Feature/download client media eligibility

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
 

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.

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`

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",

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

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"

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)
 	}

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)

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 {

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)
 		}

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,23 +1385,51 @@ 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
 	}
 	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)

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"})

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)