diff --git a/CLAUDE.md b/CLAUDE.md index fd4e4ca..a3214e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,8 +37,11 @@ Go 1.26, один статический бинарь (`CGO_ENABLED=0`). Module Исключения два, и оба — не наши операции с файловой системой, а вызов `torrents/delete` qBittorrent с `deleteFiles=true`: (1) `Delete` из `done`/`orphaned`/`target_missing` по явному подтверждению человека — гард - последней копии там выключен сознательно - ([state-reconciliation](openspec/specs/state-reconciliation/spec.md)); + последней копии там выключен сознательно; подтверждение допустимо **одно на + пачку**, если называет каждую загрузку поимённо, и условия допуска групповой + путь не смягчает + ([state-reconciliation](openspec/specs/state-reconciliation/spec.md), + [web-ui](openspec/specs/web-ui/spec.md)); (2) уборка воркером **собственного** торрента, добавленного этим же `add` секундами ранее, когда закрытие **любым** путём (`Cancel` или `Dismiss`) увело задачу из `catched` в окне после `add` — уборка привязана к состоянию, diff --git a/docs/adr/ADR-2026-08-10-last-copy-warning-from-state.md b/docs/adr/ADR-2026-08-10-last-copy-warning-from-state.md new file mode 100644 index 0000000..b664eb6 --- /dev/null +++ b/docs/adr/ADR-2026-08-10-last-copy-warning-from-state.md @@ -0,0 +1,52 @@ +# Отметка «последняя копия» на подтверждении удаления выводится из состояния, а не из файловой системы + +- **Дата:** 2026-08-10 +- **Источник:** openspec/changes/archive/2026-08-10-bulk-delete-page/design.md + +## Решение + +Экран подтверждения группового удаления помечает загрузку как последнюю копию +данных по её **состоянию** (`orphaned`), не спрашивая файловую систему. Случай, +когда байты источника исчезли с диска, а раздача осталась в списке qBittorrent, +такой отметки не получает — и это записано границей в спеке `web-ui`, а не +оставлено умолчанием. + +## Почему + +Гард последней копии в `Delete` выключен сознательно (инвариант «источник +неприкосновенен», исключение 1), поэтому осведомлённость человека — единственный +оставшийся предохранитель. Отсюда решение D2 источника: + +> Признак берётся из состояния (`orphaned` по определению значит «источник +> пропал, цель — последняя копия»), а не обходом файловой системы. + +Враждебный проход ревью построил путь, где это неверно: сверка берёт присутствие +источника из ответа `torrents/info`, а не с диска, поэтому задача с пропавшими +байтами остаётся `done` сколько угодно долго и отметки не получает. Дыра +признана и оставлена открытой по решению человека: поштучное удаление такой +отметки не несёт **вовсе**, то есть групповой путь не ухудшил положение, а +улучшил его не до конца. Закрывать её обходом файловой системы на экране +подтверждения значит завести чтение диска в транспорте ради предупреждения, +которое и сегодня лучше прежнего. + +## Рассмотренные варианты + +- **Спрашивать файловую систему на подтверждении** (`nlink` по живым ссылкам + последнего батча) — отметка стала бы правдой, но транспорт начал бы ходить в + файловую систему ради показа, а пачка ограничена двадцатью строками только + сегодня. +- **Вернуть гард последней копии в `Delete`** — отменяет само назначение + команды: она затем и существует, чтобы снять последнюю копию осознанно. +- **Убрать отметку совсем** — честно, но теряет полезный сигнал про пропавший + источник, который в подавляющем большинстве случаев и есть последняя копия. + +## Последствия + +- `+` Подтверждение предупреждает о последней копии там, где раньше не + предупреждало ничто; признак берётся из домена, второго перечня состояний не + заводится. +- `+` Транспорт не ходит в файловую систему ради показа. +- `−` Случай «`done` с пропавшими байтами источника» отметки не получает. Дыра + названа в спеке прямо, чтобы отметка не читалась как гарантия. +- `−` Пока сверка берёт присутствие источника из списка раздач, а не с диска, + закрыть дыру нельзя ни на одном экране. diff --git a/docs/adr/README.md b/docs/adr/README.md index 71de7d7..b9e251b 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -42,6 +42,7 @@ | Дата | Запись | Статус | | --- | --- | --- | +| 2026-08-10 | [Отметка «последняя копия» на подтверждении удаления выводится из состояния, а не из файловой системы](ADR-2026-08-10-last-copy-warning-from-state.md) | — | | 2026-08-10 | [Наблюдаемость поверхности не выводится из терминальности задачи](ADR-2026-08-10-observability-is-not-terminality.md) | — | | 2026-08-10 | [Причина, по которой человек не видит плана, считается на показе, а не читается из состояния](ADR-2026-08-10-reason-computed-on-read.md) | — | | 2026-08-10 | [Значение метабазы чистится на каждой точке входа в план, три санитайзера не сводятся в один](ADR-2026-08-10-sanitize-at-every-entry.md) | — | diff --git a/docs/architecture.md b/docs/architecture.md index c1ac0d1..04ead93 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -118,6 +118,7 @@ | Хардлинки и удаление своих ссылок | `internal/layout` — единственное место, которое пишет в файловую систему библиотеки | | Построение и проверка целевого пути | `layout.BuildLinks` — единственная сборка пути; там же обе проверки, и порядок значим: нахождение под корнем библиотеки, затем длина компонента. Отсюда же строятся оба предпросмотра ревью, поэтому показанное и применённое совпадают устройством, а не договорённостью | | Причина, по которой человек не видит плана | считается **на показе** (`worker.ReviewData.PreviewError`) и предпочитается записанной в состоянии: записанной может не быть вовсе, а после смены источника она уже про другой план — [ADR-2026-08-10-reason-computed-on-read](adr/ADR-2026-08-10-reason-computed-on-read.md) | +| Условие допуска полного удаления | `store.State.CanDelete()` — «из этого состояния удаление с файлами разрешено»; своего перечня состояний не заводит ни один транспорт (веб-UI, Telegram, страница группового удаления), а проверку в ядре предикат не заменяет: допуск держится без транспорта | | Условие самообновления веб-UI | `store.State.IsObservable()` — «состояние ещё может измениться без человека»; транспорт своего перечня состояний не заводит, а поверхность (карточка списка, страница загрузки) держит **ровно один** поллер на обновляемый корень — [ADR-2026-08-10-observability-is-not-terminality](adr/ADR-2026-08-10-observability-is-not-terminality.md), правило разметки — [conventions/web-ui.md](conventions/web-ui.md) | | Трансляция доменной ошибки в код ответа | внешняя граница транспорта (`httpapi`, `tgbot`); правило — [conventions/errors.md](conventions/errors.md) | | Логирующий чекпоинт | доменная граница, один на операцию; правило — [conventions/logging.md](conventions/logging.md) | diff --git a/docs/conventions/errors.md b/docs/conventions/errors.md index b6bc511..59913d9 100644 --- a/docs/conventions/errors.md +++ b/docs/conventions/errors.md @@ -86,6 +86,7 @@ jellybit — **приложение, а не библиотека**: внешн | `worker.ErrInvalidInput` (промах ввода команды) | 400 | «некорректный ввод» | | `errManualSource` (ручной ввод источника, локальный sentinel `httpapi`) | 400 | текст самой ошибки | | `errInvalidCandidate` (выбран несуществующий кандидат, локальный sentinel `httpapi`) | 400 | текст самой ошибки | + | `errBatchEmpty` / `errBatchTooLarge` / `errBatchBadID` (разбор пачки группового удаления, локальные sentinel'ы `httpapi`) | 400 | текст самой ошибки | | `worker.ErrNotReady` (источник ещё качается) | 409 | «торрент ещё качается…» | | `layout.ErrCollision` (цель занята, ушло в review) | 409 | «целевой файл уже существует…» | | `layout.ErrNameTooLong` (целевое имя не помещается, ушло в review) | 409 | «целевое имя слишком длинное…» | diff --git a/docs/database.md b/docs/database.md index c311170..d620c7b 100644 --- a/docs/database.md +++ b/docs/database.md @@ -214,6 +214,9 @@ erDiagram | `ingest.MaxTorrentSize` | `8 MiB` | предел размера принимаемого `.torrent`; проверяется **до** разбора, поэтому bencode-аллокации на эту величину не масштабируются (см. [research/torrent-bencode-limits.md](research/torrent-bencode-limits.md)) | | `httpapi.pollFast` | `5s` | интервал самообновления поверхности с живыми цифрами качания (карточка в `downloading`). Держится вровень с `[worker].poll_interval`: снимок телеметрии обновляется тиком воркера, и опрос чаще возвращает тот же снимок. Меняется `poll_interval` — меняется и эта константа | | `httpapi.pollSlow` | `15s` | интервал самообновления прочих наблюдаемых поверхностей: карточек вне `downloading` и страницы `/download/{id}` в любом состоянии. Тик страницы считает предпросмотр раскладки и ходит в ФС, поэтому частота у него ниже | +| `httpapi.maxBulkDelete` | `20` загрузок | предел размера одной пачки группового удаления. Подтверждение, перечисляющее больше, человек не читает — то есть перестаёт быть подтверждением; плюс один синхронный запрос упирается в столько же последовательных вызовов qBittorrent. Предел называет сама страница выбора; отказ по пределу возвращает выбор с сохранёнными отметками | +| `httpapi.bulkFailThreshold` | `3` отказа подряд | сколько подряд идущих отказов внешнего сервиса прекращают проход группового удаления. Удаление снимает библиотечные ссылки раньше, чем сносит раздачу: при недоступном qBittorrent каждая единица успевает выполнить необратимый локальный шаг и упасть на внешнем. Счётчик сбрасывается на успехе; конфликт состояния системным отказом не считается | +| `httpapi.bulkBudget` | `2` минуты | потолок времени на один проход группового удаления. Удаление держит общий замок воркера на всё время обращения к qBittorrent, поэтому медленно, но успешно отвечающий сосед остановил бы фоновую работу целиком, а порог отказов такого не ловит. Проверяется между единицами: начатое удаление не обрывается, иначе оно встанет между снятием ссылок и сносом раздачи | | `layout.maxComponentBytes` | `255` байт | предел длины компонента целевого пути (`NAME_MAX` у ext4/xfs/btrfs); меряется в байтах UTF-8, проверяется **до** первой операции с ФС, отказ уводит задачу в `review` с кодом `name_too_long`. У ядра не выясняется; на ФС с меньшим пределом остаётся отказ ядра — лечение правкой константы, а не настройкой | **Ретеншена нет ни у одной таблицы**, лимита на размер тела ответа LLM нет, diff --git a/docs/review.md b/docs/review.md index b7c08ac..8b6b15a 100644 --- a/docs/review.md +++ b/docs/review.md @@ -179,8 +179,8 @@ Go-сервиса и что здесь уже проскакивало. Устр - `requirements`: не завелось ли поведение, которого спека не заказывала — тихий дефолт, проглоченная ошибка, ретрай «на всякий случай», отброшенное поле? - `security`: читается ли тело ответа внешнего сервиса целиком без предела — - лимита на размер ответа LLM в проекте нет, и это единственный недоверенный - канал, где предел не стоит ([security.md](security.md) → «Что вне модели») + у LLM (8 MiB) и метабаз (4 MiB) предел стоит, и новый исходящий вызов обязан + заводить свой ([security.md](security.md) → «Что вне модели») - `operations`: гарантия, которую вводит изменение, поставлена на запись или на чтение — и что будет с данными, записанными до деплоя, которые обычный путь не перезаписывает? (журнал, 2026-08-10: чистка названия стояла на записи, и @@ -231,6 +231,13 @@ Go-сервиса и что здесь уже проскакивало. Устр (`internal/metadata`, `metadata-match`); merge-раскладка при повторном добавлении раздачи. +- **новая поверхность поверх необратимой операции** — вторая точка входа в + команду, которая удаляет файлы или снимает последнюю копию. Форма решения + здесь нащупывается по ходу: подтверждение, порядок отказов, остаток, + наблюдаемость. Выведено по факту на `bulk-delete-page` (журнал, 2026-08-10): + метка `medium` не дала ни враждебного прохода, ни замера, а именно они нашли + четыре дефекта класса «необратимо». + - «Поведение, видимое снаружи» здесь включает **тексты и карточки Telegram** — для единственного пользователя это и есть интерфейс. @@ -325,6 +332,49 @@ Go-сервиса и что здесь уже проскакивало. Устр случаи до этой даты не восстанавливались — восстановленная постфактум причина непоймания недостоверна, а именно она и нужна. +## 2026-08-10 — метка занижена: новая поверхность поверх необратимой операции прошла как среднее знакомое [пойман] + +- **Где:** конвейер, а не код — разметка задачи `bulk-delete-page` +- **Симптом:** прогон по метке `medium` закончился шестью находками, из них ни + одной про необратимое. Сигнал «метка, вероятно, занижена» вернули два прохода + из четырёх — `code` и `basics`, с одинаковым основанием: дифф трогает `store`, + `worker`, `httpapi`, `tgbot` и шаблоны и заводит новую точку входа поверх + команды, удаляющей файлы +- **Причина:** обе оси считались по объёму и по знакомости узлов, и по ним + изменение честно выходило средним и знакомым — цикл над готовым `Delete`. Ни + один триггер `large` не описывал случай «поверхность новая, а операция за ней + необратимая» +- **Чем воспроизведён:** повторная разметка после правок дельта-спек вернула + `large`; догнанные проходы дали четыре находки класса «необратимо», из них три + с прогнанными падающими тестами (`TestAdversaryDoneRowIsLastCopyWithoutWarning`, + `TestAdversaryDeleteWipesSourceOfAnotherActiveDownload`) и одна с замером + удержания общего замка воркера (`280.450631ms` при задержке соседа `300ms`) +- **Что меняем:** в «Триггеры метки», ось «незнакомое», добавлен пункт про новую + поверхность поверх необратимой операции + +## 2026-08-10 — три дефекта поштучного удаления жили незамеченными, пока рядом не появилась пачка [проскочил] + +- **Где:** `internal/worker/review.go` (`Delete`), `internal/worker/worker.go` + (`Poll`) +- **Симптом:** враждебный и эксплуатационный проходы на задаче + `bulk-delete-page` нашли три дефекта, ни один из которых эта задача не + вносила: удаление сносит раздачу, которой владеет **другая активная** загрузка + с тем же инфохэшем; удаление держит общий замок воркера через сетевой вызов и + останавливает фоновую работу на это время; при недоступном qBittorrent задача остаётся `done` весь простой + соседа, потому что сверка возвращается на первой же ошибке и до коррекции не + доходит +- **Причина:** поштучное удаление ни разу не проверялось меткой `large` — ни + построенного пути, ни замера против него не гонял никто +- **Чем воспроизведён:** тесты и замеры перечислены в + `openspec/changes/archive/2026-08-10-bulk-delete-page/review/report.md`, + находки 2–4 +- **Почему не поймали:** проходы, находящие этот класс, живут в метке `large`, а + задачи, заводившие и правившие `Delete`, шли ниже. Дефект не «пропустил + проход» — проход не запускался +- **Что меняем:** три записи в беклоге со ссылкой на оракулы; триггер метки + дополнен (см. запись выше), чтобы следующая поверхность над необратимой + операцией шла сразу с доказательными проходами + ## 2026-08-10 — тест остался зелёным навсегда, потому что проверял снятый атрибут [пойман] - **Где:** `internal/httpapi/live_test.go` — `TestFragProgressStopsWhenNotDownloading` diff --git a/docs/security.md b/docs/security.md index 7e0f87a..33e49ff 100644 --- a/docs/security.md +++ b/docs/security.md @@ -32,6 +32,7 @@ REST API работают **без авторизации** осознанно; | Ответы метабаз | HTTP к TMDB/TVDB/TVMaze | канонические названия, из которых тоже строится путь; чистятся наравне с выходом LLM на каждой точке входа в план ([ADR-2026-08-10-sanitize-at-every-entry](adr/ADR-2026-08-10-sanitize-at-every-entry.md)) | | Ответы qBittorrent | HTTP | пути, состояния, размеры | | Запросы веб-UI и REST | LAN | идентификаторы, параметры действий | +| Пачка идентификаторов и признак подтверждения на групповом удалении | форма веб-UI | необратимое действие сразу по многим загрузкам; разбор, схлопывание дублей и предел размера стоят на **обеих** границах — подтверждении и исполнении, потому что вторая получает пачку формой заново | **Выход LLM не отвечает за безопасность.** Инъекция в промпт считается состоявшейся по умолчанию; защита стоит ниже — на валидации целевого пути. @@ -103,8 +104,8 @@ REST API работают **без авторизации** осознанно; - **Отказ в обслуживании изнутри контура.** Огромная раздача, тысяча файлов, бесконечный ответ LLM — это вопросы устойчивости и ресурсов ([architecture.md](architecture.md) → «Эксплуатация»), а не безопасности. - Лимита на размер ответа LLM нет — известный пробел - ([architecture.md](architecture.md) → «Открытые вопросы»). + Тело ответа внешнего сервиса при этом читается с пределом: LLM — 8 MiB + (`internal/llm`), метабазы — 4 MiB (`internal/metadata`). - **Целостность содержимого медиафайлов.** Что в контейнере mkv — не наша забота. - **Цепочка поставки** — модули Go, базовый образ distroless, плагины тулинга. - **Приватность запросов к внешним сервисам.** Названия раздач уезжают в LLM и diff --git a/internal/httpapi/bulkdelete.go b/internal/httpapi/bulkdelete.go new file mode 100644 index 0000000..21163b9 --- /dev/null +++ b/internal/httpapi/bulkdelete.go @@ -0,0 +1,289 @@ +package httpapi + +import ( + "context" + "errors" + "net/http" + "strconv" + "time" + + "git.vakhrushev.me/av/jellybit/internal/ident" + "git.vakhrushev.me/av/jellybit/internal/store" + "git.vakhrushev.me/av/jellybit/internal/worker" +) + +// maxBulkDelete — верхний предел числа загрузок в одной пачке. Подтверждение, +// перечисляющее больше, человек не читает — то есть перестаёт быть +// подтверждением; плюс один синхронный запрос упирается в столько же +// последовательных вызовов qBittorrent. Число названо на самой странице выбора: +// предел, о котором узнают только из отказа, отнимает уже сделанную работу. +const maxBulkDelete = 20 + +// bulkFailThreshold — сколько подряд идущих отказов внешнего сервиса +// прекращают проход. Удаление снимает библиотечные ссылки раньше, чем сносит +// раздачу: при лежащем qBittorrent каждая единица успевает выполнить +// необратимый локальный шаг и упасть на внешнем, оставив тайтл без раскладки и +// не освободив места. Счётчик сбрасывается на успехе — одиночная сетевая +// ошибка пачку не рвёт. +const bulkFailThreshold = 3 + +// bulkBudget — потолок времени на один проход пачки. Удаление держит общий +// замок воркера на всё время обращения к qBittorrent, поэтому медленно, но +// успешно отвечающий сосед останавливает фоновую работу целиком, а порог +// отказов такого не ловит — он считает только ошибки. Проверяется МЕЖДУ +// единицами, а не отменой контекста: начатое удаление обрывать нельзя, иначе +// оно встанет между снятием библиотечных ссылок и сносом раздачи. +// +// Переменная, а не константа, ровно по одной причине: тест укорачивает её — +// иначе проверка потолка стоила бы двух минут прогона. +var bulkBudget = 2 * time.Minute + +// Отказы разбора пачки. Текст — публичного канала: он показывается человеку +// как есть, как у прочих sentinel'ов транспорта. Трансляция в статус и +// сообщение живёт в единой точке `classifyErr`, а не рядом. +var ( + errBatchEmpty = errors.New("ни одна загрузка не выбрана") + errBatchTooLarge = errors.New("за один раз можно удалить не больше " + + strconv.Itoa(maxBulkDelete) + " загрузок") + errBatchBadID = errors.New("некорректный идентификатор загрузки — запрос отклонён целиком") + errBatchForm = errors.New("форма запроса не разобрана — запрос отклонён целиком") +) + +// bulkRow — строка загрузки на любом из трёх экранов группового удаления. +type bulkRow struct { + ID string + Title string + State string + Selected bool // отметка сохранена при возврате отказа + LastCopy bool // orphaned: библиотечная ссылка осталась последней копией + Missing bool // записи в хранилище нет + Unread bool // состояние прочитать не удалось (отказ хранилища) + Reason string // причина отказа (только на экране результата) +} + +// bulkSelectView — страница выбора (`GET /delete`) и она же ответ на отказ +// разбора: отметки при этом сохраняются, иначе проверка стирает всю работу. +type bulkSelectView struct { + Error string + Max int + Rows []bulkRow +} + +// bulkConfirmView — страница подтверждения: выбранные названы поимённо. +type bulkConfirmView struct { + Rows []bulkRow +} + +// bulkResultView — отчёт: обе половины исхода поимённо плюс остаток, если +// проход остановлен системным отказом. +type bulkResultView struct { + Deleted []bulkRow + Failed []bulkRow + Skipped []bulkRow + StopReason string +} + +// handleBulkDeletePage — страница выбора. Самообновления не несёт сознательно: +// своп разметки унёс бы отметки, и человек подтвердил бы необратимое удаление +// по выбору, которого уже не видит (см. openspec/specs/web-ui). +func (s *server) handleBulkDeletePage(w http.ResponseWriter, r *http.Request) { + s.renderBulkSelect(w, r, "", nil) +} + +// renderBulkSelect отрисовывает страницу выбора, помечая отмеченными те строки, +// чьи идентификаторы человек уже выбрал (selected). Общий путь для чистого +// открытия страницы и для любого отказа разбора. +func (s *server) renderBulkSelect(w http.ResponseWriter, r *http.Request, msg string, selected []string) { + ds, err := s.deps.Reader.ListDeletableDownloads(r.Context()) + if err != nil { + s.deps.Logger.Error("list deletable downloads", "error", err) + http.Error(w, "внутренняя ошибка", http.StatusInternalServerError) + return + } + mark := make(map[string]bool, len(selected)) + for _, id := range selected { + mark[id] = true + } + view := bulkSelectView{Error: msg, Max: maxBulkDelete} + for _, d := range ds { + view.Rows = append(view.Rows, bulkRow{ + ID: d.ID, + Title: downloadTitle(d), + State: string(d.State), + Selected: mark[d.ID], + LastCopy: d.State == store.StateOrphaned, + }) + } + s.render(w, "delete.html", view) +} + +// handleBulkDeleteConfirm — экран подтверждения. Ничего не меняет: разбирает +// вход, читает выбранные загрузки и называет каждую поимённо. +func (s *server) handleBulkDeleteConfirm(w http.ResponseWriter, r *http.Request) { + ids, err := s.parseBulkBatch(r) + if err != nil { + s.renderBulkSelect(w, r, bulkErrMsg(err), ids) + return + } + s.render(w, "delete_confirm.html", bulkConfirmView{Rows: s.bulkRows(r.Context(), ids)}) +} + +// handleBulkDelete — исполнение пачки. Признак подтверждения проверяется ДО +// разбора и до единого вызова удаления: подтверждение — условие операции, а не +// украшение экрана. +func (s *server) handleBulkDelete(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil || r.PostForm.Get("confirm") != "1" { + s.renderBulkSelect(w, r, "Удаление уходит только со страницы подтверждения.", nil) + return + } + // Исполняющий запрос — самостоятельная входная граница: идентификаторы + // приходят формой заново, состояния между шагами сервис не хранит. + ids, err := s.parseBulkBatch(r) + if err != nil { + s.renderBulkSelect(w, r, bulkErrMsg(err), ids) + return + } + + // Контекст исполнения отвязан от запроса: обрыв связи не вправе оборвать + // необратимую операцию на середине — в том числе внутри одной загрузки, + // между снятием библиотечных ссылок и сносом раздачи. Строки отчёта читаются + // тем же контекстом: собранные отменённым, они превратили бы весь отчёт в + // «загрузка не найдена» ровно там, где удаление идёт штатно. + ctx := context.WithoutCancel(r.Context()) + rows := s.bulkRows(ctx, ids) + + var res bulkResultView + streak := 0 + deadline := store.Now().Add(bulkBudget) + for i, row := range rows { + if res.StopReason != "" { + res.Skipped = append(res.Skipped, rows[i]) + continue + } + if store.Now().After(deadline) { + res.StopReason = "Проход занял дольше отведённого времени и остановлен: " + + "пока идёт пачка, остальная работа сервиса ждёт." + res.Skipped = append(res.Skipped, rows[i]) + continue + } + err := s.deps.Reviewer.Delete(ctx, row.ID) + if err == nil { + streak = 0 + res.Deleted = append(res.Deleted, row) + continue + } + row.Reason = userErr(r, err, row.ID) + res.Failed = append(res.Failed, row) + // Конфликт состояния и отсутствие записи — про саму задачу, а не про + // доступность соседа: счётчик системных отказов они не двигают. + if errors.Is(err, worker.ErrConflict) || errors.Is(err, store.ErrNotFound) { + continue + } + streak++ + if streak >= bulkFailThreshold { + res.StopReason = "Внешний сервис отказывает подряд — проход остановлен, " + + "чтобы не снимать раскладку у остальных без освобождения места." + } + } + // Исход каждой единицы поимённо: ответ мог не дойти (вкладку закрыли), и + // журнал — единственное, по чему потом видно, что снесено, что отказало и до + // чего проход не дошёл. На воркер полагаться нельзя: отказы по конфликту и + // отсутствию записи он пишет на DEBUG. + for _, row := range res.Deleted { + s.deps.Logger.Info("bulk delete item", "download_id", row.ID, "outcome", "deleted") + } + for _, row := range res.Failed { + s.deps.Logger.Info("bulk delete item", "download_id", row.ID, "outcome", "failed", + "reason", row.Reason) + } + for _, row := range res.Skipped { + s.deps.Logger.Info("bulk delete item", "download_id", row.ID, "outcome", "skipped") + } + s.deps.Logger.Info("bulk delete finished", + "requested", len(rows), "deleted", len(res.Deleted), + "failed", len(res.Failed), "skipped", len(res.Skipped), + "stopped", res.StopReason != "") + + s.render(w, "delete_result.html", res) +} + +// bulkRows читает выбранные загрузки для показа поимённо. Идентификатор без +// записи в хранилище не выбрасывается молча — он идёт своей строкой: человек +// подтверждает пачку, и она обязана совпадать с тем, что он выбрал. +func (s *server) bulkRows(ctx context.Context, ids []string) []bulkRow { + rows := make([]bulkRow, 0, len(ids)) + for _, id := range ids { + d, err := s.deps.Reader.GetDownload(ctx, id) + switch { + case errors.Is(err, store.ErrNotFound): + rows = append(rows, bulkRow{ID: id, Missing: true, Title: "загрузка не найдена"}) + continue + case err != nil || d == nil: + // Отказ хранилища — это НЕ «записи нет». Выдав одно за другое, экран + // сказал бы «удалять нечего» о загрузке, которую пачка снесёт + // по-настоящему, и для orphaned унёс бы отметку последней копии — + // единственный оставшийся предохранитель. Приватный канал: пишем + // здесь, потому что выше эта ошибка не всплывает. + s.deps.Logger.Error("bulk delete: read download", "download_id", id, "error", err) + rows = append(rows, bulkRow{ID: id, Unread: true, Title: "состояние прочитать не удалось"}) + continue + } + rows = append(rows, bulkRow{ + ID: d.ID, + Title: downloadTitle(*d), + State: string(d.State), + LastCopy: d.State == store.StateOrphaned, + }) + } + return rows +} + +// parseBulkBatch разбирает пачку идентификаторов с формы. Проверки одинаковы на +// обеих границах — подтверждения и исполнения. Возвращает разобранные +// идентификаторы даже вместе с отказом: страница выбора возвращает по ним +// отметки, чтобы отказ не стирал проделанную работу. +func (s *server) parseBulkBatch(r *http.Request) ([]string, error) { + if err := r.ParseForm(); err != nil { + // Приватный канал: выше эта ошибка не всплывает, а человеку про + // идентификаторы говорить нечего — тело не прочиталось целиком. + s.deps.Logger.Error("bulk delete: parse form", "error", err) + return nil, errBatchForm + } + // Только тело: признак подтверждения и пачка приходят формой, и принимать + // их из строки запроса значит принимать подтверждение оттуда, откуда + // требование его не заказывало. + raw := r.PostForm["id"] + seen := make(map[string]bool, len(raw)) + ids := make([]string, 0, len(raw)) + for _, v := range raw { + id, err := ident.Parse(v) + if err != nil { + // Молча пропустить нельзя: человек подтвердил удаление поимённо, и + // выброшенный идентификатор развёл бы подтверждённое с исполненным. + return ids, errBatchBadID + } + if seen[id] { + continue + } + seen[id] = true + ids = append(ids, id) + } + if len(ids) == 0 { + return nil, errBatchEmpty + } + if len(ids) > maxBulkDelete { + return ids, errBatchTooLarge + } + return ids, nil +} + +// bulkErrMsg — сообщение человеку об отказе разбора. Сам текст берётся из +// единой точки трансляции (`classifyErr`); здесь добавляется только подсказка, +// что делать дальше, — она осмысленна ровно на этой странице. +func bulkErrMsg(err error) string { + _, msg := classifyErr(err) + if errors.Is(err, errBatchTooLarge) { + return msg + ". Отметки сохранены — сними лишние." + } + return msg + "." +} diff --git a/internal/httpapi/bulkdelete_test.go b/internal/httpapi/bulkdelete_test.go new file mode 100644 index 0000000..335c34f --- /dev/null +++ b/internal/httpapi/bulkdelete_test.go @@ -0,0 +1,611 @@ +package httpapi + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "testing" + "time" + + "git.vakhrushev.me/av/jellybit/internal/store" + "git.vakhrushev.me/av/jellybit/internal/worker" +) + +// Валидные lowercase-ULID для форм (parseBulkBatch прогоняет через ident.Parse). +const ( + bid1 = "01arz3ndektsv4rrffq69g5fa1" + bid2 = "01arz3ndektsv4rrffq69g5fa2" + bid3 = "01arz3ndektsv4rrffq69g5fa3" + bid4 = "01arz3ndektsv4rrffq69g5fa4" +) + +// bulkReviewer — Reviewer, который считает вызовы удаления и умеет отказать по +// конкретному идентификатору. Указатель: тест проверяет «ни одного вызова», а +// значение-копия этого не покажет. +type bulkReviewer struct { + stubReviewer + calls *[]string + errs map[string]error + allErr error // отказ по любому идентификатору (лежащий внешний сервис) + sleep time.Duration // медленный, но исправный сосед +} + +func (b bulkReviewer) Delete(ctx context.Context, id string) error { + *b.calls = append(*b.calls, id) + // Контекст исполнения обязан пережить отмену запроса: необратимую операцию + // нельзя обрывать на середине. Проверяем во всех тестах, а не только в том, + // что об этом, — гарантия одна на все пути. + if err := ctx.Err(); err != nil { + return fmt.Errorf("bulk delete stub: %w", err) + } + time.Sleep(b.sleep) + if b.allErr != nil { + return b.allErr + } + return b.errs[id] +} + +func newBulkReviewer(errs map[string]error) (bulkReviewer, *[]string) { + calls := &[]string{} + return bulkReviewer{calls: calls, errs: errs}, calls +} + +func dl(id string, st store.State, title string) store.Download { + return store.Download{ID: id, State: st, DisplayName: title} +} + +// byID собирает карту для поштучного чтения на экранах подтверждения и отчёта. +func byID(ds ...store.Download) map[string]store.Download { + m := make(map[string]store.Download, len(ds)) + for _, d := range ds { + m[d.ID] = d + } + return m +} + +// postCancelled отправляет POST с уже отменённым контекстом запроса — так +// выглядит закрытая вкладка или оборванная связь на середине пачки. +func postCancelled(t *testing.T, h http.Handler, path string, form url.Values) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + ctx, cancel := context.WithCancel(req.Context()) + req = req.WithContext(ctx) + cancel() + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + return rr +} + +func idForm(ids ...string) url.Values { + f := url.Values{} + for _, id := range ids { + f.Add("id", id) + } + return f +} + +// П1: страница показывает строки только тех загрузок, для которых удаление +// разрешено поштучно. Читатель отдаёт задачи во всех состояниях — на странице +// оказываются ровно разрешённые. +func TestBulkDeletePageShowsOnlyDeletable(t *testing.T) { + all := []store.State{ + store.StateCatched, store.StateDownloading, store.StateCompleted, + store.StateRecognizing, store.StateReview, store.StateLinking, + store.StateDone, store.StateDeferred, store.StateStuck, + store.StateFailed, store.StateCancelled, store.StateReverted, + store.StateTargetMissing, store.StateOrphaned, store.StateDeleted, + } + // Читатель отдаёт всё подряд — отбирает страница по домену, а не тест. + var rows []store.Download + for i, st := range all { + if st.CanDelete() { + rows = append(rows, dl(fmt.Sprintf("%026d", i), st, "задача "+string(st))) + } + } + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{deletable: rows}, rv, stubCommander{}, stubLive{}) + + body := get(t, h, "/delete").Body.String() + for _, st := range all { + marker := "задача " + string(st) + if got := strings.Contains(body, marker); got != st.CanDelete() { + t.Errorf("%s: строка на странице=%v, CanDelete=%v", st, got, st.CanDelete()) + } + } +} + +// Страница не опрашивает сервер: своп разметки стёр бы отметки, и человек +// подтвердил бы необратимое удаление по выбору, которого уже не видит. +func TestBulkDeletePageDoesNotSelfPoll(t *testing.T) { + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, + stubReader{deletable: []store.Download{dl(bid1, store.StateDone, "Дюна")}}, + rv, stubCommander{}, stubLive{}) + + body := get(t, h, "/delete").Body.String() + if strings.Contains(body, `hx-trigger="every`) { + t.Error("страница выбора не должна самообновляться") + } + // Предел пачки назван до отправки — иначе отказ по нему отнимает работу. + if !strings.Contains(body, strconv.Itoa(maxBulkDelete)) { + t.Errorf("предел пачки не назван на странице:\n%s", body) + } +} + +// Пустое состояние: разрешённых нет — удаление не предлагается. +func TestBulkDeletePageEmpty(t *testing.T) { + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{}, rv, stubCommander{}, stubLive{}) + + body := get(t, h, "/delete").Body.String() + if strings.Contains(body, `action="/ui/delete/confirm"`) { + t.Error("на пустой странице не должно быть формы удаления") + } + if !strings.Contains(body, "Удалять нечего") { + t.Errorf("нет пустого состояния:\n%s", body) + } +} + +// П2 (первая половина): подтверждение называет каждую выбранную поимённо и не +// делает ни одного вызова удаления. +func TestBulkConfirmNamesRowsAndDeletesNothing(t *testing.T) { + rv, calls := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateOrphaned, "Фарго"), + )}, rv, stubCommander{}, stubLive{}) + + body := post(t, h, "/ui/delete/confirm", idForm(bid1, bid2), false).Body.String() + for _, want := range []string{"Дюна", "Фарго", bid1, bid2} { + if !strings.Contains(body, want) { + t.Errorf("подтверждение не называет %q:\n%s", want, body) + } + } + if len(*calls) != 0 { + t.Errorf("подтверждение не должно удалять, вызовы: %v", *calls) + } +} + +// Строка orphaned на подтверждении предупреждает о последней копии данных: гард +// последней копии в удалении выключен сознательно, и осведомлённость человека — +// единственный оставшийся предохранитель. +func TestBulkConfirmWarnsAboutLastCopy(t *testing.T) { + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateOrphaned, "Фарго"), + )}, rv, stubCommander{}, stubLive{}) + + body := post(t, h, "/ui/delete/confirm", idForm(bid1), false).Body.String() + if !strings.Contains(body, "последняя копия данных") { + t.Errorf("нет предупреждения о последней копии:\n%s", body) + } +} + +// П2 (вторая половина): без признака подтверждения не удаляется ничего. +func TestBulkDeleteWithoutConfirmDeletesNothing(t *testing.T) { + rv, calls := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + )}, rv, stubCommander{}, stubLive{}) + + body := post(t, h, "/ui/delete", idForm(bid1), false).Body.String() + if len(*calls) != 0 { + t.Errorf("без подтверждения не должно быть вызовов удаления: %v", *calls) + } + if !strings.Contains(body, "страницы подтверждения") { + t.Errorf("отказ не объяснён:\n%s", body) + } +} + +// Гарды разбора одинаковы на обеих границах: исполняющий запрос получает +// идентификаторы формой заново и на проверки подтверждения опираться не вправе. +func TestBulkBatchGuardsOnBothBoundaries(t *testing.T) { + over := make([]string, 0, maxBulkDelete+1) + for i := range maxBulkDelete + 1 { + over = append(over, fmt.Sprintf("%026d", i)) + } + + cases := []struct { + name string + form url.Values + want string + }{ + {"неразобранный идентификатор", idForm(bid1, "не-ulid"), "некорректный идентификатор"}, + {"пачка сверх предела", idForm(over...), "не больше"}, + {"пустой набор", url.Values{}, "ни одна загрузка не выбрана"}, + } + for _, c := range cases { + for _, path := range []string{"/ui/delete/confirm", "/ui/delete"} { + t.Run(c.name+" "+path, func(t *testing.T) { + rv, calls := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{ + deletable: []store.Download{dl(bid1, store.StateDone, "Дюна")}, + byID: byID(dl(bid1, store.StateDone, "Дюна")), + }, rv, stubCommander{}, stubLive{}) + + form := url.Values{} + for k, v := range c.form { + form[k] = v + } + if path == "/ui/delete" { + form.Set("confirm", "1") + } + body := post(t, h, path, form, false).Body.String() + + if len(*calls) != 0 { + t.Errorf("отказ разбора не должен удалять: %v", *calls) + } + if !strings.Contains(body, c.want) { + t.Errorf("нет объяснения %q:\n%s", c.want, body) + } + }) + } + } +} + +// Отказ по пределу возвращает страницу выбора с сохранёнными отметками: иначе +// проверка отнимает всю проделанную человеком работу. +func TestBulkOverLimitKeepsSelection(t *testing.T) { + rows := []store.Download{dl(bid1, store.StateDone, "Дюна")} + over := []string{bid1} + for i := range maxBulkDelete { + over = append(over, fmt.Sprintf("%026d", i)) + } + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{deletable: rows}, rv, stubCommander{}, stubLive{}) + + body := post(t, h, "/ui/delete/confirm", idForm(over...), false).Body.String() + if !strings.Contains(body, `value="`+bid1+`" checked`) { + t.Errorf("отметка выбора не сохранена:\n%s", body) + } +} + +// Дубликаты в пачке схлопываются: повторный вызов по той же задаче дал бы +// ложный конфликт во второй строке отчёта. +func TestBulkDeleteCollapsesDuplicates(t *testing.T) { + rv, calls := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid1, bid1) + form.Set("confirm", "1") + post(t, h, "/ui/delete", form, false) + + if len(*calls) != 1 { + t.Errorf("дубликаты не схлопнуты: %v", *calls) + } +} + +// П3: отказ на одной загрузке не отменяет остальных, отчёт называет обе +// половины поимённо. +func TestBulkDeletePartialFailure(t *testing.T) { + rv, calls := newBulkReviewer(map[string]error{ + bid2: errors.New("qbittorrent: connection refused"), + }) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateDone, "Фарго"), + dl(bid3, store.StateDone, "Оппенгеймер"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid2, bid3) + form.Set("confirm", "1") + body := post(t, h, "/ui/delete", form, false).Body.String() + + if len(*calls) != 3 { + t.Fatalf("удаление должно уйти по всем трём: %v", *calls) + } + for _, want := range []string{"Дюна", "Фарго", "Оппенгеймер", "Удалено — 2", "Отказ — 1"} { + if !strings.Contains(body, want) { + t.Errorf("отчёт не называет %q:\n%s", want, body) + } + } + // Сырой текст ошибки внешнего сервиса наружу не идёт — только публичный канал. + if strings.Contains(body, "connection refused") { + t.Errorf("сырая ошибка просочилась в разметку:\n%s", body) + } +} + +// П4: групповой путь прав поштучного не расширяет — недопустимое состояние +// отклоняется тем же конфликтом, остальные выбранные удаляются. +func TestBulkDeleteConflictDoesNotWidenRights(t *testing.T) { + rv, calls := newBulkReviewer(map[string]error{ + bid2: fmt.Errorf("delete: download in state downloading: %w", worker.ErrConflict), + }) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateDownloading, "Фарго"), + dl(bid3, store.StateDone, "Оппенгеймер"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid2, bid3) + form.Set("confirm", "1") + body := post(t, h, "/ui/delete", form, false).Body.String() + + if len(*calls) != 3 { + t.Fatalf("допуск проверяет ядро — звать надо все три: %v", *calls) + } + if !strings.Contains(body, "Удалено — 2") || !strings.Contains(body, "Отказ — 1") { + t.Errorf("отчёт не разделил исходы:\n%s", body) + } +} + +// Все удалены — отчёт называет обе загрузки и отказов не содержит. +func TestBulkDeleteAllSucceed(t *testing.T) { + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateOrphaned, "Фарго"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid2) + form.Set("confirm", "1") + body := post(t, h, "/ui/delete", form, false).Body.String() + + if !strings.Contains(body, "Удалено — 2") || strings.Contains(body, "Отказ — ") { + t.Errorf("ожидались две удалённые без отказов:\n%s", body) + } +} + +// Идентификатор без записи в хранилище назван строкой и на подтверждении, и в +// отчёте: молча выброшенный, он развёл бы подтверждённое с исполненным. +func TestBulkMissingDownloadIsNamed(t *testing.T) { + rv, _ := newBulkReviewer(map[string]error{ + bid2: fmt.Errorf("delete: %w", store.ErrNotFound), + }) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid3, store.StateDone, "Оппенгеймер"), + )}, rv, stubCommander{}, stubLive{}) + + confirm := post(t, h, "/ui/delete/confirm", idForm(bid1, bid2, bid3), false).Body.String() + if !strings.Contains(confirm, bid2) || !strings.Contains(confirm, "не найдена") { + t.Errorf("подтверждение не назвало ненайденную загрузку:\n%s", confirm) + } + + form := idForm(bid1, bid2, bid3) + form.Set("confirm", "1") + res := post(t, h, "/ui/delete", form, false).Body.String() + if !strings.Contains(res, bid2) { + t.Errorf("отчёт не назвал ненайденную загрузку:\n%s", res) + } + if !strings.Contains(res, "Удалено — 2") { + t.Errorf("остальные должны быть удалены:\n%s", res) + } +} + +// Системный отказ останавливает пачку: удаление снимает библиотечные ссылки +// раньше, чем сносит раздачу, поэтому при лежащем qBittorrent проход без +// остановки оставил бы без раскладки все выбранные тайтлы разом. +func TestBulkDeleteStopsOnConsecutiveSystemFailures(t *testing.T) { + calls := &[]string{} + rv := bulkReviewer{calls: calls, allErr: errors.New("qbittorrent: unreachable")} + ids := []string{bid1, bid2, bid3, bid4} + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateDone, "Фарго"), + dl(bid3, store.StateDone, "Оппенгеймер"), + dl(bid4, store.StateDone, "Интерстеллар"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(ids...) + form.Set("confirm", "1") + body := post(t, h, "/ui/delete", form, false).Body.String() + + if len(*calls) != bulkFailThreshold { + t.Fatalf("проход должен остановиться после %d отказов, вызовов: %v", + bulkFailThreshold, *calls) + } + if !strings.Contains(body, "Не выполнено — 1") { + t.Errorf("остаток пачки не назван невыполненным:\n%s", body) + } + if !strings.Contains(body, "проход остановлен") { + t.Errorf("причина остановки не названа:\n%s", body) + } +} + +// Одиночный отказ пачку не рвёт: счётчик подряд идущих отказов сбрасывается на +// каждом успехе, иначе случайная сетевая ошибка обрывала бы всю уборку. +func TestBulkDeleteSingleFailureDoesNotStop(t *testing.T) { + rv, calls := newBulkReviewer(map[string]error{ + bid2: errors.New("qbittorrent: temporary"), + }) + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateDone, "Фарго"), + dl(bid3, store.StateDone, "Оппенгеймер"), + dl(bid4, store.StateDone, "Интерстеллар"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid2, bid3, bid4) + form.Set("confirm", "1") + body := post(t, h, "/ui/delete", form, false).Body.String() + + if len(*calls) != 4 { + t.Fatalf("одиночный отказ не должен останавливать проход: %v", *calls) + } + if strings.Contains(body, "Не выполнено") { + t.Errorf("остановки быть не должно:\n%s", body) + } +} + +// Отмена запроса не прекращает необратимую операцию: закрытая вкладка не вправе +// оборвать пачку на середине, в том числе внутри одной загрузки. +func TestBulkDeleteSurvivesRequestCancel(t *testing.T) { + calls := &[]string{} + // Reviewer, который проверяет, что контекст исполнения жив, хотя контекст + // запроса уже отменён. + rv := bulkReviewer{calls: calls, errs: map[string]error{}} + h := testRouterAction(t, stubReader{byID: byID( + dl(bid1, store.StateDone, "Дюна"), + dl(bid2, store.StateDone, "Фарго"), + )}, rv, stubCommander{}, stubLive{}) + + form := idForm(bid1, bid2) + form.Set("confirm", "1") + rr := postCancelled(t, h, "/ui/delete", form) + + if len(*calls) != 2 { + t.Fatalf("пачка должна дойти до конца при обрыве: %v", *calls) + } + if !strings.Contains(rr.Body.String(), "Удалено — 2") { + t.Errorf("исход не собран:\n%s", rr.Body.String()) + } +} + +// Формы страниц удаления работают без JavaScript: обычный POST с рабочим action +// и никаких hx-атрибутов на пути к необратимому действию. +func TestBulkDeleteWorksWithoutJS(t *testing.T) { + rv, _ := newBulkReviewer(nil) + h := testRouterAction(t, stubReader{ + deletable: []store.Download{dl(bid1, store.StateDone, "Дюна")}, + byID: byID(dl(bid1, store.StateDone, "Дюна")), + }, rv, stubCommander{}, stubLive{}) + + sel := get(t, h, "/delete").Body.String() + if !strings.Contains(sel, `