From 282600bb2031f5361afd70c5f37cb0e9407f04ab Mon Sep 17 00:00:00 2001 From: vavallee Date: Fri, 16 Oct 2026 00:51:18 -0300 Subject: [PATCH 1/2] test: cover the pin, ABS match state, staggered refresh and remap paths merged since v1.42.1 Test-only. Closes the largest patch coverage gaps on #3283, #3296, #3275, #3276, #3281 and #3285 with assertions on outcomes: pin resolution and its refusal messages, delete refusal, scheduler pin skipping and the failure record dedupe, pending row gating, ABS series match state expiry and bootstrap adoption, staggered refresh boundaries and write guards, path visibility warnings, and the content check and title relevance helpers. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- .../import_series_match_state_unit_test.go | 205 ++++++++++++++ internal/api/pin_edges_test.go | 264 ++++++++++++++++++ internal/db/abs_series_provenance_test.go | 109 ++++++++ .../db/author_metadata_refresh_edges_test.go | 170 +++++++++++ internal/db/download_client_pin_test.go | 249 +++++++++++++++++ internal/downloader/pathcheck_edges_test.go | 126 +++++++++ internal/importer/content_check_edges_test.go | 220 +++++++++++++++ .../indexer/title_relevance_edges_test.go | 121 ++++++++ internal/models/indexer_test.go | 43 +++ .../scheduler/metadata_refresh_edges_test.go | 255 +++++++++++++++++ .../scheduler_indexer_pin_unit_test.go | 252 +++++++++++++++++ 11 files changed, 2014 insertions(+) create mode 100644 internal/abs/import_series_match_state_unit_test.go create mode 100644 internal/api/pin_edges_test.go create mode 100644 internal/db/abs_series_provenance_test.go create mode 100644 internal/db/author_metadata_refresh_edges_test.go create mode 100644 internal/db/download_client_pin_test.go create mode 100644 internal/downloader/pathcheck_edges_test.go create mode 100644 internal/importer/content_check_edges_test.go create mode 100644 internal/indexer/title_relevance_edges_test.go create mode 100644 internal/models/indexer_test.go create mode 100644 internal/scheduler/metadata_refresh_edges_test.go create mode 100644 internal/scheduler/scheduler_indexer_pin_unit_test.go diff --git a/internal/abs/import_series_match_state_unit_test.go b/internal/abs/import_series_match_state_unit_test.go new file mode 100644 index 000000000..e69bbbb42 --- /dev/null +++ b/internal/abs/import_series_match_state_unit_test.go @@ -0,0 +1,205 @@ +package abs + +import ( + "context" + "testing" + "time" + + "github.com/vavallee/bindery/internal/db" + "github.com/vavallee/bindery/internal/models" +) + +const unitSource, unitLib, unitItem = DefaultSourceID, "lib-unit", "item-unit" + +func unitCfg() ImportConfig { + return ImportConfig{SourceID: unitSource, LibraryID: unitLib} +} + +func closedDB(t *testing.T) *db.ABSSeriesMatchRepo { + t.Helper() + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + database.Close() + return db.NewABSSeriesMatchRepo(database) +} + +func seedMatchState(t *testing.T, imp *Importer, hash, outcome string, checkedAt time.Time) { + t.Helper() + err := imp.seriesMatchState.Upsert(context.Background(), &models.ABSSeriesMatchState{ + SourceID: unitSource, LibraryID: unitLib, ItemID: unitItem, InputHash: hash, Outcome: outcome, + }, checkedAt) + if err != nil { + t.Fatal(err) + } +} + +func TestSeriesMatchAlreadyDone_StateReadFailureRematches(t *testing.T) { + t.Parallel() + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + imp.seriesMatchState = closedDB(t) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), &models.Book{}, unitItem, "h") { + t.Fatal("an unreadable state table must never skip the match") + } +} + +func TestSeriesMatchAlreadyDone_UnknownOutcomeIsNotTrusted(t *testing.T) { + t.Parallel() + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + seedMatchState(t, imp, "h", "something-from-the-future", time.Now()) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), &models.Book{}, unitItem, "h") { + t.Fatal("an outcome this build does not know must re-match") + } +} + +func TestSeriesMatchAlreadyDone_NoStateRepoOrFullRun(t *testing.T) { + t.Parallel() + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + seedMatchState(t, imp, "h", seriesMatchOutcomeMatched, time.Now()) + ctx := context.Background() + if !imp.seriesMatchAlreadyDone(ctx, unitCfg(), &models.Book{}, unitItem, "h") { + t.Fatal("setup: a matching recorded state must skip") + } + full := unitCfg() + full.Full = true + if imp.seriesMatchAlreadyDone(ctx, full, &models.Book{}, unitItem, "h") { + t.Fatal("a Full run must ignore recorded state") + } + imp.seriesMatchState = nil + if imp.seriesMatchAlreadyDone(ctx, unitCfg(), &models.Book{}, unitItem, "h") { + t.Fatal("without a state repo nothing can be skipped") + } +} + +func TestSeriesMatchAlreadyDone_MissExpiryBoundary(t *testing.T) { + t.Parallel() + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + checked := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC) + hash := "ffff" + seedMatchState(t, imp, hash, seriesMatchOutcomeMiss, checked) + expiry := seriesMatchMissExpiry(checked, hash) + + cases := []struct { + name string + now time.Time + want bool + }{ + {"one nanosecond before expiry still skips", expiry.Add(-time.Nanosecond), true}, + {"exactly at expiry is due again", expiry, false}, + {"after expiry is due again", expiry.Add(time.Second), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + imp.seriesMatchClock = func() time.Time { return tc.now } + if got := imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), &models.Book{}, unitItem, hash); got != tc.want { + t.Fatalf("seriesMatchAlreadyDone = %v, want %v", got, tc.want) + } + }) + } +} + +func TestSeriesMatchAlreadyDone_ChangedHashNeverSkips(t *testing.T) { + t.Parallel() + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + seedMatchState(t, imp, "old", seriesMatchOutcomeMatched, time.Now()) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), &models.Book{}, unitItem, "new") { + t.Fatal("a matched outcome recorded for different inputs must not skip") + } +} + +// linkedSeriesFor makes a series, optionally with a Hardcover link, and records +// ABS series provenance from unitItem to it. +func linkedSeriesFor(t *testing.T, seriesRepo *db.SeriesRepo, prov *db.ABSProvenanceRepo, name string, withLink bool) { + t.Helper() + ctx := context.Background() + s, err := seriesRepo.CreateManual(ctx, name) + if err != nil { + t.Fatal(err) + } + if withLink { + if err := seriesRepo.UpsertHardcoverLink(ctx, &models.SeriesHardcoverLink{SeriesID: s.ID, HardcoverSeriesID: "hc-series:" + name}); err != nil { + t.Fatal(err) + } + } + if err := prov.Upsert(ctx, &models.ABSProvenance{ + SourceID: unitSource, LibraryID: unitLib, EntityType: "series", ExternalID: name, LocalID: s.ID, ItemID: unitItem, + }); err != nil { + t.Fatal(err) + } +} + +func TestSeriesMatchBootstrap_Adoption(t *testing.T) { + t.Parallel() + hc := &models.Book{MetadataProvider: providerHardcover} + ol := &models.Book{MetadataProvider: "openlibrary"} + + t.Run("hardcover book with a linked series is adopted and recorded", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "saga", true) + if !imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), hc, unitItem, "hash1") { + t.Fatal("want adoption") + } + st, _ := imp.seriesMatchState.Get(context.Background(), unitSource, unitLib, unitItem) + if st == nil || st.Outcome != seriesMatchOutcomeMatched || st.InputHash != "hash1" { + t.Fatalf("recorded = %+v, want matched with the current hash", st) + } + }) + + t.Run("a link on the second series row is still found after an unlinked first row", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "unlinked", false) + linkedSeriesFor(t, seriesRepo, prov, "linked", true) + if !imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), hc, unitItem, "h") { + t.Fatal("want adoption through the second row") + } + }) + + t.Run("series with no hardcover link is not adopted and nothing is recorded", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "unlinked", false) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), hc, unitItem, "h") { + t.Fatal("must not adopt without a link") + } + if st, _ := imp.seriesMatchState.Get(context.Background(), unitSource, unitLib, unitItem); st != nil { + t.Fatalf("recorded %+v for a refused adoption", st) + } + }) + + t.Run("non hardcover book is not adopted", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "saga", true) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), ol, unitItem, "h") { + t.Fatal("book without a Hardcover identity must not be adopted") + } + }) + + t.Run("nil book is not adopted", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "saga", true) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), nil, unitItem, "h") { + t.Fatal("nil book must not be adopted") + } + }) + + t.Run("another item's series membership is not evidence", func(t *testing.T) { + imp, _, _, seriesRepo, _, prov, _, _, _, _ := newABSImporterFixture(t) + linkedSeriesFor(t, seriesRepo, prov, "saga", true) + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), hc, "some-other-item", "h") { + t.Fatal("provenance of a different item must not be adopted") + } + }) + + t.Run("provenance read failure falls through to a normal match", func(t *testing.T) { + imp, _, _, _, _, _, _, _, _, _ := newABSImporterFixture(t) + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + imp.provenance = db.NewABSProvenanceRepo(database) + database.Close() + if imp.seriesMatchAlreadyDone(context.Background(), unitCfg(), hc, unitItem, "h") { + t.Fatal("a failed provenance read must not adopt") + } + }) +} diff --git a/internal/api/pin_edges_test.go b/internal/api/pin_edges_test.go new file mode 100644 index 000000000..6dc835a3d --- /dev/null +++ b/internal/api/pin_edges_test.go @@ -0,0 +1,264 @@ +package api + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/vavallee/bindery/internal/auth" + "github.com/vavallee/bindery/internal/db" + "github.com/vavallee/bindery/internal/models" +) + +func adminCtx() context.Context { return auth.WithUserRole(context.Background(), "admin") } +func userCtx() context.Context { return auth.WithUserRole(context.Background(), "user") } + +func TestSendFailedText_PinSuffixNamesClientOnlyForAdmins(t *testing.T) { + err := &sendError{cause: errors.New("connection refused"), pin: &pinTarget{indexer: "TrackerX", client: "sab-private"}} + + admin := sendFailedText(adminCtx(), err) + if !strings.Contains(admin, `Indexer "TrackerX" is pinned to download client "sab-private", so no other client was tried`) { + t.Fatalf("admin text = %q", admin) + } + user := sendFailedText(userCtx(), err) + if !strings.Contains(user, `Indexer "TrackerX" is pinned to one download client, so no other client was tried`) { + t.Fatalf("user text = %q", user) + } + if strings.Contains(user, "sab-private") { + t.Fatalf("non admin text leaks the client name: %q", user) + } + + plain := sendFailedText(adminCtx(), &sendError{cause: errors.New("connection refused")}) + if strings.Contains(plain, "pinned") { + t.Fatalf("an unpinned send failure must not mention a pin: %q", plain) + } +} + +func TestQueueItemMessage_PinFailuresRespectRole(t *testing.T) { + perr := &db.PinnedClientError{IndexerName: "TrackerX", ClientID: 4, ClientName: "sab-private", Reason: db.PinClientDisabled} + if got := queueItemMessage(adminCtx(), perr); !strings.Contains(got, "sab-private") { + t.Errorf("admin message = %q, want the client named", got) + } + if got := queueItemMessage(userCtx(), perr); strings.Contains(got, "sab-private") || !strings.Contains(got, "TrackerX") { + t.Errorf("user message = %q, want the indexer and not the client", got) + } + + aerr := &ambiguousPinError{reason: pinRefuseAmbiguous, pinned: []string{"TrackerX"}} + if got := queueItemMessage(adminCtx(), aerr); !strings.Contains(got, "(TrackerX)") { + t.Errorf("admin ambiguous message = %q, want the pinned indexer named", got) + } + if got := queueItemMessage(userCtx(), aerr); strings.Contains(got, "TrackerX") { + t.Errorf("user ambiguous message = %q, must not name pinned indexers", got) + } +} + +func TestClientForGrab_Routing(t *testing.T) { + f := newPinFixture(t) + ctx := f.ctx + id := func(n int64) *int64 { return &n } + + t.Run("no indexer id takes the protocol default", func(t *testing.T) { + c, pin, err := f.h.clientForGrab(ctx, nil, "usenet", "") + if err != nil || pin != nil || c == nil || c.ID != f.pubC.ID { + t.Fatalf("got %v, %v, %v; want the priority 1 client and no pin", c, pin, err) + } + }) + t.Run("indexer id zero is not an indexer", func(t *testing.T) { + f.pin(t, &f.privC.ID) + c, pin, err := f.h.clientForGrab(ctx, id(0), "usenet", "") + if err != nil || pin != nil || c.ID != f.pubC.ID { + t.Fatalf("got %v, %v, %v", c, pin, err) + } + }) + t.Run("an id that matches no indexer falls back to the default", func(t *testing.T) { + c, pin, err := f.h.clientForGrab(ctx, id(424242), "usenet", "") + if err != nil || pin != nil || c.ID != f.pubC.ID { + t.Fatalf("got %v, %v, %v", c, pin, err) + } + }) + t.Run("a pinned indexer gets its client and a pin target", func(t *testing.T) { + c, pin, err := f.h.clientForGrab(ctx, &f.idx.ID, "usenet", "") + if err != nil || c == nil || c.ID != f.privC.ID { + t.Fatalf("got %v, %v", c, err) + } + if pin == nil || pin.indexer != "TrackerX" || pin.client != "sab-private" { + t.Fatalf("pin = %+v", pin) + } + }) + t.Run("a pinned indexer never silently uses the default when its client is unusable", func(t *testing.T) { + f.privC.Enabled = false + if err := f.clients.Update(ctx, f.privC); err != nil { + t.Fatal(err) + } + c, pin, err := f.h.clientForGrab(ctx, &f.idx.ID, "usenet", "") + var perr *db.PinnedClientError + if c != nil || pin != nil || !errors.As(err, &perr) || perr.Reason != db.PinClientDisabled { + t.Fatalf("got %v, %v, %v", c, pin, err) + } + }) + t.Run("an unpinned existing indexer takes the default", func(t *testing.T) { + f.pin(t, nil) + c, pin, err := f.h.clientForGrab(ctx, &f.idx.ID, "usenet", "") + if err != nil || pin != nil || c.ID != f.pubC.ID { + t.Fatalf("got %v, %v, %v", c, pin, err) + } + }) + t.Run("no indexer repo ignores the id", func(t *testing.T) { + f.pin(t, &f.privC.ID) + h := *f.h + h.indexers = nil + c, pin, err := h.clientForGrab(ctx, &f.idx.ID, "usenet", "") + if err != nil || pin != nil || c.ID != f.pubC.ID { + t.Fatalf("got %v, %v, %v", c, pin, err) + } + }) + t.Run("an indexer lookup failure is returned, not routed around", func(t *testing.T) { + broken, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + h := *f.h + h.indexers = db.NewIndexerRepo(broken) + broken.Close() + c, pin, err := h.clientForGrab(ctx, &f.idx.ID, "usenet", "") + if err == nil || c != nil || pin != nil { + t.Fatalf("got %v, %v, %v; want only an error", c, pin, err) + } + var perr *db.PinnedClientError + if errors.As(err, &perr) { + t.Fatalf("a storage fault must not read as a pin failure: %v", err) + } + }) +} + +func TestNoProtocolClientError_NamesTheMismatch(t *testing.T) { + h, _, _, clients, _, ctx := queueFixture(t) + torrent := &models.DownloadClient{Name: "qbit", Type: "qbittorrent", Host: "h", Port: 1, Enabled: true} + if err := clients.Create(ctx, torrent); err != nil { + t.Fatal(err) + } + + err := h.noProtocolClientError(ctx, "usenet") + var nce *noClientError + if !errors.As(err, &nce) { + t.Fatalf("err = %T", err) + } + for _, want := range []string{"usenet (nzb)", "this release is usenet", "only enabled client(s) are torrent", "SABnzbd/NZBGet"} { + if !strings.Contains(nce.msg, want) { + t.Errorf("usenet release message %q missing %q", nce.msg, want) + } + } + + h2, _, _, clients2, _, ctx2 := queueFixture(t) + sab := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: "h", Port: 1, Enabled: true} + if err := clients2.Create(ctx2, sab); err != nil { + t.Fatal(err) + } + err = h2.noProtocolClientError(ctx2, "torrent") + if !errors.As(err, &nce) { + t.Fatalf("err = %T", err) + } + for _, want := range []string{"no enabled torrent download client", "this release is torrent", "are usenet", "qBittorrent/Transmission/Deluge/rTorrent"} { + if !strings.Contains(nce.msg, want) { + t.Errorf("torrent release message %q missing %q", nce.msg, want) + } + } + + h3, _, _, _, _, ctx3 := queueFixture(t) + err = h3.noProtocolClientError(ctx3, "usenet") + if !errors.As(err, &nce) || !strings.Contains(nce.msg, "add a download client") || strings.Contains(nce.msg, "this release is") { + t.Fatalf("with no clients at all got %v", err) + } + if !errors.Is(err, errNoDownloadClient) { + t.Fatal("must unwrap to errNoDownloadClient so the API answers 400 with the setup code") + } +} + +func TestValidateDownloadClientPin_Matrix(t *testing.T) { + f := newPinAPIFixture(t) + ctx := context.Background() + idp := func(n int64) *int64 { return &n } + usenetPin, torrentPin, offPin := idp(f.usenet.ID), idp(f.torrent.ID), idp(f.off.ID) + + cases := []struct { + name string + idx models.Indexer + stored *models.Indexer + wantErr string + }{ + {"no pin is always valid", models.Indexer{Type: "newznab"}, nil, ""}, + {"new valid pin", models.Indexer{Type: "torznab", DownloadClientID: torrentPin}, nil, ""}, + {"new pin with wrong protocol", models.Indexer{Type: "newznab", DownloadClientID: torrentPin}, nil, "is a torrent client but this indexer is usenet"}, + {"new pin to disabled client", models.Indexer{Type: "torznab", DownloadClientID: offPin}, nil, "is disabled"}, + {"unchanged pin to a disabled client keeps saving", + models.Indexer{Type: "torznab", DownloadClientID: offPin}, + &models.Indexer{Type: "torznab", DownloadClientID: offPin}, ""}, + {"pin moved to a disabled client is refused", + models.Indexer{Type: "torznab", DownloadClientID: offPin}, + &models.Indexer{Type: "torznab", DownloadClientID: torrentPin}, "is disabled"}, + {"first pin on a stored unpinned indexer is checked", + models.Indexer{Type: "torznab", DownloadClientID: offPin}, + &models.Indexer{Type: "torznab"}, "is disabled"}, + {"type change under an unchanged pin is checked", + models.Indexer{Type: "newznab", DownloadClientID: torrentPin}, + &models.Indexer{Type: "torznab", DownloadClientID: torrentPin}, "is a torrent client but this indexer is usenet"}, + {"empty type reads as newznab, not a type change", + models.Indexer{Type: "", DownloadClientID: torrentPin}, + &models.Indexer{Type: "newznab", DownloadClientID: torrentPin}, ""}, + {"empty stored type against newznab is not a type change", + models.Indexer{Type: "newznab", DownloadClientID: usenetPin}, + &models.Indexer{Type: "", DownloadClientID: usenetPin}, ""}, + {"missing client", models.Indexer{Type: "newznab", DownloadClientID: idp(99999)}, nil, "does not exist"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := f.ih.validateDownloadClientPin(ctx, tc.idx, tc.stored) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } + var bad pinInvalidError + if !errors.As(err, &bad) || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("err = %v, want a pinInvalidError containing %q", err, tc.wantErr) + } + }) + } + + t.Run("no client repo refuses rather than storing an unchecked pin", func(t *testing.T) { + h := &IndexerHandler{} + err := h.validateDownloadClientPin(ctx, models.Indexer{Type: "torznab", DownloadClientID: torrentPin}, nil) + var bad pinInvalidError + if !errors.As(err, &bad) { + t.Fatalf("err = %v, want a pinInvalidError", err) + } + }) +} + +func TestIndexerPin_StorageFaultIsA500WithoutDetail(t *testing.T) { + f := newPinAPIFixture(t) + broken, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + f.ih.clients = db.NewDownloadClientRepo(broken) + broken.Close() + + rec := f.create(`{"name":"X","url":"http://192.168.1.60:9117/api","type":"newznab","downloadClientId":1}`) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500 for a storage fault: %s", rec.Code, rec.Body.String()) + } + if body := rec.Body.String(); strings.Contains(strings.ToLower(body), "sql") || strings.Contains(body, "closed") { + t.Fatalf("body leaks the storage error: %s", body) + } + + rec2 := httptest.NewRecorder() + writePinError(rec2, httptest.NewRequest(http.MethodPost, "/x", nil), pinInvalidError("bad pin")) + if rec2.Code != http.StatusBadRequest || !strings.Contains(rec2.Body.String(), "bad pin") { + t.Fatalf("pinInvalidError = %d %s, want a 400 carrying the text", rec2.Code, rec2.Body.String()) + } +} diff --git a/internal/db/abs_series_provenance_test.go b/internal/db/abs_series_provenance_test.go new file mode 100644 index 000000000..158e4c4e0 --- /dev/null +++ b/internal/db/abs_series_provenance_test.go @@ -0,0 +1,109 @@ +package db + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/vavallee/bindery/internal/models" +) + +func TestListSeriesByItem_ScopesToSourceLibraryItemAndSeriesOnly(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewABSProvenanceRepo(database) + + rows := []models.ABSProvenance{ + {SourceID: "a", LibraryID: "lib", EntityType: "series", ExternalID: "s-second", LocalID: 12, ItemID: "item"}, + {SourceID: "a", LibraryID: "lib", EntityType: "series", ExternalID: "s-first", LocalID: 11, ItemID: "item"}, + {SourceID: "a", LibraryID: "lib", EntityType: "book", ExternalID: "b", LocalID: 5, ItemID: "item"}, + {SourceID: "b", LibraryID: "lib", EntityType: "series", ExternalID: "s-other-source", LocalID: 13, ItemID: "item"}, + {SourceID: "a", LibraryID: "other-lib", EntityType: "series", ExternalID: "s-other-lib", LocalID: 14, ItemID: "item"}, + {SourceID: "a", LibraryID: "lib", EntityType: "series", ExternalID: "s-other-item", LocalID: 15, ItemID: "other-item"}, + } + for i := range rows { + if err := repo.Upsert(ctx, &rows[i]); err != nil { + t.Fatal(err) + } + } + + got, err := repo.ListSeriesByItem(ctx, "a", "lib", "item") + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got[0].LocalID != 12 || got[1].LocalID != 11 { + t.Fatalf("got %+v, want the two series rows of that item in insertion order", got) + } + for _, p := range got { + if p.EntityType != "series" || p.SourceID != "a" || p.LibraryID != "lib" || p.ItemID != "item" { + t.Fatalf("leaked row %+v", p) + } + } + + none, err := repo.ListSeriesByItem(ctx, "a", "lib", "missing") + if err != nil || len(none) != 0 { + t.Fatalf("missing item = %v, %v; want empty", none, err) + } +} + +func TestListSeriesByItem_ClosedDatabaseNamesTheItem(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + repo := NewABSProvenanceRepo(database) + database.Close() + _, err = repo.ListSeriesByItem(context.Background(), "a", "lib", "item-xyz") + if err == nil { + t.Fatal("expected an error") + } + if got := err.Error(); !strings.Contains(got, "item-xyz") { + t.Fatalf("error %q should name the item", got) + } +} + +func TestABSSeriesMatchRepo_ScopedByAllThreeKeys(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewABSSeriesMatchRepo(database) + + zone := time.FixedZone("x", 5*3600) + at := time.Date(2026, 10, 1, 17, 0, 0, 0, zone) + if err := repo.Upsert(ctx, &models.ABSSeriesMatchState{SourceID: "a", LibraryID: "l", ItemID: "i", InputHash: "h", Outcome: "miss"}, at); err != nil { + t.Fatal(err) + } + for _, key := range [][3]string{{"b", "l", "i"}, {"a", "m", "i"}, {"a", "l", "j"}} { + if got, err := repo.Get(ctx, key[0], key[1], key[2]); err != nil || got != nil { + t.Fatalf("Get%v = %+v, %v; want nil, nil", key, got, err) + } + } + got, err := repo.Get(ctx, "a", "l", "i") + if err != nil || got == nil || !got.CheckedAt.Equal(at) { + t.Fatalf("Get = %+v, %v", got, err) + } +} + +func TestABSSeriesMatchRepo_ClosedDatabaseErrors(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + repo := NewABSSeriesMatchRepo(database) + database.Close() + ctx := context.Background() + if got, err := repo.Get(ctx, "a", "l", "i"); err == nil || got != nil { + t.Fatalf("Get = %v, %v; want an error and no state", got, err) + } + if err := repo.Upsert(ctx, &models.ABSSeriesMatchState{SourceID: "a", LibraryID: "l", ItemID: "i"}, time.Now()); err == nil { + t.Fatal("Upsert on a closed database returned no error") + } +} diff --git a/internal/db/author_metadata_refresh_edges_test.go b/internal/db/author_metadata_refresh_edges_test.go new file mode 100644 index 000000000..258704818 --- /dev/null +++ b/internal/db/author_metadata_refresh_edges_test.go @@ -0,0 +1,170 @@ +package db + +import ( + "context" + "testing" + "time" +) + +func TestListMetadataRefreshDue_NonPositiveLimitReturnsNothing(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + repo := NewAuthorRepo(database) + newDiscoveryTestAuthor(t, repo, "OL1A", nil) + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + for _, limit := range []int{0, -3} { + got, err := repo.ListMetadataRefreshDue(context.Background(), now, now.Add(-time.Hour), limit) + if err != nil || len(got) != 0 { + t.Fatalf("limit %d = %v, %v; want nothing", limit, got, err) + } + } +} + +// The cutoff is strict: an author refreshed exactly at the cutoff is not yet +// due, one a nanosecond older is. A stamp equal to now is a real recent +// refresh, not a future one, so it must not read as never refreshed. +func TestListMetadataRefreshDue_Boundaries(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewAuthorRepo(database) + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + cutoff := now.Add(-24 * time.Hour) + + atCutoff := newDiscoveryTestAuthor(t, repo, "OL1A", nil) + justOlder := newDiscoveryTestAuthor(t, repo, "OL2A", nil) + atNow := newDiscoveryTestAuthor(t, repo, "OL3A", nil) + for id, when := range map[int64]time.Time{ + atCutoff.ID: cutoff, + justOlder.ID: cutoff.Add(-time.Nanosecond), + atNow.ID: now, + } { + if err := repo.StampMetadataRefresh(ctx, id, when); err != nil { + t.Fatal(err) + } + } + got, err := repo.ListMetadataRefreshDue(ctx, now, cutoff, 10) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].ID != justOlder.ID { + var ids []int64 + for _, a := range got { + ids = append(ids, a.ID) + } + t.Fatalf("due = %v, want only %d (the author one nanosecond past the cutoff)", ids, justOlder.ID) + } +} + +// An unreadable stamp must not exempt an author from refresh forever, and +// stamps written in the older text layouts compare by time, not by string. +func TestListMetadataRefreshDue_UnparseableAndLegacyLayouts(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewAuthorRepo(database) + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + cutoff := now.Add(-24 * time.Hour) + + garbage := newDiscoveryTestAuthor(t, repo, "OL1A", nil) + legacyOld := newDiscoveryTestAuthor(t, repo, "OL2A", nil) + legacyFresh := newDiscoveryTestAuthor(t, repo, "OL3A", nil) + canonicalOlder := newDiscoveryTestAuthor(t, repo, "OL4A", nil) + + set := func(id int64, raw string) { + t.Helper() + if _, err := database.ExecContext(ctx, `UPDATE authors SET last_metadata_refresh_at=? WHERE id=?`, raw, id); err != nil { + t.Fatal(err) + } + } + set(garbage.ID, "not a time") + set(legacyOld.ID, "2026-10-10 08:00:00 +0000 UTC") + set(legacyFresh.ID, "2026-10-14 06:00:00 +0000 UTC") + if err := repo.StampMetadataRefresh(ctx, canonicalOlder.ID, now.Add(-200*time.Hour)); err != nil { + t.Fatal(err) + } + + got, err := repo.ListMetadataRefreshDue(ctx, now, cutoff, 10) + if err != nil { + t.Fatal(err) + } + var ids []int64 + for _, a := range got { + ids = append(ids, a.ID) + } + want := []int64{garbage.ID, canonicalOlder.ID, legacyOld.ID} + if len(ids) != len(want) { + t.Fatalf("due = %v, want %v (garbage first as never refreshed, fresh legacy stamp excluded)", ids, want) + } + for i := range want { + if ids[i] != want[i] { + t.Fatalf("due = %v, want %v", ids, want) + } + } +} + +func TestListMetadataRefreshDue_EqualStampsBreakTiesByID(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewAuthorRepo(database) + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + same := now.Add(-48 * time.Hour) + + a := newDiscoveryTestAuthor(t, repo, "OL1A", nil) + b := newDiscoveryTestAuthor(t, repo, "OL2A", nil) + c := newDiscoveryTestAuthor(t, repo, "OL3A", nil) + for _, id := range []int64{c.ID, a.ID, b.ID} { + if err := repo.StampMetadataRefresh(ctx, id, same); err != nil { + t.Fatal(err) + } + } + got, err := repo.ListMetadataRefreshDue(ctx, now, now.Add(-24*time.Hour), 2) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got[0].ID != a.ID || got[1].ID != b.ID { + t.Fatalf("got %v, want the two lowest ids in order", got) + } +} + +// The stamp is not an edit: updated_at must not move. +func TestStampMetadataRefresh_LeavesUpdatedAt(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + repo := NewAuthorRepo(database) + a := newDiscoveryTestAuthor(t, repo, "OL1A", nil) + before, err := repo.GetByID(ctx, a.ID) + if err != nil { + t.Fatal(err) + } + if err := repo.StampMetadataRefresh(ctx, a.ID, time.Date(2030, 1, 1, 0, 0, 0, 0, time.UTC)); err != nil { + t.Fatal(err) + } + after, err := repo.GetByID(ctx, a.ID) + if err != nil { + t.Fatal(err) + } + if !after.UpdatedAt.Equal(before.UpdatedAt) { + t.Fatalf("updated_at moved from %v to %v", before.UpdatedAt, after.UpdatedAt) + } + if err := repo.StampMetadataRefresh(ctx, 99999, time.Now()); err != nil { + t.Fatalf("stamping an unknown author is a no-op, got %v", err) + } +} diff --git a/internal/db/download_client_pin_test.go b/internal/db/download_client_pin_test.go new file mode 100644 index 000000000..5b728cf7e --- /dev/null +++ b/internal/db/download_client_pin_test.go @@ -0,0 +1,249 @@ +package db + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/vavallee/bindery/internal/models" +) + +func TestResolvePinnedClient_NotPinned(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + clients := NewDownloadClientRepo(database) + ctx := context.Background() + + if c, err := clients.ResolvePinnedClient(ctx, nil, "usenet"); c != nil || err != nil { + t.Fatalf("nil indexer = %v, %v; want nil, nil", c, err) + } + if c, err := clients.ResolvePinnedClient(ctx, &models.Indexer{Name: "free", Type: "newznab"}, "usenet"); c != nil || err != nil { + t.Fatalf("unpinned indexer = %v, %v; want nil, nil so the caller falls back to the default client", c, err) + } +} + +func TestResolvePinnedClient_MissingClientNeverSubstitutes(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + clients := NewDownloadClientRepo(database) + ctx := context.Background() + other := &models.DownloadClient{Name: "other", Type: "sabnzbd", Host: "h", Port: 1, Enabled: true} + if err := clients.Create(ctx, other); err != nil { + t.Fatal(err) + } + + gone := other.ID + 100 + c, err := clients.ResolvePinnedClient(ctx, &models.Indexer{Name: "ix", Type: "newznab", DownloadClientID: &gone}, "usenet") + var perr *PinnedClientError + if !errors.As(err, &perr) { + t.Fatalf("err = %v, want a *PinnedClientError", err) + } + if c != nil { + t.Fatalf("a missing pinned client must not be replaced by another one, got %v", c) + } + if perr.Reason != PinClientMissing || perr.ClientID != gone || perr.ClientName != "" || perr.IndexerName != "ix" { + t.Fatalf("perr = %+v", perr) + } +} + +func TestResolvePinnedClient_DisabledClient(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + clients := NewDownloadClientRepo(database) + ctx := context.Background() + off := &models.DownloadClient{Name: "off", Type: "qbittorrent", Host: "h", Port: 1, Enabled: false} + if err := clients.Create(ctx, off); err != nil { + t.Fatal(err) + } + c, err := clients.ResolvePinnedClient(ctx, &models.Indexer{Name: "tor", Type: "torznab", DownloadClientID: &off.ID}, "torrent") + var perr *PinnedClientError + if !errors.As(err, &perr) || c != nil { + t.Fatalf("got %v, %v; want a PinnedClientError", c, err) + } + if perr.Reason != PinClientDisabled || perr.ClientName != "off" { + t.Fatalf("perr = %+v", perr) + } +} + +func TestResolvePinnedClient_DatabaseErrorIsNotAPinError(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + clients := NewDownloadClientRepo(database) + id := int64(1) + database.Close() + + c, err := clients.ResolvePinnedClient(context.Background(), &models.Indexer{Name: "ix", Type: "newznab", DownloadClientID: &id}, "usenet") + if err == nil || c != nil { + t.Fatalf("got %v, %v; want a database error", c, err) + } + var perr *PinnedClientError + if errors.As(err, &perr) { + t.Fatalf("a database failure must not read as a missing client: %v", err) + } +} + +func TestPinnedClientError_MessageRedactsClientForNonAdmins(t *testing.T) { + cases := []struct { + name string + e PinnedClientError + includeClient bool + want []string + notWant []string + }{ + { + name: "missing, admin", + e: PinnedClientError{IndexerName: "ix", ClientID: 7, ClientName: "", Reason: PinClientMissing}, + includeClient: true, + want: []string{`indexer "ix" is pinned to a download client (id 7), which no longer exists`, "Nothing was sent"}, + }, + { + name: "missing, non admin hides the id", + e: PinnedClientError{IndexerName: "ix", ClientID: 7, Reason: PinClientMissing}, + includeClient: false, + want: []string{"that no longer exists"}, + notWant: []string{"id 7", "(id"}, + }, + { + name: "disabled, admin names the client", + e: PinnedClientError{IndexerName: "ix", ClientID: 7, ClientName: "qbit-main", Reason: PinClientDisabled}, + includeClient: true, + want: []string{`download client "qbit-main"`, "which is disabled"}, + }, + { + name: "disabled, non admin does not see the name", + e: PinnedClientError{IndexerName: "ix", ClientID: 7, ClientName: "qbit-main", Reason: PinClientDisabled}, + includeClient: false, + want: []string{`indexer "ix" is pinned to a download client, which is disabled`}, + notWant: []string{"qbit-main"}, + }, + { + name: "wrong protocol", + e: PinnedClientError{IndexerName: "ix", ClientID: 7, ClientName: "sab", Reason: PinClientWrongProtocol, ClientProtocol: "usenet", WantProtocol: "torrent"}, + includeClient: true, + want: []string{"which is a usenet client, but the indexer is torrent"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := tc.e.Message(tc.includeClient) + for _, w := range tc.want { + if !strings.Contains(got, w) { + t.Errorf("message %q missing %q", got, w) + } + } + for _, nw := range tc.notWant { + if strings.Contains(got, nw) { + t.Errorf("message %q must not contain %q", got, nw) + } + } + }) + } + + e := &PinnedClientError{IndexerName: "ix", ClientID: 7, ClientName: "sab", Reason: PinClientDisabled} + if e.Error() != e.Message(true) { + t.Fatalf("Error() must be the admin rendering, got %q", e.Error()) + } +} + +func TestDownloadClientDelete_PinnedClientListsEveryIndexerAndKeepsTheClient(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + clients := NewDownloadClientRepo(database) + indexers := NewIndexerRepo(database) + c := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: "h", Port: 1, Enabled: true} + if err := clients.Create(ctx, c); err != nil { + t.Fatal(err) + } + for _, name := range []string{"zeta", "alpha"} { + idx := &models.Indexer{Name: name, Type: "newznab", URL: "http://x", Enabled: true, DownloadClientID: &c.ID} + if err := indexers.Create(ctx, idx); err != nil { + t.Fatal(err) + } + } + // A third indexer on a different client must not be named. + idx := &models.Indexer{Name: "unrelated", Type: "newznab", URL: "http://x", Enabled: true} + if err := indexers.Create(ctx, idx); err != nil { + t.Fatal(err) + } + + err = clients.Delete(ctx, c.ID) + var pinned *ClientPinnedError + if !errors.As(err, &pinned) { + t.Fatalf("Delete = %v, want *ClientPinnedError", err) + } + if strings.Join(pinned.Indexers, ",") != "alpha,zeta" || pinned.ClientID != c.ID { + t.Fatalf("pinned = %+v, want alpha,zeta sorted", pinned) + } + msg := pinned.Error() + if !strings.Contains(msg, "2 indexer(s)") || !strings.Contains(msg, "alpha, zeta") || strings.Contains(msg, "unrelated") { + t.Fatalf("message = %q", msg) + } + if got, err := clients.GetByID(ctx, c.ID); err != nil || got == nil { + t.Fatalf("a refused delete must leave the client in place, got %v, %v", got, err) + } +} + +func TestDownloadClientDelete_UnpinnedAndUnknown(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + clients := NewDownloadClientRepo(database) + c := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: "h", Port: 1, Enabled: true} + if err := clients.Create(ctx, c); err != nil { + t.Fatal(err) + } + if err := clients.Delete(ctx, c.ID); err != nil { + t.Fatalf("deleting an unpinned client: %v", err) + } + if got, err := clients.GetByID(ctx, c.ID); err != nil || got != nil { + t.Fatalf("GetByID after delete = %v, %v; want nil, nil", got, err) + } + if err := clients.Delete(ctx, 9999); err != nil { + t.Fatalf("deleting an unknown id is a no-op, got %v", err) + } +} + +func TestDownloadClientDelete_ReleasedAfterIndexerUnpinned(t *testing.T) { + database, err := OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + ctx := context.Background() + clients := NewDownloadClientRepo(database) + indexers := NewIndexerRepo(database) + c := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: "h", Port: 1, Enabled: true} + if err := clients.Create(ctx, c); err != nil { + t.Fatal(err) + } + idx := &models.Indexer{Name: "ix", Type: "newznab", URL: "http://x", Enabled: true, DownloadClientID: &c.ID} + if err := indexers.Create(ctx, idx); err != nil { + t.Fatal(err) + } + idx.DownloadClientID = nil + if err := indexers.Update(ctx, idx); err != nil { + t.Fatal(err) + } + if err := clients.Delete(ctx, c.ID); err != nil { + t.Fatalf("delete after clearing the pin: %v", err) + } +} diff --git a/internal/downloader/pathcheck_edges_test.go b/internal/downloader/pathcheck_edges_test.go new file mode 100644 index 000000000..d75461a07 --- /dev/null +++ b/internal/downloader/pathcheck_edges_test.go @@ -0,0 +1,126 @@ +package downloader + +import ( + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/vavallee/bindery/internal/models" +) + +func TestCheckCompletedPathVisibility_NilClientIsUnknown(t *testing.T) { + got := CheckCompletedPathVisibility(t.Context(), nil, "/d", "/a", "") + if got.Status != PathUnknown || got.Message != "" || got.Path != "" { + t.Fatalf("got %+v, want a bare unknown", got) + } +} + +func TestCreatedOnGrabVisibility(t *testing.T) { + parent := t.TempDir() + missing := filepath.Join(parent, "ebooks-category") + warn := PathVisibility{Status: PathNotVisible, Message: "original warning", Path: missing, RemotePath: "/remote/ebooks-category"} + + t.Run("a readable parent means the client can create the folder", func(t *testing.T) { + got := createdOnGrabVisibility(warn) + if got.Status != PathVisible || got.Path != missing || got.RemotePath != "/remote/ebooks-category" { + t.Fatalf("got %+v", got) + } + if !strings.Contains(got.Message, parent) || !strings.Contains(got.Message, missing) { + t.Fatalf("message %q should name the parent and the folder to be created", got.Message) + } + }) + t.Run("a missing parent keeps the warning untouched", func(t *testing.T) { + w := warn + w.Path = filepath.Join(parent, "nope", "ebooks-category") + if got := createdOnGrabVisibility(w); got != w { + t.Fatalf("got %+v, want the original warning", got) + } + }) + t.Run("a parent that is a file keeps the warning", func(t *testing.T) { + file := filepath.Join(parent, "plainfile") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + w := warn + w.Path = filepath.Join(file, "child") + if got := createdOnGrabVisibility(w); got != w { + t.Fatalf("got %+v, want the original warning", got) + } + }) + t.Run("no local path keeps the warning", func(t *testing.T) { + w := warn + w.Path = "" + if got := createdOnGrabVisibility(w); got != w { + t.Fatalf("got %+v", got) + } + }) +} + +func TestStatRemappedPath_FileBlankAndRemapPrecedence(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "afile") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + client := &models.DownloadClient{Type: "qbittorrent"} + + if got := statRemappedPath(client, file, "", ""); got.Status != PathVisible || !strings.Contains(got.Message, "completed-downloads path at") { + t.Fatalf("a file where a folder is expected is still reachable: %+v", got) + } + if got := statRemappedPath(client, dir, "", ""); got.Status != PathVisible || !strings.Contains(got.Message, "completed-downloads folder at") { + t.Fatalf("directory: %+v", got) + } + for _, blank := range []string{"", " ", "."} { + if got := statRemappedPath(client, blank, "", ""); got.Status != PathUnknown { + t.Errorf("blank path %q = %+v, want unknown", blank, got) + } + } + + // The client's own remap wins over the global one. + remapped := &models.DownloadClient{Type: "qbittorrent", PathRemap: "/remote:" + dir} + if got := statRemappedPath(remapped, "/remote", "", "/remote:/does/not/exist"); got.Status != PathVisible || got.Path != dir { + t.Fatalf("client remap should beat the global remap: %+v", got) + } + // With no client remap the global one applies. + if got := statRemappedPath(client, "/remote", "", "/remote:"+dir); got.Status != PathVisible || got.Path != dir { + t.Fatalf("global remap fallback: %+v", got) + } +} + +func TestStatRemappedPath_WarningTextVariants(t *testing.T) { + missing := filepath.Join(t.TempDir(), "gone") + + t.Run("same path both sides without a remap", func(t *testing.T) { + got := statRemappedPath(&models.DownloadClient{}, missing, "", "") + if got.Status != PathNotVisible || !strings.Contains(got.Message, "mount the same storage at the same path") || strings.Contains(got.Message, "maps to") { + t.Fatalf("got %+v", got) + } + }) + t.Run("a configured remap that lands on nothing names both paths", func(t *testing.T) { + client := &models.DownloadClient{PathRemap: "/remote:" + missing} + got := statRemappedPath(client, "/remote", "", "") + if got.Status != PathNotVisible || !strings.Contains(got.Message, `maps to`) || !strings.Contains(got.Message, missing) { + t.Fatalf("got %+v", got) + } + if strings.Contains(got.Message, "to translate the client's path") { + t.Fatalf("a client that already has a remap must not be told to add one: %q", got.Message) + } + if got.RemotePath != "/remote" { + t.Fatalf("RemotePath = %q", got.RemotePath) + } + }) + t.Run("windows drive path says a remap is required", func(t *testing.T) { + got := statRemappedPath(&models.DownloadClient{}, `Z:\Downloads`, "", "") + if got.Status != PathNotVisible || !strings.Contains(got.Message, "Windows drive path") || !strings.Contains(got.Message, strconv.Quote(`Z:\Downloads:/downloads`)) { + t.Fatalf("got %+v", got) + } + }) + t.Run("expected directories are appended when known", func(t *testing.T) { + got := statRemappedPath(&models.DownloadClient{}, missing, `the ebook download directory "/books"`, "") + if !strings.HasSuffix(got.Message, `Bindery is configured to read from the ebook download directory "/books".`) { + t.Fatalf("message = %q", got.Message) + } + }) +} diff --git a/internal/importer/content_check_edges_test.go b/internal/importer/content_check_edges_test.go new file mode 100644 index 000000000..28c8a00a6 --- /dev/null +++ b/internal/importer/content_check_edges_test.go @@ -0,0 +1,220 @@ +package importer + +import ( + "context" + "testing" + + "github.com/vavallee/bindery/internal/db" + "github.com/vavallee/bindery/internal/models" +) + +func TestBookIsCompilation_Shapes(t *testing.T) { + cases := []struct { + title string + want bool + }{ + {"The Dune Saga Collection", true}, + {"Dune Saga Collection", true}, + {"The Memory Collection", false}, + {"A Collection of Curses", false}, + {"Dune Chronicles 1-3", true}, + {"Dune Chronicles 3-1", false}, + {"Dune Chronicles 2-2", false}, + {"1984", false}, + {"Project Hail Mary", false}, + } + for _, tc := range cases { + if got := bookIsCompilation(tc.title); got != tc.want { + t.Errorf("bookIsCompilation(%q) = %v, want %v", tc.title, got, tc.want) + } + } +} + +func TestBookIsCompilationBy_BestOfNeedsTheAuthorsName(t *testing.T) { + cases := []struct { + title, author string + want bool + }{ + {"The Best of Poe", "Edgar Allan Poe", true}, + {"Best of Poe and Others", "Edgar Allan Poe", true}, + {"The Best of Me", "Nicholas Sparks", false}, + {"Best of Friends", "Nicholas Sparks", false}, + {"The Best of Poe", "", false}, + {"The Best of Li", "Wei Li", false}, + {"The Best of Poet Laureates", "Edgar Allan Poe", false}, + {"Some Ordinary Title", "Edgar Allan Poe", false}, + } + for _, tc := range cases { + if got := bookIsCompilationBy(tc.title, tc.author); got != tc.want { + t.Errorf("bookIsCompilationBy(%q, %q) = %v, want %v", tc.title, tc.author, got, tc.want) + } + } +} + +func TestEbookAuthorMatches_Scripts(t *testing.T) { + cases := []struct { + name string + names []string + creator string + want bool + }{ + {"no creator says nothing", []string{"Jane Austen"}, "", true}, + {"no author on record says nothing", nil, "Mary Trevelyan", true}, + {"only blank names say nothing is known", []string{" "}, "Mary Trevelyan", true}, + {"placeholder creator", []string{"Jane Austen"}, "Unknown", true}, + {"placeholder creator converter name", []string{"Jane Austen"}, "calibre", true}, + {"last, first against first last", []string{"J.R.R. Tolkien"}, "Tolkien, J. R. R.", true}, + {"co author credit", []string{"James Patterson"}, "James Patterson and Maxine Paetro", true}, + {"clear mismatch", []string{"Jane Austen"}, "Mary Trevelyan", false}, + {"a placeholder alongside a real mismatching name is still a mismatch", []string{"Jane Austen"}, "Unknown; Mary Trevelyan", false}, + {"cyrillic creator with only latin names says nothing", []string{"Leo Tolstoy"}, "Лев Толстой", true}, + {"latin creator disagrees even though the author has a cyrillic alias", []string{"Лев Толстой", "Leo Tolstoy"}, "Mary Trevelyan", false}, + {"latin creator with only a cyrillic name on record says nothing", []string{"Лев Толстой"}, "Mary Trevelyan", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ebookAuthorMatches(tc.names, tc.creator); got != tc.want { + t.Fatalf("ebookAuthorMatches(%q, %q) = %v, want %v", tc.names, tc.creator, got, tc.want) + } + }) + } +} + +func TestAuthorNameAndAuthorNamesFor(t *testing.T) { + if authorName(nil) != "" || authorName(&models.Author{Name: "Jane Austen"}) != "Jane Austen" { + t.Error("authorName") + } + ctx := context.Background() + if got := (&Scanner{}).authorNamesFor(ctx, nil); got != nil { + t.Fatalf("no author = %v, want nil", got) + } + a := &models.Author{ID: 1, Name: "Jane Austen"} + if got := (&Scanner{}).authorNamesFor(ctx, a); len(got) != 1 || got[0] != "Jane Austen" { + t.Fatalf("no alias repo = %v, want just the name on record", got) + } + + database, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + defer database.Close() + authors := db.NewAuthorRepo(database) + real := &models.Author{ForeignID: "OL1A", Name: "Jane Austen", SortName: "Austen, Jane", Monitored: true} + if err := authors.Create(ctx, real); err != nil { + t.Fatal(err) + } + aliases := db.NewAuthorAliasRepo(database) + if err := aliases.Create(ctx, &models.AuthorAlias{AuthorID: real.ID, Name: "J. Austen"}); err != nil { + t.Fatal(err) + } + s := &Scanner{authorAliases: aliases} + got := s.authorNamesFor(ctx, real) + if len(got) != 2 || got[0] != "Jane Austen" || got[1] != "J. Austen" { + t.Fatalf("with aliases = %v, want the name then each alias", got) + } + + closed, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + s2 := &Scanner{authorAliases: db.NewAuthorAliasRepo(closed)} + closed.Close() + if got := s2.authorNamesFor(ctx, real); len(got) != 1 || got[0] != "Jane Austen" { + t.Fatalf("alias lookup failure = %v, want the name on record alone", got) + } +} + +func TestContentTitleHelpers(t *testing.T) { + t.Run("genericSubtitleTail", func(t *testing.T) { + cases := map[string]bool{ + "": false, "A Novel": true, "Part Two": true, "Book 3": true, "An Unabridged Audiobook": true, + "Messiah": false, "The Chronicles of Narnia": false, + } + for in, want := range cases { + if got := genericSubtitleTail(in); got != want { + t.Errorf("genericSubtitleTail(%q) = %v, want %v", in, got, want) + } + } + }) + t.Run("containsWholeWords", func(t *testing.T) { + cases := []struct { + hay, needle string + want bool + }{ + {"dune messiah", "dune", true}, + {"dune messiah", "messiah", true}, + {"dunes of mars", "dune", false}, + {"children of dune", "dune", true}, + {"dune", "", false}, + {"", "dune", false}, + {"dune messiah", "dune messiah", true}, + {"dune messiah", "une messiah", false}, + } + for _, tc := range cases { + if got := containsWholeWords(tc.hay, tc.needle); got != tc.want { + t.Errorf("containsWholeWords(%q, %q) = %v, want %v", tc.hay, tc.needle, got, tc.want) + } + } + }) + t.Run("isDigits", func(t *testing.T) { + for in, want := range map[string]bool{"": false, "0": true, "123": true, "12a": false, "1.5": false, "-1": false} { + if got := isDigits(in); got != want { + t.Errorf("isDigits(%q) = %v, want %v", in, got, want) + } + } + }) + t.Run("stripSeriesSuffix", func(t *testing.T) { + cases := map[string]string{ + "Mary, Mary: Alex Cross, Book 11": "Mary, Mary", + "Dune #3": "Dune", + "Dune # 3": "Dune", + "Dune Messiah, Book Eleven": "Dune Messiah", + "Dune - Volume 3": "Dune", + "Dune": "Dune", + " Dune ": "Dune", + "Book 3": "Book 3", + } + for in, want := range cases { + if got := stripSeriesSuffix(in); got != want { + t.Errorf("stripSeriesSuffix(%q) = %q, want %q", in, got, want) + } + } + }) + t.Run("genericTrackTitle", func(t *testing.T) { + cases := map[string]bool{ + "": false, "Chapter 12": true, "Track 03": true, "Chapter 3 of 12": true, "12": true, + "Chapter One Two Three Four Five": false, + "Prologue": true, + "The Hobbit": false, + } + for in, want := range cases { + if got := genericTrackTitle(in); got != want { + t.Errorf("genericTrackTitle(%q) = %v, want %v", in, got, want) + } + } + }) + t.Run("volumeStem", func(t *testing.T) { + cases := []struct { + in string + wantNum string + wantOK bool + wantStem string + }{ + {"Foundation #4", "4", true, "foundation"}, + {"Foundation, Book 4", "4", true, "foundation"}, + {"Foundation Vol. 2.5", "2.5", true, "foundation"}, + {"NYPD Red 3", "3", true, "nypd red"}, + {"Book 3", "", false, ""}, + {"#3", "", false, ""}, + {"1984", "", false, ""}, + {"", "", false, ""}, + {"Dune", "", false, ""}, + } + for _, tc := range cases { + stem, num, ok := volumeStem(tc.in) + if ok != tc.wantOK || num != tc.wantNum || stem != tc.wantStem { + t.Errorf("volumeStem(%q) = %q, %q, %v; want %q, %q, %v", tc.in, stem, num, ok, tc.wantStem, tc.wantNum, tc.wantOK) + } + } + }) +} diff --git a/internal/indexer/title_relevance_edges_test.go b/internal/indexer/title_relevance_edges_test.go new file mode 100644 index 000000000..5039c9410 --- /dev/null +++ b/internal/indexer/title_relevance_edges_test.go @@ -0,0 +1,121 @@ +package indexer + +import ( + "reflect" + "testing" +) + +func TestNewSubtitleInfo(t *testing.T) { + cases := []struct { + title string + has bool + seriesLike bool + n int + }{ + {"Plain Title", false, false, 0}, + {": Leading Colon", false, false, 0}, + {"Main:", false, false, 0}, + {"Main: ", false, false, 0}, + {"Main: Book 2", true, true, 2}, + {"Main: A Real Subtitle", true, false, 0}, + } + for _, tc := range cases { + si := newSubtitleInfo(tc.title) + if si.has != tc.has || si.seriesLike != tc.seriesLike || si.n != tc.n { + t.Errorf("newSubtitleInfo(%q) = %+v, want has=%v seriesLike=%v n=%d", tc.title, si, tc.has, tc.seriesLike, tc.n) + } + } + if si := newSubtitleInfo("Main: A Real Subtitle"); len(si.ident) == 0 { + t.Error("a real subtitle must carry identity words") + } +} + +func TestMainTitleOnly_ReleaseNamingTheSubtitleIsKept(t *testing.T) { + si := newSubtitleInfo("Main: A Real Subtitle") + anyFull := func(string) bool { t.Fatal("anyFull consulted although the release names the subtitle"); return false } + got := si.mainTitleOnly(nil, "Main A Real Subtitle epub", "main a real subtitle epub", "main real subtitle", anyFull) + if got != "" { + t.Fatalf("got %q, want keep", got) + } +} + +func TestMainTitleOnly_FullTitleElsewhereInBatchDropsTheSibling(t *testing.T) { + si := newSubtitleInfo("Main: A Real Subtitle") + var asked string + anyFull := func(mt string) bool { asked = mt; return true } + got := si.mainTitleOnly(nil, "Main epub", "main epub", "main", anyFull) + if got != reasonMainTitleOnlyFullFound { + t.Fatalf("got %q, want %q", got, reasonMainTitleOnlyFullFound) + } + if asked != "ebook" { + t.Fatalf("anyFull asked about %q, want the release's own media type", asked) + } + + // A release that names no format has no media type to compare, so the + // batch rule cannot apply and it is kept. + anyFull = func(string) bool { t.Fatal("anyFull consulted for a release with no media type"); return false } + if got := si.mainTitleOnly(nil, "Main", "main", "main", anyFull); got != "" { + t.Fatalf("got %q, want keep", got) + } +} + +func TestFullTitleCounts(t *testing.T) { + ident := []string{"main", "title"} + authors := [][]string{{"jane", "austen"}} + count := func(raw string) bool { + return fullTitleCounts(raw, NormalizeRelease(raw), authors, ident) + } + cases := []struct { + raw string + want bool + }{ + {"Main Title", true}, + {"Main Title epub", true}, + {"Main Title - Jane Austen epub", true}, + {"Main Title Summary epub", false}, + {"Main Title Study Guide", false}, + {"Main Title: Insights and Key Takeaways", false}, + {"Unrelated Words epub", false}, + {"Main Title and Some Unexplained Words", false}, + } + for _, tc := range cases { + if got := count(tc.raw); got != tc.want { + t.Errorf("fullTitleCounts(%q) = %v, want %v", tc.raw, got, tc.want) + } + } + if fullTitleCounts("Main Title epub", NormalizeRelease("Main Title epub"), authors) { + t.Error("with no identity phrase the title cannot be located and must not count") + } +} + +func TestNumbersAfter(t *testing.T) { + ident := []string{"main", "title"} + cases := []struct { + name string + raw string + guarded bool + want []int + }{ + {"nothing follows", "Main Title epub", true, nil}, + {"book marker then digit", "Main Title Book 3 epub", true, []int{3}}, + {"bare digit ending the release", "Main Title 4", true, []int{4}}, + {"a digit before an ordinary word is a count when guarded", "Main Title 4 epub", true, nil}, + {"and a number when not guarded", "Main Title 4 epub", false, []int{4}}, + {"capital roman numeral", "Main Title III epub", true, []int{3}}, + {"number word after a marker", "Main Title Book Two epub", true, []int{2}}, + {"a bitrate is a count when guarded", "Main Title 64 kbps", true, nil}, + {"the same digits are read when not guarded", "Main Title 64 kbps", false, []int{64}}, + {"title not in the release", "Other Words 3", true, nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := numbersAfter(tc.raw, NormalizeRelease(tc.raw), tc.guarded, ident) + if len(got) == 0 && len(tc.want) == 0 { + return + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("numbersAfter(%q, guarded=%v) = %v, want %v", tc.raw, tc.guarded, got, tc.want) + } + }) + } +} diff --git a/internal/models/indexer_test.go b/internal/models/indexer_test.go new file mode 100644 index 000000000..5ae670b37 --- /dev/null +++ b/internal/models/indexer_test.go @@ -0,0 +1,43 @@ +package models + +import "testing" + +func TestIndexerProtocol(t *testing.T) { + cases := map[string]string{ + "torznab": "torrent", + "newznab": "usenet", + "": "usenet", + "Torznab": "usenet", + "whatever": "usenet", + } + for typ, want := range cases { + if got := (Indexer{Type: typ}).Protocol(); got != want { + t.Errorf("Indexer{Type: %q}.Protocol() = %q, want %q", typ, got, want) + } + } +} + +func TestIndexerNeedsAttention(t *testing.T) { + msg := "refused" + code := func(n int) *int { return &n } + cases := []struct { + name string + idx Indexer + want bool + }{ + {"no error", Indexer{}, false}, + {"error without a code is a transport failure", Indexer{LastError: &msg}, false}, + {"code without an error text", Indexer{LastErrorCode: code(100)}, false}, + {"99 is below the human range", Indexer{LastError: &msg, LastErrorCode: code(99)}, false}, + {"100 bad credentials", Indexer{LastError: &msg, LastErrorCode: code(100)}, true}, + {"102 forbidden", Indexer{LastError: &msg, LastErrorCode: code(102)}, true}, + {"199 is the top of the range", Indexer{LastError: &msg, LastErrorCode: code(199)}, true}, + {"200 is outside it", Indexer{LastError: &msg, LastErrorCode: code(200)}, false}, + {"500 rate limit clears on its own", Indexer{LastError: &msg, LastErrorCode: code(500)}, false}, + } + for _, tc := range cases { + if got := tc.idx.NeedsAttention(); got != tc.want { + t.Errorf("%s: NeedsAttention() = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/scheduler/metadata_refresh_edges_test.go b/internal/scheduler/metadata_refresh_edges_test.go new file mode 100644 index 000000000..d8a4830f9 --- /dev/null +++ b/internal/scheduler/metadata_refresh_edges_test.go @@ -0,0 +1,255 @@ +package scheduler + +import ( + "context" + "testing" + "time" + + "github.com/vavallee/bindery/internal/metadata" + "github.com/vavallee/bindery/internal/models" +) + +func TestMetadataRefreshBatchSize(t *testing.T) { + cases := []struct{ eligible, want int }{ + {-5, 1}, {0, 1}, {1, 1}, {24, 1}, {25, 2}, {48, 2}, {49, 3}, + {2400, 100}, {2401, 100}, {1000000, 100}, + } + for _, tc := range cases { + if got := metadataRefreshBatchSize(tc.eligible); got != tc.want { + t.Errorf("metadataRefreshBatchSize(%d) = %d, want %d", tc.eligible, got, tc.want) + } + } +} + +func TestCoolingAuthors_CooldownBoundaryAndPruning(t *testing.T) { + s := &Scheduler{} + base := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + s.coolAuthor(1, base) + until := base.Add(metadataRefreshAuthorCooldown) + + if got := s.coolingAuthors(until.Add(-time.Nanosecond)); !got[1] { + t.Fatal("author must still be cooling one nanosecond before the end") + } + if got := s.coolingAuthors(until); got[1] { + t.Fatal("author must be released exactly at the end of the cooldown") + } + s.metadataCooldownMu.Lock() + _, kept := s.metadataCooldown[1] + s.metadataCooldownMu.Unlock() + if kept { + t.Fatal("an expired cooldown must be forgotten, not kept forever") + } +} + +func TestCoolingAuthors_PrunesStaleBreakerTrips(t *testing.T) { + s := &Scheduler{} + base := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + s.noteBreakerTrip(1, base) + s.noteBreakerTrip(2, base.Add(time.Hour)) + + s.coolingAuthors(base.Add(breakerTripMemory)) + if s.breakerTripCount(1) != 1 { + t.Fatal("a trip exactly breakerTripMemory old is still remembered") + } + s.coolingAuthors(base.Add(breakerTripMemory + time.Second)) + if s.breakerTripCount(1) != 0 { + t.Fatal("a trip older than breakerTripMemory must be pruned") + } + if s.breakerTripCount(2) != 1 { + t.Fatal("a newer trip must survive the pruning of an older one") + } +} + +func TestNoteBreakerTrip_CoolsOnTheThirdAndRestartsCount(t *testing.T) { + s := &Scheduler{} + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + for i := 1; i < metadataRefreshBreakerTripsToCool; i++ { + s.noteBreakerTrip(7, now) + if s.coolingAuthors(now)[7] { + t.Fatalf("author cooled after only %d trips", i) + } + if s.breakerTripCount(7) != i { + t.Fatalf("trip count = %d, want %d", s.breakerTripCount(7), i) + } + } + s.noteBreakerTrip(7, now) + if !s.coolingAuthors(now)[7] { + t.Fatal("author must be cooled on the third consecutive trip") + } + if s.breakerTripCount(7) != 0 { + t.Fatal("cooling must restart the trip count") + } + s.noteBreakerTrip(8, now) + s.clearBreakerTrips(8) + if s.breakerTripCount(8) != 0 { + t.Fatal("clearBreakerTrips must forget the author's trips") + } +} + +// refreshProvider answers GetAuthor through fn so a test can return a sparse +// record, nothing at all, or act mid call. +type refreshProvider struct { + mockMetaProvider + calls []string + fn func(foreignID string) (*models.Author, error) +} + +func (p *refreshProvider) GetAuthor(_ context.Context, foreignID string) (*models.Author, error) { + p.calls = append(p.calls, foreignID) + return p.fn(foreignID) +} + +func newRefreshEdgeFixture(t *testing.T, n int, fn func(string) (*models.Author, error)) (*refreshFixture, *refreshProvider) { + t.Helper() + f := newRefreshFixture(t, n) + p := &refreshProvider{fn: fn} + f.s.meta = metadata.NewAggregator(p) + return f, p +} + +func TestRefreshMetadata_NothingEligibleCallsNoProvider(t *testing.T) { + f, p := newRefreshEdgeFixture(t, 0, func(string) (*models.Author, error) { return &models.Author{}, nil }) + f.s.refreshMetadata() + if len(p.calls) != 0 { + t.Fatalf("provider called %v with no eligible authors", p.calls) + } +} + +// An author unmonitored while the pass was running is skipped without a +// provider call and without using one of the batch slots. +func TestRefreshMetadata_AuthorUnmonitoredMidPassIsSkippedWithoutUsingASlot(t *testing.T) { + var f *refreshFixture + var p *refreshProvider + f, p = newRefreshEdgeFixture(t, 60, func(fid string) (*models.Author, error) { + if fid == "OL000A" { + a, err := f.authors.GetByID(context.Background(), f.ids["OL001A"]) + if err != nil || a == nil { + t.Fatalf("reload: %v", err) + } + a.Monitored = false + if err := f.authors.Update(context.Background(), a); err != nil { + t.Fatal(err) + } + } + return &models.Author{ForeignID: fid, Description: "bio"}, nil + }) + // batch = ceil(60/24) = 3. OL000A runs first and unmonitors OL001A. + f.s.refreshMetadata() + + want := []string{"OL000A", "OL002A", "OL003A"} + if len(p.calls) != len(want) { + t.Fatalf("calls = %v, want %v", p.calls, want) + } + for i := range want { + if p.calls[i] != want[i] { + t.Fatalf("calls = %v, want %v", p.calls, want) + } + } + if f.stamped("OL001A") != nil { + t.Fatal("the unmonitored author must not be stamped") + } +} + +func TestRefreshMetadata_CancelledContextMidPassStopsBeforeTheNextAuthor(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + f, p := newRefreshEdgeFixture(t, 60, func(fid string) (*models.Author, error) { + cancel() + return &models.Author{ForeignID: fid, Description: "bio"}, nil + }) + f.s.appCtx = ctx + f.s.refreshMetadata() + if len(p.calls) != 1 { + t.Fatalf("calls = %v, want the pass to stop after the author in flight", p.calls) + } +} + +// A provider that owns no prefix answers (nil, nil): the author is not +// stamped and is cooled so it does not head every tick. +func TestRefreshMetadata_NoProviderForAuthorCoolsItWithoutStamping(t *testing.T) { + f, p := newRefreshEdgeFixture(t, 3, func(fid string) (*models.Author, error) { + if fid == "OL000A" { + return nil, nil + } + return &models.Author{ForeignID: fid, Description: "bio"}, nil + }) + f.s.refreshMetadata() + f.s.refreshMetadata() + var first int + for _, c := range p.calls { + if c == "OL000A" { + first++ + } + } + if first != 1 { + t.Fatalf("OL000A asked %d times over two ticks, want once (calls %v)", first, p.calls) + } + if f.stamped("OL000A") != nil { + t.Fatal("an author nobody answered for must not be stamped") + } + if f.stamped("OL001A") == nil { + t.Fatal("the next author should have been reached on the second tick") + } +} + +// A sparse provider answer must not wipe enrichment already stored (#1463). +func TestRefreshMetadata_SparseAnswerKeepsStoredProfile(t *testing.T) { + f, _ := newRefreshEdgeFixture(t, 1, func(fid string) (*models.Author, error) { + return &models.Author{ForeignID: fid}, nil + }) + ctx := context.Background() + a, _ := f.authors.GetByID(ctx, f.ids["OL000A"]) + a.Description, a.ImageURL, a.AverageRating, a.RatingsCount = "stored bio", "http://img/a.jpg", 4.5, 120 + if err := f.authors.Update(ctx, a); err != nil { + t.Fatal(err) + } + f.s.refreshMetadata() + + got, _ := f.authors.GetByID(ctx, f.ids["OL000A"]) + if got.Description != "stored bio" || got.ImageURL != "http://img/a.jpg" || got.AverageRating != 4.5 || got.RatingsCount != 120 { + t.Fatalf("sparse refresh clobbered the profile: %+v", got) + } + if got.LastMetadataRefreshAt == nil { + t.Fatal("a successful refresh must stamp even when the answer is sparse") + } +} + +func TestRefreshMetadata_RichAnswerOverwritesEveryField(t *testing.T) { + f, _ := newRefreshEdgeFixture(t, 1, func(fid string) (*models.Author, error) { + return &models.Author{ForeignID: fid, Description: "new bio", ImageURL: "http://img/new.jpg", AverageRating: 3.9, RatingsCount: 7}, nil + }) + ctx := context.Background() + a, _ := f.authors.GetByID(ctx, f.ids["OL000A"]) + a.Description, a.ImageURL, a.AverageRating, a.RatingsCount = "old", "http://img/old.jpg", 1, 1 + if err := f.authors.Update(ctx, a); err != nil { + t.Fatal(err) + } + f.s.refreshMetadata() + got, _ := f.authors.GetByID(ctx, f.ids["OL000A"]) + if got.Description != "new bio" || got.ImageURL != "http://img/new.jpg" || got.AverageRating != 3.9 || got.RatingsCount != 7 { + t.Fatalf("profile = %+v, want the provider's values", got) + } +} + +// An edit saved while the provider was being asked wins: the refresh is +// dropped, the edit survives and the author stays unstamped for a later tick. +func TestRefreshMetadata_EditDuringProviderCallWinsAndAuthorStaysStale(t *testing.T) { + var f *refreshFixture + f, _ = newRefreshEdgeFixture(t, 1, func(fid string) (*models.Author, error) { + ctx := context.Background() + a, _ := f.authors.GetByID(ctx, f.ids["OL000A"]) + a.Description = "edited by the user" + if err := f.authors.Update(ctx, a); err != nil { + t.Fatal(err) + } + return &models.Author{ForeignID: fid, Description: "from provider"}, nil + }) + f.s.refreshMetadata() + got, _ := f.authors.GetByID(context.Background(), f.ids["OL000A"]) + if got.Description != "edited by the user" { + t.Fatalf("description = %q, the user's edit was overwritten", got.Description) + } + if got.LastMetadataRefreshAt != nil { + t.Fatal("a dropped refresh must not stamp the author") + } +} diff --git a/internal/scheduler/scheduler_indexer_pin_unit_test.go b/internal/scheduler/scheduler_indexer_pin_unit_test.go new file mode 100644 index 000000000..c3bb5a01e --- /dev/null +++ b/internal/scheduler/scheduler_indexer_pin_unit_test.go @@ -0,0 +1,252 @@ +package scheduler + +import ( + "errors" + "testing" + + "github.com/vavallee/bindery/internal/db" + "github.com/vavallee/bindery/internal/decision" + "github.com/vavallee/bindery/internal/indexer/newznab" + "github.com/vavallee/bindery/internal/models" +) + +func (f *schedPinFixture) result(guid, protocol string) *newznab.SearchResult { + return &newznab.SearchResult{GUID: guid, Title: "Pin Author Pinned Book epub", Protocol: protocol, IndexerID: f.idx.ID, IndexerName: f.idx.Name} +} + +func TestPinnedClient_Routing(t *testing.T) { + f := newSchedPinFixture(t) + ctx := f.ctx + + t.Run("unpinned indexer yields no client", func(t *testing.T) { + c, err := f.s.pinnedClient(ctx, f.result("g", "usenet")) + if c != nil || err != nil { + t.Fatalf("got %v, %v; want nil, nil", c, err) + } + }) + t.Run("result without an indexer id is never pinned", func(t *testing.T) { + f.pin(t, f.privC.ID) + res := f.result("g", "usenet") + res.IndexerID = 0 + if c, err := f.s.pinnedClient(ctx, res); c != nil || err != nil { + t.Fatalf("got %v, %v; want nil, nil", c, err) + } + }) + t.Run("no indexer repo means no pin", func(t *testing.T) { + s := &Scheduler{clients: f.s.clients, history: f.s.history} + if c, err := s.pinnedClient(ctx, f.result("g", "usenet")); c != nil || err != nil { + t.Fatalf("got %v, %v; want nil, nil", c, err) + } + }) + t.Run("pinned indexer yields exactly its client, not the higher priority one", func(t *testing.T) { + f.pin(t, f.privC.ID) + c, err := f.s.pinnedClient(ctx, f.result("g", "usenet")) + if err != nil || c == nil || c.ID != f.privC.ID { + t.Fatalf("got %v, %v; want client %d", c, err, f.privC.ID) + } + }) + t.Run("release protocol must match the pinned client", func(t *testing.T) { + c, err := f.s.pinnedClient(ctx, f.result("g", "torrent")) + var perr *db.PinnedClientError + if c != nil || !errors.As(err, &perr) || perr.Reason != db.PinClientWrongProtocol { + t.Fatalf("got %v, %v; want a wrong protocol PinnedClientError", c, err) + } + }) + t.Run("indexer lookup failure is an error and not a silent fallback", func(t *testing.T) { + broken, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + s := &Scheduler{indexers: db.NewIndexerRepo(broken), clients: f.s.clients, history: f.s.history} + broken.Close() + c, err := s.pinnedClient(ctx, f.result("g", "usenet")) + if c != nil || err == nil { + t.Fatalf("got %v, %v; want an error", c, err) + } + var perr *db.PinnedClientError + if errors.As(err, &perr) { + t.Fatalf("a database failure must not look like a pin refusal: %v", err) + } + }) +} + +func TestCheckPin_Outcomes(t *testing.T) { + f := newSchedPinFixture(t) + ctx := f.ctx + + t.Run("an indexer already skipped this sweep is skipped without a lookup or a record", func(t *testing.T) { + skipped := map[int64]bool{f.idx.ID: true} + c, skip, err := f.s.checkPin(ctx, f.book, f.result("g", "usenet"), skipped) + if c != nil || !skip || err != nil { + t.Fatalf("got %v, %v, %v", c, skip, err) + } + if n := len(f.pinnedFailures(t)); n != 0 { + t.Fatalf("recorded %d failures for a quiet skip", n) + } + }) + + t.Run("a database error stops the grab and does not mark the indexer skipped", func(t *testing.T) { + broken, err := db.OpenMemory() + if err != nil { + t.Fatal(err) + } + s := &Scheduler{indexers: db.NewIndexerRepo(broken), clients: f.s.clients, history: f.s.history} + broken.Close() + skipped := map[int64]bool{} + c, skip, err := s.checkPin(ctx, f.book, f.result("g", "usenet"), skipped) + if c != nil || skip || err == nil { + t.Fatalf("got %v, %v, %v; want nil, false, error", c, skip, err) + } + if skipped[f.idx.ID] { + t.Fatal("a transient database error must not blacklist the indexer for the sweep") + } + }) + + t.Run("a refused pin skips, marks the indexer and records once", func(t *testing.T) { + f.pin(t, f.privC.ID) + f.privC.Enabled = false + if err := f.clients.Update(ctx, f.privC); err != nil { + t.Fatal(err) + } + skipped := map[int64]bool{} + c, skip, err := f.s.checkPin(ctx, f.book, f.result("g1", "usenet"), skipped) + if c != nil || !skip || err != nil { + t.Fatalf("got %v, %v, %v; want nil, true, nil", c, skip, err) + } + if !skipped[f.idx.ID] { + t.Fatal("indexer not marked skipped") + } + c, skip, err = f.s.checkPin(ctx, f.book, f.result("g2", "usenet"), skipped) + if c != nil || !skip || err != nil { + t.Fatalf("second release: got %v, %v, %v", c, skip, err) + } + if n := len(f.pinnedFailures(t)); n != 1 { + t.Fatalf("failures recorded = %d, want 1 for two releases of one indexer", n) + } + }) + + t.Run("a healthy pin returns the client and does not skip", func(t *testing.T) { + f.privC.Enabled = true + if err := f.clients.Update(ctx, f.privC); err != nil { + t.Fatal(err) + } + c, skip, err := f.s.checkPin(ctx, f.book, f.result("g3", "usenet"), map[int64]bool{}) + if err != nil || skip || c == nil || c.ID != f.privC.ID { + t.Fatalf("got %v, %v, %v", c, skip, err) + } + }) +} + +func TestRecordPinnedFailure_Dedupe(t *testing.T) { + f := newSchedPinFixture(t) + n := &recordingNotifier{} + f.s.notif = n + disabled := &db.PinnedClientError{IndexerName: "TrackerX", ClientID: 1, ClientName: "c", Reason: db.PinClientDisabled} + missing := &db.PinnedClientError{IndexerName: "TrackerX", ClientID: 1, Reason: db.PinClientMissing} + + f.s.recordPinnedFailure(f.ctx, f.book.ID, f.result("g1", "usenet"), disabled) + f.s.recordPinnedFailure(f.ctx, f.book.ID, f.result("g2", "usenet"), disabled) + if got := len(f.pinnedFailures(t)); got != 1 { + t.Fatalf("identical failure for the same indexer recorded %d times, want 1", got) + } + + other := f.result("g3", "usenet") + other.IndexerID = f.idx.ID + 50 + f.s.recordPinnedFailure(f.ctx, f.book.ID, other, disabled) + if got := len(f.pinnedFailures(t)); got != 2 { + t.Fatalf("a different indexer must record its own failure, events = %d", got) + } + + f.s.recordPinnedFailure(f.ctx, f.book.ID, f.result("g4", "usenet"), missing) + if got := len(f.pinnedFailures(t)); got != 3 { + t.Fatalf("a changed reason is new information and must record, events = %d", got) + } + + f.s.recordPinnedFailure(f.ctx, f.book.ID+99, f.result("g5", "usenet"), disabled) + if got := len(f.pinnedFailures(t)); got != 3 { + t.Fatalf("another book has no history of its own here, events listed for the first book = %d", got) + } + failed := 0 + for _, typ := range n.types { + if typ == "downloadFailed" { + failed++ + } + } + if failed != 4 { + t.Fatalf("notifications = %d, want 4 (one per recorded failure)", failed) + } +} + +func TestPendingApproved_Gate(t *testing.T) { + newFixture := func(t *testing.T) (*schedPinFixture, *db.PendingReleaseRepo) { + f := newSchedPinFixture(t) + f.s.searcher.(*perFormatSearcher).byFormat[models.MediaTypeEbook] = nil + p := db.NewPendingReleaseRepo(f.database) + f.s.pending = p + return f, p + } + + t.Run("no pending repo", func(t *testing.T) { + f := newSchedPinFixture(t) + res, c, err := f.s.pendingApproved(f.ctx, f.book, models.MediaTypeEbook, nil, nil) + if res != nil || c != nil || err != nil { + t.Fatalf("got %v, %v, %v", res, c, err) + } + }) + + t.Run("a gate error stops the sweep and keeps the row", func(t *testing.T) { + f, p := newFixture(t) + f.holdPending(t, p, "g-held", f.idx) + boom := errors.New("boom") + dm := decision.New() + res, c, err := f.s.pendingApproved(f.ctx, f.book, models.MediaTypeEbook, dm, + func(*newznab.SearchResult) (*models.DownloadClient, bool, error) { return nil, false, boom }) + if res != nil || c != nil || !errors.Is(err, boom) { + t.Fatalf("got %v, %v, %v; want the gate error", res, c, err) + } + rows, _ := p.ListByBookAndMediaType(f.ctx, f.book.ID, models.MediaTypeEbook) + if len(rows) != 1 { + t.Fatalf("rows = %d, want the held release kept", len(rows)) + } + }) + + t.Run("an accepted row is returned with its client and deleted", func(t *testing.T) { + f, p := newFixture(t) + f.holdPending(t, p, "g-held", f.idx) + dm := decision.New() + res, c, err := f.s.pendingApproved(f.ctx, f.book, models.MediaTypeEbook, dm, + func(*newznab.SearchResult) (*models.DownloadClient, bool, error) { return f.privC, false, nil }) + if err != nil || res == nil || res.GUID != "g-held" || c == nil || c.ID != f.privC.ID { + t.Fatalf("got %v, %v, %v", res, c, err) + } + rows, _ := p.ListByBookAndMediaType(f.ctx, f.book.ID, models.MediaTypeEbook) + if len(rows) != 0 { + t.Fatalf("rows = %d, want the taken release deleted", len(rows)) + } + }) + + t.Run("a corrupt stored release is passed over, not fatal", func(t *testing.T) { + f, p := newFixture(t) + f.holdPending(t, p, "g-good", f.idx) + if err := p.Upsert(f.ctx, &models.PendingRelease{ + BookID: f.book.ID, MediaType: models.MediaTypeEbook, Title: "bad", GUID: "g-bad", Protocol: "usenet", Reason: "delay", ReleaseJSON: "{not json", + }); err != nil { + t.Fatal(err) + } + dm := decision.New() + res, _, err := f.s.pendingApproved(f.ctx, f.book, models.MediaTypeEbook, dm, nil) + if err != nil || res == nil || res.GUID != "g-good" { + t.Fatalf("got %v, %v; want the good row", res, err) + } + }) + + t.Run("other media type rows are not considered", func(t *testing.T) { + f, p := newFixture(t) + f.holdPending(t, p, "g-held", f.idx) + dm := decision.New() + res, _, err := f.s.pendingApproved(f.ctx, f.book, models.MediaTypeAudiobook, dm, nil) + if err != nil || res != nil { + t.Fatalf("got %v, %v; want nothing for the other format", res, err) + } + }) +} From 13df90dbc7a81a335514831784a02591fa9d3a4b Mon Sep 17 00:00:00 2001 From: vavallee Date: Fri, 16 Oct 2026 01:40:05 -0300 Subject: [PATCH 2/2] test: pin the release protocol in clientForGrab and drop questionable assertions Adds a clientForGrab subtest where the release protocol disagrees with the pinned client, in both directions. Removes the unknown indexer id fallback row, the Torznab case row and the two letter surname row, and deletes the low value indexer relevance test file and three content helper subtests. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9 Signed-off-by: vavallee --- internal/api/pin_edges_test.go | 26 +++- internal/importer/content_check_edges_test.go | 32 ----- .../indexer/title_relevance_edges_test.go | 121 ------------------ internal/models/indexer_test.go | 1 - 4 files changed, 22 insertions(+), 158 deletions(-) delete mode 100644 internal/indexer/title_relevance_edges_test.go diff --git a/internal/api/pin_edges_test.go b/internal/api/pin_edges_test.go index 6dc835a3d..b408f94cc 100644 --- a/internal/api/pin_edges_test.go +++ b/internal/api/pin_edges_test.go @@ -73,10 +73,28 @@ func TestClientForGrab_Routing(t *testing.T) { t.Fatalf("got %v, %v, %v", c, pin, err) } }) - t.Run("an id that matches no indexer falls back to the default", func(t *testing.T) { - c, pin, err := f.h.clientForGrab(ctx, id(424242), "usenet", "") - if err != nil || pin != nil || c.ID != f.pubC.ID { - t.Fatalf("got %v, %v, %v", c, pin, err) + t.Run("the release protocol must match the pinned client", func(t *testing.T) { + f.pin(t, &f.privC.ID) + c, pin, err := f.h.clientForGrab(ctx, &f.idx.ID, "torrent", "") + var perr *db.PinnedClientError + if c != nil || pin != nil || !errors.As(err, &perr) || perr.Reason != db.PinClientWrongProtocol { + t.Fatalf("usenet pinned indexer, torrent release: got %v, %v, %v", c, pin, err) + } + tor := &models.DownloadClient{Name: "qbit", Type: "qbittorrent", Host: "h", Port: 1, Enabled: true} + if err := f.clients.Create(ctx, tor); err != nil { + t.Fatal(err) + } + tix := &models.Indexer{Name: "TorX", Type: "torznab", URL: "http://tor.invalid/api", Enabled: true, DownloadClientID: &tor.ID} + if err := f.indexers.Create(ctx, tix); err != nil { + t.Fatal(err) + } + c, pin, err = f.h.clientForGrab(ctx, &tix.ID, "usenet", "") + if c != nil || pin != nil || !errors.As(err, &perr) || perr.Reason != db.PinClientWrongProtocol { + t.Fatalf("torrent pinned indexer, usenet release: got %v, %v, %v", c, pin, err) + } + c, _, err = f.h.clientForGrab(ctx, &tix.ID, "torrent", "") + if err != nil || c == nil || c.ID != tor.ID { + t.Fatalf("matching protocol: got %v, %v", c, err) } }) t.Run("a pinned indexer gets its client and a pin target", func(t *testing.T) { diff --git a/internal/importer/content_check_edges_test.go b/internal/importer/content_check_edges_test.go index 28c8a00a6..14d294584 100644 --- a/internal/importer/content_check_edges_test.go +++ b/internal/importer/content_check_edges_test.go @@ -40,7 +40,6 @@ func TestBookIsCompilationBy_BestOfNeedsTheAuthorsName(t *testing.T) { {"The Best of Me", "Nicholas Sparks", false}, {"Best of Friends", "Nicholas Sparks", false}, {"The Best of Poe", "", false}, - {"The Best of Li", "Wei Li", false}, {"The Best of Poet Laureates", "Edgar Allan Poe", false}, {"Some Ordinary Title", "Edgar Allan Poe", false}, } @@ -125,17 +124,6 @@ func TestAuthorNameAndAuthorNamesFor(t *testing.T) { } func TestContentTitleHelpers(t *testing.T) { - t.Run("genericSubtitleTail", func(t *testing.T) { - cases := map[string]bool{ - "": false, "A Novel": true, "Part Two": true, "Book 3": true, "An Unabridged Audiobook": true, - "Messiah": false, "The Chronicles of Narnia": false, - } - for in, want := range cases { - if got := genericSubtitleTail(in); got != want { - t.Errorf("genericSubtitleTail(%q) = %v, want %v", in, got, want) - } - } - }) t.Run("containsWholeWords", func(t *testing.T) { cases := []struct { hay, needle string @@ -156,13 +144,6 @@ func TestContentTitleHelpers(t *testing.T) { } } }) - t.Run("isDigits", func(t *testing.T) { - for in, want := range map[string]bool{"": false, "0": true, "123": true, "12a": false, "1.5": false, "-1": false} { - if got := isDigits(in); got != want { - t.Errorf("isDigits(%q) = %v, want %v", in, got, want) - } - } - }) t.Run("stripSeriesSuffix", func(t *testing.T) { cases := map[string]string{ "Mary, Mary: Alex Cross, Book 11": "Mary, Mary", @@ -180,19 +161,6 @@ func TestContentTitleHelpers(t *testing.T) { } } }) - t.Run("genericTrackTitle", func(t *testing.T) { - cases := map[string]bool{ - "": false, "Chapter 12": true, "Track 03": true, "Chapter 3 of 12": true, "12": true, - "Chapter One Two Three Four Five": false, - "Prologue": true, - "The Hobbit": false, - } - for in, want := range cases { - if got := genericTrackTitle(in); got != want { - t.Errorf("genericTrackTitle(%q) = %v, want %v", in, got, want) - } - } - }) t.Run("volumeStem", func(t *testing.T) { cases := []struct { in string diff --git a/internal/indexer/title_relevance_edges_test.go b/internal/indexer/title_relevance_edges_test.go deleted file mode 100644 index 5039c9410..000000000 --- a/internal/indexer/title_relevance_edges_test.go +++ /dev/null @@ -1,121 +0,0 @@ -package indexer - -import ( - "reflect" - "testing" -) - -func TestNewSubtitleInfo(t *testing.T) { - cases := []struct { - title string - has bool - seriesLike bool - n int - }{ - {"Plain Title", false, false, 0}, - {": Leading Colon", false, false, 0}, - {"Main:", false, false, 0}, - {"Main: ", false, false, 0}, - {"Main: Book 2", true, true, 2}, - {"Main: A Real Subtitle", true, false, 0}, - } - for _, tc := range cases { - si := newSubtitleInfo(tc.title) - if si.has != tc.has || si.seriesLike != tc.seriesLike || si.n != tc.n { - t.Errorf("newSubtitleInfo(%q) = %+v, want has=%v seriesLike=%v n=%d", tc.title, si, tc.has, tc.seriesLike, tc.n) - } - } - if si := newSubtitleInfo("Main: A Real Subtitle"); len(si.ident) == 0 { - t.Error("a real subtitle must carry identity words") - } -} - -func TestMainTitleOnly_ReleaseNamingTheSubtitleIsKept(t *testing.T) { - si := newSubtitleInfo("Main: A Real Subtitle") - anyFull := func(string) bool { t.Fatal("anyFull consulted although the release names the subtitle"); return false } - got := si.mainTitleOnly(nil, "Main A Real Subtitle epub", "main a real subtitle epub", "main real subtitle", anyFull) - if got != "" { - t.Fatalf("got %q, want keep", got) - } -} - -func TestMainTitleOnly_FullTitleElsewhereInBatchDropsTheSibling(t *testing.T) { - si := newSubtitleInfo("Main: A Real Subtitle") - var asked string - anyFull := func(mt string) bool { asked = mt; return true } - got := si.mainTitleOnly(nil, "Main epub", "main epub", "main", anyFull) - if got != reasonMainTitleOnlyFullFound { - t.Fatalf("got %q, want %q", got, reasonMainTitleOnlyFullFound) - } - if asked != "ebook" { - t.Fatalf("anyFull asked about %q, want the release's own media type", asked) - } - - // A release that names no format has no media type to compare, so the - // batch rule cannot apply and it is kept. - anyFull = func(string) bool { t.Fatal("anyFull consulted for a release with no media type"); return false } - if got := si.mainTitleOnly(nil, "Main", "main", "main", anyFull); got != "" { - t.Fatalf("got %q, want keep", got) - } -} - -func TestFullTitleCounts(t *testing.T) { - ident := []string{"main", "title"} - authors := [][]string{{"jane", "austen"}} - count := func(raw string) bool { - return fullTitleCounts(raw, NormalizeRelease(raw), authors, ident) - } - cases := []struct { - raw string - want bool - }{ - {"Main Title", true}, - {"Main Title epub", true}, - {"Main Title - Jane Austen epub", true}, - {"Main Title Summary epub", false}, - {"Main Title Study Guide", false}, - {"Main Title: Insights and Key Takeaways", false}, - {"Unrelated Words epub", false}, - {"Main Title and Some Unexplained Words", false}, - } - for _, tc := range cases { - if got := count(tc.raw); got != tc.want { - t.Errorf("fullTitleCounts(%q) = %v, want %v", tc.raw, got, tc.want) - } - } - if fullTitleCounts("Main Title epub", NormalizeRelease("Main Title epub"), authors) { - t.Error("with no identity phrase the title cannot be located and must not count") - } -} - -func TestNumbersAfter(t *testing.T) { - ident := []string{"main", "title"} - cases := []struct { - name string - raw string - guarded bool - want []int - }{ - {"nothing follows", "Main Title epub", true, nil}, - {"book marker then digit", "Main Title Book 3 epub", true, []int{3}}, - {"bare digit ending the release", "Main Title 4", true, []int{4}}, - {"a digit before an ordinary word is a count when guarded", "Main Title 4 epub", true, nil}, - {"and a number when not guarded", "Main Title 4 epub", false, []int{4}}, - {"capital roman numeral", "Main Title III epub", true, []int{3}}, - {"number word after a marker", "Main Title Book Two epub", true, []int{2}}, - {"a bitrate is a count when guarded", "Main Title 64 kbps", true, nil}, - {"the same digits are read when not guarded", "Main Title 64 kbps", false, []int{64}}, - {"title not in the release", "Other Words 3", true, nil}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got := numbersAfter(tc.raw, NormalizeRelease(tc.raw), tc.guarded, ident) - if len(got) == 0 && len(tc.want) == 0 { - return - } - if !reflect.DeepEqual(got, tc.want) { - t.Fatalf("numbersAfter(%q, guarded=%v) = %v, want %v", tc.raw, tc.guarded, got, tc.want) - } - }) - } -} diff --git a/internal/models/indexer_test.go b/internal/models/indexer_test.go index 5ae670b37..8aac456cb 100644 --- a/internal/models/indexer_test.go +++ b/internal/models/indexer_test.go @@ -7,7 +7,6 @@ func TestIndexerProtocol(t *testing.T) { "torznab": "torrent", "newznab": "usenet", "": "usenet", - "Torznab": "usenet", "whatever": "usenet", } for typ, want := range cases {