Ревью: preflight готовности источника для команд ревью (MAJOR-5)
Команды ревью проверяли только наличие раздачи в qBittorrent, но не её готовность. Недокачанную задачу можно припарковать в deferred, затем «Распознать заново» → recognizing → авто-раскладка (Rerecognize/Refine/ SetType не ставят force_review) → хардлинки на неполные файлы. Даже ручной Apply не имел preflight завершённости. Вводим ensureSourceReady (classify(t.State)==classReady) вместо ensureSourcePresent во всех командах, которым нужен источник (Relink/ Rerecognize/Refine/SetType), и inline-проверку класса в Apply — последний рубеж перед хардлинками. Недокачанный источник → отдельный sentinel ErrNotReady (409) с actionable-текстом «торрент ещё качается» в web и Telegram, без reconcile (состояние deferred/review легитимно). Change review-readiness-preflight заархивирован, дельта влита в openspec/specs/review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -6,3 +6,10 @@ import "errors"
|
||||
// вне review/deferred, undo вне done). Это нормальный конфликт состояния, а не
|
||||
// сбой сервера: транспорт матчит его через errors.Is и отвечает 409, не 500.
|
||||
var ErrConflict = errors.New("conflict")
|
||||
|
||||
// ErrNotReady — источник (раздача) присутствует в qBittorrent, но ещё не
|
||||
// докачан (не в классе classReady), поэтому команда, ведущая к распознаванию
|
||||
// или раскладке, отклонена: хардлинки на неполные файлы недопустимы. Отдельно
|
||||
// от ErrConflict (тоже 409), потому что причина actionable — «дождись докачки»
|
||||
// — и транспорт показывает её конкретным текстом, а не генериком конфликта.
|
||||
var ErrNotReady = errors.New("source not ready")
|
||||
|
||||
@@ -245,23 +245,33 @@ func recoveredState(state string) store.State {
|
||||
|
||||
// --- Синхронный preflight перед действием (не доверяем state в БД) ---
|
||||
|
||||
// ensureSourcePresent синхронно (без дебаунса) проверяет, что раздача есть в
|
||||
// qBittorrent прямо сейчас. При отсутствии приводит состояние к реальности и
|
||||
// возвращает ErrConflict. Недоступность qBittorrent — честный отказ операции.
|
||||
func (w *Worker) ensureSourcePresent(ctx context.Context, d *store.Download, op string) error {
|
||||
// ensureSourceReady синхронно (без дебаунса) проверяет, что раздача есть в
|
||||
// qBittorrent прямо сейчас И докачана (класс classReady). Команды ревью, ведущие
|
||||
// к распознаванию или раскладке, работают только с готовым источником — иначе
|
||||
// хардлинки легли бы на неполные файлы (qBittorrent отдаёт имена до завершения).
|
||||
// - источник исчез → приводим состояние к реальности (orphaned/deleted) и
|
||||
// ErrConflict;
|
||||
// - источник есть, но ещё качается → ErrNotReady БЕЗ reconcile: нахождение
|
||||
// задачи в review/deferred/… легитимно, приводить нечего.
|
||||
//
|
||||
// Недоступность qBittorrent — честный отказ операции.
|
||||
func (w *Worker) ensureSourceReady(ctx context.Context, d *store.Download, op string) error {
|
||||
if len(d.Infohashes) == 0 {
|
||||
return fmt.Errorf("%s: download %s has no infohash", op, d.ID)
|
||||
}
|
||||
_, ok, err := w.torrentByInfohash(ctx, d.HashList())
|
||||
t, ok, err := w.torrentByInfohash(ctx, d.HashList())
|
||||
if err != nil {
|
||||
return fmt.Errorf("%s: %w", op, err)
|
||||
}
|
||||
if ok {
|
||||
return nil
|
||||
if !ok {
|
||||
// Источник пропал — немедленно приводим состояние к реальности.
|
||||
w.reconcileToReality(ctx, *d, false)
|
||||
return fmt.Errorf("%s: источник удалён из qBittorrent: %w", op, ErrConflict)
|
||||
}
|
||||
// Источник пропал — немедленно приводим состояние к реальности.
|
||||
w.reconcileToReality(ctx, *d, false)
|
||||
return fmt.Errorf("%s: источник удалён из qBittorrent: %w", op, ErrConflict)
|
||||
if classify(t.State) != classReady {
|
||||
return fmt.Errorf("%s: торрент ещё качается: %w", op, ErrNotReady)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// reconcileToReality выводит и проставляет состояние по уже известному факту об
|
||||
|
||||
@@ -41,7 +41,9 @@ func newReconcileFixture(t *testing.T, state store.State, sourcePresent, makeTar
|
||||
|
||||
var torrents []qbt.Torrent
|
||||
if sourcePresent {
|
||||
torrents = []qbt.Torrent{{Hash: ihTest}}
|
||||
// Раздача докачана (seeding) — classReady: preflight Relink требует
|
||||
// готовности источника, а не только присутствия.
|
||||
torrents = []qbt.Torrent{{Hash: ihTest, State: "uploading"}}
|
||||
}
|
||||
w := testWorkerWith(st, &fakeQbt{torrents: torrents}, nil, nil)
|
||||
w.cfg.SourceMissingThreshold = 1 // помечаем при первой же пропаже (без задержки)
|
||||
|
||||
@@ -247,6 +247,11 @@ func (w *Worker) Apply(ctx context.Context, id string) error {
|
||||
w.reconcileToReality(ctx, *d, false)
|
||||
return fmt.Errorf("apply: источник удалён из qBittorrent: %w", ErrConflict)
|
||||
}
|
||||
if classify(t.State) != classReady {
|
||||
// Последний рубеж перед хардлинками: источник ещё качается — не
|
||||
// линкуем неполные файлы (задача могла войти в review иным путём).
|
||||
return fmt.Errorf("apply: торрент ещё качается: %w", ErrNotReady)
|
||||
}
|
||||
|
||||
w.transition(ctx, *d, store.StateLinking, "", "")
|
||||
if err := w.linkPlan(ctx, d, plan, tag, translatePath(t.SavePath, w.cfg.PathMap)); err != nil {
|
||||
@@ -321,7 +326,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize
|
||||
// (cancelled) задачу: возвращает её на распознавание, и поллинг-цикл
|
||||
// перезапустит recognize. Авто-раскладку при этом не делаем — ручная
|
||||
// перепривязка всегда проходит через ревью с подтверждением (force_review).
|
||||
// Источник (раздача в qBittorrent) для этого должен быть на месте.
|
||||
// Источник (раздача в qBittorrent) для этого должен быть на месте и докачан.
|
||||
func (w *Worker) Relink(ctx context.Context, id string) error {
|
||||
w.mu.Lock()
|
||||
defer w.mu.Unlock()
|
||||
@@ -333,9 +338,10 @@ func (w *Worker) Relink(ctx context.Context, id string) error {
|
||||
if d.State != store.StateReverted && d.State != store.StateCancelled && d.State != store.StateTargetMissing {
|
||||
return fmt.Errorf("relink: download %s is in state %s (expected reverted/cancelled/target_missing): %w", id, d.State, ErrConflict)
|
||||
}
|
||||
// Источник нужен для распознавания — проверяем синхронно (без дебаунса) и при
|
||||
// его отсутствии приводим состояние к реальности (orphaned/deleted).
|
||||
if err := w.ensureSourcePresent(ctx, d, "relink"); err != nil {
|
||||
// Источник нужен для распознавания и должен быть докачан — проверяем
|
||||
// синхронно (без дебаунса); отсутствие приводит состояние к реальности
|
||||
// (orphaned/deleted), недокачанный — ErrNotReady.
|
||||
if err := w.ensureSourceReady(ctx, d, "relink"); err != nil {
|
||||
return err
|
||||
}
|
||||
// Ручная перепривязка — всегда с подтверждением, без авто-раскладки.
|
||||
@@ -366,7 +372,7 @@ func (w *Worker) Rerecognize(ctx context.Context, id string) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := w.ensureSourcePresent(ctx, d, "rerecognize"); err != nil {
|
||||
if err := w.ensureSourceReady(ctx, d, "rerecognize"); err != nil {
|
||||
return err
|
||||
}
|
||||
ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash())
|
||||
@@ -388,7 +394,7 @@ func (w *Worker) Refine(ctx context.Context, id string, hint string) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := w.ensureSourcePresent(ctx, d, "refine"); err != nil {
|
||||
if err := w.ensureSourceReady(ctx, d, "refine"); err != nil {
|
||||
return err
|
||||
}
|
||||
ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash())
|
||||
@@ -413,7 +419,7 @@ func (w *Worker) SetType(ctx context.Context, id string, mediaType string) error
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := w.ensureSourcePresent(ctx, d, "set type"); err != nil {
|
||||
if err := w.ensureSourceReady(ctx, d, "set type"); err != nil {
|
||||
return err
|
||||
}
|
||||
ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash())
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
@@ -107,7 +108,7 @@ func revertedDownload(id string) *store.Download {
|
||||
func TestRelink_RevertedToRecognizing(t *testing.T) {
|
||||
st := newMemStore()
|
||||
st.put(revertedDownload("1"))
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d"}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{result: seriesResult()}, nil)
|
||||
|
||||
if err := w.Relink(context.Background(), "1"); err != nil {
|
||||
@@ -126,7 +127,7 @@ func TestRelink_CancelledToRecognizing(t *testing.T) {
|
||||
d := revertedDownload("1")
|
||||
d.State = store.StateCancelled
|
||||
st.put(d)
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d"}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{result: seriesResult()}, nil)
|
||||
|
||||
if err := w.Relink(context.Background(), "1"); err != nil {
|
||||
@@ -156,7 +157,7 @@ func TestRerecognize_ReviewToRecognizing(t *testing.T) {
|
||||
d := completedDownload("1")
|
||||
d.State = store.StateReview
|
||||
st.put(d)
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{}, nil)
|
||||
|
||||
if err := w.Rerecognize(context.Background(), "1"); err != nil {
|
||||
@@ -219,6 +220,87 @@ func TestRelink_ForceReviewSkipsAuto(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestReviewCommands_RejectNotReadySource — readiness-preflight: команды,
|
||||
// вводящие задачу в recognizing, отклоняют недокачанный (downloading-класс)
|
||||
// источник с ErrNotReady и НЕ трогают состояние. Иначе недокачанную задачу
|
||||
// можно было бы провести в авто-раскладку и захардлинкать неполные файлы.
|
||||
func TestReviewCommands_RejectNotReadySource(t *testing.T) {
|
||||
notReady := []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "downloading"}}
|
||||
|
||||
// review/deferred-команды на задаче в deferred/review.
|
||||
cases := []struct {
|
||||
name string
|
||||
state store.State
|
||||
call func(w *Worker) error
|
||||
}{
|
||||
{"rerecognize", store.StateDeferred, func(w *Worker) error { return w.Rerecognize(context.Background(), "1") }},
|
||||
{"refine", store.StateReview, func(w *Worker) error { return w.Refine(context.Background(), "1", "подсказка") }},
|
||||
{"set type", store.StateReview, func(w *Worker) error { return w.SetType(context.Background(), "1", "series") }},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
st := newMemStore()
|
||||
d := completedDownload("1")
|
||||
d.State = tc.state
|
||||
st.put(d)
|
||||
w := testWorkerWith(st, &fakeQbt{torrents: notReady}, &fakeRecognizer{}, nil)
|
||||
|
||||
if err := tc.call(w); !errors.Is(err, ErrNotReady) {
|
||||
t.Fatalf("err = %v, want ErrNotReady", err)
|
||||
}
|
||||
if got := st.downloads["1"].State; got != tc.state {
|
||||
t.Errorf("state = %q, want %q (не тронуто)", got, tc.state)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Relink из reverted — тоже требует готовности источника.
|
||||
t.Run("relink", func(t *testing.T) {
|
||||
st := newMemStore()
|
||||
st.put(revertedDownload("1"))
|
||||
w := testWorkerWith(st, &fakeQbt{torrents: notReady}, &fakeRecognizer{}, nil)
|
||||
|
||||
if err := w.Relink(context.Background(), "1"); !errors.Is(err, ErrNotReady) {
|
||||
t.Fatalf("err = %v, want ErrNotReady", err)
|
||||
}
|
||||
if got := st.downloads["1"].State; got != store.StateReverted {
|
||||
t.Errorf("state = %q, want reverted (не тронуто)", got)
|
||||
}
|
||||
if _, ok := st.overrides["1"][ovrForceReview]; ok {
|
||||
t.Errorf("force_review проставлен, а не должен: отказ до записи override")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestApply_RejectsNotReadySource — последний рубеж: Apply не создаёт хардлинки
|
||||
// на недокачанный источник, даже если задача оказалась в review.
|
||||
func TestApply_RejectsNotReadySource(t *testing.T) {
|
||||
st := newMemStore()
|
||||
d := completedDownload("1")
|
||||
d.State = store.StateReview
|
||||
st.put(d)
|
||||
planJSON, _ := json.Marshal(seriesResult().Plan)
|
||||
st.recs = append(st.recs, &store.Recognition{
|
||||
ID: "1", DownloadID: "1", IsCurrent: true, Plan: store.NullString(string(planJSON)),
|
||||
})
|
||||
lay, err := layout.New(layout.Config{MoviesDir: t.TempDir(), SeriesDir: t.TempDir()}, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: "/d", State: "downloading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{}, lay)
|
||||
|
||||
if err := w.Apply(context.Background(), "1"); !errors.Is(err, ErrNotReady) {
|
||||
t.Fatalf("err = %v, want ErrNotReady", err)
|
||||
}
|
||||
if got := st.downloads["1"].State; got != store.StateReview {
|
||||
t.Errorf("state = %q, want review (не тронуто)", got)
|
||||
}
|
||||
if len(st.links) != 0 {
|
||||
t.Errorf("file_links = %d, want 0 (хардлинки не создаём)", len(st.links))
|
||||
}
|
||||
}
|
||||
|
||||
// memStore — полноценный in-memory store для тестов Ф3.
|
||||
type memStore struct {
|
||||
downloads map[string]*store.Download
|
||||
@@ -691,7 +773,7 @@ func TestRefine_AddsHintAndRerecognizes(t *testing.T) {
|
||||
d := completedDownload("1")
|
||||
d.State = store.StateReview
|
||||
st.put(d)
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{}, nil)
|
||||
|
||||
if err := w.Refine(context.Background(), "1", "это второй сезон"); err != nil {
|
||||
@@ -713,7 +795,7 @@ func TestSetType(t *testing.T) {
|
||||
d := completedDownload("1")
|
||||
d.State = store.StateReview
|
||||
st.put(d)
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{}, nil)
|
||||
|
||||
if err := w.SetType(context.Background(), "1", "series"); err != nil {
|
||||
@@ -807,7 +889,7 @@ func newApplyFixture(t *testing.T, plan recognize.Plan) applyFixture {
|
||||
st.recs = append(st.recs, &store.Recognition{
|
||||
ID: "1", DownloadID: "1", IsCurrent: true, Plan: store.NullString(string(planJSON)),
|
||||
})
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: downloads, Category: "jellybit"}}}
|
||||
qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: downloads, Category: "jellybit", State: "uploading"}}}
|
||||
w := testWorkerWith(st, qb, &fakeRecognizer{}, lay)
|
||||
|
||||
return applyFixture{w: w, st: st, downloads: downloads, movies: movies, series: series}
|
||||
|
||||
Reference in New Issue
Block a user