diff --git a/changelog.d/2046-remove-torrent-on-import.md b/changelog.d/2046-remove-torrent-on-import.md new file mode 100644 index 000000000..acd190db4 --- /dev/null +++ b/changelog.d/2046-remove-torrent-on-import.md @@ -0,0 +1,2 @@ +### Added +- **Remove a torrent from the download client after it imports** (#2046): a new per client toggle, Remove torrent after import, for qBittorrent and Transmission. It is off by default. When on, Bindery removes the finished torrent from the client once the book is imported and leaves the downloaded files on disk, so a hard linked or copied import is unaffected. Leave it off on private trackers where you need to keep seeding. Deluge and rTorrent do not offer it yet. Thanks ccarpinteri for building it. diff --git a/internal/db/download_clients.go b/internal/db/download_clients.go index f7f2f3f11..de0a2926f 100644 --- a/internal/db/download_clients.go +++ b/internal/db/download_clients.go @@ -17,7 +17,7 @@ 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, created_at, updated_at` + category, category_audiobook, path_remap, priority, enabled, 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 @@ -142,13 +142,15 @@ func (r *DownloadClientRepo) List(ctx context.Context) ([]models.DownloadClient, for rows.Next() { var c models.DownloadClient var enabled, useSSL int + var 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, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) } @@ -157,13 +159,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 int + var enabled, useSSL, 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, &c.CreatedAt, &c.UpdatedAt) + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -172,6 +174,7 @@ func (r *DownloadClientRepo) GetByID(ctx context.Context, id int64) (*models.Dow } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil } @@ -190,13 +193,15 @@ func (r *DownloadClientRepo) ListEnabled(ctx context.Context) ([]models.Download for rows.Next() { var c models.DownloadClient var enabled, useSSL int + var 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, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) } @@ -205,13 +210,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 int + var enabled, useSSL, 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, &c.CreatedAt, &c.UpdatedAt) + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -220,6 +225,7 @@ func (r *DownloadClientRepo) GetFirstEnabled(ctx context.Context) (*models.Downl } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil } @@ -229,14 +235,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 int + var enabled, useSSL, 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, &c.CreatedAt, &c.UpdatedAt) + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt) if err != nil && !errors.Is(err, sql.ErrNoRows) { return nil, err } @@ -245,6 +251,7 @@ func (r *DownloadClientRepo) GetFirstEnabledByProtocol(ctx context.Context, prot } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) return &c, nil } @@ -266,13 +273,15 @@ func (r *DownloadClientRepo) GetEnabledByProtocol(ctx context.Context, protocol for rows.Next() { var c models.DownloadClient var enabled, useSSL int + var 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, &c.CreatedAt, &c.UpdatedAt); err != nil { + &enabled, &removeOnImport, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } c.Enabled = enabled == 1 c.UseSSL = useSSL == 1 + c.RemoveOnImport = removeOnImport == 1 hydrateClientCredentials(&c) clients = append(clients, c) } @@ -283,9 +292,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, 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, 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, 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) if err != nil { return fmt.Errorf("create download client: %w", err) } @@ -304,9 +313,9 @@ 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=?, updated_at=? + url_base=?, username=?, password=?, category=?, category_audiobook=?, path_remap=?, priority=?, enabled=?, 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, 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.RemoveOnImport, now, c.ID) return err } diff --git a/internal/db/migrations/090_download_client_remove_on_import.sql b/internal/db/migrations/090_download_client_remove_on_import.sql new file mode 100644 index 000000000..d55d3076a --- /dev/null +++ b/internal/db/migrations/090_download_client_remove_on_import.sql @@ -0,0 +1 @@ +ALTER TABLE download_clients ADD COLUMN remove_on_import INTEGER NOT NULL DEFAULT 0; diff --git a/internal/importer/scanner.go b/internal/importer/scanner.go index 2997f3c0b..c59f02ad7 100644 --- a/internal/importer/scanner.go +++ b/internal/importer/scanner.go @@ -1453,6 +1453,11 @@ func (s *Scanner) tryImportInternal(ctx context.Context, dl *models.Download, do // Walk: importPending → importing → imported. s.updateDownloadStatus(ctx, dl.ID, models.StateImporting) s.updateDownloadStatus(ctx, dl.ID, models.StateImported) + if cleanupFunc != nil { + if err := cleanupFunc(); err != nil { + slog.Warn("cleanup failed", cleanupWarnAttrs(cleanupClientType, cleanupRemoteID, err)...) + } + } return } // Distinguish "path doesn't exist on this host" from "path exists but has diff --git a/internal/importer/scanner_client_wrappers_test.go b/internal/importer/scanner_client_wrappers_test.go index 0ea3c647d..3f22f6024 100644 --- a/internal/importer/scanner_client_wrappers_test.go +++ b/internal/importer/scanner_client_wrappers_test.go @@ -16,6 +16,7 @@ import ( "path/filepath" "testing" + "github.com/vavallee/bindery/internal/downloader" "github.com/vavallee/bindery/internal/downloader/nzbget" "github.com/vavallee/bindery/internal/downloader/sabnzbd" "github.com/vavallee/bindery/internal/models" @@ -55,8 +56,10 @@ func TestTryImportTransmission_Delegates(t *testing.T) { Title: "Transmission Delegate Test", Status: models.StateCompleted, } - // Transmission wrapper passes nil cleanupFunc; empty dir → fails fast. - s.tryImportTransmission(ctx, dl, t.TempDir(), nil) + client := &models.DownloadClient{Type: "transmission"} + trans := downloader.TransmissionFor(client) + // RemoveOnImport left unset (false) → nil cleanupFunc; empty dir → fails fast. + s.tryImportTransmission(ctx, trans, client, dl, t.TempDir(), nil) } // TestTryImportNZBGet_CleanupCalledOnSuccess verifies that the cleanup closure diff --git a/internal/importer/scanner_poll.go b/internal/importer/scanner_poll.go index 5bb28a966..9ba6c8c68 100644 --- a/internal/importer/scanner_poll.go +++ b/internal/importer/scanner_poll.go @@ -201,35 +201,35 @@ func transmissionCompletion(status int, percentDone float64) (complete, stopped func (s *Scanner) checkTransmissionDownloads(ctx context.Context, client *models.DownloadClient) { trans := downloader.TransmissionFor(client) - // Poll every category this client may have grabbed under. Audiobook grabs - // use CategoryAudiobook when it is set, ebook grabs use Category, and - // polling only Category leaves audiobook torrents invisible: their - // downloads then hang at "downloading" for good. CategoriesToPoll returns - // both. GetTorrents normalises the path comparison and also accepts - // Transmission 3.0+ labels, so "books" matches both a downloadDir of - // "/data/books/" and a label "books". - // + // Poll every category this client may have grabbed under. Audiobook grabs use + // CategoryAudiobook (when set) while ebook grabs use Category; polling only + // Category leaves audiobook torrents invisible and their downloads hang at + // "downloading" forever. CategoriesToPoll returns both. GetTorrents normalises + // the path comparison and also accepts Transmission 3.0+ labels, so "books" + // matches both a downloadDir of "/data/books/" and a label "books". // Torrents are indexed by info hash, which is stable for the life of the - // torrent. The numeric id is not: Transmission renumbers every torrent when - // the daemon restarts. + // torrent. The session-scoped numeric id is indexed separately and used + // only to reconcile rows grabbed before hashes were persisted: Transmission + // renumbers every torrent when the daemon restarts, so an id stored at grab + // time either matches nothing (the download strands at "downloading" + // forever) or matches whichever unrelated torrent inherited the number. torrentsByHash := make(map[string]transmission.Torrent) - var torrents []transmission.Torrent + var allTorrents []transmission.Torrent seenTorrentIDs := make(map[int64]bool) for _, cat := range downloader.CategoriesToPoll(client) { - found, err := trans.GetTorrents(ctx, cat) + torrents, err := trans.GetTorrents(ctx, cat) if err != nil { // Warn, not Debug: see checkSABnzbdDownloads (#1019 failure mode). slog.Warn("download poll: failed to fetch Transmission torrents — downloads will not be imported", "client", client.Name, "category", cat, "error", err) return } - // The two categories can overlap, so a torrent seen twice is kept once. - for _, t := range found { + for _, t := range torrents { if seenTorrentIDs[t.ID] { continue } seenTorrentIDs[t.ID] = true - torrents = append(torrents, t) + allTorrents = append(allTorrents, t) if hash := strings.ToLower(strings.TrimSpace(t.HashString)); hash != "" { torrentsByHash[hash] = t } @@ -239,7 +239,7 @@ func (s *Scanner) checkTransmissionDownloads(ctx context.Context, client *models // Surface a misconfiguration when the Category filter returns nothing but the // daemon actually holds torrents (#1091). A silent zero-match means every // Bindery grab permanently sits at "downloading" with no indication of why. - if client.Category != "" && len(torrents) == 0 { + if client.Category != "" && len(allTorrents) == 0 { if all, allErr := trans.GetTorrents(ctx, ""); allErr == nil && len(all) > 0 { slog.Warn("transmission: Category filter matched zero torrents — verify Category matches the torrent download directory path or a torrent label", "client", client.Name, "category", client.Category, "category_audiobook", client.CategoryAudiobook, "total_torrents", len(all)) @@ -274,7 +274,7 @@ func (s *Scanner) checkTransmissionDownloads(ctx context.Context, client *models } } - legacyMatches := s.reconcileLegacyTransmissionIDs(ctx, client, allDownloads, torrents, claimedHashes) + legacyMatches := s.reconcileLegacyTransmissionIDs(ctx, client, allDownloads, allTorrents, claimedHashes) for _, dl := range allDownloads { if dl.DownloadClientID == nil || *dl.DownloadClientID != client.ID || dl.TorrentID == nil { @@ -307,7 +307,7 @@ func (s *Scanner) checkTransmissionDownloads(ctx context.Context, client *models bookFiles := s.transmissionFilesFor(ctx, trans, client, torrent) slog.Info("download completed", "title", dl.Title, "path", downloadPath, "files", len(bookFiles)) s.updateDownloadStatus(ctx, dl.ID, models.StateCompleted) - s.tryImportTransmission(ctx, &dl, downloadPath, bookFiles) + s.tryImportTransmission(ctx, trans, client, &dl, downloadPath, bookFiles) } else if isComplete && dl.Status == models.StateImportFailed && dl.ImportRetryCount < importRetryLimit { // Bug #7: retry a previously failed import. downloadPath := s.remapDownloadClientPath(client, torrent.DownloadDir) @@ -321,7 +321,7 @@ func (s *Scanner) checkTransmissionDownloads(ctx context.Context, client *models if err := s.downloads.IncrementImportRetryCount(ctx, dl.ID); err != nil { slog.Warn("failed to increment import retry count", "download_id", dl.ID, "error", err) } - s.tryImportTransmission(ctx, &dl, downloadPath, bookFiles) + s.tryImportTransmission(ctx, trans, client, &dl, downloadPath, bookFiles) } else if isStopped && !isComplete && dl.Status != models.StateFailed { if stopError == "" { // Transmission also reports user-paused torrents as stopped. @@ -623,6 +623,11 @@ func (s *Scanner) checkQbittorrentDownloads(ctx context.Context, client *models. } s.updateDownloadStatus(ctx, dl.ID, models.StateImporting) s.updateDownloadStatus(ctx, dl.ID, models.StateImported) + if cleanup := qbittorrentRemoveOnImportCleanup(ctx, qb, client, &dl); cleanup != nil { + if err := cleanup(); err != nil { + slog.Warn("cleanup failed", cleanupWarnAttrs("qbittorrent", safeRemoteID(dl.TorrentID), err)...) + } + } continue } // Path doesn't exist on disk yet (qBittorrent may sanitise characters @@ -660,7 +665,7 @@ func (s *Scanner) checkQbittorrentDownloads(ctx context.Context, client *models. if dl.Status == models.StateDownloading || dl.Status == models.StateGrabbed { s.updateDownloadStatus(ctx, dl.ID, models.StateCompleted) } - s.tryImportQbittorrent(ctx, &dl, downloadPath, bookFiles) + s.tryImportQbittorrent(ctx, qb, client, &dl, downloadPath, bookFiles) } else if isComplete && dl.Status == models.StateImportFailed && dl.ImportRetryCount < importRetryLimit { // Bug #7: a previous import attempt failed (e.g. transient filesystem // error, path mismatch). The torrent is still seeding so we have the @@ -676,6 +681,11 @@ func (s *Scanner) checkQbittorrentDownloads(ctx context.Context, client *models. s.updateDownloadStatus(ctx, dl.ID, models.StateImportPending) s.updateDownloadStatus(ctx, dl.ID, models.StateImporting) s.updateDownloadStatus(ctx, dl.ID, models.StateImported) + if cleanup := qbittorrentRemoveOnImportCleanup(ctx, qb, client, &dl); cleanup != nil { + if err := cleanup(); err != nil { + slog.Warn("cleanup failed", cleanupWarnAttrs("qbittorrent", safeRemoteID(dl.TorrentID), err)...) + } + } continue } // The files are still not here. Count the miss through the same @@ -698,7 +708,7 @@ func (s *Scanner) checkQbittorrentDownloads(ctx context.Context, client *models. if err := s.downloads.IncrementImportRetryCount(ctx, dl.ID); err != nil { slog.Warn("failed to increment import retry count", "download_id", dl.ID, "error", err) } - s.tryImportQbittorrent(ctx, &dl, downloadPath, bookFiles) + s.tryImportQbittorrent(ctx, qb, client, &dl, downloadPath, bookFiles) } else if isFailed && dl.Status != models.StateFailed { slog.Warn("download failed", "title", dl.Title, "state", torrent.State) s.markDownloadFailed(ctx, &dl, "Torrent failed in qBittorrent") @@ -1006,8 +1016,21 @@ func (s *Scanner) tryImportSABnzbd(ctx context.Context, sab *sabnzbd.Client, dl // of bug where a single-file torrent at a shared download root would cause // every unrelated sibling to be imported. Pass nil to fall back to the // directory walk. -func (s *Scanner) tryImportTransmission(ctx context.Context, dl *models.Download, downloadPath string, explicitFiles []string) { - s.tryImportInternal(ctx, dl, downloadPath, "transmission", safeRemoteID(dl.TorrentID), "", nil, explicitFiles) +func (s *Scanner) tryImportTransmission(ctx context.Context, trans *transmission.Client, client *models.DownloadClient, dl *models.Download, downloadPath string, explicitFiles []string) { + var cleanup func() error + if client.RemoveOnImport && dl.TorrentID != nil && strings.TrimSpace(*dl.TorrentID) != "" { + ref := strings.TrimSpace(*dl.TorrentID) + cleanup = func() error { + slog.Info("removing torrent from Transmission after import", "torrent", ref, "title", dl.Title) + // deleteFiles=false: the payload has been imported (hard linked or + // copied) but the torrent may still be seeding from those files. + if err := downloader.RemoveTransmissionTorrent(ctx, trans, ref, false); err != nil { + return fmt.Errorf("remove torrent %s: %w", ref, err) + } + return nil + } + } + s.tryImportInternal(ctx, dl, downloadPath, "transmission", safeRemoteID(dl.TorrentID), "", cleanup, explicitFiles) } // legacyTransmissionMatchWindow is how far apart Bindery's grab timestamp and @@ -1204,8 +1227,28 @@ func normaliseReleaseName(s string) string { // tryImportQbittorrent attempts to import a completed qBittorrent download. See // tryImportTransmission for the semantics of explicitFiles. -func (s *Scanner) tryImportQbittorrent(ctx context.Context, dl *models.Download, downloadPath string, explicitFiles []string) { - s.tryImportInternal(ctx, dl, downloadPath, "qbittorrent", safeRemoteID(dl.TorrentID), "", nil, explicitFiles) +func (s *Scanner) tryImportQbittorrent(ctx context.Context, qb *qbittorrent.Client, client *models.DownloadClient, dl *models.Download, downloadPath string, explicitFiles []string) { + cleanup := qbittorrentRemoveOnImportCleanup(ctx, qb, client, dl) + s.tryImportInternal(ctx, dl, downloadPath, "qbittorrent", safeRemoteID(dl.TorrentID), "", cleanup, explicitFiles) +} + +// qbittorrentRemoveOnImportCleanup builds the post-import removal callback +// shared by tryImportQbittorrent and the "book already in library" shortcuts +// in checkQbittorrentDownloads (issue #2046) — those shortcuts close out a +// download without ever calling tryImportInternal, so they need the same +// cleanup construction rather than only the normal import path getting it. +func qbittorrentRemoveOnImportCleanup(ctx context.Context, qb *qbittorrent.Client, client *models.DownloadClient, dl *models.Download) func() error { + if !client.RemoveOnImport || dl.TorrentID == nil { + return nil + } + hash := *dl.TorrentID + return func() error { + slog.Info("removing torrent from qBittorrent after import", "hash", hash, "title", dl.Title) + if err := qb.DeleteTorrent(ctx, hash, false); err != nil { + return fmt.Errorf("remove torrent %s: %w", hash, err) + } + return nil + } } // torrentFile is the minimal shape resolveTorrentFiles consumes; it matches diff --git a/internal/models/download.go b/internal/models/download.go index 913f0cc98..08bdc879e 100644 --- a/internal/models/download.go +++ b/internal/models/download.go @@ -20,6 +20,7 @@ type DownloadClient struct { 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"` diff --git a/web/src/api/downloadclients.ts b/web/src/api/downloadclients.ts index a57aec7cc..a04afa74c 100644 --- a/web/src/api/downloadclients.ts +++ b/web/src/api/downloadclients.ts @@ -22,6 +22,10 @@ export interface DownloadClient { // fall back to `category`. categoryAudiobook?: string pathRemap?: string + // removeOnImport removes the torrent from the client once Bindery has + // imported it (the data is left on disk). Torrent clients only: usenet + // clients always clear their own history entry on import. + removeOnImport?: boolean enabled: boolean health?: DownloadClientHealth } diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index d0e7b15a8..6b9f4d704 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -956,6 +956,8 @@ "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.", + "removeOnImportLabel": "Remove torrent after import", + "removeOnImportHelp": "When Bindery imports a download from this client, remove the torrent from it. The downloaded files are left on disk. Off by default: on a private tracker this stops the torrent seeding, which can cost you ratio.", "rtorrentUrlBaseHelp": "Full path of rTorrent's XML-RPC endpoint, not a prefix. Usually /RPC2; ruTorrent installs often expose /plugins/rpc/rpc.php instead. Leave blank to use /RPC2. For a plain rTorrent with no web server in front of it, use rTorrent's own SCGI listener: scgi:// for the host and port above, scgi://host:port, or scgi:///path/to/socket. SCGI carries no credentials, so keep it off-network.", "rtorrentScgiIgnoredHint": "{{fields}} will be ignored — rTorrent's SCGI listener has no TLS and no authentication. Put rTorrent behind a web server and use an HTTP path instead if you need either.", "passwordEditLabel": "Password (leave blank to keep current)", diff --git a/web/src/pages/SettingsPage.test.tsx b/web/src/pages/SettingsPage.test.tsx index cc8652d22..de589913b 100644 --- a/web/src/pages/SettingsPage.test.tsx +++ b/web/src/pages/SettingsPage.test.tsx @@ -1780,6 +1780,10 @@ describe('SettingsPage', () => { enabled: true, useSsl: false, urlBase: '', + // 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. + ...(type === 'qbittorrent' || type === 'transmission' ? { removeOnImport: false } : {}), }) }) }) @@ -1819,6 +1823,9 @@ describe('SettingsPage', () => { category: 'ebooks', categoryAudiobook: '', pathRemap: '/media:/books', + // The type switched to a torrent client, so the toggle is now part of + // the payload; the form leaves it off unless the user ticks it. + removeOnImport: false, useSsl: true, urlBase: '/qbittorrent', }) diff --git a/web/src/pages/settings/ClientsTab.test.tsx b/web/src/pages/settings/ClientsTab.test.tsx index 52aa29dfc..784b96bfe 100644 --- a/web/src/pages/settings/ClientsTab.test.tsx +++ b/web/src/pages/settings/ClientsTab.test.tsx @@ -256,3 +256,51 @@ describe('download client diagnose', () => { expect(await screen.findByRole('alert')).toHaveTextContent('settings.clients.diagnose.failed') }) }) + +describe('download client remove-on-import toggle', () => { + it('is hidden for a usenet client, which always clears its own history', () => { + renderTab([makeClient({ type: 'sabnzbd' })]) + openEditForm() + + expect(screen.queryByLabelText('settings.clients.removeOnImportLabel')).not.toBeInTheDocument() + }) + + it.each(['deluge', 'rtorrent'])('is hidden for %s, whose import path does not remove torrents', type => { + renderTab([makeClient({ type })]) + openEditForm() + + expect(screen.queryByLabelText('settings.clients.removeOnImportLabel')).not.toBeInTheDocument() + }) + + it('is shown for a torrent client and reflects the stored value', () => { + renderTab([makeClient({ type: 'transmission', removeOnImport: true })]) + openEditForm() + + const toggle = screen.getByLabelText('settings.clients.removeOnImportLabel') as HTMLInputElement + expect(toggle.checked).toBe(true) + }) + + it('defaults to off and sends the change on save', async () => { + renderTab([makeClient({ type: 'transmission' })]) + openEditForm() + + const toggle = screen.getByLabelText('settings.clients.removeOnImportLabel') as HTMLInputElement + expect(toggle.checked).toBe(false) + fireEvent.click(toggle) + save() + + await waitFor(() => expect(api.updateDownloadClient).toHaveBeenCalled()) + const [, payload] = vi.mocked(api.updateDownloadClient).mock.calls[0] + expect(payload.removeOnImport).toBe(true) + }) + + it('never sends it enabled for a usenet client', async () => { + renderTab([makeClient({ type: 'sabnzbd', removeOnImport: true })]) + openEditForm() + save() + + await waitFor(() => expect(api.updateDownloadClient).toHaveBeenCalled()) + const [, payload] = vi.mocked(api.updateDownloadClient).mock.calls[0] + expect(payload.removeOnImport).toBe(false) + }) +}) diff --git a/web/src/pages/settings/ClientsTab.tsx b/web/src/pages/settings/ClientsTab.tsx index e3ada7d9d..393a568ca 100644 --- a/web/src/pages/settings/ClientsTab.tsx +++ b/web/src/pages/settings/ClientsTab.tsx @@ -210,6 +210,7 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; const [saving, setSaving] = useState(false) const [saveError, setSaveError] = useState(null) const [pathRemap, setPathRemap] = useState(client.pathRemap || '') + const [removeOnImport, setRemoveOnImport] = useState(client.removeOnImport || false) const [testing, setTesting] = useState(false) const [testResult, setTestResult] = useState<{ ok: boolean; msg: string; warn?: string } | null>(null) const labelCls = 'block text-xs text-slate-600 dark:text-zinc-400 mb-1' @@ -256,6 +257,7 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; category, categoryAudiobook: categoryAudiobook.trim(), pathRemap: pathRemap.trim(), + removeOnImport: supportsRemoveOnImport(type) ? removeOnImport : false, useSsl: useSSL, urlBase: urlBase.trim(), } @@ -394,6 +396,21 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; placeholder={type === 'qbittorrent' ? '/downloads:/media/books' : '/media:/books'} help={downloadClientPathRemapHelp(type)} /> + {supportsRemoveOnImport(type) && ( +
+
+ setRemoveOnImport(e.target.checked)} + className="rounded border-slate-300 dark:border-zinc-700" + /> + +
+

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

+
+ )} {saveError &&

{saveError}

} {testResult && (
@@ -416,6 +433,14 @@ function EditClientForm({ client, onClose, onSaved }: { client: DownloadClient; ) } +// The clients whose import path removes the torrent when the toggle is on: +// tryImportQbittorrent and tryImportTransmission in the scanner. A usenet +// client always clears its own history entry once Bindery has imported the +// job, so there is nothing for the toggle to control there. Deluge and +// rTorrent have no removal wired up yet, so showing them the toggle would +// store a choice that does nothing. +const supportsRemoveOnImport = (t: string) => t === 'qbittorrent' || t === 'transmission' + function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c: DownloadClient) => void }) { const { t } = useTranslation() const [name, setName] = useState('SABnzbd') @@ -431,6 +456,7 @@ function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c: const [saving, setSaving] = useState(false) const [saveError, setSaveError] = useState(null) const [pathRemap, setPathRemap] = useState('') + const [removeOnImport, setRemoveOnImport] = useState(false) const [testing, setTesting] = useState(false) const [testResult, setTestResult] = useState<{ ok: boolean; msg: string; warn?: string } | null>(null) const labelCls = 'block text-xs text-slate-600 dark:text-zinc-400 mb-1' @@ -477,9 +503,15 @@ function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c: setPort('8080') } - const buildData = () => 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() } + // removeOnImport is only sent for a torrent client: a usenet client has no + // torrent to remove, and a new row defaults to off anyway, so sending it + // there would be noise on the wire. + const buildData = () => ({ + ...(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() }), + ...(supportsRemoveOnImport(type) ? { removeOnImport } : {}), + }) const submit = async () => { const data = buildData() @@ -594,6 +626,21 @@ function AddClientForm({ onClose, onAdded }: { onClose: () => void; onAdded: (c: placeholder={type === 'qbittorrent' ? '/downloads:/media/books' : '/media:/books'} help={downloadClientPathRemapHelp(type)} /> + {supportsRemoveOnImport(type) && ( +
+
+ setRemoveOnImport(e.target.checked)} + className="rounded border-slate-300 dark:border-zinc-700" + /> + +
+

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

+
+ )} {saveError &&

{saveError}

} {testResult && (