diff --git a/README.md b/README.md index d50d06aeb..a45ab2ba0 100644 --- a/README.md +++ b/README.md @@ -263,7 +263,7 @@ The full reference (path remapping, API-key seeding, telemetry, trusted-proxy, r | **Authentication** | Local (argon2id), API key, OIDC (Google, GitHub via Dex, Authelia, Keycloak, …), forward-auth proxy | | **Reading apps** | OPDS 1.2 catalogue at `/opds/` (KOReader, Moon+ Reader, Aldiko, …) | -All download clients support **Use SSL** and **URL Base** for connections through a reverse-proxy subpath. +All download clients support **Use SSL** and **URL Base** for connections through a reverse-proxy subpath, plus independent **eligible for books / eligible for audiobooks** toggles (both on by default) so a grab only reaches clients configured for that media type. Bindery sends to the highest-ranked eligible client; if sending fails, it reports the error without retrying another client because the first may already have accepted the release. ## Architecture diff --git a/changelog.d/download-client-media-eligibility.md b/changelog.d/download-client-media-eligibility.md new file mode 100644 index 000000000..30e82afe8 --- /dev/null +++ b/changelog.d/download-client-media-eligibility.md @@ -0,0 +1,2 @@ +### Added +- **Download client media eligibility and priority** — download clients now feature independent "Eligible for books" and "Eligible for audiobooks" toggles (both on by default) and a configurable Priority. Priority is considered before category hints, so existing installs may select a different client. Grabs route to the highest-ranked eligible client; a failed send is reported rather than retried because the client may already have accepted it. diff --git a/docs/API.md b/docs/API.md index af55737ff..374ec01d1 100644 --- a/docs/API.md +++ b/docs/API.md @@ -883,6 +883,15 @@ secret stays on the row. Booleans follow the same rule: omitting `enabled` or `useSsl` leaves them as they were, and an explicitly sent `false` still turns them off. +`enabledForBooks` and `enabledForAudiobooks` gate which media type a client is +considered for at grab time, independent of `category`/`categoryAudiobook`. +Both default `true` on create when omitted, so a client handles every media +type until you opt it out of one. A grab is routed only to clients eligible +for its media type, ranked by priority and category hints, and sent to the +highest-ranked client. A failed send is not retried on another client because +the first may already have accepted it. If none are eligible, the grab fails +with an error naming the media type. + `POST /api/v1/downloadclient/test` probes a config without saving it. Include the saved client's `id` and the handler fills in a credential you left blank from that row, but only while `type`, `host`, `port`, `useSsl` and `urlBase` diff --git a/internal/api/destructive_handlers_test.go b/internal/api/destructive_handlers_test.go index 32e37bd6f..08a71af19 100644 --- a/internal/api/destructive_handlers_test.go +++ b/internal/api/destructive_handlers_test.go @@ -395,7 +395,7 @@ func TestPendingGrab_DispatchesDownloadAndClearsPending(t *testing.T) { pending := db.NewPendingReleaseRepo(database) host, port := testServerHostPort(t, srv.URL) - client := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true} + client := &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, EnabledForBooks: false, EnabledForAudiobooks: true} if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) } @@ -404,7 +404,7 @@ func TestPendingGrab_DispatchesDownloadAndClearsPending(t *testing.T) { if err := authors.Create(ctx, author); err != nil { t.Fatalf("create author: %v", err) } - book := &models.Book{ForeignID: "grab-book", AuthorID: author.ID, Title: "Grab Book", SortTitle: "grab book", MediaType: models.MediaTypeEbook} + book := &models.Book{ForeignID: "grab-book", AuthorID: author.ID, Title: "Grab Book", SortTitle: "grab book", MediaType: models.MediaTypeBoth} if err := books.Create(ctx, book); err != nil { t.Fatalf("create book: %v", err) } @@ -420,7 +420,7 @@ func TestPendingGrab_DispatchesDownloadAndClearsPending(t *testing.T) { }) pr := &models.PendingRelease{ BookID: book.ID, - MediaType: models.MediaTypeEbook, + MediaType: models.MediaTypeAudiobook, Title: "Pending Release Title", GUID: "pending-guid", Protocol: "usenet", diff --git a/internal/api/download_clients.go b/internal/api/download_clients.go index 43eecdc6d..421345892 100644 --- a/internal/api/download_clients.go +++ b/internal/api/download_clients.go @@ -194,8 +194,14 @@ func (h *DownloadClientHandler) Get(w http.ResponseWriter, r *http.Request) { } func (h *DownloadClientHandler) Create(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + writeBodyReadError(w, err) + return + } var c models.DownloadClient - if !decodeJSON(w, r, &c) { + if err := json.Unmarshal(body, &c); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid request body"}) return } if c.Name == "" || c.Host == "" { @@ -214,6 +220,24 @@ func (h *DownloadClientHandler) Create(w http.ResponseWriter, r *http.Request) { if c.Category == "" { c.Category = "books" } + // Eligible for both media types unless the caller explicitly opts a + // field out — omitted means "default true", but an explicit false (e.g. + // a deliberately book-only or audiobook-only client) must stick. Go's + // bool zero value can't tell "omitted" from "false" on its own, so this + // checks the raw body instead of the decoded struct (#2401 fallout: a + // naive "both false -> default true" at the DB layer silently overrode a + // user's genuine "eligible for neither" choice). + raw := map[string]json.RawMessage{} + if err := json.Unmarshal(body, &raw); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid request body"}) + return + } + if _, ok := raw["enabledForBooks"]; !ok { + c.EnabledForBooks = true + } + if _, ok := raw["enabledForAudiobooks"]; !ok { + c.EnabledForAudiobooks = true + } if err := httpsec.ValidateOutboundURL(downloadClientURL(&c), httpsec.PolicyService); err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) return diff --git a/internal/api/error_leak_test.go b/internal/api/error_leak_test.go index 026bb0ff1..97eeb109c 100644 --- a/internal/api/error_leak_test.go +++ b/internal/api/error_leak_test.go @@ -114,7 +114,7 @@ func leakyQueue(t *testing.T) (*QueueHandler, *db.DownloadRepo, string) { h, _, downloads, clients, _, ctx := queueFixture(t) host, port := testServerHostPort(t, sab.URL) - if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true}); err != nil { + if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true}); err != nil { t.Fatal(err) } return h, downloads, indexerSrv.URL + "/x.nzb" diff --git a/internal/api/indexer_pin_grab_test.go b/internal/api/indexer_pin_grab_test.go index 31b781320..46170bb94 100644 --- a/internal/api/indexer_pin_grab_test.go +++ b/internal/api/indexer_pin_grab_test.go @@ -84,12 +84,12 @@ func newPinFixture(t *testing.T) *pinFixture { rel := newPinSAB(t) pubHost, pubPort := testServerHostPort(t, pub.srv.URL) - pubC := &models.DownloadClient{Name: "sab-public", Type: "sabnzbd", Host: pubHost, Port: pubPort, Enabled: true, Priority: 1} + pubC := &models.DownloadClient{Name: "sab-public", Type: "sabnzbd", Host: pubHost, Port: pubPort, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, Priority: 1} if err := clients.Create(ctx, pubC); err != nil { t.Fatalf("create public client: %v", err) } privHost, privPort := testServerHostPort(t, priv.srv.URL) - privC := &models.DownloadClient{Name: "sab-private", Type: "sabnzbd", Host: privHost, Port: privPort, Enabled: true, Priority: 50} + privC := &models.DownloadClient{Name: "sab-private", Type: "sabnzbd", Host: privHost, Port: privPort, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, Priority: 50} if err := clients.Create(ctx, privC); err != nil { t.Fatalf("create private client: %v", err) } diff --git a/internal/api/indexers.go b/internal/api/indexers.go index e8c517bea..c962146ac 100644 --- a/internal/api/indexers.go +++ b/internal/api/indexers.go @@ -1081,6 +1081,18 @@ func (h *IndexerHandler) SearchQuery(w http.ResponseWriter, r *http.Request) { h.searchResults.remember(results) // Strip every credential before returning to the client (see SearchBook). for i := range results { + if results[i].MediaType == "" { + formats := indexer.ReleaseFormats(results[i].Title) + if len(formats) > 0 { + results[i].MediaType = models.MediaTypeEbook + for _, format := range formats { + if indexer.MediaTypeForFormat(format) == models.MediaTypeAudiobook { + results[i].MediaType = models.MediaTypeAudiobook + break + } + } + } + } redactSearchResult(&results[i]) } writeJSON(w, http.StatusOK, results) diff --git a/internal/api/indexers_test.go b/internal/api/indexers_test.go index 156292b76..96008ffd8 100644 --- a/internal/api/indexers_test.go +++ b/internal/api/indexers_test.go @@ -24,6 +24,7 @@ import ( type mockIndexerSearcher struct { ebookResults []newznab.SearchResult audioResults []newznab.SearchResult + queryResults []newznab.SearchResult // SearchBook runs the ebook and audiobook legs of a dual-format book // concurrently, so both goroutines land here. mu guards the recorded @@ -81,7 +82,7 @@ func (m *mockIndexerSearcher) SearchBookWithDebug(_ context.Context, _ []models. } func (m *mockIndexerSearcher) SearchQuery(_ context.Context, _ []models.Indexer, _ string) []newznab.SearchResult { - return nil + return m.queryResults } func indexerFixture(t *testing.T) *IndexerHandler { @@ -369,6 +370,32 @@ func TestIndexerSearchQuery_MissingQ(t *testing.T) { } } +func TestIndexerSearchQuery_DetectsMediaTypeFromReleaseTitle(t *testing.T) { + h := indexerFixture(t) + h.searcher = &mockIndexerSearcher{queryResults: []newznab.SearchResult{ + {Title: "A Novel.m4b + PDF"}, + {Title: "A Novel.epub"}, + }} + rec := httptest.NewRecorder() + h.SearchQuery(rec, httptest.NewRequest(http.MethodGet, "/indexer/search?q=novel", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", rec.Code, rec.Body.String()) + } + var results []newznab.SearchResult + if err := json.NewDecoder(rec.Body).Decode(&results); err != nil { + t.Fatal(err) + } + if len(results) != 2 { + t.Fatalf("got %d results, want 2", len(results)) + } + if results[0].MediaType != models.MediaTypeAudiobook { + t.Errorf("mixed audiobook release media type = %q, want audiobook", results[0].MediaType) + } + if results[1].MediaType != models.MediaTypeEbook { + t.Errorf("ebook release media type = %q, want ebook", results[1].MediaType) + } +} + func TestSearchBook_DualFormat_MediaTypeTagging(t *testing.T) { database, err := db.OpenMemory() if err != nil { diff --git a/internal/api/pin_edges_test.go b/internal/api/pin_edges_test.go index b408f94cc..a7194b95b 100644 --- a/internal/api/pin_edges_test.go +++ b/internal/api/pin_edges_test.go @@ -80,7 +80,7 @@ func TestClientForGrab_Routing(t *testing.T) { 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} + tor := &models.DownloadClient{Name: "qbit", Type: "qbittorrent", Host: "h", Port: 1, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true} if err := f.clients.Create(ctx, tor); err != nil { t.Fatal(err) } diff --git a/internal/api/queue.go b/internal/api/queue.go index 1d8dfd78e..bb59d0985 100644 --- a/internal/api/queue.go +++ b/internal/api/queue.go @@ -119,6 +119,14 @@ func pinnedMessage(ctx context.Context, perr *db.PinnedClientError) string { return perr.Message(isAdminCtx(ctx)) } +func isNoEligibleMediaTypeError(err error) bool { + if err == nil { + return false + } + message := err.Error() + return strings.Contains(message, "no enabled ") && strings.Contains(message, " download client is eligible for") +} + // writeGrabError answers an error from grab or resendRelease. A missing client // is a setup problem the user can fix (400, with a code the web app keys on); // a refused or unreachable client is a generic 502, since the text comes from @@ -135,6 +143,8 @@ func writeGrabError(w http.ResponseWriter, r *http.Request, err error) { writeJSON(w, http.StatusBadRequest, map[string]string{"error": pinnedMessage(r.Context(), perr), "code": codeIndexerClientUnavailable}) case errors.Is(err, errNoDownloadClient): writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error(), "code": codeNoDownloadClient}) + case isNoEligibleMediaTypeError(err): + writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) case errors.Is(err, errSendFailed): slog.Warn("grab: download client did not take the release", "method", r.Method, "path", r.URL.Path, "error", err) writeJSON(w, http.StatusBadGateway, map[string]string{"error": sendFailedText(r.Context(), err)}) @@ -161,7 +171,8 @@ func queueItemMessage(ctx context.Context, err error) string { case errors.As(err, &perr): return pinnedMessage(ctx, perr) case errors.Is(err, errNoDownloadClient), errors.Is(err, errAlreadyGrabbed), - errors.Is(err, errRetryNotResendable), errors.Is(err, errGrabBookNotFound): + errors.Is(err, errRetryNotResendable), errors.Is(err, errGrabBookNotFound), + isNoEligibleMediaTypeError(err): return err.Error() case errors.Is(err, errSendFailed): slog.Warn("bulk queue item: download client did not take the release", "error", err) @@ -1374,15 +1385,19 @@ func (h *QueueHandler) BulkRetry(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, queueRetryResponse{Results: results}) } -// selectClient picks the best enabled client for the given protocol and media type. -// It prefers a client whose category hints match the media type when multiple -// clients of the same protocol type are configured. +// selectClients returns every enabled client of the given protocol that is +// eligible for the given media type, ranked best-first (priority, then +// category hint) — see db.FilterEligibleForMediaType and db.RankClientsForMediaType. +// grab() sends to the highest-ranked eligible client. A failed response can +// be ambiguous about whether the client accepted the release, so it must not +// be retried against another client automatically. // // When no client of the requested protocol is enabled, the returned error names // the protocol and — if the user has enabled clients of the *other* protocol — // spells out the mismatch so they don't re-check the client they already -// verified. See noProtocolClientError. -func (h *QueueHandler) selectClient(ctx context.Context, protocol, mediaType string) (*models.DownloadClient, error) { +// verified. See noProtocolClientError. When clients of the right protocol exist +// but none opted in to this media type, see noEligibleMediaTypeError. +func (h *QueueHandler) selectClients(ctx context.Context, protocol, mediaType string) ([]models.DownloadClient, error) { candidates, err := h.clients.GetEnabledByProtocol(ctx, protocol) if err != nil { return nil, err @@ -1390,7 +1405,31 @@ func (h *QueueHandler) selectClient(ctx context.Context, protocol, mediaType str if len(candidates) == 0 { return nil, h.noProtocolClientError(ctx, protocol) } - return db.PickClientForMediaType(candidates, mediaType), nil + eligible := db.FilterEligibleForMediaType(candidates, mediaType) + if len(eligible) == 0 { + otherProtocol := otherProtocol(protocol) + otherCandidates, otherErr := h.clients.GetEnabledByProtocol(ctx, otherProtocol) + if otherErr == nil && len(db.FilterEligibleForMediaType(otherCandidates, mediaType)) > 0 { + mediaLabel := "books" + if mediaType == models.MediaTypeAudiobook { + mediaLabel = "audiobooks" + } + return nil, fmt.Errorf("no enabled %s download client is eligible for %s; the eligible %s client cannot handle this release, so configure an eligible %s client", protocolLabel(protocol), mediaLabel, otherProtocol, protocolLabel(protocol)) + } + return nil, noEligibleMediaTypeError(mediaType) + } + return db.RankClientsForMediaType(eligible, mediaType), nil +} + +// noEligibleMediaTypeError builds the "no enabled download client is eligible" +// error for when every enabled client of the right protocol has opted out of +// this release's media type (EnabledForBooks / EnabledForAudiobooks). +func noEligibleMediaTypeError(mediaType string) error { + label := "books" + if mediaType == models.MediaTypeAudiobook { + label = "audiobooks" + } + return fmt.Errorf("no enabled download client is eligible for %s — enable a download client for %s in Settings", label, label) } // clientForGrab picks the client for a grab. An indexer pinned to a download @@ -1411,11 +1450,17 @@ func (h *QueueHandler) clientForGrab(ctx context.Context, indexerID *int64, prot if err != nil { return nil, nil, err } + if len(db.FilterEligibleForMediaType([]models.DownloadClient{*c}, mediaType)) == 0 { + return nil, nil, noEligibleMediaTypeError(mediaType) + } return c, &pinTarget{indexer: idx.Name, client: c.Name}, nil } } - c, err := h.selectClient(ctx, protocol, mediaType) - return c, nil, err + candidates, err := h.selectClients(ctx, protocol, mediaType) + if err != nil { + return nil, nil, err + } + return &candidates[0], nil, nil } // noProtocolClientError builds an actionable "no enabled download client" error. @@ -1623,7 +1668,6 @@ func (h *QueueHandler) grab(ctx context.Context, req grabRequest) (*models.Downl // per-indexer overrides are lost (#2053). indexerID = hostMatched } - // The client is chosen only now, after the host match above has settled // which indexer the release came from: an indexer pinned to a download // client (#3080) must be found even when the caller sent no indexer id. @@ -1638,7 +1682,6 @@ func (h *QueueHandler) grab(ctx context.Context, req grabRequest) (*models.Downl return nil, h.noProtocolClientError(ctx, req.Protocol) } protocol := downloader.ProtocolForClient(client.Type) - dl := &models.Download{ GUID: req.GUID, BookID: bookID, @@ -1690,17 +1733,17 @@ func (h *QueueHandler) grab(ctx context.Context, req grabRequest) (*models.Downl GlobalRemap: h.downloadPathRemap, }.WithSeedLimits(h.resolveSeedLimits(ctx, indexerID))) if err != nil { - slog.Error("failed to send download", "client_type", client.Type, "error", err, "title", req.Title) - if setErr := h.downloads.SetError(ctx, dl.ID, err.Error()); setErr != nil { + slog.Error("failed to send download to eligible client", "client", client.Name, "error", err, "title", req.Title) + wrapped := fmt.Errorf("failed to send to eligible download client %s: %w", client.Name, err) + if setErr := h.downloads.SetError(ctx, dl.ID, wrapped.Error()); setErr != nil { slog.Warn("failed to persist download error", "download_id", dl.ID, "error", setErr) } - h.recordHistory(ctx, models.HistoryEventDownloadFailed, req.Title, bookID, map[string]any{"guid": req.GUID, "message": err.Error()}) + h.recordHistory(ctx, models.HistoryEventDownloadFailed, req.Title, bookID, map[string]any{"guid": req.GUID, "message": wrapped.Error()}) if h.notif != nil { - h.notif.Send(ctx, notifier.EventDownloadFailed, map[string]any{"title": req.Title, "message": err.Error()}) + h.notif.Send(ctx, notifier.EventDownloadFailed, map[string]any{"title": req.Title, "message": wrapped.Error()}) } return nil, &sendError{cause: err, pin: pin} } - if remoteID := sendRes.RemoteID; remoteID != "" { if sendRes.UsesTorrentID { normalised := strings.ToLower(remoteID) diff --git a/internal/api/queue_coverage_test.go b/internal/api/queue_coverage_test.go index 1272fe2d4..74e37b5ee 100644 --- a/internal/api/queue_coverage_test.go +++ b/internal/api/queue_coverage_test.go @@ -329,7 +329,7 @@ func TestQueueRetryDownloadClientFailureIs502(t *testing.T) { h, _, downloads, clients, _, ctx := queueFixture(t) host, port := testServerHostPort(t, sab.URL) - if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true}); err != nil { + if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true}); err != nil { t.Fatal(err) } dl := &models.Download{GUID: "covq-502", Title: "Refused", Status: models.StateFailed, Protocol: "usenet", NZBURL: indexerSrv.URL + "/x.nzb"} @@ -435,7 +435,7 @@ func TestQueueGrabRecordsFailureHistory(t *testing.T) { t.Cleanup(sab.Close) h, database, downloads, clients, _, ctx := queueFixture(t) host, port := testServerHostPort(t, sab.URL) - if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true}); err != nil { + if err := clients.Create(ctx, &models.DownloadClient{Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true}); err != nil { t.Fatal(err) } _, err := h.grab(ctx, grabRequest{GUID: "covq-grabfail", Title: "Some.Book.epub", NZBURL: sab.URL + "/x.nzb"}) diff --git a/internal/api/queue_grab_category_test.go b/internal/api/queue_grab_category_test.go index bbb0652cd..af620b7b9 100644 --- a/internal/api/queue_grab_category_test.go +++ b/internal/api/queue_grab_category_test.go @@ -70,6 +70,7 @@ func categoryFixture(t *testing.T, srvURL string) (*QueueHandler, *sql.DB, *mode host, port := testServerHostPort(t, srvURL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, Category: ebookCategory, CategoryAudiobook: audiobookCategory, }); err != nil { t.Fatalf("create client: %v", err) diff --git a/internal/api/queue_grab_history_indexer_test.go b/internal/api/queue_grab_history_indexer_test.go index 86ace20dd..4b75d3062 100644 --- a/internal/api/queue_grab_history_indexer_test.go +++ b/internal/api/queue_grab_history_indexer_test.go @@ -39,6 +39,7 @@ func TestQueueGrab_HistoryRecordsResolvedIndexerID(t *testing.T) { host, port := testServerHostPort(t, sab.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_grab_remap_test.go b/internal/api/queue_grab_remap_test.go index 1377b8d8c..3ebc696a3 100644 --- a/internal/api/queue_grab_remap_test.go +++ b/internal/api/queue_grab_remap_test.go @@ -42,6 +42,7 @@ func TestQueueGrab_SavePathUsesGlobalRemap(t *testing.T) { host, port := testServerHostPort(t, srv.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "qbit", Type: "qbittorrent", Host: host, Port: port, Username: "u", Password: "p", Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_grab_security_test.go b/internal/api/queue_grab_security_test.go index c98ef7d25..7e2d03f67 100644 --- a/internal/api/queue_grab_security_test.go +++ b/internal/api/queue_grab_security_test.go @@ -117,6 +117,7 @@ func newGrabSecFixture(t *testing.T) *grabSecFixture { host, port := testServerHostPort(t, sab.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_grab_sign_test.go b/internal/api/queue_grab_sign_test.go index 9aaa32a39..2d5641c54 100644 --- a/internal/api/queue_grab_sign_test.go +++ b/internal/api/queue_grab_sign_test.go @@ -42,6 +42,7 @@ func TestQueueGrab_SignsDownloadURLWithoutIndexerID(t *testing.T) { host, port := testServerHostPort(t, sab.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } @@ -178,6 +179,7 @@ func TestQueueGrab_ResponseAndListNeverCarryIndexerAPIKey(t *testing.T) { host, port := testServerHostPort(t, sab.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_regrab_test.go b/internal/api/queue_regrab_test.go index 19f42a89f..85ac24356 100644 --- a/internal/api/queue_regrab_test.go +++ b/internal/api/queue_regrab_test.go @@ -117,6 +117,7 @@ func TestQueueGrab_ImportBlockedIsRegrabbable(t *testing.T) { host, port := testServerHostPort(t, srv.URL) if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } @@ -400,6 +401,7 @@ func regrabDownloadClient(t *testing.T, clients *db.DownloadClientRepo) (string, host, port := testServerHostPort(t, srv.URL) if err := clients.Create(context.Background(), &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_retry_test.go b/internal/api/queue_retry_test.go index 0e415cf50..1a00cae93 100644 --- a/internal/api/queue_retry_test.go +++ b/internal/api/queue_retry_test.go @@ -58,11 +58,13 @@ func retryFixture(t *testing.T, adds *sabAddRecorder) (*QueueHandler, *sql.DB, * h, database, downloads, clients, _, ctx := queueFixture(t) host, port := testServerHostPort(t, sab.URL) if err := clients.Create(ctx, &models.DownloadClient{ - Name: "sab", - Type: "sabnzbd", - Host: host, - Port: port, - Enabled: true, + Name: "sab", + Type: "sabnzbd", + Host: host, + Port: port, + Enabled: true, + EnabledForBooks: true, + EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create client: %v", err) } diff --git a/internal/api/queue_test.go b/internal/api/queue_test.go index 02ae6c980..d1b2ee073 100644 --- a/internal/api/queue_test.go +++ b/internal/api/queue_test.go @@ -76,13 +76,15 @@ func TestQueueGrab_NoDownloadClient(t *testing.T) { func TestQueueGrab_ProtocolMismatchNamesUsenet(t *testing.T) { h, _, _, clients, _, ctx := queueFixture(t) if err := clients.Create(ctx, &models.DownloadClient{ - Name: "qb", - Type: "qbittorrent", - Host: "127.0.0.1", - Port: 8080, - Username: "user", - Password: "pass", - Enabled: true, + Name: "qb", + Type: "qbittorrent", + Host: "127.0.0.1", + Port: 8080, + Username: "user", + Password: "pass", + Enabled: true, + EnabledForBooks: true, + EnabledForAudiobooks: true, }); err != nil { t.Fatalf("create torrent client: %v", err) } @@ -175,11 +177,13 @@ func TestQueueGrab_RetriesFailedGUID(t *testing.T) { h, database, downloads, clients, books, ctx := queueFixture(t) host, port := testServerHostPort(t, srv.URL) client := &models.DownloadClient{ - Name: "sab", - Type: "sabnzbd", - Host: host, - Port: port, - Enabled: true, + Name: "sab", + Type: "sabnzbd", + Host: host, + Port: port, + Enabled: true, + EnabledForBooks: true, + EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) @@ -311,11 +315,13 @@ func TestQueueGrab_FailedRetryFailureRemainsRetryable(t *testing.T) { h, _, downloads, clients, _, ctx := queueFixture(t) host, port := testServerHostPort(t, srv.URL) client := &models.DownloadClient{ - Name: "sab", - Type: "sabnzbd", - Host: host, - Port: port, - Enabled: true, + Name: "sab", + Type: "sabnzbd", + Host: host, + Port: port, + Enabled: true, + EnabledForBooks: true, + EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) @@ -632,6 +638,7 @@ func TestQueueBulkDelete_HonoursRemoveFromClient(t *testing.T) { client := &models.DownloadClient{ Name: "qb", Type: "qbittorrent", Host: host, Port: port, Username: "user", Password: "pass", Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) @@ -783,6 +790,7 @@ func queueDeleteFilesProbe(t *testing.T, urlSuffix string) string { client := &models.DownloadClient{ Name: "qb", Type: "qbittorrent", Host: host, Port: port, Username: "user", Password: "pass", Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) @@ -854,6 +862,7 @@ func queueClientCallProbe(t *testing.T, urlSuffix string) (clientCalled bool, ro client := &models.DownloadClient{ Name: "qb", Type: "qbittorrent", Host: host, Port: port, Username: "user", Password: "pass", Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatalf("create client: %v", err) @@ -1824,8 +1833,230 @@ func newQueueTestHandler(t *testing.T) *QueueHandler { return NewQueueHandler(db.NewDownloadRepo(database), db.NewDownloadClientRepo(database), nil, nil) } +// sabTestServer starts a fake SABnzbd server whose addfile response reports +// SAB-side success or failure, and counts how many times it was called. +func sabTestServer(t *testing.T, ok bool) (*httptest.Server, *int) { + t.Helper() + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + if !ok { + _ = json.NewEncoder(w).Encode(map[string]any{"status": false}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"status": true, "nzo_ids": []string{"nzo-ok"}}) + })) + t.Cleanup(srv.Close) + return srv, &calls +} + +// TestQueueGrab_SkipsClientIneligibleForMediaType verifies that a client +// opted out of a media type is never sent an audiobook grab, even though it +// is otherwise enabled and protocol-matched. +func TestQueueGrab_SkipsClientIneligibleForMediaType(t *testing.T) { + defer httpsec.AllowLoopbackForTests()() + indexerSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(``)) + })) + defer indexerSrv.Close() + bookOnlySrv, bookOnlyCalls := sabTestServer(t, true) + audioSrv, audioCalls := sabTestServer(t, true) + + h, _, _, clients, _, ctx := queueFixture(t) + bookOnlyHost, bookOnlyPort := testServerHostPort(t, bookOnlySrv.URL) + if err := clients.Create(ctx, &models.DownloadClient{ + Name: "book-only", Type: "sabnzbd", Host: bookOnlyHost, Port: bookOnlyPort, + Priority: 1, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: false, + }); err != nil { + t.Fatalf("create book-only client: %v", err) + } + audioHost, audioPort := testServerHostPort(t, audioSrv.URL) + if err := clients.Create(ctx, &models.DownloadClient{ + Name: "audio-eligible", Type: "sabnzbd", Host: audioHost, Port: audioPort, + Priority: 2, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + }); err != nil { + t.Fatalf("create audiobook-eligible client: %v", err) + } + + body := bytes.NewBufferString(`{"guid":"audio-guid","nzbUrl":"` + indexerSrv.URL + `/x.nzb","title":"t","mediaType":"audiobook"}`) + rec := httptest.NewRecorder() + h.Grab(rec, httptest.NewRequest(http.MethodPost, "/api/v1/queue/grab", body)) + + if rec.Code != http.StatusAccepted { + t.Fatalf("expected 202, got %d: %s", rec.Code, rec.Body.String()) + } + if *bookOnlyCalls != 0 { + t.Errorf("expected the book-only client never to be contacted, got %d calls", *bookOnlyCalls) + } + if *audioCalls != 1 { + t.Errorf("expected the audiobook-eligible client to receive the grab, got %d calls", *audioCalls) + } +} + +// TestQueueGrab_DoesNotRetryAmbiguousSendFailure verifies that a failed +// response from the best-ranked client is not retried against another client. +func TestQueueGrab_DoesNotRetryAmbiguousSendFailure(t *testing.T) { + defer httpsec.AllowLoopbackForTests()() + indexerSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(``)) + })) + defer indexerSrv.Close() + failingSrv, failingCalls := sabTestServer(t, false) + workingSrv, workingCalls := sabTestServer(t, true) + + h, _, downloads, clients, _, ctx := queueFixture(t) + failHost, failPort := testServerHostPort(t, failingSrv.URL) + failing := &models.DownloadClient{ + Name: "failing", Type: "sabnzbd", Host: failHost, Port: failPort, + Priority: 1, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + } + if err := clients.Create(ctx, failing); err != nil { + t.Fatalf("create failing client: %v", err) + } + workHost, workPort := testServerHostPort(t, workingSrv.URL) + working := &models.DownloadClient{ + Name: "working", Type: "sabnzbd", Host: workHost, Port: workPort, + Priority: 2, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + } + if err := clients.Create(ctx, working); err != nil { + t.Fatalf("create working client: %v", err) + } + + body := bytes.NewBufferString(`{"guid":"fallback-guid","nzbUrl":"` + indexerSrv.URL + `/x.nzb","title":"t"}`) + rec := httptest.NewRecorder() + h.Grab(rec, httptest.NewRequest(http.MethodPost, "/api/v1/queue/grab", body)) + + if rec.Code != http.StatusBadGateway { + t.Fatalf("expected 502, got %d: %s", rec.Code, rec.Body.String()) + } + if *failingCalls != 1 || *workingCalls != 0 { + t.Fatalf("expected only the top-ranked client to be attempted, got failing=%d working=%d", *failingCalls, *workingCalls) + } + got, err := downloads.GetByGUID(ctx, "fallback-guid") + if err != nil || got == nil { + t.Fatalf("reload download: %v", err) + } + if got.DownloadClientID == nil || *got.DownloadClientID != failing.ID { + t.Fatalf("expected the download to remain attributed to the selected client %d, got %v", failing.ID, got.DownloadClientID) + } + if got.Status != models.StateFailed { + t.Fatalf("expected an error status after the selected client failed, got %q", got.Status) + } +} + +// TestQueueGrab_SendFailureDoesNotTryAnotherClient verifies that only the +// highest-ranked eligible client's ambiguous send failure is reported. +func TestQueueGrab_SendFailureDoesNotTryAnotherClient(t *testing.T) { + defer httpsec.AllowLoopbackForTests()() + indexerSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(``)) + })) + defer indexerSrv.Close() + firstSrv, firstCalls := sabTestServer(t, false) + secondSrv, secondCalls := sabTestServer(t, false) + + h, _, _, clients, _, ctx := queueFixture(t) + firstHost, firstPort := testServerHostPort(t, firstSrv.URL) + if err := clients.Create(ctx, &models.DownloadClient{ + Name: "first", Type: "sabnzbd", Host: firstHost, Port: firstPort, + Priority: 1, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + }); err != nil { + t.Fatalf("create first client: %v", err) + } + secondHost, secondPort := testServerHostPort(t, secondSrv.URL) + if err := clients.Create(ctx, &models.DownloadClient{ + Name: "second", Type: "sabnzbd", Host: secondHost, Port: secondPort, + Priority: 2, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + }); err != nil { + t.Fatalf("create second client: %v", err) + } + + body := bytes.NewBufferString(`{"guid":"all-fail-guid","nzbUrl":"` + indexerSrv.URL + `/x.nzb","title":"t"}`) + rec := httptest.NewRecorder() + h.Grab(rec, httptest.NewRequest(http.MethodPost, "/api/v1/queue/grab", body)) + + if rec.Code != http.StatusBadGateway { + t.Fatalf("expected 502, got %d: %s", rec.Code, rec.Body.String()) + } + if *firstCalls != 1 || *secondCalls != 0 { + t.Fatalf("expected only the first-ranked client to be attempted, got first=%d second=%d", *firstCalls, *secondCalls) + } + var resp map[string]string + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp["error"] != sendFailedMessage { + t.Errorf("expected a generic send-failure message, got: %q", resp["error"]) + } +} + +// TestQueueGrab_NoEligibleClientForMediaTypeNamesIt verifies the actionable +// 400 when every enabled, protocol-matched client has opted out of the +// release's media type. +func TestQueueGrab_NoEligibleClientForMediaTypeNamesIt(t *testing.T) { + h, _, _, clients, _, ctx := queueFixture(t) + if err := clients.Create(ctx, &models.DownloadClient{ + Name: "book-only", Type: "sabnzbd", Host: "127.0.0.1", Port: 8080, + Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: false, + }); err != nil { + t.Fatalf("create book-only client: %v", err) + } + + body := bytes.NewBufferString(`{"guid":"no-eligible-guid","nzbUrl":"http://example/x.nzb","title":"t","mediaType":"audiobook"}`) + rec := httptest.NewRecorder() + h.Grab(rec, httptest.NewRequest(http.MethodPost, "/api/v1/queue/grab", body)) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) + } + var resp map[string]string + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if !strings.Contains(resp["error"], "audiobooks") { + t.Errorf("expected error to name the media type (audiobooks), got: %q", resp["error"]) + } +} + +func TestQueueGrab_NoEligibleClientExplainsProtocolMismatch(t *testing.T) { + h, _, _, clients, _, ctx := queueFixture(t) + for _, client := range []models.DownloadClient{ + {Name: "book-usenet", Type: "sabnzbd", Host: "127.0.0.1", Port: 8080, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: false}, + {Name: "audio-torrent", Type: "qbittorrent", Host: "127.0.0.1", Port: 8081, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true}, + } { + if err := clients.Create(ctx, &client); err != nil { + t.Fatalf("create %s client: %v", client.Name, err) + } + } + + body := bytes.NewBufferString(`{"guid":"wrong-protocol-guid","nzbUrl":"http://example/x.nzb","title":"t","protocol":"usenet","mediaType":"audiobook"}`) + rec := httptest.NewRecorder() + h.Grab(rec, httptest.NewRequest(http.MethodPost, "/api/v1/queue/grab", body)) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) + } + var resp map[string]string + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatal(err) + } + for _, want := range []string{"usenet", "audiobooks", "torrent", "cannot handle this release"} { + if !strings.Contains(resp["error"], want) { + t.Errorf("error %q does not explain %q", resp["error"], want) + } + } +} + +// createTestDownloadClient creates client, defaulting the media-type +// eligibility fields to true/true when the caller left both at their zero +// value — most tests here predate eligibility and don't care about it; tests +// that do exercise eligibility set both fields explicitly, which this leaves +// untouched. func createTestDownloadClient(t *testing.T, h *QueueHandler, client *models.DownloadClient) *models.DownloadClient { t.Helper() + if !client.EnabledForBooks && !client.EnabledForAudiobooks { + client.EnabledForBooks = true + client.EnabledForAudiobooks = true + } if err := h.clients.Create(context.Background(), client); err != nil { t.Fatalf("create download client: %v", err) } diff --git a/internal/db/db_test.go b/internal/db/db_test.go index 6549c13c1..fa78bddc3 100644 --- a/internal/db/db_test.go +++ b/internal/db/db_test.go @@ -911,6 +911,75 @@ func TestPickClientForMediaType(t *testing.T) { } } +func TestFilterEligibleForMediaType(t *testing.T) { + bookOnly := models.DownloadClient{ID: 1, Name: "book-only", EnabledForBooks: true, EnabledForAudiobooks: false} + audiobookOnly := models.DownloadClient{ID: 2, Name: "audiobook-only", EnabledForBooks: false, EnabledForAudiobooks: true} + both := models.DownloadClient{ID: 3, Name: "both", EnabledForBooks: true, EnabledForAudiobooks: true} + neither := models.DownloadClient{ID: 4, Name: "neither", EnabledForBooks: false, EnabledForAudiobooks: false} + + tests := []struct { + name string + clients []models.DownloadClient + mediaType string + wantIDs []int64 + }{ + {"ebook keeps book-eligible clients only", []models.DownloadClient{bookOnly, audiobookOnly, both, neither}, models.MediaTypeEbook, []int64{1, 3}}, + {"audiobook keeps audiobook-eligible clients only", []models.DownloadClient{bookOnly, audiobookOnly, both, neither}, models.MediaTypeAudiobook, []int64{2, 3}}, + {"empty media type treated as books", []models.DownloadClient{bookOnly, audiobookOnly}, "", []int64{1}}, + {"none eligible returns empty", []models.DownloadClient{neither}, models.MediaTypeAudiobook, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FilterEligibleForMediaType(tt.clients, tt.mediaType) + if len(got) != len(tt.wantIDs) { + t.Fatalf("expected %d eligible clients, got %d", len(tt.wantIDs), len(got)) + } + for i, id := range tt.wantIDs { + if got[i].ID != id { + t.Errorf("expected client %d at position %d, got %d", id, i, got[i].ID) + } + } + }) + } +} + +func TestRankClientsForMediaType(t *testing.T) { + low := models.DownloadClient{ID: 1, Name: "low-priority", Category: "books"} + high := models.DownloadClient{ID: 2, Name: "high-priority", Category: "books"} + audio := models.DownloadClient{ID: 3, Name: "audio", Category: "audiobooks"} + + // All three share the same (zero) Priority, so with nothing to break the + // tie on Priority the legacy category heuristic decides order — this is + // the pre-Priority-field behavior, preserved for anyone who never set it. + ranked := RankClientsForMediaType([]models.DownloadClient{high, low, audio}, models.MediaTypeEbook) + wantOrder := []int64{2, 1, 3} + for i, id := range wantOrder { + if ranked[i].ID != id { + t.Errorf("position %d: expected client %d, got %d", i, id, ranked[i].ID) + } + } + + // Same tie-on-Priority case: the audio-category client ranks first for an + // audiobook grab even though it was listed last. + ranked = RankClientsForMediaType([]models.DownloadClient{high, low, audio}, models.MediaTypeAudiobook) + if ranked[0].ID != audio.ID { + t.Fatalf("expected audio client ranked first for audiobook, got %d", ranked[0].ID) + } + + // #2412 regression: a client with an explicit CategoryAudiobook must not + // jump ahead of a client with a numerically lower (higher-precedence) + // Priority that has no category hint at all. Priority is the primary, + // user-controlled signal; the category heuristic only breaks a tie + // between clients that share the same Priority. + audiobooksPriority := models.DownloadClient{ID: 10, Name: "Audiobooks", Priority: 30} + bothCategoryAudiobook := models.DownloadClient{ID: 11, Name: "Both", Priority: 50, CategoryAudiobook: "audiobooks"} + ranked = RankClientsForMediaType([]models.DownloadClient{audiobooksPriority, bothCategoryAudiobook}, models.MediaTypeAudiobook) + if ranked[0].ID != audiobooksPriority.ID { + t.Fatalf("expected the higher-priority client (30) ranked first regardless of the category hint, got client %d first", ranked[0].ID) + } +} + func TestDownloadClientRepoCredentialFields(t *testing.T) { database, err := OpenMemory() if err != nil { diff --git a/internal/db/download_clients.go b/internal/db/download_clients.go index ed1562847..d64b37792 100644 --- a/internal/db/download_clients.go +++ b/internal/db/download_clients.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "sort" "strings" "time" @@ -17,7 +18,8 @@ type DownloadClientRepo struct { const downloadClientSelectColumns = ` id, name, type, host, port, api_key, use_ssl, url_base, username, password, - category, category_audiobook, path_remap, priority, enabled, remove_on_import, created_at, updated_at` + category, category_audiobook, path_remap, priority, enabled, + enabled_for_books, enabled_for_audiobooks, remove_on_import, created_at, updated_at` // torrentClientTypes and usenetClientTypes are the download-client types each // protocol can be served by. They back the protocol-scoped queries below; a new @@ -141,15 +143,16 @@ func (r *DownloadClientRepo) List(ctx context.Context) ([]models.DownloadClient, var clients []models.DownloadClient for rows.Next() { var c models.DownloadClient - var enabled, useSSL int - var removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int if err := rows.Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) @@ -159,13 +162,13 @@ func (r *DownloadClientRepo) List(ctx context.Context) ([]models.DownloadClient, func (r *DownloadClientRepo) GetByID(ctx context.Context, id int64) (*models.DownloadClient, error) { var c models.DownloadClient - var enabled, useSSL, removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int err := r.db.QueryRowContext(ctx, ` SELECT `+downloadClientSelectColumns+` FROM download_clients WHERE id=?`, id). Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -174,6 +177,8 @@ func (r *DownloadClientRepo) GetByID(ctx context.Context, id int64) (*models.Dow } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil @@ -192,15 +197,16 @@ func (r *DownloadClientRepo) ListEnabled(ctx context.Context) ([]models.Download var clients []models.DownloadClient for rows.Next() { var c models.DownloadClient - var enabled, useSSL int - var removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int if err := rows.Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) @@ -210,13 +216,13 @@ func (r *DownloadClientRepo) ListEnabled(ctx context.Context) ([]models.Download func (r *DownloadClientRepo) GetFirstEnabled(ctx context.Context) (*models.DownloadClient, error) { var c models.DownloadClient - var enabled, useSSL, removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int err := r.db.QueryRowContext(ctx, ` SELECT `+downloadClientSelectColumns+` FROM download_clients WHERE enabled=1 ORDER BY priority LIMIT 1`). Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -225,6 +231,8 @@ func (r *DownloadClientRepo) GetFirstEnabled(ctx context.Context) (*models.Downl } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil @@ -235,14 +243,14 @@ func (r *DownloadClientRepo) GetFirstEnabled(ctx context.Context) (*models.Downl // Returns (nil, nil) if no matching client is configured. func (r *DownloadClientRepo) GetFirstEnabledByProtocol(ctx context.Context, protocol string) (*models.DownloadClient, error) { var c models.DownloadClient - var enabled, useSSL, removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int placeholders, args := clientTypesForProtocol(protocol) err := r.db.QueryRowContext(ctx, ` SELECT `+downloadClientSelectColumns+` FROM download_clients WHERE enabled=1 AND type IN (`+placeholders+`) ORDER BY priority LIMIT 1`, args...). Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if err != nil && !errors.Is(err, sql.ErrNoRows) { return nil, err } @@ -251,6 +259,8 @@ func (r *DownloadClientRepo) GetFirstEnabledByProtocol(ctx context.Context, prot } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil @@ -272,15 +282,16 @@ func (r *DownloadClientRepo) GetEnabledByProtocol(ctx context.Context, protocol var clients []models.DownloadClient for rows.Next() { var c models.DownloadClient - var enabled, useSSL int - var removeOnImport int + var enabled, useSSL, enabledForBooks, enabledForAudiobooks, removeOnImport int if err := rows.Scan(&c.ID, &c.Name, &c.Type, &c.Host, &c.Port, &c.APIKey, &useSSL, &c.URLBase, &c.Username, &c.Password, &c.Category, &c.CategoryAudiobook, &c.PathRemap, &c.Priority, - &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &enabledForBooks, &enabledForAudiobooks, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.EnabledForBooks = enabledForBooks == 1 + c.EnabledForAudiobooks = enabledForAudiobooks == 1 c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) @@ -292,9 +303,9 @@ func (r *DownloadClientRepo) Create(ctx context.Context, c *models.DownloadClien normalizeClientCredentialStorage(c) now := time.Now().UTC() result, err := r.db.ExecContext(ctx, ` - INSERT INTO download_clients (name, type, host, port, api_key, use_ssl, url_base, username, password, category, category_audiobook, path_remap, priority, enabled, remove_on_import, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - c.Name, c.Type, c.Host, c.Port, c.APIKey, c.UseSSL, c.URLBase, c.Username, c.Password, c.Category, c.CategoryAudiobook, c.PathRemap, c.Priority, c.Enabled, c.RemoveOnImport, now, now) + INSERT INTO download_clients (name, type, host, port, api_key, use_ssl, url_base, username, password, category, category_audiobook, path_remap, priority, enabled, enabled_for_books, enabled_for_audiobooks, remove_on_import, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + c.Name, c.Type, c.Host, c.Port, c.APIKey, c.UseSSL, c.URLBase, c.Username, c.Password, c.Category, c.CategoryAudiobook, c.PathRemap, c.Priority, c.Enabled, c.EnabledForBooks, c.EnabledForAudiobooks, c.RemoveOnImport, now, now) if err != nil { return fmt.Errorf("create download client: %w", err) } @@ -313,9 +324,11 @@ func (r *DownloadClientRepo) Update(ctx context.Context, c *models.DownloadClien now := time.Now().UTC() _, err := r.db.ExecContext(ctx, ` UPDATE download_clients SET name=?, type=?, host=?, port=?, api_key=?, use_ssl=?, - url_base=?, username=?, password=?, category=?, category_audiobook=?, path_remap=?, priority=?, enabled=?, remove_on_import=?, updated_at=? + url_base=?, username=?, password=?, category=?, category_audiobook=?, path_remap=?, priority=?, enabled=?, + enabled_for_books=?, enabled_for_audiobooks=?, remove_on_import=?, updated_at=? WHERE id=?`, - c.Name, c.Type, c.Host, c.Port, c.APIKey, c.UseSSL, c.URLBase, c.Username, c.Password, c.Category, c.CategoryAudiobook, c.PathRemap, c.Priority, c.Enabled, c.RemoveOnImport, now, c.ID) + c.Name, c.Type, c.Host, c.Port, c.APIKey, c.UseSSL, c.URLBase, c.Username, c.Password, c.Category, c.CategoryAudiobook, c.PathRemap, c.Priority, c.Enabled, + c.EnabledForBooks, c.EnabledForAudiobooks, c.RemoveOnImport, now, c.ID) return err } @@ -345,27 +358,73 @@ func (r *DownloadClientRepo) Delete(ctx context.Context, id int64) error { // fuzzy "audio in category" heuristic so existing single-client setups keep // working. func PickClientForMediaType(clients []models.DownloadClient, mediaType string) *models.DownloadClient { - if len(clients) == 0 { + ranked := RankClientsForMediaType(clients, mediaType) + if len(ranked) == 0 { return nil } - if len(clients) == 1 { - return &clients[0] + return &ranked[0] +} + +// RankClientsForMediaType orders clients by Priority first (lower is tried +// first — the user-controlled ordering signal), and only falls back to the +// category heuristic to break a tie between clients that share the same +// Priority: an explicit CategoryAudiobook match, then the legacy "audio in +// category" heuristic, then everything else. Unlike PickClientForMediaType it +// returns every client, so a caller can retry the next-best one if sending to +// the first choice fails. +// +// Priority must win outright over the category hint — #2412 was a client +// with CategoryAudiobook set jumping ahead of a higher-priority (lower +// Priority number) client that had no category hint at all, because the +// category heuristic used to partition the whole list before priority was +// ever consulted. +func RankClientsForMediaType(clients []models.DownloadClient, mediaType string) []models.DownloadClient { + ranked := append([]models.DownloadClient(nil), clients...) + sort.SliceStable(ranked, func(i, j int) bool { + if ranked[i].Priority != ranked[j].Priority { + return ranked[i].Priority < ranked[j].Priority + } + return categoryPreferenceScore(ranked[i], mediaType) < categoryPreferenceScore(ranked[j], mediaType) + }) + return ranked +} + +// categoryPreferenceScore ranks a client's category hint for the given media +// type; lower is more preferred. Only used to break a Priority tie — see +// RankClientsForMediaType. +func categoryPreferenceScore(c models.DownloadClient, mediaType string) int { + if mediaType == models.MediaTypeAudiobook && strings.TrimSpace(c.CategoryAudiobook) != "" { + return 0 } - // First pass: prefer a client whose explicit fields match. - for i := range clients { - if mediaType == models.MediaTypeAudiobook && strings.TrimSpace(clients[i].CategoryAudiobook) != "" { - return &clients[i] + audioCategory := strings.Contains(strings.ToLower(c.Category), "audio") + if mediaType == models.MediaTypeAudiobook { + if audioCategory { + return 1 } + return 2 + } + if !audioCategory { + return 1 } - // Second pass: legacy heuristic for clients without CategoryAudiobook set. - for i := range clients { - cat := strings.ToLower(clients[i].Category) - if mediaType == models.MediaTypeAudiobook && strings.Contains(cat, "audio") { - return &clients[i] + return 2 +} + +// FilterEligibleForMediaType keeps only clients that opted in to handling the +// given media type. Audiobooks require EnabledForAudiobooks; anything else +// (mirroring the binary "audiobook vs everything else" treatment used +// elsewhere in this file) requires EnabledForBooks. +func FilterEligibleForMediaType(clients []models.DownloadClient, mediaType string) []models.DownloadClient { + eligible := make([]models.DownloadClient, 0, len(clients)) + for _, c := range clients { + if mediaType == models.MediaTypeAudiobook { + if c.EnabledForAudiobooks { + eligible = append(eligible, c) + } + continue } - if mediaType != models.MediaTypeAudiobook && !strings.Contains(cat, "audio") { - return &clients[i] + if c.EnabledForBooks { + eligible = append(eligible, c) } } - return &clients[0] + return eligible } diff --git a/internal/db/downloads.go b/internal/db/downloads.go index a489b6add..028afae24 100644 --- a/internal/db/downloads.go +++ b/internal/db/downloads.go @@ -615,6 +615,12 @@ func (r *DownloadRepo) SetNzoID(ctx context.Context, id int64, nzoID string) err return err } +// SetDownloadClientID records the download client selected for a download. +func (r *DownloadRepo) SetDownloadClientID(ctx context.Context, id, clientID int64) error { + _, err := r.db.ExecContext(ctx, "UPDATE downloads SET download_client_id=? WHERE id=?", clientID, id) + return err +} + func (r *DownloadRepo) SetTorrentID(ctx context.Context, id int64, torrentID string) error { torrentID = strings.ToLower(torrentID) _, err := r.db.ExecContext(ctx, "UPDATE downloads SET torrent_id=? WHERE id=?", torrentID, id) diff --git a/internal/db/migrations/116_download_client_media_eligibility.sql b/internal/db/migrations/116_download_client_media_eligibility.sql new file mode 100644 index 000000000..59e0d69ef --- /dev/null +++ b/internal/db/migrations/116_download_client_media_eligibility.sql @@ -0,0 +1,6 @@ +-- +migrate Up +-- Per-client media-type eligibility. Both default to enabled so existing +-- clients keep receiving every grab exactly as before until a user opts a +-- client out of a media type. +ALTER TABLE download_clients ADD COLUMN enabled_for_books INTEGER NOT NULL DEFAULT 1; +ALTER TABLE download_clients ADD COLUMN enabled_for_audiobooks INTEGER NOT NULL DEFAULT 1; \ No newline at end of file diff --git a/internal/db/pending_releases.go b/internal/db/pending_releases.go index f96c6d127..103fb0f6e 100644 --- a/internal/db/pending_releases.go +++ b/internal/db/pending_releases.go @@ -72,7 +72,7 @@ func (r *PendingReleaseRepo) ListByBookAndMediaType(ctx context.Context, bookID SELECT id, book_id, media_type, title, indexer_id, guid, protocol, size, age_minutes, quality, custom_score, reason, first_seen, release_json FROM pending_releases WHERE book_id = ? AND media_type = ? - ORDER BY first_seen DESC`, bookID, mediaType) + ORDER BY first_seen DESC, id DESC`, bookID, mediaType) if err != nil { return nil, err } diff --git a/internal/migrate/readarr.go b/internal/migrate/readarr.go index 10d0b7af9..ca1ef78ff 100644 --- a/internal/migrate/readarr.go +++ b/internal/migrate/readarr.go @@ -276,16 +276,18 @@ func importReadarrDownloadClients(ctx context.Context, src *sql.DB, repo *db.Dow } c := &models.DownloadClient{ - Name: name, - Type: t, - Host: host, - Port: s.Port, - APIKey: s.APIKey, - Username: s.Username, - Password: s.Password, - Category: cat, - UseSSL: s.UseSsl, - Enabled: enable, + Name: name, + Type: t, + Host: host, + Port: s.Port, + APIKey: s.APIKey, + Username: s.Username, + Password: s.Password, + Category: cat, + UseSSL: s.UseSsl, + Enabled: enable, + EnabledForBooks: true, + EnabledForAudiobooks: true, } // Same SSRF check the Add Download Client form runs (#2349). The RPC // clients have no dial-time guard of their own, so an unvalidated host diff --git a/internal/migrate/readarr_test.go b/internal/migrate/readarr_test.go index 89764c3b7..be03dcfd9 100644 --- a/internal/migrate/readarr_test.go +++ b/internal/migrate/readarr_test.go @@ -279,10 +279,10 @@ func TestImportReadarr_HappyPath(t *testing.T) { qbt = &clients[i] } } - if sab == nil || sab.Type != "sabnzbd" || sab.Category != "ebooks" || sab.APIKey != "sabkey" { + if sab == nil || sab.Type != "sabnzbd" || sab.Category != "ebooks" || sab.APIKey != "sabkey" || !sab.EnabledForBooks || !sab.EnabledForAudiobooks { t.Errorf("SAB unexpected: %+v", sab) } - if qbt == nil || qbt.Type != "qbittorrent" || qbt.Username != "u" || qbt.Password != "p" { + if qbt == nil || qbt.Type != "qbittorrent" || qbt.Username != "u" || qbt.Password != "p" || !qbt.EnabledForBooks || !qbt.EnabledForAudiobooks { t.Errorf("QBT unexpected: %+v", qbt) } // QBT has no tvCategory — should default to "books". diff --git a/internal/models/download.go b/internal/models/download.go index 8c6c4318a..55fb3fab0 100644 --- a/internal/models/download.go +++ b/internal/models/download.go @@ -16,14 +16,20 @@ type DownloadClient struct { // audiobook download. When empty, audiobooks fall back to Category, which // preserves pre-#700 behaviour for clients that have not opted in to the // per-media-type split. - CategoryAudiobook string `json:"categoryAudiobook"` - PathRemap string `json:"pathRemap"` - Priority int `json:"priority"` - Enabled bool `json:"enabled"` - RemoveOnImport bool `json:"removeOnImport"` - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` - Health *DownloadClientHealth `json:"health,omitempty"` + CategoryAudiobook string `json:"categoryAudiobook"` + PathRemap string `json:"pathRemap"` + Priority int `json:"priority"` + Enabled bool `json:"enabled"` + // EnabledForBooks and EnabledForAudiobooks gate whether this client is + // ever considered for a grab of that media type — independent of which + // category label Category/CategoryAudiobook assigns it. Both default true + // so a client handles everything until a user opts it out of one type. + EnabledForBooks bool `json:"enabledForBooks"` + EnabledForAudiobooks bool `json:"enabledForAudiobooks"` + RemoveOnImport bool `json:"removeOnImport"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + Health *DownloadClientHealth `json:"health,omitempty"` // Username and Password are used by download clients that authenticate with // credentials rather than an API key (e.g. qBittorrent, Transmission). diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index de84d38f6..4282a496f 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -1326,6 +1326,50 @@ func (s *Scheduler) searchAndGrabFormat(ctx context.Context, book models.Book, m specs = append(specs, decision.FreeleechOnlySpec{IndexerIDs: freeleechOnly}) } dm := decision.New(specs...) + enabledProtocols := map[string]bool{"usenet": true, "torrent": true} + eligibleProtocols := map[string]bool{"usenet": true, "torrent": true} + if s.clients != nil { + enabledClients, err := s.clients.ListEnabled(ctx) + if err != nil { + slog.Error("SearchAndGrabBook: failed to list download clients", "error", err) + outcome = "download client lookup failed" + return + } + enabledProtocols = make(map[string]bool) + eligibleProtocols = make(map[string]bool) + for _, client := range enabledClients { + protocol := downloader.ProtocolForClient(client.Type) + enabledProtocols[protocol] = true + if len(db.FilterEligibleForMediaType([]models.DownloadClient{client}, mediaType)) > 0 { + eligibleProtocols[protocol] = true + } + } + } + resultProtocolEnabled := false + for _, result := range results { + if enabledProtocols[result.Protocol] { + resultProtocolEnabled = true + break + } + } + resultCountBeforeClientFilter := len(results) + filteredResults := results[:0] + for _, result := range results { + if eligibleProtocols[result.Protocol] { + filteredResults = append(filteredResults, result) + } + } + results = filteredResults + if len(results) == 0 && resultCountBeforeClientFilter > 0 { + if resultProtocolEnabled { + slog.Warn("SearchAndGrabBook: no enabled download client eligible for media type", "book", book.Title, "mediaType", mediaType) + outcome = "no eligible download client for media type" + } else { + slog.Warn("SearchAndGrabBook: no enabled download client for protocol", "book", book.Title) + outcome = "no download client for protocol" + } + return + } releases := make([]decision.Release, len(results)) for i, res := range results { releases[i] = decision.ReleaseFromSearchResult(res) @@ -1415,24 +1459,34 @@ func (s *Scheduler) searchAndGrabFormat(ctx context.Context, book models.Book, m // An indexer pinned to a download client (#3080) sends every grab to that // client or, above, was skipped. There is deliberately no fallback to the // default pick for its releases. - client := bestClient - if client == nil { - candidates, err := s.clients.GetEnabledByProtocol(ctx, best.Protocol) + var candidates []models.DownloadClient + if bestClient != nil { + candidates = []models.DownloadClient{*bestClient} + } else { + var err error + candidates, err = s.clients.GetEnabledByProtocol(ctx, best.Protocol) if err != nil { slog.Error("SearchAndGrabBook: failed to list download clients", "protocol", best.Protocol, "error", err) outcome = "download client lookup failed" return } - client = db.PickClientForMediaType(candidates, mediaType) + } + if len(candidates) == 0 { + slog.Warn("SearchAndGrabBook: no enabled download client for protocol", "book", book.Title, "protocol", best.Protocol) + outcome = "no download client for protocol" + return } // No cross-protocol fallback: a usenet release must not be pushed to a // torrent client (qBittorrent would accept the .nzb URL, fail to parse it // as a torrent, and report "hash could not be determined"), and vice versa. - if client == nil { - slog.Warn("SearchAndGrabBook: no enabled download client for protocol", "book", book.Title, "protocol", best.Protocol) - outcome = "no download client for protocol" + eligible := db.FilterEligibleForMediaType(candidates, mediaType) + if len(eligible) == 0 { + slog.Warn("SearchAndGrabBook: no enabled download client eligible for media type", "book", book.Title, "protocol", best.Protocol, "mediaType", mediaType) + outcome = "no eligible download client for media type" return } + ranked := db.RankClientsForMediaType(eligible, mediaType) + client := &ranked[0] slog.Info("auto-grabbing book", "book", book.Title, @@ -1441,7 +1495,7 @@ func (s *Scheduler) searchAndGrabFormat(ctx context.Context, book models.Book, m "result", best.Title, "indexer", best.IndexerName, "protocol", best.Protocol, - "client", client.Name, + "client", ranked[0].Name, "size", best.Size, ) @@ -1475,7 +1529,7 @@ func (s *Scheduler) searchAndGrabFormat(ctx context.Context, book models.Book, m BookID: &book.ID, OwnerUserID: book.OwnerUserID, // tenancy (#1457): background grab inherits the book's owner IndexerID: &best.IndexerID, - DownloadClientID: &client.ID, + DownloadClientID: &ranked[0].ID, Title: best.Title, NZBURL: best.NZBURL, Size: best.Size, diff --git a/internal/scheduler/scheduler_freeleech_test.go b/internal/scheduler/scheduler_freeleech_test.go index 2e122906b..6ad87a024 100644 --- a/internal/scheduler/scheduler_freeleech_test.go +++ b/internal/scheduler/scheduler_freeleech_test.go @@ -52,6 +52,7 @@ func freeleechFixture(t *testing.T, freeleechOnly bool, results []newznab.Search } if err := clients.Create(ctx, &models.DownloadClient{ Name: "qbit", Type: "qbittorrent", Host: "127.0.0.1", Port: 1, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("client create: %v", err) } diff --git a/internal/scheduler/scheduler_grab_category_test.go b/internal/scheduler/scheduler_grab_category_test.go index c2b81e495..520efca4f 100644 --- a/internal/scheduler/scheduler_grab_category_test.go +++ b/internal/scheduler/scheduler_grab_category_test.go @@ -84,6 +84,7 @@ func TestSearchAndGrabBook_DualFormatUsesEachFormatsCategory(t *testing.T) { } if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: host, Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, Category: "readarr-ebook", CategoryAudiobook: "readarr-audio", }); err != nil { t.Fatalf("client create: %v", err) diff --git a/internal/scheduler/scheduler_indexer_pin_test.go b/internal/scheduler/scheduler_indexer_pin_test.go index 24d710358..019dcc36c 100644 --- a/internal/scheduler/scheduler_indexer_pin_test.go +++ b/internal/scheduler/scheduler_indexer_pin_test.go @@ -98,12 +98,12 @@ func newSchedPinFixture(t *testing.T) *schedPinFixture { } pubHost, pubPort := stallServerHostPort(t, pub.srv.URL) - pubC := &models.DownloadClient{Name: "sab-public", Type: "sabnzbd", Host: pubHost, Port: pubPort, Enabled: true, Priority: 1} + pubC := &models.DownloadClient{Name: "sab-public", Type: "sabnzbd", Host: pubHost, Port: pubPort, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, Priority: 1} if err := clients.Create(ctx, pubC); err != nil { t.Fatalf("client create: %v", err) } privHost, privPort := stallServerHostPort(t, priv.srv.URL) - privC := &models.DownloadClient{Name: "sab-private", Type: "sabnzbd", Host: privHost, Port: privPort, Enabled: true, Priority: 50} + privC := &models.DownloadClient{Name: "sab-private", Type: "sabnzbd", Host: privHost, Port: privPort, Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, Priority: 50} if err := clients.Create(ctx, privC); err != nil { t.Fatalf("client create: %v", err) } diff --git a/internal/scheduler/scheduler_language_test.go b/internal/scheduler/scheduler_language_test.go index 9fb617bfc..815570a95 100644 --- a/internal/scheduler/scheduler_language_test.go +++ b/internal/scheduler/scheduler_language_test.go @@ -64,6 +64,7 @@ func languageFixture(t *testing.T, allowedLanguages string, results []newznab.Se } if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: "127.0.0.1", Port: 1, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("client create: %v", err) } diff --git a/internal/scheduler/scheduler_media_eligibility_test.go b/internal/scheduler/scheduler_media_eligibility_test.go new file mode 100644 index 000000000..afe1a1b2a --- /dev/null +++ b/internal/scheduler/scheduler_media_eligibility_test.go @@ -0,0 +1,268 @@ +package scheduler + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "sync/atomic" + "testing" + + "github.com/vavallee/bindery/internal/db" + "github.com/vavallee/bindery/internal/httpsec" + "github.com/vavallee/bindery/internal/indexer/newznab" + "github.com/vavallee/bindery/internal/models" +) + +// eligibilityFixture builds a DB-backed scheduler with one wanted book, one +// enabled usenet indexer, and the SABnzbd clients the caller supplies. Each +// stub SAB server counts its own requests and reports success or failure per +// clientOK, so a test can tell which client the scheduler actually used. +func eligibilityFixture(t *testing.T, clientSpecs []struct { + name string + priority int + enabledForBooks bool + enabledForAudiobooks bool + ok bool +}) (s *Scheduler, downloads *db.DownloadRepo, book models.Book, calls map[string]*atomic.Int32) { + t.Helper() + t.Cleanup(httpsec.AllowLoopbackForTests()) + + indexerSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(``)) + })) + t.Cleanup(indexerSrv.Close) + + database, err := db.OpenMemory() + if err != nil { + t.Fatalf("OpenMemory: %v", err) + } + t.Cleanup(func() { database.Close() }) + ctx := context.Background() + + authors := db.NewAuthorRepo(database) + books := db.NewBookRepo(database) + clients := db.NewDownloadClientRepo(database) + downloadsRepo := db.NewDownloadRepo(database) + indexers := db.NewIndexerRepo(database) + + a := &models.Author{ + ForeignID: "OL-ELIG-A", Name: "Eligibility Author", SortName: "Author, Eligibility", + MetadataProvider: "ol", Monitored: true, + } + if err := authors.Create(ctx, a); err != nil { + t.Fatalf("author create: %v", err) + } + b := models.Book{ + ForeignID: "OL-ELIG-B", AuthorID: a.ID, Title: "Eligibility Book", + SortTitle: "Eligibility Book", Status: models.BookStatusWanted, + Genres: []string{}, MetadataProvider: "ol", Monitored: true, + MediaType: models.MediaTypeAudiobook, + } + if err := books.Create(ctx, &b); err != nil { + t.Fatalf("book create: %v", err) + } + + calls = make(map[string]*atomic.Int32) + for _, spec := range clientSpecs { + count := &atomic.Int32{} + calls[spec.name] = count + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + count.Add(1) + if !spec.ok { + _ = json.NewEncoder(w).Encode(map[string]any{"status": false}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"status": true, "nzo_ids": []string{"nzo-" + spec.name}}) + })) + t.Cleanup(srv.Close) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(u.Port()) + if err != nil { + t.Fatal(err) + } + if err := clients.Create(ctx, &models.DownloadClient{ + Name: spec.name, Type: "sabnzbd", Host: u.Hostname(), Port: port, + Priority: spec.priority, Enabled: true, + EnabledForBooks: spec.enabledForBooks, + EnabledForAudiobooks: spec.enabledForAudiobooks, + }); err != nil { + t.Fatalf("client create %s: %v", spec.name, err) + } + } + + idx := &models.Indexer{Name: "stub", Type: "newznab", URL: indexerSrv.URL, Enabled: true} + if err := indexers.Create(ctx, idx); err != nil { + t.Fatalf("indexer create: %v", err) + } + + s = &Scheduler{ + searcher: &fixedResultsSearcher{results: []newznab.SearchResult{{ + GUID: "g-eligibility", Title: "Eligibility Book.m4b", NZBURL: indexerSrv.URL + "/new.nzb", + Protocol: "usenet", IndexerID: idx.ID, + }}}, + indexers: indexers, + authors: authors, + settings: db.NewSettingsRepo(database), + blocklist: db.NewBlocklistRepo(database), + downloads: downloadsRepo, + clients: clients, + profiles: db.NewMetadataProfileRepo(database), + } + return s, downloadsRepo, b, calls +} + +// TestSearchAndGrabFormat_SkipsClientIneligibleForMediaType verifies that an +// audiobook auto-grab never reaches a client that opted out of audiobooks, +// even though it is otherwise enabled. +func TestSearchAndGrabFormat_SkipsClientIneligibleForMediaType(t *testing.T) { + s, downloads, book, calls := eligibilityFixture(t, []struct { + name string + priority int + enabledForBooks bool + enabledForAudiobooks bool + ok bool + }{ + {name: "book-only", priority: 1, enabledForBooks: true, enabledForAudiobooks: false, ok: true}, + {name: "audio-eligible", priority: 2, enabledForBooks: true, enabledForAudiobooks: true, ok: true}, + }) + + s.searchAndGrabFormat(context.Background(), book, models.MediaTypeAudiobook, nil) + + if calls["book-only"].Load() != 0 { + t.Errorf("expected the book-only client never to be contacted, got %d calls", calls["book-only"].Load()) + } + if calls["audio-eligible"].Load() != 1 { + t.Errorf("expected the audiobook-eligible client to receive the grab, got %d calls", calls["audio-eligible"].Load()) + } + rows, err := downloads.List(context.Background()) + if err != nil { + t.Fatalf("downloads list: %v", err) + } + if len(rows) != 1 || rows[0].Status != models.StateDownloading { + t.Fatalf("expected one downloading row, got %+v", rows) + } +} + +// TestSearchAndGrabFormat_DoesNotRetryAmbiguousSendFailure ensures a failed +// client response cannot cause the same release to be submitted elsewhere. +func TestSearchAndGrabFormat_DoesNotRetryAmbiguousSendFailure(t *testing.T) { + s, downloads, book, calls := eligibilityFixture(t, []struct { + name string + priority int + enabledForBooks bool + enabledForAudiobooks bool + ok bool + }{ + {name: "failing", priority: 1, enabledForBooks: true, enabledForAudiobooks: true, ok: false}, + {name: "working", priority: 2, enabledForBooks: true, enabledForAudiobooks: true, ok: true}, + }) + + s.searchAndGrabFormat(context.Background(), book, models.MediaTypeAudiobook, nil) + + if calls["failing"].Load() != 1 || calls["working"].Load() != 0 { + t.Fatalf("expected only the highest-ranked client to be attempted, got failing=%d working=%d", calls["failing"].Load(), calls["working"].Load()) + } + rows, err := downloads.List(context.Background()) + if err != nil { + t.Fatalf("downloads list: %v", err) + } + if len(rows) != 1 || rows[0].Status != models.StateFailed { + t.Fatalf("expected one errored row after the selected client's failure, got %+v", rows) + } +} + +// TestSearchAndGrabFormat_NoEligibleClientForMediaType verifies that an +// audiobook auto-grab with only book-eligible clients configured produces no +// download record, rather than being silently sent somewhere wrong. +func TestSearchAndGrabFormat_NoEligibleClientForMediaType(t *testing.T) { + s, downloads, book, calls := eligibilityFixture(t, []struct { + name string + priority int + enabledForBooks bool + enabledForAudiobooks bool + ok bool + }{ + {name: "book-only", priority: 1, enabledForBooks: true, enabledForAudiobooks: false, ok: true}, + }) + + s.searchAndGrabFormat(context.Background(), book, models.MediaTypeAudiobook, nil) + + if calls["book-only"].Load() != 0 { + t.Errorf("expected no client to be contacted, got %d calls", calls["book-only"].Load()) + } + rows, err := downloads.List(context.Background()) + if err != nil { + t.Fatalf("downloads list: %v", err) + } + if len(rows) != 0 { + t.Fatalf("expected no download record when no client is eligible, got %+v", rows) + } +} + +func TestSearchAndGrabFormat_SkipsProtocolWithoutEligibleClient(t *testing.T) { + s, downloads, book, calls := eligibilityFixture(t, []struct { + name string + priority int + enabledForBooks bool + enabledForAudiobooks bool + ok bool + }{ + {name: "usenet-book-only", priority: 1, enabledForBooks: true, enabledForAudiobooks: false, ok: true}, + }) + + transmissionCalls := 0 + transmissionServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + transmissionCalls++ + _ = json.NewEncoder(w).Encode(map[string]any{ + "result": "success", + "arguments": map[string]any{"torrent-added": map[string]any{"id": 7, "name": "Audiobook"}}, + }) + })) + defer transmissionServer.Close() + serverURL, err := url.Parse(transmissionServer.URL) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(serverURL.Port()) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + torrentClient := &models.DownloadClient{ + Name: "torrent-audio", Type: "transmission", Host: serverURL.Hostname(), Port: port, + Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, + } + if err := s.clients.Create(ctx, torrentClient); err != nil { + t.Fatalf("create torrent client: %v", err) + } + indexers, err := s.indexers.List(ctx) + if err != nil || len(indexers) == 0 { + t.Fatalf("list indexers: %v", err) + } + s.searcher = &fixedResultsSearcher{results: []newznab.SearchResult{ + {GUID: "g-usenet", Title: "Eligibility Book.m4b", NZBURL: "https://example.test/a.nzb", Protocol: "usenet", IndexerID: indexers[0].ID}, + {GUID: "g-torrent", Title: "Eligibility Book.m4b", NZBURL: "magnet:?xt=urn:btih:audio", Protocol: "torrent", IndexerID: indexers[0].ID}, + }} + + s.searchAndGrabFormat(ctx, book, models.MediaTypeAudiobook, nil) + + if calls["usenet-book-only"].Load() != 0 { + t.Errorf("ineligible usenet client received %d calls", calls["usenet-book-only"].Load()) + } + if transmissionCalls != 1 { + t.Errorf("eligible torrent client calls = %d, want 1", transmissionCalls) + } + rows, err := downloads.List(ctx) + if err != nil { + t.Fatalf("downloads list: %v", err) + } + if len(rows) != 1 || rows[0].Protocol != "torrent" || rows[0].Status != models.StateDownloading { + t.Fatalf("expected the eligible torrent result to be grabbed, got %+v", rows) + } +} diff --git a/internal/scheduler/scheduler_multibookpack_test.go b/internal/scheduler/scheduler_multibookpack_test.go index 82041cc4b..57d6b0693 100644 --- a/internal/scheduler/scheduler_multibookpack_test.go +++ b/internal/scheduler/scheduler_multibookpack_test.go @@ -40,7 +40,7 @@ func packGrabFixture(t *testing.T, results []newznab.SearchResult) (*Scheduler, client := &models.DownloadClient{ Name: "qbit", Type: "qbittorrent", Host: "127.0.0.1", Port: 1, - Enabled: true, + Enabled: true, EnabledForBooks: true, EnabledForAudiobooks: true, } if err := clients.Create(ctx, client); err != nil { t.Fatal(err) diff --git a/internal/scheduler/scheduler_quality_test.go b/internal/scheduler/scheduler_quality_test.go index 916689666..7fcb2798b 100644 --- a/internal/scheduler/scheduler_quality_test.go +++ b/internal/scheduler/scheduler_quality_test.go @@ -62,6 +62,7 @@ func qualityFixture(t *testing.T, attachProfile bool, items []models.QualityItem } if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: "127.0.0.1", Port: 1, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("client create: %v", err) } diff --git a/internal/scheduler/scheduler_regrab_test.go b/internal/scheduler/scheduler_regrab_test.go index f815441f4..9e89c5f98 100644 --- a/internal/scheduler/scheduler_regrab_test.go +++ b/internal/scheduler/scheduler_regrab_test.go @@ -100,6 +100,7 @@ func newRegrabFixture(t *testing.T) *regrabFixture { } if err := clients.Create(ctx, &models.DownloadClient{ Name: "sab", Type: "sabnzbd", Host: u.Hostname(), Port: port, Enabled: true, + EnabledForBooks: true, EnabledForAudiobooks: true, }); err != nil { t.Fatalf("client create: %v", err) } diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 33f75bd02..41c4ce36c 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -34,7 +34,7 @@ import { adoptionApi } from './adoption' import { requestsApi } from './requests' // Shared core: public constant, error class, helpers, and CSRF init. -export { ApiError, BINDERY_BASE, isNoDownloadClientError, initCSRF } from './core' +export { ApiError, BINDERY_BASE, isNoDownloadClientError, isNoEligibleClientError, initCSRF } from './core' // Re-export every domain module's types so // `import { Book, Author, ImportList, … } from '../api/client'` still resolves. diff --git a/web/src/api/core.ts b/web/src/api/core.ts index a6a5b88f9..d6f54d3e5 100644 --- a/web/src/api/core.ts +++ b/web/src/api/core.ts @@ -91,6 +91,17 @@ export function isNoDownloadClientError(err: unknown): boolean { return err instanceof ApiError && err.status === 400 && err.body.code === NO_DOWNLOAD_CLIENT_CODE } +// True when an error is the backend's "no enabled download client is +// eligible" failure (internal/api/queue.go's noEligibleMediaTypeError) — +// every enabled, protocol-matched client has opted out of this release's +// media type (book/audiobook eligibility checkboxes). Distinct from +// isNoDownloadClientError: the fix here is enabling an existing client for +// this media type, not adding a new one. +export function isNoEligibleClientError(err: unknown): boolean { + const msg = err instanceof Error ? err.message : '' + return msg.includes('no enabled download client is eligible for') +} + export async function request(path: string, options?: RequestInit): Promise { // Merge caller-supplied headers on top of the defaults so we can't lose // the CSRF header if a caller passes their own `headers`. diff --git a/web/src/api/downloadclients.ts b/web/src/api/downloadclients.ts index c93cd9185..99d0f58d1 100644 --- a/web/src/api/downloadclients.ts +++ b/web/src/api/downloadclients.ts @@ -27,6 +27,15 @@ export interface DownloadClient { // clients always clear their own history entry on import. removeOnImport?: boolean enabled: boolean + // Priority orders clients when more than one is eligible for a grab — + // lower is tried first. Ties are broken by insertion order. + priority: number + // Eligibility gates (independent of category/pathRemap): a client not + // eligible for a media type is skipped when a grab of that type is routed. + // Both default true, so an existing client keeps handling everything until + // a user opts it out of one. + enabledForBooks: boolean + enabledForAudiobooks: boolean health?: DownloadClientHealth } diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index f46b0cfa5..8e523f64b 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -384,6 +384,11 @@ "body": "Nothing was grabbed — no enabled download client can handle this release. Add or enable a download client to start downloads.", "action": "Set up Download Clients" }, + "noEligibleClient": { + "bodyBooks": "Nothing was grabbed — no enabled download client is eligible for books. Enable a client for books, or add a new one.", + "bodyAudiobooks": "Nothing was grabbed — no enabled download client is eligible for audiobooks. Enable a client for audiobooks, or add a new one.", + "action": "Manage Download Clients" + }, "forceTitle": "Grab this release again?", "forceBody": "Bindery already imported this release once. Grab it anyway only if the files from that import are missing: while they are still there, the new download is skipped as a duplicate. Nothing is removed from your download client and the book's current files stay where they are.", "forceConfirm": "Grab anyway", @@ -1708,6 +1713,11 @@ "audiobookCategoryLabel": "Audiobook Category / Label", "audiobookCategoryPlaceholder": "audiobooks", "audiobookCategoryHelp": "Optional. When set, audiobook grabs use this category instead of the one above. Leave blank to use the same category as ebooks.", + "eligibilityLabel": "Media type eligibility", + "enabledForBooks": "Eligible for books", + "enabledForAudiobooks": "Eligible for audiobooks", + "priorityLabel": "Priority", + "priorityHelp": "Lower numbers are tried first when more than one client is eligible for a grab. Ties fall back to the order clients were added in.", "pathNotVisible": "Connected, but Bindery can't read the client's completed-downloads folder. Configure a path remap or a shared mount so both point at the same storage, otherwise downloads won't import.", "loopbackHint": "The host is localhost — if Bindery runs in Docker, localhost is the Bindery container itself, not the machine. Use the host's LAN IP or the client's container/service name instead.", "rtorrentCategoryHelp": "Stored as the ruTorrent label (d.custom1) and used to filter which torrents Bindery polls. Leave blank to poll every torrent in the client.", diff --git a/web/src/pages/SearchPage.test.tsx b/web/src/pages/SearchPage.test.tsx index b364897f9..b52f78c7e 100644 --- a/web/src/pages/SearchPage.test.tsx +++ b/web/src/pages/SearchPage.test.tsx @@ -140,6 +140,63 @@ describe('SearchPage no-download-client nudge', () => { }) }) +describe('SearchPage no-eligible-client nudge', () => { + it('shows the audiobook-specific nudge when a grab fails with the eligibility error', async () => { + vi.mocked(api.searchIndexers).mockResolvedValue([ + makeResult({ guid: 'audio-guid', title: 'Some Book M4B', mediaType: 'audiobook' }), + ]) + renderSearchPage() + fireEvent.change(screen.getByPlaceholderText(en.search.placeholder), { target: { value: 'some book' } }) + fireEvent.submit(screen.getByPlaceholderText(en.search.placeholder).closest('form')!) + const grabButton = await screen.findByRole('button', { name: en.search.grab }) + vi.mocked(api.grab).mockRejectedValue( + new ApiError(400, { error: 'no enabled download client is eligible for audiobooks — enable a download client for audiobooks in Settings' }, 'Bad Request'), + ) + + fireEvent.click(grabButton) + + expect(await screen.findByText(en.search.noEligibleClient.bodyAudiobooks)).toBeInTheDocument() + const link = screen.getByRole('link', { name: en.search.noEligibleClient.action }) + expect(link).toHaveAttribute('href', '/settings?tab=clients') + }) + + it('does not show the no-client nudge for an eligibility failure', async () => { + const grabButton = await searchAndGetGrabButton() + vi.mocked(api.grab).mockRejectedValue( + new ApiError(400, { error: 'no enabled download client is eligible for books — enable a download client for books in Settings' }, 'Bad Request'), + ) + + fireEvent.click(grabButton) + + expect(await screen.findByText(en.search.noEligibleClient.bodyBooks)).toBeInTheDocument() + expect(screen.queryByText(noClientBody)).not.toBeInTheDocument() + }) +}) + +// A free-text search result carries its own detected mediaType (the search +// page groups results as "Ebook"/"Audiobook" from exactly this field), so the +// grab payload must forward it — otherwise the backend can't route the grab +// to a client eligible for that media type (#2401 regression). +describe('SearchPage grab forwards mediaType', () => { + it('includes the search result mediaType in the grab payload', async () => { + vi.mocked(api.searchIndexers).mockResolvedValue([ + makeResult({ guid: 'audio-guid', title: 'Some Book M4B', mediaType: 'audiobook' }), + ]) + vi.mocked(api.grab).mockResolvedValue(makeDownload({ guid: 'audio-guid' })) + renderSearchPage() + fireEvent.change(screen.getByPlaceholderText(en.search.placeholder), { + target: { value: 'some book' }, + }) + fireEvent.submit(screen.getByPlaceholderText(en.search.placeholder).closest('form')!) + const grabButton = await screen.findByRole('button', { name: en.search.grab }) + + fireEvent.click(grabButton) + + await waitFor(() => expect(api.grab).toHaveBeenCalledWith( + expect.objectContaining({ guid: 'audio-guid', mediaType: 'audiobook' }), + )) + }) +}) // #2289: a release Bindery already imported is refused with forceAvailable, // and the page offers to grab it anyway instead of leaving the user stuck. describe('SearchPage forced re-grab (#2289)', () => { diff --git a/web/src/pages/SearchPage.tsx b/web/src/pages/SearchPage.tsx index 9c8892263..99cdf79d1 100644 --- a/web/src/pages/SearchPage.tsx +++ b/web/src/pages/SearchPage.tsx @@ -1,7 +1,7 @@ import { useEffect, useState, useRef } from 'react' import { Link, useSearchParams } from 'react-router' import { useTranslation } from 'react-i18next' -import { api, isNoDownloadClientError, SearchResult } from '../api/client' +import { api, isNoDownloadClientError, isNoEligibleClientError, SearchResult } from '../api/client' import { formatBytes } from '../util/format' import { useConfirmDialog } from '../components/useConfirmDialog' import { grabWithForceConfirm } from '../util/forceGrab' @@ -25,6 +25,10 @@ export default function SearchPage() { // linking to the client settings — shown regardless of whether the library is // empty, which the first-run guidance (#960) didn't cover (#968). const [needsClient, setNeedsClient] = useState(false) + // Same idea, for a grab that failed because every enabled client of the + // right protocol has opted out of this release's media type (book/audiobook + // eligibility checkboxes) rather than none being enabled at all. + const [needsEligibleClient, setNeedsEligibleClient] = useState<'ebook' | 'audiobook' | null>(null) const inputRef = useRef(null) const lastRun = useRef('') // Two quick header searches: only the latest one's answer is shown, even @@ -46,6 +50,7 @@ export default function SearchPage() { setSearching(true) setError(null) setNeedsClient(false) + setNeedsEligibleClient(null) setResults(null) const seq = ++searchSeq.current try { @@ -78,6 +83,7 @@ export default function SearchPage() { setGrabbing(r.guid) setError(null) setNeedsClient(false) + setNeedsEligibleClient(null) try { const dl = await grabWithForceConfirm({ guid: r.guid, @@ -86,11 +92,14 @@ export default function SearchPage() { size: r.size, indexerId: r.indexerId, protocol: r.protocol, + mediaType: r.mediaType, }, confirm, t) if (!dl) return setGrabbed(prev => new Set(prev).add(r.guid)) } catch (e) { - if (isNoDownloadClientError(e)) { + if (isNoEligibleClientError(e)) { + setNeedsEligibleClient(r.mediaType === 'audiobook' ? 'audiobook' : 'ebook') + } else if (isNoDownloadClientError(e)) { setNeedsClient(true) } else { setError(errorText(e, t('search.grabFailed', 'Grab failed'))) @@ -148,6 +157,18 @@ export default function SearchPage() { )} + {needsEligibleClient && ( +
+

{t(needsEligibleClient === 'audiobook' ? 'search.noEligibleClient.bodyAudiobooks' : 'search.noEligibleClient.bodyBooks')}

+ + {t('search.noEligibleClient.action')} + +
+ )} + {results !== null && results.length === 0 && (
{t('search.noResults')} diff --git a/web/src/pages/SettingsPage.test.tsx b/web/src/pages/SettingsPage.test.tsx index 398bc920f..dfe459ff5 100644 --- a/web/src/pages/SettingsPage.test.tsx +++ b/web/src/pages/SettingsPage.test.tsx @@ -147,6 +147,9 @@ function makeClient(overrides: Partial = {}): DownloadClient { category: 'books', pathRemap: '', enabled: true, + priority: 0, + enabledForBooks: true, + enabledForAudiobooks: true, ...overrides, } } @@ -1888,6 +1891,9 @@ describe('SettingsPage', () => { enabled: true, useSsl: true, urlBase: '/sab', + enabledForBooks: true, + enabledForAudiobooks: true, + priority: 0, }) }) expect(await screen.findByText('SAB Books')).toBeInTheDocument() @@ -1967,6 +1973,9 @@ describe('SettingsPage', () => { enabled: true, useSsl: false, urlBase: '', + enabledForBooks: true, + enabledForAudiobooks: true, + priority: 0, // qBittorrent and Transmission carry the remove-on-import toggle, off // by default. The other clients have no removal wired up, so the form // omits it. @@ -2015,6 +2024,9 @@ describe('SettingsPage', () => { removeOnImport: false, useSsl: true, urlBase: '/qbittorrent', + enabledForBooks: true, + enabledForAudiobooks: true, + priority: 0, }) }) expect(await screen.findByText('qBit Books')).toBeInTheDocument() @@ -2043,6 +2055,39 @@ describe('SettingsPage', () => { }) }) + it('defaults media-type eligibility to both, and persists opting a client out of one', async () => { + renderSettings() + await openClientsTab() + + fireEvent.click(screen.getByRole('button', { name: 'settings.clients.addButton' })) + fireEvent.change(screen.getByPlaceholderText('Host'), { target: { value: 'sabnzbd' } }) + fireEvent.change(screen.getByPlaceholderText('API Key'), { target: { value: 'k' } }) + expect(screen.getByRole('checkbox', { name: 'settings.clients.enabledForBooks' })).toBeChecked() + expect(screen.getByRole('checkbox', { name: 'settings.clients.enabledForAudiobooks' })).toBeChecked() + + fireEvent.click(screen.getByRole('checkbox', { name: 'settings.clients.enabledForAudiobooks' })) + fireEvent.click(screen.getByRole('button', { name: 'common.save' })) + + await waitFor(() => { + expect(api.addDownloadClient).toHaveBeenCalledWith(expect.objectContaining({ + enabledForBooks: true, + enabledForAudiobooks: false, + })) + }) + }) + + it('displays and sorts download clients by priority', async () => { + const low = makeClient({ id: 51, name: 'Low priority', priority: 5 }) + const high = makeClient({ id: 52, name: 'High priority', priority: 1 }) + + renderSettings({ clients: [low, high] }) + await openClientsTab() + + const names = screen.getAllByRole('heading', { level: 4 }).map(h => h.textContent) + expect(names).toEqual(['High priority', 'Low priority']) + expect(screen.getByText(/High priority/).closest('div')?.parentElement?.textContent).toContain('settings.clients.priorityLabel: 1') + }) + it('shows qBittorrent path health errors under the client', async () => { const client = makeClient({ id: 46, diff --git a/web/src/pages/settings/ClientsTab.test.tsx b/web/src/pages/settings/ClientsTab.test.tsx index 04ee0123a..fc03f4b94 100644 --- a/web/src/pages/settings/ClientsTab.test.tsx +++ b/web/src/pages/settings/ClientsTab.test.tsx @@ -45,6 +45,9 @@ function makeClient(overrides: Partial = {}): DownloadClient { categoryAudiobook: '', pathRemap: '', enabled: true, + priority: 0, + enabledForBooks: true, + enabledForAudiobooks: true, ...overrides, } } diff --git a/web/src/pages/settings/ClientsTab.tsx b/web/src/pages/settings/ClientsTab.tsx index bff85c560..debaa01c2 100644 --- a/web/src/pages/settings/ClientsTab.tsx +++ b/web/src/pages/settings/ClientsTab.tsx @@ -120,6 +120,12 @@ export default function ClientsTab({ clients, setClients }: Props) { return next }) + // The API already returns clients ORDER BY priority, but that order goes + // stale the moment a client is added, edited, or reordered without a full + // refetch (setClients just appends/replaces in place) — sort on every + // render so the list always reflects the order grabs are actually tried in. + const sortedClients = [...clients].sort((a, b) => a.priority - b.priority) + return (
@@ -132,7 +138,7 @@ export default function ClientsTab({ clients, setClients }: Props) {

{t('settings.clients.empty')}

) : (
- {clients.map(c => ( + {sortedClients.map(c => (
@@ -146,7 +152,7 @@ export default function ClientsTab({ clients, setClients }: Props) { />

{c.name}

-

{c.host}:{c.port} ({c.category})

+

{c.host}:{c.port} ({c.category}) · {t('settings.clients.priorityLabel')}: {c.priority}

@@ -270,6 +276,9 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; const [urlBase, setUrlBase] = useState(client.urlBase || '') const [category, setCategory] = useState(client.category) const [categoryAudiobook, setCategoryAudiobook] = useState(client.categoryAudiobook || '') + const [enabledForBooks, setEnabledForBooks] = useState(client.enabledForBooks ?? true) + const [enabledForAudiobooks, setEnabledForAudiobooks] = useState(client.enabledForAudiobooks ?? true) + const [priority, setPriority] = useState(String(client.priority ?? 0)) const [saving, setSaving] = useState(false) const [saveError, setSaveError] = useState(null) const [pathRemap, setPathRemap] = useState(client.pathRemap || '') @@ -323,6 +332,9 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; removeOnImport: supportsRemoveOnImport(type) ? removeOnImport : false, useSsl: useSSL, urlBase: urlBase.trim(), + enabledForBooks, + enabledForAudiobooks, + priority: parseInt(priority) || 0, } if (isPasswordClient(type)) { data.clearApiKey = true @@ -452,6 +464,24 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient;

{t('settings.clients.audiobookCategoryHelp')}

)} +
+ +
+
+ setEnabledForBooks(e.target.checked)} className="rounded border-slate-300 dark:border-zinc-700" /> + +
+
+ setEnabledForAudiobooks(e.target.checked)} className="rounded border-slate-300 dark:border-zinc-700" /> + +
+
+
+
+ + setPriority(e.target.value)} className={`${inputCls} max-w-[8rem]`} /> +

{t('settings.clients.priorityHelp')}

+
void; onAdded: (c: const [urlBase, setUrlBase] = useState('') const [category, setCategory] = useState('books') const [categoryAudiobook, setCategoryAudiobook] = useState('') + const [enabledForBooks, setEnabledForBooks] = useState(true) + const [enabledForAudiobooks, setEnabledForAudiobooks] = useState(true) + const [priority, setPriority] = useState('0') const [saving, setSaving] = useState(false) const [saveError, setSaveError] = useState(null) const [pathRemap, setPathRemap] = useState('') @@ -575,6 +608,9 @@ function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c: ...(isPasswordClient(type) ? { name, host, port: parseInt(port), username: hasUsername(type) ? username : '', password: credential, apiKey: '', category, categoryAudiobook: categoryAudiobook.trim(), pathRemap: pathRemap.trim(), type, enabled: true, useSsl: useSSL, urlBase: urlBase.trim() } : { name, host, port: parseInt(port), apiKey: credential, username: '', password: '', category, categoryAudiobook: categoryAudiobook.trim(), pathRemap: pathRemap.trim(), type, enabled: true, useSsl: useSSL, urlBase: urlBase.trim() }), + enabledForBooks, + enabledForAudiobooks, + priority: parseInt(priority) || 0, ...(supportsRemoveOnImport(type) ? { removeOnImport } : {}), }) @@ -683,6 +719,24 @@ function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c:

{t('settings.clients.audiobookCategoryHelp')}

)} +
+ +
+
+ setEnabledForBooks(e.target.checked)} className="rounded border-slate-300 dark:border-zinc-700" /> + +
+
+ setEnabledForAudiobooks(e.target.checked)} className="rounded border-slate-300 dark:border-zinc-700" /> + +
+
+
+
+ + setPriority(e.target.value)} className={`${inputCls} max-w-[8rem]`} /> +

{t('settings.clients.priorityHelp')}

+
): DownloadClient { return { id: 1, name: 'c', type: 'qbittorrent', host: 'h', port: 1, apiKey: '', username: '', password: '', apiKeyConfigured: false, passwordConfigured: false, useSsl: false, urlBase: '', category: '', - enabled: true, ...overrides, + enabled: true, enabledForBooks: true, enabledForAudiobooks: true, priority: 0, ...overrides, } } diff --git a/web/src/pages/settings/RemapAssistant.test.tsx b/web/src/pages/settings/RemapAssistant.test.tsx index b71566551..85fcbcf97 100644 --- a/web/src/pages/settings/RemapAssistant.test.tsx +++ b/web/src/pages/settings/RemapAssistant.test.tsx @@ -42,6 +42,9 @@ function makeClient(overrides: Partial = {}): DownloadClient { categoryAudiobook: '', pathRemap: '', enabled: true, + enabledForBooks: true, + enabledForAudiobooks: true, + priority: 0, ...overrides, } }