From 8d2ee249e8ad3a2084587f16d22b2d663c5b5704 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:14:05 +0000 Subject: [PATCH 1/4] feat(requests): show download progress while a request downloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Request-router plugins can now report how far a target's downloads are (TargetStatus.progress, declared with request_router.reports_download_progress). Store it on the target in new download_* columns, and show it on the Requests page, the title page's request bar and the admin queue: a bar once the size is known and "Downloading · 43% · about 12 min left", or the phase (waiting, paused, stalled, importing, import blocked). The reconcile pass records a download's first progress. A new one-minute refresh_request_downloads task then refreshes only downloading targets that have progress, from plugins that declare it. It shares a target-write lock with reconcile, which waits for it rather than skipping, and runs within a 90-second budget. Progress writes leave the target's updated_at, the request status and its history alone; progress clears on completion or failure, when the plugin stops reporting it, or 15 minutes after its server stops answering. A target's raw external status is kept in step with its phase. The v2 API adds RequestDownload on request targets, requests and the title detail's request state, and download_progress_supported on the requests status capability. Clients poll every 30 seconds while something they show downloads. TMDB title details are cached for two minutes so title pages polling a download share one fetch. The plugin SDK is pinned to the silo-plugin-sdk PR commit until v0.19.0 is tagged. Refs #1643 Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/silo/main.go | 1 + .../api/v2/fixtures/get_system_info_ok.json | 2 +- .../api/v2/fixtures/list_my_requests_ok.json | 28 +- .../api/v2/fixtures/request_status_ok.json | 5 +- contracts/api/v2/openapi.json | 92 ++ docs/architecture/api-contract.md | 7 + docs/architecture/media-requests.md | 102 +- go.mod | 2 +- go.sum | 4 +- internal/api/requests_wiring.go | 5 +- internal/apiv2/request_downloads_test.go | 143 +++ internal/apiv2/request_lifecycle.go | 5 +- internal/apiv2/requests.go | 51 +- internal/apiv2/requests_test.go | 22 +- internal/database/pglock/pglock.go | 52 + internal/database/pglock/pglock_test.go | 109 ++ internal/metadata/tmdb/client.go | 119 +- internal/metadata/tmdb/client_test.go | 232 ++++ internal/plugins/request_router_test.go | 16 +- internal/requests/downloads.go | 216 ++++ internal/requests/downloads_test.go | 1135 +++++++++++++++++ internal/requests/provider.go | 36 + internal/requests/repository.go | 41 + internal/requests/router_seasons.go | 34 +- internal/requests/service.go | 96 +- internal/requests/service_test.go | 188 ++- internal/requests/store.go | 18 + internal/requests/targets.go | 94 +- internal/requests/types.go | 101 ++ .../taskmanager/tasks/reconcile_requests.go | 40 +- .../tasks/refresh_all_library_metadata.go | 23 +- .../tasks/refresh_request_downloads.go | 97 ++ .../tasks/refresh_request_downloads_test.go | 339 +++++ internal/workmetrics/work.go | 2 +- ...43912_request_target_download_progress.sql | 29 + web/src/api/types.ts | 27 + web/src/api/v2/requests.test.ts | 69 + web/src/api/v2/schema.ts | 51 + .../RequestDownloadProgress.test.tsx | 86 ++ .../components/RequestDownloadProgress.tsx | 49 + web/src/hooks/queries/admin/requests.ts | 6 +- web/src/hooks/queries/useRequests.test.tsx | 84 ++ web/src/hooks/queries/useRequests.ts | 28 +- web/src/lib/requestDownload.test.ts | 90 ++ web/src/lib/requestDownload.ts | 72 ++ web/src/pages/AdminRequests.v2.test.tsx | 48 + .../components/RequestActionBar.tsx | 20 +- web/src/pages/Requests.test.tsx | 32 + web/src/pages/Requests.tsx | 7 + web/src/pages/TitleDetail.test.tsx | 44 +- web/src/pages/admin-requests/RequestQueue.tsx | 8 + .../admin-requests/RequestQueueDialog.tsx | 8 + 52 files changed, 4120 insertions(+), 95 deletions(-) create mode 100644 internal/apiv2/request_downloads_test.go create mode 100644 internal/requests/downloads.go create mode 100644 internal/requests/downloads_test.go create mode 100644 internal/taskmanager/tasks/refresh_request_downloads.go create mode 100644 internal/taskmanager/tasks/refresh_request_downloads_test.go create mode 100644 migrations/sql/20260928143912_request_target_download_progress.sql create mode 100644 web/src/api/v2/requests.test.ts create mode 100644 web/src/components/RequestDownloadProgress.test.tsx create mode 100644 web/src/components/RequestDownloadProgress.tsx create mode 100644 web/src/lib/requestDownload.test.ts create mode 100644 web/src/lib/requestDownload.ts diff --git a/cmd/silo/main.go b/cmd/silo/main.go index bb0c6d0289..1e9c92f9c8 100644 --- a/cmd/silo/main.go +++ b/cmd/silo/main.go @@ -2979,6 +2979,7 @@ func main() { requestReconcileSvc.SetFulfillmentNotifier(notifications.NewRequestFulfillmentNotifier(notificationSystem)) } taskMgr.Register(tasks.NewReconcileRequestsTask(requestReconcileSvc, 100, deps.DB)) + taskMgr.Register(tasks.NewRefreshRequestDownloadsTask(requestReconcileSvc, 200, deps.DB)) if deps.FolderRepo != nil && deps.LibraryScanQueue != nil && pluginService != nil && pluginInstallationStore != nil { autoscanRepo := autoscan.NewRepository(deps.DB, deps.SecretCipher) if err := autoscanRepo.MarkInterruptedEvents(appCtx); err != nil { diff --git a/contracts/api/v2/fixtures/get_system_info_ok.json b/contracts/api/v2/fixtures/get_system_info_ok.json index ad85e85ae1..dd59d9add6 100644 --- a/contracts/api/v2/fixtures/get_system_info_ok.json +++ b/contracts/api/v2/fixtures/get_system_info_ok.json @@ -1,7 +1,7 @@ { "server_version": "unavailable", "api_major": 2, - "contract_digest": "948f4237fbedbbaa2b4c00fb14cdc2d9f85534718e2c8bc7f37eeaac0a5f77c7", + "contract_digest": "1b7ed48bc565d83aad564fd8ccd16806c5e4589d2a4fe589bc08a6676599a17e", "links": { "openapi": "/api/v2/openapi.json", "capabilities": "/api/v2/capabilities", diff --git a/contracts/api/v2/fixtures/list_my_requests_ok.json b/contracts/api/v2/fixtures/list_my_requests_ok.json index f7c1a240d9..01624f3db0 100644 --- a/contracts/api/v2/fixtures/list_my_requests_ok.json +++ b/contracts/api/v2/fixtures/list_my_requests_ok.json @@ -38,9 +38,9 @@ "tmdb_id": 949, "title": "Heat", "year": 1995, - "status": "approved", + "status": "downloading", "outcome": "active", - "state": "approved", + "state": "processing", "seasons": [], "season_progress": [], "requested_by_user_id": "1", @@ -53,14 +53,32 @@ "request_id": "r-1", "quality": "1080p", "is_anime": false, - "status": "queued", + "status": "downloading", "created_at": "2026-01-02T03:04:05.678Z", - "updated_at": "2026-01-02T03:04:05.678Z" + "updated_at": "2026-01-02T03:04:05.678Z", + "download": { + "phase": "downloading", + "percent": 43, + "bytes_total": 4294967296, + "bytes_left": 2448131358, + "estimated_completion_at": "2026-01-02T03:16:05.678Z", + "downloads": 1, + "updated_at": "2026-01-02T03:04:05.678Z" + } } ], "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z", - "approved_at": "2026-01-02T03:04:05.678Z" + "approved_at": "2026-01-02T03:04:05.678Z", + "download": { + "phase": "downloading", + "percent": 43, + "bytes_total": 4294967296, + "bytes_left": 2448131358, + "estimated_completion_at": "2026-01-02T03:16:05.678Z", + "downloads": 1, + "updated_at": "2026-01-02T03:04:05.678Z" + } } ], "page": { diff --git a/contracts/api/v2/fixtures/request_status_ok.json b/contracts/api/v2/fixtures/request_status_ok.json index fb5c4df0ff..a1bb72355b 100644 --- a/contracts/api/v2/fixtures/request_status_ok.json +++ b/contracts/api/v2/fixtures/request_status_ok.json @@ -1,10 +1,11 @@ { - "revision": "2eed5bf1bef8a5c20e02b089577b35d088cd5442fb8af1b85156ff2772b2037a", + "revision": "dc6458db8af9354952b69baa1805dcb12a4cee23a40517a0dea2c3f02e441bb2", "state": "available", "allowed": true, "requests_enabled": true, "rating_restrictions_enforced": true, "follow_supported": true, "season_requests_supported": true, - "missing_seasons_requestable": false + "missing_seasons_requestable": false, + "download_progress_supported": true } diff --git a/contracts/api/v2/openapi.json b/contracts/api/v2/openapi.json index 87752d1ef9..efab7c8207 100644 --- a/contracts/api/v2/openapi.json +++ b/contracts/api/v2/openapi.json @@ -24669,6 +24669,10 @@ "description": "Whether the current principal may use the capability", "type": "boolean" }, + "download_progress_supported": { + "description": "Whether the server reports download progress (download on requests, their targets, and the title detail's request state). Whether a given request has any depends on its download server's request plugin.", + "type": "boolean" + }, "follow_supported": { "type": "boolean" }, @@ -24706,6 +24710,7 @@ "follow_supported", "season_requests_supported", "missing_seasons_requestable", + "download_progress_supported", "revision", "state", "allowed" @@ -28534,6 +28539,10 @@ "patternDescription": "a non-zero RFC 3339 instant", "type": "string" }, + "download": { + "$ref": "#/components/schemas/RequestDownload", + "description": "How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time" + }, "external_id": { "type": "string" }, @@ -36324,6 +36333,81 @@ }, "type": "object" }, + "RequestDownload": { + "additionalProperties": false, + "properties": { + "bytes_left": { + "description": "Bytes still to download; present whenever bytes_total is", + "examples": [ + 2448131358 + ], + "format": "int64", + "minimum": 0, + "type": "integer" + }, + "bytes_total": { + "description": "Size of the downloads in bytes; absent while unknown", + "examples": [ + 4294967296 + ], + "format": "int64", + "minimum": 1, + "type": "integer" + }, + "downloads": { + "description": "Distinct downloads in flight; a season pack counts once", + "examples": [ + 1 + ], + "format": "int64", + "minimum": 0, + "type": "integer" + }, + "estimated_completion_at": { + "description": "When the download server expects the downloads to finish; absent when it cannot tell", + "examples": [ + "2026-01-02T03:16:05.000Z" + ], + "format": "date-time", + "pattern": "^(?:[1-9][0-9]{3}|0[1-9][0-9]{2}|00[1-9][0-9]|000[2-9])-", + "patternDescription": "a non-zero RFC 3339 instant", + "type": "string" + }, + "percent": { + "description": "How much has downloaded, rounded down; absent while the size is unknown", + "examples": [ + 43 + ], + "format": "int64", + "maximum": 100, + "minimum": 0, + "type": "integer" + }, + "phase": { + "description": "queued, downloading, paused, stalled, importing or import_blocked. More values may be added: read an unknown one as downloading, without a percentage", + "examples": [ + "downloading" + ], + "type": "string" + }, + "updated_at": { + "description": "When the server last heard from the download server. A client may hide figures older than about ten minutes", + "examples": [ + "2026-01-02T03:04:05.000Z" + ], + "format": "date-time", + "pattern": "^(?:[1-9][0-9]{3}|0[1-9][0-9]{2}|00[1-9][0-9]|000[2-9])-", + "patternDescription": "a non-zero RFC 3339 instant", + "type": "string" + } + }, + "required": [ + "phase", + "downloads", + "updated_at" + ], + "type": "object" + }, "RequestMediaCastMember": { "additionalProperties": false, "properties": { @@ -36741,6 +36825,10 @@ "RequestMediaState": { "additionalProperties": false, "properties": { + "download": { + "$ref": "#/components/schemas/RequestDownload", + "description": "How far the active request's downloads are, while its download server reports them. Only the title detail (getRequestMediaDetail) carries it" + }, "following": { "description": "Whether the viewer will be notified when the media becomes available: they requested it or follow it", "examples": [ @@ -36846,6 +36934,10 @@ "patternDescription": "a non-zero RFC 3339 instant", "type": "string" }, + "download": { + "$ref": "#/components/schemas/RequestDownload", + "description": "How far this target's downloads are, while its download server reports them" + }, "external_id": { "description": "The integration's own identifier", "type": "string" diff --git a/docs/architecture/api-contract.md b/docs/architecture/api-contract.md index 178b6cea2f..05a40281bf 100644 --- a/docs/architecture/api-contract.md +++ b/docs/architecture/api-contract.md @@ -1818,6 +1818,13 @@ series detail lists the regular seasons with availability and request coverage; requests carry `seasons`, `season_progress` and the `partially_available` state; `GET /api/v2/requests/status` advertises `season_requests_supported`. +While a request downloads, requests, their targets and the title detail's +request state carry `download` (phase, percent, bytes, estimated completion, +and when the server last heard from the download server), and +`GET /api/v2/requests/status` advertises `download_progress_supported`. The +phase is an open set: clients render an unknown one like `downloading`. See +[Media requests](media-requests.md#download-progress). + A profile can follow a title another profile already requested, to be notified when it becomes available, with `PUT` and `DELETE /api/v2/requests/follows/{media_type}/{tmdb_id}`. Both are naturally idempotent. diff --git a/docs/architecture/media-requests.md b/docs/architecture/media-requests.md index 832f649a5e..fa9f0efac0 100644 --- a/docs/architecture/media-requests.md +++ b/docs/architecture/media-requests.md @@ -4,7 +4,8 @@ A user asks for a movie or series the server does not have, an admin approves it (or the user's policy approves it automatically), a request-router plugin sends it to a downstream service such as Sonarr or Radarr, and the request completes when the media is in the library. The code lives in `internal/requests`; the reconcile -pass is the `reconcile_requests` task in `internal/taskmanager/tasks`. +pass is the `reconcile_requests` task in `internal/taskmanager/tasks`, and the +download refresh pass is `refresh_request_downloads` beside it. ## State @@ -352,6 +353,105 @@ candidates in `last_reconciled_at` order, stamps each one when checked, even if it errors, and looks presence up in one batch per media type. The 30-day bound keeps an upgrade from completing, and notifying, a backlog of old failures. +A second task, `refresh_request_downloads`, runs every minute between reconcile +passes (see [Download progress](#download-progress)). It asks only about +targets in `downloading` that already have download progress and whose plugin +declares `reports_download_progress`, and applies what the plugin answers +through the same code as reconcile: status transitions and download progress. +It submits nothing, checks no library presence and sends no notification; those +stay with the reconcile pass. `queued` targets stay on the five-minute cadence +on purpose: a requested but unreleased movie can sit `queued` for months, and +polling it every minute would multiply calls to the download server for +nothing. A scheduled run is skipped, without a task history entry, while no +downloading target has progress. + +Two advisory locks keep the passes apart. The reconcile lock lets one server +run each reconcile pass; the others skip. The request target write lock keeps +the two passes from writing the same target at once. A reconcile pass takes it +after its own lock and waits up to two minutes for a refresh pass that holds +it, so a refresh never makes a reconcile pass skip; a wait that runs out fails +the pass. A refresh pass only tries the write lock, so it runs on one server at +a time and skips while a reconcile pass runs anywhere in the cluster. A refresh +pass also stops after 90 seconds, cutting the plugin call in flight and leaving +the requests it has not reached for the next pass, so it always ends inside the +reconcile pass's wait, even while a download server stops answering and every +call to it runs to the router's 60-second deadline. + +## Download progress + +While a target downloads, its router plugin can report how far along it is in +`CheckStatus` (`TargetStatus.progress`): a phase, the size and bytes left summed +over the target's distinct downloads (a season pack counts once), the latest +estimated completion, and the number of downloads. A plugin declares that it +does with `request_router.reports_download_progress` in its manifest, which the +host reads from the stored capability metadata without launching the plugin. +The host keeps progress only from a declaring plugin; any other plugin has none +and keeps the five-minute cadence. + +The phase is `queued`, `downloading`, `paused`, `stalled`, `importing` or +`import_blocked`. The host reads an unknown or empty phase as `downloading`, +and a report with neither a phase nor a size as none. The set is open: clients +must render an unknown phase like `downloading`, without a percentage. When +several downloads or targets are combined, the phase follows +`import_blocked` > `stalled` > `downloading` > `importing` > `paused` > +`queued`, so a download that needs attention shows, and otherwise +`downloading` wins while anything still downloads. + +Progress lives on the target row (`download_*` columns on +`media_request_targets`), so any node serves the latest value and a node dying +loses nothing. `download_updated_at` records when the server last heard from +the plugin. A progress write touches only those columns, and only while the +target is `queued` or `downloading`, so a late report cannot give a finished +target progress again. It does not move the target's `updated_at`, which dates +status changes for the stalled-target backstop, recompute the request's status, +or add to its history. When the server's own state moves without changing the +target's status (an import that stalls), the pass records the new raw +`external_status` the same way, so the raw status and the progress shown +beside it agree. Progress is cleared when a target completes or fails, +and when the plugin stops reporting any for a live target (the download left +the queue). A pass writes nothing for a target that has no progress and +reports none. A target with progress that a pass asks about without getting +its status back (the plugin skipped a server that errored or no longer has the +title, or the call failed) keeps its progress, since one missed answer is +usually a blip, until the last report is 15 minutes old; the next unanswered +pass then clears it, so a frozen figure stops showing and clients stop polling +for it. + +The reconcile pass records a download's first progress, so progress appears +within one reconcile pass of a download starting. From then on the refresh +task refreshes it every minute, taking requests by their downloading target +with progress asked about longest ago (`download_checked_at`), at most 200 a +minute. A target counts as asked about whether or not its server answered, so +one that stops answering takes its turn and moves to the back instead of +heading every batch. A target leaves the refresh task as soon as its progress +clears. A downloading target without progress stays with the reconcile pass +however long it waits: a plugin can report a title downloading with nothing in +its download queue for it (Seerr keeps media at Processing until a release +arrives), and such targets must not crowd out the ones that are downloading. +The refresh task also clears, without a call, the progress of a target whose +server is gone, disabled or unusable, or whose plugin no longer declares +progress, since nothing will refresh it. + +The v2 API carries `download` on each request target, on the request itself, +and on the title detail's request state. The request's figure combines its live +targets (1080p and 4K together): bytes and downloads summed, the phase by the +order above, the latest estimate, and the oldest report's time, so a client +that hides figures older than about ten minutes hides a partly stale one too. +Its size is unknown while any target's is. `percent` is +`floor((total - left) * 100 / total)`, and the byte counts and percent are +absent while the size is unknown. Only the title detail +(`GET /api/v2/requests/detail/{media_type}/{tmdb_id}`) fills the request +state's `download`, with one targets query for an active request that is queued +or downloading; search and discovery do not load targets per result. It +carries no requester identity, so anyone who can see the title's request state +sees it. `GET /api/v2/requests/status` advertises +`download_progress_supported`. v1 does not carry progress. Release names, +indexers, download clients, paths and queue messages never reach a client. +Clients poll every 30 seconds while anything they show has `download`, and stop +polling otherwise. The TMDB client keeps a title's detail for two minutes, so +title pages polling a download share one TMDB fetch per server however many are +open. + ## Re-requesting a failed title Creating a request deletes the requester's own failed requests for the same diff --git a/go.mod b/go.mod index 5c2bcbfa82..a77f215cba 100644 --- a/go.mod +++ b/go.mod @@ -129,7 +129,7 @@ require ( ) require ( - github.com/Silo-Server/silo-plugin-sdk v0.18.0 + github.com/Silo-Server/silo-plugin-sdk v0.18.1-0.20260928194839-1abd582d0304 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.14 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.30 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.30 // indirect diff --git a/go.sum b/go.sum index d89afb2455..c6fd89f43d 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,8 @@ github.com/PuerkitoBio/goquery v1.8.0 h1:PJTF7AmFCFKk1N6V6jmKfrNH9tV5pNE6lZMkG0g github.com/PuerkitoBio/goquery v1.8.0/go.mod h1:ypIiRMtY7COPGk+I/YbZLbxsxn9g5ejnI2HSMtkjZvI= github.com/SherClockHolmes/webpush-go v1.4.0 h1:ocnzNKWN23T9nvHi6IfyrQjkIc0oJWv1B1pULsf9i3s= github.com/SherClockHolmes/webpush-go v1.4.0/go.mod h1:XSq8pKX11vNV8MJEMwjrlTkxhAj1zKfxmyhdV7Pd6UA= -github.com/Silo-Server/silo-plugin-sdk v0.18.0 h1:82KRDre+c8CY4jqqh51JVe1wM4rVOdiYOogqD5ij9dQ= -github.com/Silo-Server/silo-plugin-sdk v0.18.0/go.mod h1:abwsCEKuPAAgeAqpNGbwoaut2eQlC/Kj97u89Vvg9qM= +github.com/Silo-Server/silo-plugin-sdk v0.18.1-0.20260928194839-1abd582d0304 h1:D8lB4rqvGxOQTw9DeJiE7Dwr6qTVk2xqjI5uuyJM+i0= +github.com/Silo-Server/silo-plugin-sdk v0.18.1-0.20260928194839-1abd582d0304/go.mod h1:abwsCEKuPAAgeAqpNGbwoaut2eQlC/Kj97u89Vvg9qM= github.com/TwiN/go-color v1.4.1 h1:mqG0P/KBgHKVqmtL5ye7K0/Gr4l6hTksPgTgMk3mUzc= github.com/TwiN/go-color v1.4.1/go.mod h1:WcPf/jtiW95WBIsEeY1Lc/b8aaWoiqQpu5cf8WFxu+s= github.com/abadojack/whatlanggo v1.0.1 h1:19N6YogDnf71CTHm3Mp2qhYfkRdyvbgwWdd2EPxJRG4= diff --git a/internal/api/requests_wiring.go b/internal/api/requests_wiring.go index fcfd857c1d..9bca586b26 100644 --- a/internal/api/requests_wiring.go +++ b/internal/api/requests_wiring.go @@ -33,7 +33,10 @@ func (a PluginRequestRouterAdapter) RouterFeatures(ctx context.Context, installa if err != nil { return mediarequests.RouterFeatures{}, err } - return mediarequests.RouterFeatures{SupportsSeasons: descriptor.GetSupportsSeasons()}, nil + return mediarequests.RouterFeatures{ + SupportsSeasons: descriptor.GetSupportsSeasons(), + ReportsDownloadProgress: descriptor.GetReportsDownloadProgress(), + }, nil } // AttachRequestRouter wires the plugin-backed router provider onto a requests diff --git a/internal/apiv2/request_downloads_test.go b/internal/apiv2/request_downloads_test.go new file mode 100644 index 0000000000..ed0fd84cb5 --- /dev/null +++ b/internal/apiv2/request_downloads_test.go @@ -0,0 +1,143 @@ +package apiv2 + +import ( + "encoding/json" + "math" + "net/http" + "strings" + "testing" + "time" + + mediarequests "github.com/Silo-Server/silo-server/internal/requests" +) + +func TestRequestDownloadOf(t *testing.T) { + updated := fixedTime() + eta := updated.Add(10 * time.Minute) + progress := func(total, left int64) *mediarequests.DownloadProgress { + return &mediarequests.DownloadProgress{Phase: mediarequests.DownloadPhaseDownloading, BytesTotal: total, BytesLeft: left, Downloads: 1, UpdatedAt: updated} + } + for _, tc := range []struct { + name string + total, left int64 + want int + }{ + {"just started", 1000, 1000, 0}, + {"rounded down", 3, 1, 66}, + {"almost done", 1000, 1, 99}, + {"done", 1000, 0, 100}, + {"left above total", 1000, 5000, 0}, + {"negative left", 1000, -5, 100}, + {"huge", math.MaxInt64, math.MaxInt64 / 2, 50}, + } { + t.Run(tc.name, func(t *testing.T) { + got := requestDownloadOf(progress(tc.total, tc.left)) + if got.Percent == nil || *got.Percent != tc.want { + t.Fatalf("percent = %v, want %d", got.Percent, tc.want) + } + if got.BytesTotal == nil || *got.BytesTotal != tc.total || got.BytesLeft == nil || *got.BytesLeft < 0 || *got.BytesLeft > tc.total { + t.Fatalf("bytes = %v/%v, want the total and a left within it", got.BytesTotal, got.BytesLeft) + } + }) + } + + if requestDownloadOf(nil) != nil { + t.Fatal("no progress must map to no download") + } + unknown := requestDownloadOf(&mediarequests.DownloadProgress{Phase: mediarequests.DownloadPhaseQueued, Downloads: -1, UpdatedAt: updated, EstimatedCompletion: &eta}) + if unknown.Percent != nil || unknown.BytesTotal != nil || unknown.BytesLeft != nil { + t.Fatalf("unknown size: %+v, want no percent and no byte counts", unknown) + } + if unknown.Phase != "queued" || unknown.Downloads != 0 || unknown.EstimatedCompletionAt == nil || !unknown.EstimatedCompletionAt.Equal(eta) || !unknown.UpdatedAt.Equal(updated) { + t.Fatalf("unknown size: %+v", unknown) + } + raw, err := json.Marshal(requestDownloadOf(progress(0, 0))) + if err != nil { + t.Fatal(err) + } + for _, field := range []string{"percent", "bytes_total", "bytes_left", "estimated_completion_at"} { + if strings.Contains(string(raw), `"`+field+`"`) { + t.Fatalf("%s: %s must be omitted", raw, field) + } + } +} + +func TestRequestResponsesCarryDownload(t *testing.T) { + h := newTestHandler(t, requestDeps(fixtureRequests())) + type download struct { + Phase string `json:"phase"` + Percent *int `json:"percent"` + BytesTotal *int64 `json:"bytes_total"` + BytesLeft *int64 `json:"bytes_left"` + EstimatedCompletionAt string `json:"estimated_completion_at"` + Downloads int `json:"downloads"` + UpdatedAt string `json:"updated_at"` + } + var got struct { + Download *download `json:"download"` + Targets []struct { + Download *download `json:"download"` + } `json:"targets"` + } + rec := do(t, h, http.MethodGet, "/api/v2/requests/r-1", "", requestOwner) + decodeBody(t, rec.Body, &got) + if rec.Code != 200 || got.Download == nil || len(got.Targets) != 1 || got.Targets[0].Download == nil { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } + for _, d := range []*download{got.Download, got.Targets[0].Download} { + if d.Phase != "downloading" || d.Percent == nil || *d.Percent != 43 || d.BytesTotal == nil || *d.BytesTotal != 4294967296 || + d.BytesLeft == nil || *d.BytesLeft != 2448131358 || d.Downloads != 1 || + d.EstimatedCompletionAt != "2026-01-02T03:16:05.678Z" || d.UpdatedAt != "2026-01-02T03:04:05.678Z" { + t.Fatalf("download = %+v in %s", d, rec.Body.String()) + } + } + + // A request without progress carries no download at all. + rec = do(t, h, http.MethodGet, "/api/v2/requests/r-2", "", requestOwner) + if rec.Code != 200 || strings.Contains(rec.Body.String(), `"download"`) { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } +} + +func TestRequestMediaDetailCarriesDownload(t *testing.T) { + svc := fixtureRequests() + h := newTestHandler(t, requestDeps(svc)) + rec := do(t, h, http.MethodGet, "/api/v2/requests/detail/movie/949", "", requestOwner) + if rec.Code != 200 || strings.Contains(rec.Body.String(), `"download"`) { + t.Fatalf("without progress: %d %s", rec.Code, rec.Body.String()) + } + + svc.detailDownload = fixtureDownload() + rec = do(t, h, http.MethodGet, "/api/v2/requests/detail/movie/949", "", requestOwner) + var got struct { + Request struct { + Download *struct { + Phase string `json:"phase"` + Percent *int `json:"percent"` + } `json:"download"` + } `json:"request"` + Recommendations []struct { + Request map[string]any `json:"request"` + } `json:"recommendations"` + } + decodeBody(t, rec.Body, &got) + if rec.Code != 200 || got.Request.Download == nil || got.Request.Download.Phase != "downloading" || + got.Request.Download.Percent == nil || *got.Request.Download.Percent != 43 { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } + if len(got.Recommendations) != 1 || got.Recommendations[0].Request["download"] != nil { + t.Fatalf("recommendations = %+v, want no download on them", got.Recommendations) + } +} + +func TestRequestStatusAdvertisesDownloadProgress(t *testing.T) { + h := lifecycleHandler(&fakeLifecycle{}, &fakeWatchLifecycle{}) + rec := do(t, h, http.MethodGet, Prefix+"/requests/status", "", requestOwner) + var got struct { + DownloadProgressSupported *bool `json:"download_progress_supported"` + } + decodeBody(t, rec.Body, &got) + if rec.Code != 200 || got.DownloadProgressSupported == nil || !*got.DownloadProgressSupported { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } +} diff --git a/internal/apiv2/request_lifecycle.go b/internal/apiv2/request_lifecycle.go index 1c1bc327e2..21fad4600c 100644 --- a/internal/apiv2/request_lifecycle.go +++ b/internal/apiv2/request_lifecycle.go @@ -46,6 +46,9 @@ type FeatureStatus struct { // MissingSeasonsRequestable reports whether a series already in the // library can be requested for its missing seasons. MissingSeasonsRequestable bool `json:"missing_seasons_requestable" doc:"Whether a series already in the library can be requested for the seasons it is missing. False while a download server that takes series uses a request plugin that cannot fetch individual seasons, so such a series stays already_available."` + // DownloadProgressSupported advertises download on requests, their + // targets, and the title detail's request state. + DownloadProgressSupported bool `json:"download_progress_supported" doc:"Whether the server reports download progress (download on requests, their targets, and the title detail's request state). Whether a given request has any depends on its download server's request plugin."` } type RequestFeatureStatusOutput struct { Status int @@ -185,7 +188,7 @@ func registerRequestLifecycle(reg *Registry, requests RequestLifecycleService, p return nil, requestProblem(err) } } - return &RequestFeatureStatusOutput{Body: FeatureStatus{Capability: Capability{State: enabledCapabilityState(status.RequestsEnabled), Allowed: &allowed}, RequestsEnabled: status.RequestsEnabled, RatingRestrictionsEnforced: status.RatingRestrictionsEnforced, FollowSupported: true, SeasonRequestsSupported: true, MissingSeasonsRequestable: status.MissingSeasonsRequestable}}, nil + return &RequestFeatureStatusOutput{Body: FeatureStatus{Capability: Capability{State: enabledCapabilityState(status.RequestsEnabled), Allowed: &allowed}, RequestsEnabled: status.RequestsEnabled, RatingRestrictionsEnforced: status.RatingRestrictionsEnforced, FollowSupported: true, SeasonRequestsSupported: true, MissingSeasonsRequestable: status.MissingSeasonsRequestable, DownloadProgressSupported: true}}, nil }) Register(reg, op(http.MethodPost, "/requests/{id}/cancel", "cancelRequest", "Cancel an accessible request."), func(ctx context.Context, in *RequestCancelInput) (*MediaRequestOutput, error) { if requests == nil { diff --git a/internal/apiv2/requests.go b/internal/apiv2/requests.go index 29a50efa80..0076e5bc32 100644 --- a/internal/apiv2/requests.go +++ b/internal/apiv2/requests.go @@ -4,6 +4,7 @@ import ( "cmp" "context" "errors" + "math/bits" "net/http" "slices" "strconv" @@ -30,6 +31,21 @@ type RequestMediaState struct { Following bool `json:"following" doc:"Whether the viewer will be notified when the media becomes available: they requested it or follow it" example:"false"` // RequestedByViewer tells a client whether to offer a follow toggle. RequestedByViewer bool `json:"requested_by_viewer" doc:"Whether the viewing profile made the active request, so there is nothing to follow" example:"false"` + // Download is filled on the title detail only: search and discovery do + // not load each result's targets. + Download *RequestDownload `json:"download,omitempty" doc:"How far the active request's downloads are, while its download server reports them. Only the title detail (getRequestMediaDetail) carries it"` +} + +// RequestDownload is how far downloads are, as the download server last +// reported them. +type RequestDownload struct { + Phase string `json:"phase" doc:"queued, downloading, paused, stalled, importing or import_blocked. More values may be added: read an unknown one as downloading, without a percentage" example:"downloading"` + Percent *int `json:"percent,omitempty" minimum:"0" maximum:"100" doc:"How much has downloaded, rounded down; absent while the size is unknown" example:"43"` + BytesTotal *int64 `json:"bytes_total,omitempty" minimum:"1" doc:"Size of the downloads in bytes; absent while unknown" example:"4294967296"` + BytesLeft *int64 `json:"bytes_left,omitempty" minimum:"0" doc:"Bytes still to download; present whenever bytes_total is" example:"2448131358"` + EstimatedCompletionAt *Instant `json:"estimated_completion_at,omitempty" doc:"When the download server expects the downloads to finish; absent when it cannot tell" example:"2026-01-02T03:16:05.000Z"` + Downloads int `json:"downloads" minimum:"0" doc:"Distinct downloads in flight; a season pack counts once" example:"1"` + UpdatedAt Instant `json:"updated_at" doc:"When the server last heard from the download server. A client may hide figures older than about ten minutes" example:"2026-01-02T03:04:05.000Z"` } // RequestMediaResult is one discovery or search card. @@ -184,6 +200,8 @@ type RequestTarget struct { RouteName string `json:"route_name,omitempty" doc:"The routing rule that sent this target to its server, as named when it was sent"` CreatedAt Instant `json:"created_at" example:"2026-01-02T03:04:05.000Z"` UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` + // Download is set while the target's router plugin reports progress. + Download *RequestDownload `json:"download,omitempty" doc:"How far this target's downloads are, while its download server reports them"` } // MediaRequest is one media request. @@ -218,6 +236,7 @@ type MediaRequest struct { UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` ApprovedAt *Instant `json:"approved_at,omitempty"` CompletedAt *Instant `json:"completed_at,omitempty"` + Download *RequestDownload `json:"download,omitempty" doc:"How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time"` } // MediaRequestOutput is a single-request response. @@ -810,6 +829,7 @@ func mediaRequestOf(r *mediarequests.Request) MediaRequest { UpdatedAt: NewInstant(r.UpdatedAt), ApprovedAt: instantPtr(r.ApprovedAt), CompletedAt: instantPtr(r.CompletedAt), + Download: requestDownloadOf(r.Download()), } if r.RequestedByUserID != 0 { out.RequestedByUserID = IDFromInt(int64(r.RequestedByUserID)) @@ -821,11 +841,40 @@ func mediaRequestOf(r *mediarequests.Request) MediaRequest { IntegrationKind: t.IntegrationKind, InstanceName: t.InstanceName, Quality: string(t.Quality), IsAnime: t.IsAnime, ExternalID: t.ExternalID, ExternalStatus: t.ExternalStatus, Status: string(t.Status), LastError: t.LastError, RouteName: t.RouteName, CreatedAt: NewInstant(t.CreatedAt), UpdatedAt: NewInstant(t.UpdatedAt), + Download: requestDownloadOf(t.Download), }) } return out } +// requestDownloadOf maps download progress. The byte counts and percent are +// left out while the size is unknown (a total of 0). +func requestDownloadOf(d *mediarequests.DownloadProgress) *RequestDownload { + if d == nil { + return nil + } + out := &RequestDownload{ + Phase: string(d.Phase), + EstimatedCompletionAt: instantPtr(d.EstimatedCompletion), + Downloads: max(d.Downloads, 0), + UpdatedAt: NewInstant(d.UpdatedAt), + } + if d.BytesTotal > 0 { + total, left := d.BytesTotal, min(max(d.BytesLeft, 0), d.BytesTotal) + percent := downloadPercent(total, left) + out.BytesTotal, out.BytesLeft, out.Percent = &total, &left, &percent + } + return out +} + +// downloadPercent is floor((total-left)*100/total) for 0 <= left <= total and +// total > 0. The product is taken in 128 bits, so no total can overflow it. +func downloadPercent(total, left int64) int { + hi, lo := bits.Mul64(uint64(total-left), 100) + percent, _ := bits.Div64(hi, lo, uint64(total)) + return int(percent) +} + func requestSeasonProgressOf(progress []mediarequests.SeasonProgress) []RequestSeasonProgress { out := make([]RequestSeasonProgress, 0, len(progress)) for _, p := range progress { @@ -835,7 +884,7 @@ func requestSeasonProgressOf(progress []mediarequests.SeasonProgress) []RequestS } func requestMediaStateOf(s mediarequests.RequestState) RequestMediaState { - return RequestMediaState{Status: string(s.Status), Requestable: s.Requestable, Reason: s.Reason, RequestID: ID(s.RequestID), Following: s.Following, RequestedByViewer: s.RequestedByViewer, State: string(s.State)} + return RequestMediaState{Status: string(s.Status), Requestable: s.Requestable, Reason: s.Reason, RequestID: ID(s.RequestID), Following: s.Following, RequestedByViewer: s.RequestedByViewer, State: string(s.State), Download: requestDownloadOf(s.Download)} } func requestMediaResultsOf(results []mediarequests.MediaResult) []RequestMediaResult { diff --git a/internal/apiv2/requests_test.go b/internal/apiv2/requests_test.go index 800d3afb00..dc1b11ab4b 100644 --- a/internal/apiv2/requests_test.go +++ b/internal/apiv2/requests_test.go @@ -8,6 +8,7 @@ import ( "slices" "strings" "testing" + "time" mediarequests "github.com/Silo-Server/silo-server/internal/requests" ) @@ -17,6 +18,8 @@ import ( type fakeRequests struct { requests []*mediarequests.Request err error + // detailDownload is the download progress GetDetail reports. + detailDownload *mediarequests.DownloadProgress lastViewer mediarequests.Viewer lastFilter mediarequests.ListFilter @@ -78,7 +81,7 @@ func (f *fakeRequests) GetDetail(_ context.Context, viewer mediarequests.Viewer, MediaType: mediaType, TMDBID: tmdbID, IMDbID: "tt0113277", Title: "Heat", Year: 1995, Runtime: 170, Genres: []string{"Crime"}, Cast: []mediarequests.MediaCastMember{{Name: "Al Pacino", Character: "Vincent Hanna"}}, Director: "Michael Mann", Recommendations: []mediarequests.MediaResult{fixtureResult(950)}, Availability: mediarequests.AvailabilityAvailable, - LibraryContentID: "movie:heat-1995", Request: mediarequests.RequestState{Reason: "already_available"}, + LibraryContentID: "movie:heat-1995", Request: mediarequests.RequestState{Reason: "already_available", Download: f.detailDownload}, }, nil } @@ -212,7 +215,22 @@ func fixtureRequests() *fakeRequests { other.RequestedByUserID, other.RequestedByProfileID = 2, "p-primary" pending := fixtureMediaRequest("r-2", 950) pending.Status = mediarequests.StatusPending - return &fakeRequests{requests: []*mediarequests.Request{fixtureMediaRequest("r-1", 949), pending, other}} + // r-1 is downloading, so the fixtures show a request with download + // progress next to ones without. + downloading := fixtureMediaRequest("r-1", 949) + downloading.Status = mediarequests.StatusDownloading + downloading.Targets[0].Status = mediarequests.StatusDownloading + downloading.Targets[0].Download = fixtureDownload() + return &fakeRequests{requests: []*mediarequests.Request{downloading, pending, other}} +} + +// fixtureDownload is a 4 GiB download 43% of the way. +func fixtureDownload() *mediarequests.DownloadProgress { + eta := fixedTime().Add(12 * time.Minute) + return &mediarequests.DownloadProgress{ + Phase: mediarequests.DownloadPhaseDownloading, BytesTotal: 4294967296, BytesLeft: 2448131358, + EstimatedCompletion: &eta, Downloads: 1, UpdatedAt: fixedTime(), + } } func decodeBody(t *testing.T, rec interface{ String() string }, into any) { diff --git a/internal/database/pglock/pglock.go b/internal/database/pglock/pglock.go index 9a41906358..5c36e1cc59 100644 --- a/internal/database/pglock/pglock.go +++ b/internal/database/pglock/pglock.go @@ -11,9 +11,12 @@ package pglock import ( "context" + "errors" "fmt" + "strconv" "time" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" ) @@ -58,6 +61,55 @@ func TryAcquire(ctx context.Context, pool *pgxpool.Pool, key int64) (*Lock, bool return &Lock{conn: conn, key: key}, true, nil } +// lockNotAvailable is the SQLSTATE PostgreSQL reports when lock_timeout ends a +// lock wait. +const lockNotAvailable = "55P03" + +// Acquire takes advisory lock key, waiting up to wait for the session holding +// it to release it. It reports acquired false (with a nil Lock and no error) +// when the wait runs out, or when pool is nil. A wait of zero or less does not +// wait at all. ctx still bounds the call. +// +// PostgreSQL enforces the wait with a transaction-local lock_timeout, so a +// wait that runs out leaves the lock queue and returns a clean connection to +// the pool. The lock itself is session-level and outlives that transaction. +// Any other failure closes the connection instead of returning it, because +// PostgreSQL may have granted the lock just before reporting the error. +// +// The caller owns the returned Lock and must call Release exactly once. +func Acquire(ctx context.Context, pool *pgxpool.Pool, key int64, wait time.Duration) (*Lock, bool, error) { + if wait <= 0 { + return TryAcquire(ctx, pool, key) + } + if pool == nil { + return nil, false, nil + } + conn, err := pool.Acquire(ctx) + if err != nil { + return nil, false, fmt.Errorf("acquiring connection for advisory lock %d: %w", key, err) + } + tx, err := conn.Begin(ctx) + if err == nil { + _, err = tx.Exec(ctx, `SELECT set_config('lock_timeout', $1, true)`, strconv.FormatInt(max(wait.Milliseconds(), 1), 10)) + } + if err == nil { + _, err = tx.Exec(ctx, `SELECT pg_advisory_lock($1)`, key) + if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == lockNotAvailable { + _ = tx.Rollback(ctx) + conn.Release() + return nil, false, nil + } + } + if err == nil { + err = tx.Commit(ctx) + } + if err != nil { + _ = conn.Hijack().Close(context.WithoutCancel(ctx)) + return nil, false, fmt.Errorf("acquiring advisory lock %d: %w", key, err) + } + return &Lock{conn: conn, key: key}, true, nil +} + // Conn exposes the connection holding the lock. Work that must be serialized // against the lock may run on any connection; this is for callers that want to // keep it on the locked session. diff --git a/internal/database/pglock/pglock_test.go b/internal/database/pglock/pglock_test.go index 9f7b3bf64c..445366308b 100644 --- a/internal/database/pglock/pglock_test.go +++ b/internal/database/pglock/pglock_test.go @@ -122,3 +122,112 @@ func TestTryAcquireNilPoolReportsNotAcquired(t *testing.T) { t.Fatalf("Release on nil lock: %v", err) } } + +// lockWaiters counts the sessions queued for advisory lock key. +func lockWaiters(t *testing.T, pool *pgxpool.Pool, key int64) int { + t.Helper() + var waiters int + err := pool.QueryRow(context.Background(), ` + SELECT count(*) FROM pg_locks + WHERE locktype = 'advisory' + AND NOT granted + AND ((classid::bigint << 32) | objid::bigint) = $1`, key).Scan(&waiters) + if err != nil { + t.Fatalf("inspect pg_locks: %v", err) + } + return waiters +} + +func TestAcquireWaitsForTheHolder(t *testing.T) { + pool := testPool(t) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + held, acquired, err := TryAcquire(ctx, pool, pglockTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire = (%v, %v), want acquired", acquired, err) + } + type result struct { + lock *Lock + acquired bool + err error + } + done := make(chan result, 1) + go func() { + lock, acquired, err := Acquire(ctx, pool, pglockTestKey, 20*time.Second) + done <- result{lock, acquired, err} + }() + for lockWaiters(t, pool, pglockTestKey) == 0 { + select { + case got := <-done: + t.Fatalf("Acquire returned (%v, %v) while the lock was held", got.acquired, got.err) + case <-ctx.Done(): + t.Fatal("Acquire never queued for the lock") + case <-time.After(10 * time.Millisecond): + } + } + if err := held.Release(ctx); err != nil { + t.Fatalf("Release: %v", err) + } + got := <-done + if got.err != nil || !got.acquired { + t.Fatalf("Acquire = (%v, %v), want acquired once the holder released", got.acquired, got.err) + } + // The lock outlives the transaction that set the wait. + if !lockHeld(t, pool, pglockTestKey) { + t.Fatal("advisory lock not held after Acquire") + } + if _, again, err := TryAcquire(ctx, pool, pglockTestKey); err != nil || again { + t.Fatalf("TryAcquire beside Acquire's lock = (%v, %v), want not acquired", again, err) + } + if err := got.lock.Release(ctx); err != nil { + t.Fatalf("Release: %v", err) + } + if lockHeld(t, pool, pglockTestKey) { + t.Fatal("advisory lock still held after Release") + } +} + +// A wait that runs out reports not acquired and hands the connection back +// without the lock_timeout it set. The single-connection pool makes the next +// borrower get the same session. +func TestAcquireGivesUpAfterTheWait(t *testing.T) { + pool := testPool(t) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + config := pool.Config().Copy() + config.MaxConns = 1 + single, err := pgxpool.NewWithConfig(ctx, config) + if err != nil { + t.Fatalf("connect single-connection pool: %v", err) + } + t.Cleanup(single.Close) + + held, acquired, err := TryAcquire(ctx, pool, pglockTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire = (%v, %v), want acquired", acquired, err) + } + defer func() { _ = held.Release(ctx) }() + + lock, acquired, err := Acquire(ctx, single, pglockTestKey, 100*time.Millisecond) + if err != nil || acquired || lock != nil { + t.Fatalf("Acquire = (%v, %v, %v), want (nil, false, nil) after the wait", lock, acquired, err) + } + if waiters := lockWaiters(t, pool, pglockTestKey); waiters != 0 { + t.Fatalf("%d sessions still queued for the lock, want none", waiters) + } + var timeout string + if err := single.QueryRow(ctx, `SHOW lock_timeout`).Scan(&timeout); err != nil { + t.Fatalf("read lock_timeout: %v", err) + } + if timeout != "0" { + t.Fatalf("pooled session lock_timeout = %q, want the default 0", timeout) + } +} + +func TestAcquireNilPoolReportsNotAcquired(t *testing.T) { + lock, acquired, err := Acquire(context.Background(), nil, pglockTestKey, time.Minute) + if err != nil || acquired || lock != nil { + t.Fatalf("Acquire(nil pool) = (%v, %v, %v), want (nil, false, nil)", lock, acquired, err) + } +} diff --git a/internal/metadata/tmdb/client.go b/internal/metadata/tmdb/client.go index 2f3fb235af..062ce762f4 100644 --- a/internal/metadata/tmdb/client.go +++ b/internal/metadata/tmdb/client.go @@ -31,9 +31,14 @@ const ( // never changes. The stale-window failure mode is safe — a title that // gains a cert stays hidden (fail-closed) for at most the TTL. certificationCacheTTL = 7 * 24 * time.Hour - // certificationFetchTimeout bounds the shared (caller-detached) - // singleflight fetch; see GetCertification. - certificationFetchTimeout = 30 * time.Second + // sharedFetchTimeout bounds a shared (caller-detached) singleflight + // fetch; see cachedCertification and GetMediaDetail. + sharedFetchTimeout = 30 * time.Second + // mediaDetailCacheTTL is deliberately short: a detail is large, so the + // cache holds only the titles being looked at right now. It is long enough + // that a title page polling a download's progress every 30 seconds, and + // everyone else viewing the title, share one fetch. + mediaDetailCacheTTL = 2 * time.Minute ) // Client is an HTTP client for the TMDB collection preset API surface. @@ -46,6 +51,7 @@ type Client struct { discoverPageCache *cache.TTLCache[*MediaPage] externalIDCache *cache.TTLCache[*ExternalIDs] certificationCache *cache.TTLCache[string] + mediaDetailCache *cache.TTLCache[*MediaDetail] cacheGroup singleflight.Group responseCacheTTL time.Duration } @@ -71,6 +77,7 @@ func NewClient(apiKey string, rateLimit int) *Client { discoverPageCache: cache.NewTTLCache[*MediaPage](), externalIDCache: cache.NewTTLCache[*ExternalIDs](), certificationCache: cache.NewTTLCache[string](), + mediaDetailCache: cache.NewTTLCache[*MediaDetail](), responseCacheTTL: defaultResponseCacheTTL, } } @@ -97,6 +104,9 @@ func (c *Client) Close() { if c.certificationCache != nil { c.certificationCache.Close() } + if c.mediaDetailCache != nil { + c.mediaDetailCache.Close() + } } // doGet executes a GET request against the TMDB API with rate limiting, @@ -889,29 +899,105 @@ func (c *Client) GetList(ctx context.Context, id, limit int) ([]CollectionResult // // Cast is sorted by TMDB billing order and capped at 24 entries to keep the // payload bounded. +// +// A detail is cached for mediaDetailCacheTTL, and concurrent callers for one +// title share a fetch. Each caller gets its own copy. func (c *Client) GetMediaDetail(ctx context.Context, mediaType string, id int) (*MediaDetail, error) { if id <= 0 { return nil, fmt.Errorf("tmdb: media id must be > 0 (got %d)", id) } - + var fetch func(context.Context) (*MediaDetail, error) switch mediaType { case "movie": - path := fmt.Sprintf("/movie/%d?append_to_response=credits,external_ids,recommendations,release_dates,keywords", id) - var resp movieDetailResponse - if err := c.doGet(ctx, path, &resp); err != nil { - return nil, err + fetch = func(fetchCtx context.Context) (*MediaDetail, error) { + path := fmt.Sprintf("/movie/%d?append_to_response=credits,external_ids,recommendations,release_dates,keywords", id) + var resp movieDetailResponse + if err := c.doGet(fetchCtx, path, &resp); err != nil { + return nil, err + } + return normalizeMovieDetail(&resp), nil } - return normalizeMovieDetail(&resp), nil case "series", "tv": - path := fmt.Sprintf("/tv/%d?append_to_response=credits,external_ids,recommendations,content_ratings,keywords", id) - var resp tvDetailResponse - if err := c.doGet(ctx, path, &resp); err != nil { - return nil, err + mediaType = "tv" + fetch = func(fetchCtx context.Context) (*MediaDetail, error) { + path := fmt.Sprintf("/tv/%d?append_to_response=credits,external_ids,recommendations,content_ratings,keywords", id) + var resp tvDetailResponse + if err := c.doGet(fetchCtx, path, &resp); err != nil { + return nil, err + } + return normalizeTVDetail(&resp), nil } - return normalizeTVDetail(&resp), nil default: return nil, fmt.Errorf("tmdb: invalid media type for detail: %q", mediaType) } + + cacheKey := "media_detail:" + mediaType + ":" + strconv.Itoa(id) + if c.mediaDetailCache != nil { + if cached, ok := c.mediaDetailCache.Get(cacheKey); ok { + return cloneMediaDetail(cached), nil + } + } + // DoChan + select for the same reason as cachedCertification: the shared + // fetch survives any one caller's disconnect, and each caller stops + // waiting on its own cancellation. + resultCh := c.cacheGroup.DoChan(cacheKey, func() (any, error) { + if c.mediaDetailCache != nil { + if cached, ok := c.mediaDetailCache.Get(cacheKey); ok { + return cached, nil + } + } + fetchCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sharedFetchTimeout) + defer cancel() + detail, err := fetch(fetchCtx) + if err != nil { + return nil, err + } + if c.mediaDetailCache != nil { + c.mediaDetailCache.Set(cacheKey, detail, mediaDetailCacheTTL) + } + return detail, nil + }) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case result := <-resultCh: + if result.Err != nil { + return nil, result.Err + } + detail, ok := result.Val.(*MediaDetail) + if !ok { + return nil, fmt.Errorf("tmdb: invalid cached media detail response") + } + return cloneMediaDetail(detail), nil + } +} + +// cloneMediaDetail copies a detail deeply enough that a caller changing its +// copy cannot change the cached one. +func cloneMediaDetail(detail *MediaDetail) *MediaDetail { + if detail == nil { + return nil + } + cloned := *detail + cloned.Genres = slices.Clone(detail.Genres) + if detail.Certifications != nil { + cloned.Certifications = make(map[string][]string, len(detail.Certifications)) + for country, certs := range detail.Certifications { + cloned.Certifications[country] = slices.Clone(certs) + } + } + cloned.ProductionCompanies = slices.Clone(detail.ProductionCompanies) + cloned.KeywordIDs = slices.Clone(detail.KeywordIDs) + cloned.GenreIDs = slices.Clone(detail.GenreIDs) + cloned.CompanyIDs = slices.Clone(detail.CompanyIDs) + cloned.NetworkIDs = slices.Clone(detail.NetworkIDs) + cloned.OriginCountries = slices.Clone(detail.OriginCountries) + cloned.Networks = slices.Clone(detail.Networks) + cloned.Seasons = slices.Clone(detail.Seasons) + cloned.Cast = slices.Clone(detail.Cast) + cloned.Creators = slices.Clone(detail.Creators) + cloned.Recommendations = slices.Clone(detail.Recommendations) + return &cloned } func normalizeMovieDetail(resp *movieDetailResponse) *MediaDetail { @@ -1276,7 +1362,8 @@ func cloneExternalIDs(ids *ExternalIDs) *ExternalIDs { // "TV-MA", ...), or "" when the title has no US certification. It uses the // dedicated release_dates / content_ratings sub-resources instead of the full // detail payload for the same reason GetExternalIDs does: the detail response -// is 100+ KB and uncached, while these are a country list of a few KB. +// is 100+ KB and cached only briefly, while these are a country list of a few +// KB. // // Unlike GetMediaDetail's display rating, this deliberately does NOT fall // back to another country's certification: the value feeds the US-scale @@ -1352,7 +1439,7 @@ func (c *Client) cachedCertification(ctx context.Context, cacheKey string, fetch return cached, nil } } - fetchCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), certificationFetchTimeout) + fetchCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sharedFetchTimeout) defer cancel() cert, err := fetch(fetchCtx) if err != nil { diff --git a/internal/metadata/tmdb/client_test.go b/internal/metadata/tmdb/client_test.go index 8744906e65..08cadcd24c 100644 --- a/internal/metadata/tmdb/client_test.go +++ b/internal/metadata/tmdb/client_test.go @@ -9,6 +9,7 @@ import ( "reflect" "slices" "strings" + "sync" "sync/atomic" "testing" "time" @@ -1341,3 +1342,234 @@ func TestGetListRejectsNonPositiveID(t *testing.T) { t.Fatal("GetList(0) succeeded, want error") } } + +// A title page polls its detail while a request downloads, and several people +// may have it open: they share one fetch for the cache's TTL, each with a copy +// of its own. A failure is not cached. +func TestGetMediaDetailIsCachedBriefly(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/movie/129": + _, _ = w.Write([]byte(`{"id": 129, "title": "Spirited Away", "genres": [{"id": 16, "name": "Animation"}], + "recommendations": {"results": [{"id": 4935, "title": "Howl's Moving Castle"}]}}`)) + case "/tv/95396": + _, _ = w.Write([]byte(`{"id": 95396, "name": "Severance"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + client := NewClient("key", 40) + defer client.Close() + client.SetBaseURL(server.URL) + ctx := context.Background() + + first, err := client.GetMediaDetail(ctx, "movie", 129) + if err != nil { + t.Fatal(err) + } + first.Genres[0], first.Recommendations[0].Title = "changed", "changed" + second, err := client.GetMediaDetail(ctx, "movie", 129) + if err != nil { + t.Fatal(err) + } + if calls.Load() != 1 { + t.Fatalf("upstream calls = %d, want 1 while the detail is cached", calls.Load()) + } + if second.Genres[0] != "Animation" || second.Recommendations[0].Title != "Howl's Moving Castle" { + t.Fatalf("second detail = %+v; a caller's change reached the cache", second) + } + + // "series" and "tv" name the same title. + for _, mediaType := range []string{"series", "tv"} { + if detail, err := client.GetMediaDetail(ctx, mediaType, 95396); err != nil || detail.Title != "Severance" { + t.Fatalf("GetMediaDetail(%s) = %+v, %v", mediaType, detail, err) + } + } + if calls.Load() != 2 { + t.Fatalf("upstream calls = %d, want 2", calls.Load()) + } + + for range 2 { + if _, err := client.GetMediaDetail(ctx, "movie", 404); !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } + } + if calls.Load() != 4 { + t.Fatalf("upstream calls = %d, want each failure fetched again", calls.Load()) + } +} + +// blockedDetailServer answers /movie/129 once release is closed. started is +// closed when the first request arrives; calls counts every request. +func blockedDetailServer(t *testing.T) (client *Client, started <-chan struct{}, release func(), calls *atomic.Int32) { + t.Helper() + startedCh := make(chan struct{}) + releaseCh := make(chan struct{}) + var startOnce, releaseOnce sync.Once + calls = new(atomic.Int32) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + startOnce.Do(func() { close(startedCh) }) + <-releaseCh + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id": 129, "title": "Spirited Away"}`)) + })) + release = func() { releaseOnce.Do(func() { close(releaseCh) }) } + client = NewClient("key", 1000) + client.SetBaseURL(server.URL) + t.Cleanup(func() { + release() + server.Close() + client.Close() + }) + return client, startedCh, release, calls +} + +// waitingCtx closes waiting the first time its caller selects on Done. A +// follower does that only once it has joined the in-flight fetch. +type waitingCtx struct { + context.Context + once sync.Once + waiting chan struct{} +} + +func newWaitingCtx(parent context.Context) *waitingCtx { + return &waitingCtx{Context: parent, waiting: make(chan struct{})} +} + +func (c *waitingCtx) Done() <-chan struct{} { + c.once.Do(func() { close(c.waiting) }) + return c.Context.Done() +} + +type detailResult struct { + detail *MediaDetail + err error +} + +func getMediaDetailAsync(ctx context.Context, client *Client) <-chan detailResult { + out := make(chan detailResult, 1) + go func() { + detail, err := client.GetMediaDetail(ctx, "movie", 129) + out <- detailResult{detail, err} + }() + return out +} + +func awaitDetail(t *testing.T, who string, ch <-chan detailResult) detailResult { + t.Helper() + select { + case result := <-ch: + return result + case <-time.After(5 * time.Second): + t.Fatalf("%s is still blocked on the shared detail fetch", who) + return detailResult{} + } +} + +// The caller that starts a shared detail fetch may disconnect; everyone else +// waiting on the same title still gets it. +func TestGetMediaDetailSurvivesLeaderCancellation(t *testing.T) { + client, started, release, calls := blockedDetailServer(t) + + leaderCtx, cancelLeader := context.WithCancel(context.Background()) + defer cancelLeader() + leader := getMediaDetailAsync(leaderCtx, client) + <-started + + followerCtx := newWaitingCtx(context.Background()) + follower := getMediaDetailAsync(followerCtx, client) + select { + case <-followerCtx.waiting: + case <-time.After(5 * time.Second): + t.Fatal("follower never waited on its own context") + } + + cancelLeader() + if got := awaitDetail(t, "canceled leader", leader); !errors.Is(got.err, context.Canceled) { + t.Fatalf("leader err = %v, want context.Canceled", got.err) + } + release() + got := awaitDetail(t, "follower", follower) + if got.err != nil || got.detail.Title != "Spirited Away" { + t.Fatalf("follower = %+v, %v; want the detail after the leader canceled", got.detail, got.err) + } + if calls.Load() != 1 { + t.Fatalf("upstream calls = %d, want 1 shared fetch", calls.Load()) + } +} + +// A waiting caller that disconnects stops waiting at once, and the shared +// fetch still completes for the caller that started it. +func TestGetMediaDetailFollowerStopsWaitingOnCancel(t *testing.T) { + client, started, release, calls := blockedDetailServer(t) + + leader := getMediaDetailAsync(context.Background(), client) + <-started + + followerCtx, cancelFollower := context.WithCancel(context.Background()) + defer cancelFollower() + waiting := newWaitingCtx(followerCtx) + follower := getMediaDetailAsync(waiting, client) + select { + case <-waiting.waiting: + case <-time.After(5 * time.Second): + t.Fatal("follower never waited on its own context") + } + + cancelFollower() + if got := awaitDetail(t, "canceled follower", follower); !errors.Is(got.err, context.Canceled) { + t.Fatalf("follower err = %v, want context.Canceled", got.err) + } + release() + got := awaitDetail(t, "leader", leader) + if got.err != nil || got.detail.Title != "Spirited Away" { + t.Fatalf("leader = %+v, %v; want the detail after the follower canceled", got.detail, got.err) + } + if calls.Load() != 1 { + t.Fatalf("upstream calls = %d, want 1 shared fetch", calls.Load()) + } +} + +// Every slice and map a detail holds is copied, so no caller shares one with +// the cache; a field added later is caught here. +func TestCloneMediaDetailCopiesEveryReference(t *testing.T) { + var detail MediaDetail + v := reflect.ValueOf(&detail).Elem() + for i := range v.NumField() { + field := v.Field(i) + switch field.Kind() { + case reflect.Slice: + field.Set(reflect.MakeSlice(field.Type(), 1, 1)) + case reflect.Map: + m := reflect.MakeMap(field.Type()) + m.SetMapIndex(reflect.New(field.Type().Key()).Elem(), reflect.MakeSlice(field.Type().Elem(), 1, 1)) + field.Set(m) + case reflect.Pointer, reflect.Interface, reflect.Chan, reflect.Func: + t.Fatalf("MediaDetail.%s is a %s; teach cloneMediaDetail to copy it", v.Type().Field(i).Name, field.Kind()) + } + } + cloned := reflect.ValueOf(cloneMediaDetail(&detail)).Elem() + for i := range v.NumField() { + name := v.Type().Field(i).Name + switch v.Field(i).Kind() { + case reflect.Slice: + if cloned.Field(i).Pointer() == v.Field(i).Pointer() { + t.Errorf("MediaDetail.%s shares its backing array with the cached detail", name) + } + case reflect.Map: + if cloned.Field(i).Pointer() == v.Field(i).Pointer() { + t.Errorf("MediaDetail.%s shares its map with the cached detail", name) + } + for _, key := range v.Field(i).MapKeys() { + if cloned.Field(i).MapIndex(key).Pointer() == v.Field(i).MapIndex(key).Pointer() { + t.Errorf("MediaDetail.%s[%v] shares its slice with the cached detail", name, key) + } + } + } + } +} diff --git a/internal/plugins/request_router_test.go b/internal/plugins/request_router_test.go index 8b45539b05..d58945c8e6 100644 --- a/internal/plugins/request_router_test.go +++ b/internal/plugins/request_router_test.go @@ -9,13 +9,14 @@ import ( "github.com/Silo-Server/silo-plugin-sdk/pkg/pluginsdk/capability" ) -// The host reads supports_seasons from the capability metadata stored from the -// manifest, the way install records it. +// The host reads supports_seasons and reports_download_progress from the +// capability metadata stored from the manifest, the way install records it. func TestRequestRouterDescriptorReadsStoredManifestFlag(t *testing.T) { records, err := CapabilityRecordsFromManifest(&pluginv1.PluginManifest{ Capabilities: []*pluginv1.CapabilityDescriptor{ {Type: capability.RequestRouter, Id: "arr", RequestRouter: &pluginv1.RequestRouterDescriptor{SupportsSeasons: true}}, {Type: capability.RequestRouter, Id: "legacy"}, + {Type: capability.RequestRouter, Id: "progress", RequestRouter: &pluginv1.RequestRouterDescriptor{SupportsSeasons: true, ReportsDownloadProgress: true}}, }, }) if err != nil { @@ -30,10 +31,12 @@ func TestRequestRouterDescriptorReadsStoredManifestFlag(t *testing.T) { for _, tc := range []struct { capabilityID string want bool + wantProgress bool }{ - {"arr", true}, - {"legacy", false}, // built before the descriptor existed - {"missing", false}, // not declared at all + {"arr", true, false}, // built before reports_download_progress existed + {"legacy", false, false}, // built before the descriptor existed + {"missing", false, false}, // not declared at all + {"progress", true, true}, } { got, err := service.RequestRouterDescriptor(context.Background(), 5, tc.capabilityID) if err != nil { @@ -42,6 +45,9 @@ func TestRequestRouterDescriptorReadsStoredManifestFlag(t *testing.T) { if got.GetSupportsSeasons() != tc.want { t.Errorf("%s: supports_seasons = %v, want %v", tc.capabilityID, got.GetSupportsSeasons(), tc.want) } + if got.GetReportsDownloadProgress() != tc.wantProgress { + t.Errorf("%s: reports_download_progress = %v, want %v", tc.capabilityID, got.GetReportsDownloadProgress(), tc.wantProgress) + } } } diff --git a/internal/requests/downloads.go b/internal/requests/downloads.go new file mode 100644 index 0000000000..d0baa6092f --- /dev/null +++ b/internal/requests/downloads.go @@ -0,0 +1,216 @@ +package requests + +import ( + "context" + "fmt" + "log/slog" + "slices" + "time" +) + +// staleDownloadProgress is how long a target's download progress outlives +// its last report once its server stops answering for it. A pass that asks +// about a target and gets no status back (the plugin skipped a server that +// errored or no longer has the title, or the call failed) clears progress +// older than this, so a frozen figure stops showing and clients stop polling +// for it. Until then the progress stays, since one missed answer is usually +// a blip. +const staleDownloadProgress = 15 * time.Minute + +// RefreshDownloads is the download refresh pass. For each active request with +// a downloading target that has download progress, least recently asked about +// first, it asks the plugins that declare reports_download_progress about +// those targets and applies the answer the way reconcile does: status +// transitions and download progress. It submits nothing, checks no library +// presence and sends no notification; the reconcile pass keeps those. +// +// Only a target with progress is polled. The reconcile pass records a +// download's first progress, and a target leaves this pass as soon as its +// progress clears, so a target whose plugin reports none (a legacy plugin, or +// one with nothing in its download queue for it) costs no call a minute. +// Queued targets stay on the reconcile cadence, since a title can sit queued +// for months before release. +// +// budget bounds the pass, zero or less leaving it to ctx: once it has run +// out, the call in flight is cut and no further request is asked about. The +// requests left over are the least recently asked about on the next pass, and +// the pass reports no error for them. +func (s *Service) RefreshDownloads(ctx context.Context, limit int, budget time.Duration) (DownloadRefreshResult, error) { + if s == nil || s.store == nil { + return DownloadRefreshResult{}, fmt.Errorf("request service is not configured") + } + if s.router == nil { + return DownloadRefreshResult{}, nil + } + if limit <= 0 || limit > 500 { + limit = 200 + } + checkCtx := ctx + if budget > 0 { + var cancel context.CancelFunc + checkCtx, cancel = context.WithTimeout(ctx, budget) + defer cancel() + } + reqs, err := s.store.ListDownloadingRequests(ctx, limit) + if err != nil || len(reqs) == 0 { + return DownloadRefreshResult{}, err + } + fc, err := s.newFulfillContext(ctx) + if err != nil { + return DownloadRefreshResult{}, err + } + ids := make([]string, 0, len(reqs)) + for _, req := range reqs { + ids = append(ids, req.ID) + } + byRequest, err := s.store.ListTargetsForRequests(ctx, ids) + if err != nil { + return DownloadRefreshResult{}, err + } + var result DownloadRefreshResult + for i, req := range reqs { + if err := ctx.Err(); err != nil { + return result, err + } + if checkCtx.Err() != nil { + slog.InfoContext(ctx, "request download refresh ran out of time; the rest wait for the next pass", "component", "requests", + "budget", budget, + "requests_left", len(reqs)-i, + ) + break + } + change, checked, err := s.refreshRequestDownloads(ctx, checkCtx, fc, *req, byRequest[req.ID]) + if checked { + result.Checked++ + } + if change != reconcileUnchanged { + result.Updated++ + } + if err != nil { + slog.WarnContext(ctx, "request download refresh failed", "component", "requests", + "request_id", req.ID, + "media_type", req.MediaType, + "tmdb_id", req.TMDBID, + "err", err, + ) + result.Errors++ + } + } + return result, nil +} + +// refreshRequestDownloads checks one request's downloading targets that have +// progress and whose plugin reports it, and applies what it answers. A target +// with progress that its server can no longer be asked about (gone, disabled +// or unusable), or whose plugin no longer declares progress, has the progress +// cleared: it is stale, and clearing it takes the target off this pass. +// checkCtx bounds only the plugin calls; the writes that apply their answers +// run on ctx, so a call the pass's budget cuts still moves its targets back in +// the rotation. checked is false when no target was asked about. +func (s *Service) refreshRequestDownloads(ctx, checkCtx context.Context, fc *fulfillContext, req Request, targets []Target) (change reconcileChange, checked bool, err error) { + var polled []Target + for _, t := range targets { + if t.Status != StatusDownloading || t.Download == nil { + continue + } + reports, err := s.targetReportsProgress(ctx, fc, req.MediaType, t) + if err != nil { + return reconcileUnchanged, false, err + } + if !reports { + if err := s.store.UpdateTargetDownload(ctx, t.ID, nil); err != nil { + return reconcileUnchanged, false, err + } + continue + } + polled = append(polled, t) + } + if len(polled) == 0 { + return reconcileUnchanged, false, nil + } + statuses, checkErr := s.checkTargetStatuses(checkCtx, req, polled, fc) + change, err = s.applyTargetStatuses(ctx, polled, statuses) + if err != nil { + return reconcileUnchanged, true, err + } + return change, true, checkErr +} + +// HasDownloadsToRefresh reports whether the download refresh pass has any +// work: an active request with a downloading target that has progress. +func (s *Service) HasDownloadsToRefresh(ctx context.Context) (bool, error) { + if s == nil || s.store == nil || s.router == nil { + return false, nil + } + reqs, err := s.store.ListDownloadingRequests(ctx, 1) + return len(reqs) > 0, err +} + +// settleUnansweredDownload handles a target that a pass asked about, or meant +// to, without getting its status back. A queued or downloading target with +// progress keeps it, marked as asked about so the download refresh pass takes +// the others first next time, until the progress is staleDownloadProgress +// old; then the progress is cleared. Any other target is not written. +func (s *Service) settleUnansweredDownload(ctx context.Context, t Target) error { + if t.Download == nil || (t.Status != StatusQueued && t.Status != StatusDownloading) { + return nil + } + if s.now().Sub(t.Download.UpdatedAt) > staleDownloadProgress { + return s.store.UpdateTargetDownload(ctx, t.ID, nil) + } + return s.store.MarkTargetDownloadChecked(ctx, t.ID) +} + +// targetReportsProgress reports whether the router capability that +// checkTargetStatuses would ask about a target declares +// reports_download_progress. A target it would skip reports none. +func (s *Service) targetReportsProgress(ctx context.Context, fc *fulfillContext, mediaType MediaType, t Target) (bool, error) { + var installationID int + var capabilityID string + if t.IntegrationID == "" { + conns, id, capability, err := s.resolveRouterConnections(ctx, fc, mediaType) + if err != nil || len(conns) == 0 { + return false, err + } + installationID, capabilityID = id, capability + } else { + in := integrationByID(fc, t.IntegrationID) + if !statusCheckable(in) { + return false, nil + } + installationID, capabilityID = *in.InstallationID, in.CapabilityID + } + features, err := s.routerFeatures(ctx, fc, installationID, capabilityID) + return features.ReportsDownloadProgress, err +} + +// keepsReportedProgress reports whether the progress in a router's statuses +// is recorded: only a plugin that declares reports_download_progress has its +// progress kept. The download refresh pass clears the progress of any other, +// so recording it would make it come and go between passes. Statuses without +// progress read no features, and when the declaration cannot be read the +// report stands. +func (s *Service) keepsReportedProgress(ctx context.Context, fc *fulfillContext, installationID int, capabilityID string, statuses []RouterTargetStatus) (bool, error) { + if !slices.ContainsFunc(statuses, func(st RouterTargetStatus) bool { return st.Progress != nil }) { + return true, nil + } + features, err := s.routerFeatures(ctx, fc, installationID, capabilityID) + if err != nil { + return true, err + } + return features.ReportsDownloadProgress, nil +} + +// activeRequestDownload loads an active request's targets for its download +// progress. Only a queued or downloading request can have any, so any other +// costs no query. +func (s *Service) activeRequestDownload(ctx context.Context, req *Request) (*DownloadProgress, error) { + if req == nil || req.Outcome != OutcomeActive || (req.Status != StatusQueued && req.Status != StatusDownloading) { + return nil, nil + } + targets, err := s.store.ListTargets(ctx, req.ID) + if err != nil { + return nil, err + } + return (&Request{Targets: targets}).Download(), nil +} diff --git a/internal/requests/downloads_test.go b/internal/requests/downloads_test.go new file mode 100644 index 0000000000..914a253ad2 --- /dev/null +++ b/internal/requests/downloads_test.go @@ -0,0 +1,1135 @@ +package requests + +import ( + "context" + "errors" + "fmt" + "slices" + "testing" + "time" + + pluginv1 "github.com/Silo-Server/silo-plugin-sdk/pkg/pluginproto/silo/plugin/v1" + "github.com/jackc/pgx/v5/pgxpool" + "google.golang.org/protobuf/types/known/timestamppb" + + "github.com/Silo-Server/silo-server/internal/metadata/tmdb" +) + +func TestDownloadPhasePrecedence(t *testing.T) { + // Highest first; an unknown phase ranks as downloading. + order := []DownloadPhase{ + DownloadPhaseImportBlocked, DownloadPhaseStalled, DownloadPhaseDownloading, + DownloadPhaseImporting, DownloadPhasePaused, DownloadPhaseQueued, + } + for i := 1; i < len(order); i++ { + if downloadPhaseRank(order[i-1]) <= downloadPhaseRank(order[i]) { + t.Fatalf("%s must outrank %s", order[i-1], order[i]) + } + } + if downloadPhaseRank("seeding") != downloadPhaseRank(DownloadPhaseDownloading) { + t.Fatal("an unknown phase must rank as downloading") + } +} + +func TestRequestDownloadAggregatesLiveTargets(t *testing.T) { + early := time.Date(2026, 9, 28, 12, 0, 0, 0, time.UTC) + late := early.Add(time.Hour) + older, newer := early.Add(-2*time.Minute), early.Add(-time.Minute) + req := Request{Targets: []Target{ + {Quality: Quality1080p, Status: StatusDownloading, Download: &DownloadProgress{ + Phase: DownloadPhaseDownloading, BytesTotal: 4000, BytesLeft: 1000, EstimatedCompletion: &early, Downloads: 1, UpdatedAt: newer, + }}, + {Quality: Quality2160p, Status: StatusQueued, Download: &DownloadProgress{ + Phase: DownloadPhaseStalled, BytesTotal: 6000, BytesLeft: 6000, EstimatedCompletion: &late, Downloads: 2, UpdatedAt: older, + }}, + }} + got := req.Download() + if got == nil { + t.Fatal("Download() = nil, want the aggregate") + } + if got.Phase != DownloadPhaseStalled || got.BytesTotal != 10000 || got.BytesLeft != 7000 || got.Downloads != 3 { + t.Fatalf("aggregate = %+v, want stalled, 10000/7000 bytes, 3 downloads", got) + } + if got.EstimatedCompletion == nil || !got.EstimatedCompletion.Equal(late) || !got.UpdatedAt.Equal(older) { + t.Fatalf("aggregate = %+v, want the latest estimate and the oldest report", got) + } + // The aggregate is a copy: the targets keep their own figures. + if req.Targets[0].Download.BytesTotal != 4000 || req.Targets[0].Download.Phase != DownloadPhaseDownloading { + t.Fatalf("first target changed to %+v", req.Targets[0].Download) + } +} + +func TestRequestDownloadPhaseByPrecedence(t *testing.T) { + for _, tc := range []struct { + phases []DownloadPhase + want DownloadPhase + }{ + {[]DownloadPhase{DownloadPhaseQueued, DownloadPhaseImportBlocked}, DownloadPhaseImportBlocked}, + {[]DownloadPhase{DownloadPhaseDownloading, DownloadPhaseStalled}, DownloadPhaseStalled}, + {[]DownloadPhase{DownloadPhaseImporting, DownloadPhaseDownloading}, DownloadPhaseDownloading}, + {[]DownloadPhase{DownloadPhasePaused, DownloadPhaseImporting}, DownloadPhaseImporting}, + {[]DownloadPhase{DownloadPhaseQueued, DownloadPhasePaused}, DownloadPhasePaused}, + {[]DownloadPhase{DownloadPhaseQueued, DownloadPhaseQueued}, DownloadPhaseQueued}, + } { + req := Request{Targets: []Target{ + {Quality: Quality1080p, Status: StatusDownloading, Download: &DownloadProgress{Phase: tc.phases[0], BytesTotal: 1}}, + {Quality: Quality2160p, Status: StatusDownloading, Download: &DownloadProgress{Phase: tc.phases[1], BytesTotal: 1}}, + }} + if got := req.Download(); got == nil || got.Phase != tc.want { + t.Errorf("phases %v aggregate to %+v, want %s", tc.phases, got, tc.want) + } + } +} + +func TestRequestDownloadWithAnUnknownSizeHasNoTotal(t *testing.T) { + req := Request{Targets: []Target{ + {Quality: Quality1080p, Status: StatusDownloading, Download: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 4000, BytesLeft: 1000, Downloads: 1}}, + {Quality: Quality2160p, Status: StatusDownloading, Download: &DownloadProgress{Phase: DownloadPhaseQueued, Downloads: 1}}, + }} + got := req.Download() + if got == nil || got.BytesTotal != 0 || got.BytesLeft != 0 || got.Downloads != 2 { + t.Fatalf("aggregate = %+v, want no total while one size is unknown", got) + } +} + +func TestRequestDownloadIgnoresFinishedTargetsAndTargetsWithoutProgress(t *testing.T) { + stale := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 100} + req := Request{Targets: []Target{ + {Quality: Quality1080p, Status: StatusCompleted, Download: stale}, + {Quality: Quality2160p, Status: StatusDownloading}, + }} + if got := req.Download(); got != nil { + t.Fatalf("Download() = %+v, want nil", got) + } + req.Targets = append(req.Targets, Target{Status: StatusFailed, Download: stale}) + if got := req.Download(); got != nil { + t.Fatalf("Download() = %+v, want nil", got) + } +} + +func TestDownloadProgressFromProto(t *testing.T) { + eta := time.Date(2026, 9, 28, 13, 0, 0, 0, time.UTC) + for _, tc := range []struct { + name string + in *pluginv1.DownloadProgress + want *DownloadProgress + }{ + {name: "unset", in: nil, want: nil}, + {name: "empty", in: &pluginv1.DownloadProgress{}, want: nil}, + {name: "no phase and no size", in: &pluginv1.DownloadProgress{Downloads: 1}, want: nil}, + { + name: "known phase", + in: &pluginv1.DownloadProgress{Phase: "import_blocked", BytesTotal: 100, BytesLeft: 0, Downloads: 1, EstimatedCompletion: timestamppb.New(eta)}, + want: &DownloadProgress{Phase: DownloadPhaseImportBlocked, BytesTotal: 100, Downloads: 1, EstimatedCompletion: &eta}, + }, + { + name: "empty phase with a size", + in: &pluginv1.DownloadProgress{BytesTotal: 100, BytesLeft: 40, Downloads: 1}, + want: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 100, BytesLeft: 40, Downloads: 1}, + }, + { + name: "unknown phase", + in: &pluginv1.DownloadProgress{Phase: "seeding", Downloads: 2}, + want: &DownloadProgress{Phase: DownloadPhaseDownloading, Downloads: 2}, + }, + { + name: "queued without a size", + in: &pluginv1.DownloadProgress{Phase: "queued", Downloads: 1}, + want: &DownloadProgress{Phase: DownloadPhaseQueued, Downloads: 1}, + }, + { + name: "clamped", + in: &pluginv1.DownloadProgress{Phase: "downloading", BytesTotal: 100, BytesLeft: 250, Downloads: -1}, + want: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 100, BytesLeft: 100}, + }, + { + name: "negative", + in: &pluginv1.DownloadProgress{Phase: "paused", BytesTotal: -5, BytesLeft: -5}, + want: &DownloadProgress{Phase: DownloadPhasePaused}, + }, + { + name: "invalid estimate", + in: &pluginv1.DownloadProgress{Phase: "downloading", BytesTotal: 10, EstimatedCompletion: ×tamppb.Timestamp{Nanos: -1}}, + want: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 10}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := downloadProgressFromProto(tc.in) + if !sameProgress(got, tc.want) { + t.Fatalf("downloadProgressFromProto() = %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestPluginRouterProviderCheckStatusCarriesProgress(t *testing.T) { + eta := time.Date(2026, 9, 28, 13, 0, 0, 0, time.UTC) + fc := &fakeRouterClient{statuses: []*pluginv1.TargetStatus{ + {Quality: "1080p", ConnectionId: "c1", Status: "downloading", Progress: &pluginv1.DownloadProgress{ + Phase: "downloading", BytesTotal: 2000, BytesLeft: 500, Downloads: 1, EstimatedCompletion: timestamppb.New(eta), + }}, + {Quality: "2160p", ConnectionId: "c1", Status: "queued"}, + }} + out, err := NewPluginRouterProvider(fakeRouterResolver{c: fc}).CheckStatus(context.Background(), 1, "arr", + Request{MediaType: MediaTypeMovie, TMDBID: 42}, []RouterTargetRef{{Quality: Quality1080p, ConnectionID: "c1"}}, nil) + if err != nil { + t.Fatalf("CheckStatus: %v", err) + } + want := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 2000, BytesLeft: 500, Downloads: 1, EstimatedCompletion: &eta} + if len(out) != 2 || !sameProgress(out[0].Progress, want) || out[1].Progress != nil { + t.Fatalf("statuses = %+v, want progress on the first only", out) + } +} + +// sameProgress compares progress reports, ignoring UpdatedAt, which the +// store stamps. +func sameProgress(a, b *DownloadProgress) bool { + if a == nil || b == nil { + return a == b + } + if (a.EstimatedCompletion == nil) != (b.EstimatedCompletion == nil) || + (a.EstimatedCompletion != nil && !a.EstimatedCompletion.Equal(*b.EstimatedCompletion)) { + return false + } + return a.Phase == b.Phase && a.BytesTotal == b.BytesTotal && a.BytesLeft == b.BytesLeft && a.Downloads == b.Downloads +} + +// seedDownloadTarget stores an active request with one target on connection. +func seedDownloadTarget(t *testing.T, store *fakeStore, requestID, connection string, quality Quality, status Status, download *DownloadProgress) Target { + t.Helper() + if store.requests[requestID] == nil { + store.requests[requestID] = &Request{ID: requestID, MediaType: MediaTypeMovie, TMDBID: 550, Status: status, Outcome: OutcomeActive} + } + target, err := store.CreateTarget(context.Background(), Target{ + RequestID: requestID, IntegrationID: connection, Quality: quality, Status: status, ExternalID: "123", Download: download, + }) + if err != nil { + t.Fatalf("seed target: %v", err) + } + return target +} + +func onlyTarget(t *testing.T, store *fakeStore, requestID string) Target { + t.Helper() + targets, _ := store.ListTargets(context.Background(), requestID) + if len(targets) != 1 { + t.Fatalf("targets = %+v, want one", targets) + } + return targets[0] +} + +func TestApplyTargetStatusesWritesAndClearsProgress(t *testing.T) { + progress := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 250, Downloads: 1} + earlier := &DownloadProgress{Phase: DownloadPhaseQueued, BytesTotal: 1000, BytesLeft: 1000, Downloads: 1} + for _, tc := range []struct { + name string + status Status + had *DownloadProgress + reported RouterTargetStatus + wantStatus Status + wantChange reconcileChange + wantWrites int + want *DownloadProgress + }{ + { + name: "progress without a status change", status: StatusDownloading, + reported: RouterTargetStatus{Status: StatusDownloading, Progress: progress}, + wantStatus: StatusDownloading, wantChange: reconcileUnchanged, wantWrites: 1, want: progress, + }, + { + name: "progress on the move to downloading", status: StatusQueued, had: earlier, + reported: RouterTargetStatus{Status: StatusDownloading, Progress: progress}, + wantStatus: StatusDownloading, wantChange: reconcileDownloading, wantWrites: 1, want: progress, + }, + { + name: "completion clears", status: StatusDownloading, had: earlier, + reported: RouterTargetStatus{Status: StatusCompleted, Progress: progress}, + wantStatus: StatusCompleted, wantChange: reconcileCompleted, + }, + { + name: "failure clears", status: StatusDownloading, had: earlier, + reported: RouterTargetStatus{Status: StatusFailed, Message: "no release"}, + wantStatus: StatusFailed, wantChange: reconcileFailed, + }, + { + name: "no progress clears what was reported", status: StatusDownloading, had: earlier, + reported: RouterTargetStatus{Status: StatusDownloading}, + wantStatus: StatusDownloading, wantChange: reconcileUnchanged, wantWrites: 1, + }, + { + name: "an idle target is not written", status: StatusQueued, + reported: RouterTargetStatus{Status: StatusQueued}, + wantStatus: StatusQueued, wantChange: reconcileUnchanged, + }, + { + name: "no status keeps the status and writes progress", status: StatusQueued, + reported: RouterTargetStatus{Progress: progress}, + wantStatus: StatusQueued, wantChange: reconcileUnchanged, wantWrites: 1, want: progress, + }, + } { + t.Run(tc.name, func(t *testing.T) { + store := newFakeStore() + target := seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, tc.status, tc.had) + service := newTestService(store) + reported := tc.reported + reported.Quality, reported.ConnectionID = Quality1080p, "router-1" + + targets, _ := store.ListTargets(context.Background(), "req-1") + change, err := service.applyTargetStatuses(context.Background(), targets, []RouterTargetStatus{ + reported, + // A status for a target the request does not have is ignored. + {Quality: Quality2160p, ConnectionID: "router-1", Status: StatusCompleted, Progress: progress}, + }) + if err != nil { + t.Fatalf("applyTargetStatuses: %v", err) + } + got := onlyTarget(t, store, "req-1") + if change != tc.wantChange || got.Status != tc.wantStatus { + t.Fatalf("change = %s, status = %s; want %s, %s", change, got.Status, tc.wantChange, tc.wantStatus) + } + if len(store.downloadWrites) != tc.wantWrites { + t.Fatalf("progress writes = %+v, want %d", store.downloadWrites, tc.wantWrites) + } + if tc.wantWrites > 0 && store.downloadWrites[0].targetID != target.ID { + t.Fatalf("wrote target %d, want %d", store.downloadWrites[0].targetID, target.ID) + } + if !sameProgress(got.Download, tc.want) { + t.Fatalf("stored progress = %+v, want %+v", got.Download, tc.want) + } + }) + } +} + +func TestApplyTargetStatusesReportsAProgressWriteFailure(t *testing.T) { + store := newFakeStore() + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, nil) + store.downloadErr = errors.New("database unavailable") + targets, _ := store.ListTargets(context.Background(), "req-1") + _, err := newTestService(store).applyTargetStatuses(context.Background(), targets, []RouterTargetStatus{{ + Quality: Quality1080p, ConnectionID: "router-1", Status: StatusDownloading, + Progress: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 10}, + }}) + if !errors.Is(err, store.downloadErr) { + t.Fatalf("applyTargetStatuses err = %v, want the store's", err) + } +} + +// The reconcile pass records progress too, for queued targets and for a +// download's first report, which puts the target on the download refresh pass. +func TestReconcileRequestsRecordsDownloadProgress(t *testing.T) { + store := newFakeStore() + store.integrations = []Integration{routerInst("router-1")} + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusQueued, nil) + store.candidates = []*Request{store.requests["req-1"]} + progress := &DownloadProgress{Phase: DownloadPhaseQueued, BytesTotal: 800, BytesLeft: 800, Downloads: 1} + service := newTestService(store) + service.SetRouterProvider(&fakeRouterProvider{progressCapable: map[int]bool{1: true}, statuses: []RouterTargetStatus{{ + Quality: Quality1080p, ConnectionID: "router-1", Status: StatusQueued, Progress: progress, + }}}) + + if _, err := service.ReconcileRequests(context.Background(), 100); err != nil { + t.Fatalf("ReconcileRequests: %v", err) + } + if got := onlyTarget(t, store, "req-1"); got.Status != StatusQueued || !sameProgress(got.Download, progress) { + t.Fatalf("target = %+v, want queued with the reported progress", got) + } + if len(store.statusUpdates) != 0 { + t.Fatalf("status updates = %v, want none for an unchanged status", store.statusUpdates) + } +} + +// Progress counts only from a plugin that declares reports_download_progress. +// The download refresh pass clears any other plugin's, so were reconcile to +// record it, it would come and go between the passes. +func TestReconcileRequestsIgnoresProgressFromUndeclaredPlugins(t *testing.T) { + store := newFakeStore() + store.integrations = []Integration{routerInst("router-1")} + stale := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 800, BytesLeft: 100, Downloads: 1} + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, stale) + store.candidates = []*Request{store.requests["req-1"]} + service := newTestService(store) + router := &fakeRouterProvider{statuses: []RouterTargetStatus{{ + Quality: Quality1080p, ConnectionID: "router-1", Status: StatusDownloading, + Progress: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 800, BytesLeft: 400, Downloads: 1}, + }}} + service.SetRouterProvider(router) + + result, err := service.ReconcileRequests(context.Background(), 100) + if err != nil || result.Errors != 0 { + t.Fatalf("ReconcileRequests = %+v, %v", result, err) + } + if got := onlyTarget(t, store, "req-1"); got.Status != StatusDownloading || got.Download != nil { + t.Fatalf("target = %+v, want downloading with its stale progress cleared", got) + } + + // When the declaration cannot be read, the report stands and the pass + // reports the error. + router.featuresErr = errors.New("capability metadata unavailable") + result, err = service.ReconcileRequests(context.Background(), 100) + if err != nil || result.Errors != 1 { + t.Fatalf("ReconcileRequests = %+v, %v; want one error", result, err) + } + if got := onlyTarget(t, store, "req-1"); got.Download == nil || got.Download.BytesLeft != 400 { + t.Fatalf("target = %+v, want the reported progress kept", got) + } +} + +// A server whose own state moves while the target's status does not (an +// import that stalls) has its raw status recorded beside the new progress, +// with no status write and so no history entry. An unchanged raw status is +// not written again. +func TestReconcileRequestsKeepsExternalStatusInStep(t *testing.T) { + store := newFakeStore() + store.integrations = []Integration{routerInst("router-1")} + blocked := &DownloadProgress{Phase: DownloadPhaseImportBlocked, BytesTotal: 800, BytesLeft: 0, Downloads: 1} + target := seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, blocked) + store.targets["req-1"][0].ExternalStatus = "completed/importBlocked" + store.candidates = []*Request{store.requests["req-1"]} + service := newTestService(store) + stalled := &DownloadProgress{Phase: DownloadPhaseStalled, BytesTotal: 800, BytesLeft: 480, Downloads: 1} + service.SetRouterProvider(&fakeRouterProvider{progressCapable: map[int]bool{1: true}, statuses: []RouterTargetStatus{{ + Quality: Quality1080p, ConnectionID: "router-1", Status: StatusDownloading, ExternalStatus: "warning/downloading", Progress: stalled, + }}}) + + for pass := 1; pass <= 2; pass++ { + if _, err := service.ReconcileRequests(context.Background(), 100); err != nil { + t.Fatalf("pass %d: ReconcileRequests: %v", pass, err) + } + got := onlyTarget(t, store, "req-1") + if got.Status != StatusDownloading || got.ExternalStatus != "warning/downloading" || !sameProgress(got.Download, stalled) { + t.Fatalf("pass %d: target = %+v, want downloading, stalled, with the new raw status", pass, got) + } + if len(store.statusUpdates) != 0 { + t.Fatalf("pass %d: status updates = %v, want none for an unchanged status", pass, store.statusUpdates) + } + if want := []int64{target.ID}; !slices.Equal(store.externalStatusWrites, want) { + t.Fatalf("pass %d: external status writes = %v, want %v, once", pass, store.externalStatusWrites, want) + } + } +} + +// downloadRefreshService serves two download servers: router-1 on a plugin +// that reports progress, router-2 on one that does not. +func downloadRefreshService(store *fakeStore) (*Service, *fakeRouterProvider) { + store.integrations = []Integration{routerInstOn("router-1", 1), routerInstOn("router-2", 2)} + router := &fakeRouterProvider{progressCapable: map[int]bool{1: true}} + service := newTestService(store) + service.SetRouterProvider(router) + return service, router +} + +func TestRefreshDownloadsPollsOnlyDownloadingTargetsOfProgressPlugins(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + earlier := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 900, Downloads: 1} + // req-mixed: 1080p downloading on the reporting plugin, 4K downloading + // on the other. req-queued waits in the queue on the reporting plugin. + // req-legacy downloads only through the other plugin. + seedDownloadTarget(t, store, "req-mixed", "router-1", Quality1080p, StatusDownloading, earlier) + seedDownloadTarget(t, store, "req-mixed", "router-2", Quality2160p, StatusDownloading, nil) + seedDownloadTarget(t, store, "req-queued", "router-1", Quality1080p, StatusQueued, earlier) + seedDownloadTarget(t, store, "req-legacy", "router-2", Quality1080p, StatusDownloading, nil) + progress := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 400, Downloads: 1} + router.statuses = []RouterTargetStatus{ + {Quality: Quality1080p, ConnectionID: "router-1", Status: StatusDownloading, Progress: progress}, + {Quality: Quality2160p, ConnectionID: "router-2", Status: StatusCompleted}, + } + presence := service.presence.(*fakePresence) + + result, err := service.RefreshDownloads(context.Background(), 200, 0) + if err != nil { + t.Fatalf("RefreshDownloads: %v", err) + } + if result != (DownloadRefreshResult{Checked: 1}) { + t.Fatalf("result = %+v, want one request checked", result) + } + if len(router.statusLog) != 1 || router.statusLog[0].installationID != 1 || + len(router.statusLog[0].refs) != 1 || router.statusLog[0].refs[0].Quality != Quality1080p { + t.Fatalf("status calls = %+v, want one call to installation 1 for the 1080p target", router.statusLog) + } + targets, _ := store.ListTargets(context.Background(), "req-mixed") + for _, target := range targets { + switch target.Quality { + case Quality1080p: + if !sameProgress(target.Download, progress) { + t.Fatalf("1080p progress = %+v, want %+v", target.Download, progress) + } + case Quality2160p: + // Not polled, so the completion reported for it was not applied. + if target.Status != StatusDownloading || target.Download != nil { + t.Fatalf("4K target = %+v, want it untouched", target) + } + } + } + if len(presence.got) != 0 || router.fulfillCalls != 0 { + t.Fatalf("presence lookups = %d, submissions = %d; want neither", len(presence.got), router.fulfillCalls) + } +} + +func TestRefreshDownloadsAppliesStatusTransitions(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + // Were the pass to run the notification step, req-1 would be notified: + // it is in the library and not notified yet. + notifier := &fakeNotifier{} + service.SetFulfillmentNotifier(notifier) + service.presence.(*fakePresence).available = map[MediaType]map[int]bool{MediaTypeMovie: {550: true}} + store.unnotified = []string{"req-1"} + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseImporting, BytesTotal: 10, Downloads: 1}) + router.statuses = []RouterTargetStatus{{Quality: Quality1080p, ConnectionID: "router-1", Status: StatusCompleted, ExternalStatus: "imported"}} + + result, err := service.RefreshDownloads(context.Background(), 200, 0) + if err != nil { + t.Fatalf("RefreshDownloads: %v", err) + } + if result != (DownloadRefreshResult{Checked: 1, Updated: 1}) { + t.Fatalf("result = %+v, want one request checked and updated", result) + } + if got := onlyTarget(t, store, "req-1"); got.Status != StatusCompleted || got.Download != nil { + t.Fatalf("target = %+v, want completed without progress", got) + } + if store.requests["req-1"].Status != StatusCompleted { + t.Fatalf("request status = %s, want completed", store.requests["req-1"].Status) + } + if len(notifier.requestIDs) != 0 || len(service.presence.(*fakePresence).got) != 0 { + t.Fatalf("notified %v after presence lookups %v; the reconcile pass owns both", notifier.requestIDs, service.presence.(*fakePresence).got) + } +} + +func TestRefreshDownloadsCountsPluginErrors(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 10, Downloads: 1}) + router.statusErr = errors.New("plugin unavailable") + + result, err := service.RefreshDownloads(context.Background(), 200, 0) + if err != nil { + t.Fatalf("RefreshDownloads: %v", err) + } + if result != (DownloadRefreshResult{Checked: 1, Errors: 1}) { + t.Fatalf("result = %+v, want one request checked with an error", result) + } +} + +// A downloading target without progress is left to the reconcile pass, +// however long it stays that way: a plugin can report a title downloading with +// nothing in its download queue (Seerr keeps media at Processing for as long +// as it waits for a release), and polling it every minute would cost calls for +// nothing. A target whose progress stops leaves the pass the same way. +func TestRefreshDownloadsLeavesTargetsWithoutProgressToReconcile(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + seedDownloadTarget(t, store, "req-idle", "router-1", Quality1080p, StatusDownloading, nil) + // req-live's 1080p has progress; its 4K, on the same plugin, has none. + seedDownloadTarget(t, store, "req-live", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 10, Downloads: 1}) + seedDownloadTarget(t, store, "req-live", "router-1", Quality2160p, StatusDownloading, nil) + // The plugin still calls every target downloading, with nothing in its + // queue. + router.statuses = []RouterTargetStatus{ + {Quality: Quality1080p, ConnectionID: "router-1", Status: StatusDownloading}, + {Quality: Quality2160p, ConnectionID: "router-1", Status: StatusDownloading}, + } + + result, err := service.RefreshDownloads(context.Background(), 200, 0) + if err != nil || result != (DownloadRefreshResult{Checked: 1}) { + t.Fatalf("RefreshDownloads = %+v, %v; want only req-live checked", result, err) + } + if len(router.statusLog) != 1 || len(router.statusLog[0].refs) != 1 || router.statusLog[0].refs[0].Quality != Quality1080p { + t.Fatalf("status calls = %+v, want one, for req-live's 1080p only", router.statusLog) + } + targets, _ := store.ListTargets(context.Background(), "req-live") + for _, target := range targets { + if target.Status != StatusDownloading || target.Download != nil { + t.Fatalf("req-live %s target = %+v, want downloading without progress", target.Quality, target) + } + } + + // Neither target is polled again until the reconcile pass records new + // progress, and a scheduled run has nothing to do. + result, err = service.RefreshDownloads(context.Background(), 200, 0) + if err != nil || result != (DownloadRefreshResult{}) || router.statusCalls != 1 { + t.Fatalf("second pass = %+v, %v with %d status calls; want nothing asked", result, err, router.statusCalls) + } + if has, err := service.HasDownloadsToRefresh(context.Background()); err != nil || has { + t.Fatalf("HasDownloadsToRefresh() = %v, %v; want false", has, err) + } +} + +// Progress the pass cannot refresh is stale: the pass clears it without a +// call, which takes the target off the pass. +func TestRefreshDownloadsClearsProgressItCannotRefresh(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + disabled := routerInstOn("router-3", 1) + disabled.Enabled = false + noKey := routerInstOn("router-4", 1) + noKey.APIKeyRef = " " + store.integrations = append(store.integrations, disabled, noKey) + stale := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 500, Downloads: 1} + for id, connection := range map[string]string{ + "req-undeclared": "router-2", // its plugin no longer declares progress + "req-disabled": "router-3", + "req-no-key": "router-4", + "req-gone": "router-9", // its server was deleted + } { + seedDownloadTarget(t, store, id, connection, Quality1080p, StatusDownloading, stale) + } + + result, err := service.RefreshDownloads(context.Background(), 200, 0) + if err != nil || result != (DownloadRefreshResult{}) { + t.Fatalf("RefreshDownloads = %+v, %v; want nothing checked", result, err) + } + if router.statusCalls != 0 { + t.Fatalf("status calls = %d, want none", router.statusCalls) + } + for _, id := range []string{"req-undeclared", "req-disabled", "req-no-key", "req-gone"} { + if got := onlyTarget(t, store, id); got.Status != StatusDownloading || got.Download != nil { + t.Fatalf("%s target = %+v, want downloading without progress", id, got) + } + } + if left, _ := store.ListDownloadingRequests(context.Background(), 200); len(left) != 0 { + t.Fatalf("requests left on the refresh pass = %d, want none", len(left)) + } +} + +// A target that gets no status back keeps its progress while it is fresh, +// since one missed answer is usually a blip, and loses it once it is stale, so +// a frozen figure stops showing and clients stop polling for it. That holds +// whether the plugin skipped the target (its server errored or no longer has +// the title) or the whole call failed, and in the reconcile pass too. +func TestUnansweredTargetsKeepProgressUntilItIsStale(t *testing.T) { + for _, tc := range []struct { + name string + age time.Duration + callFails bool + reconcile bool + wantKept bool + }{ + {name: "skipped, fresh", age: 2 * time.Minute, wantKept: true}, + {name: "skipped, stale", age: staleDownloadProgress + time.Minute}, + {name: "call failed, fresh", age: 2 * time.Minute, callFails: true, wantKept: true}, + {name: "call failed, stale", age: staleDownloadProgress + time.Minute, callFails: true}, + {name: "reconcile, fresh", age: 2 * time.Minute, reconcile: true, wantKept: true}, + {name: "reconcile, stale", age: staleDownloadProgress + time.Minute, reconcile: true}, + } { + t.Run(tc.name, func(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + reported := service.now().Add(-tc.age) + progress := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 400, Downloads: 1, UpdatedAt: reported} + target := seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, progress) + // The plugin answers for a target the request does not have. + router.statuses = []RouterTargetStatus{{Quality: Quality2160p, ConnectionID: "router-1", Status: StatusDownloading, Progress: progress}} + if tc.callFails { + router.statusErr = errors.New("context deadline exceeded") + } + + var err error + if tc.reconcile { + store.candidates = []*Request{store.requests["req-1"]} + _, err = service.ReconcileRequests(context.Background(), 100) + } else { + _, err = service.RefreshDownloads(context.Background(), 200, 0) + } + if err != nil { + t.Fatalf("pass: %v", err) + } + if router.statusCalls != 1 { + t.Fatalf("status calls = %d, want 1", router.statusCalls) + } + got := onlyTarget(t, store, "req-1") + if got.Status != StatusDownloading { + t.Fatalf("status = %s, want downloading", got.Status) + } + if tc.wantKept { + if !sameProgress(got.Download, progress) || !got.Download.UpdatedAt.Equal(reported) { + t.Fatalf("progress = %+v, want %+v as last reported", got.Download, progress) + } + if !slices.Equal(store.downloadChecks, []int64{target.ID}) || len(store.downloadWrites) != 0 { + t.Fatalf("checks = %v, writes = %+v; want the target marked asked about and nothing else", store.downloadChecks, store.downloadWrites) + } + return + } + if got.Download != nil { + t.Fatalf("progress = %+v, want stale progress cleared", got.Download) + } + if len(store.downloadChecks) != 0 { + t.Fatalf("checks = %v, want none once the progress is cleared", store.downloadChecks) + } + }) + } +} + +// A request whose server stops answering takes its turn and goes to the back +// instead of heading every batch, so the others still get refreshed. +func TestRefreshDownloadsRotatesPastATargetWithoutAnAnswer(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + store.integrations = append(store.integrations, routerInstOn("router-3", 1)) + now := time.Now().UTC() + seedDownloadTarget(t, store, "req-silent", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 900, Downloads: 1, UpdatedAt: now.Add(-2 * time.Minute)}) + seedDownloadTarget(t, store, "req-healthy", "router-3", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 900, Downloads: 1, UpdatedAt: now.Add(-time.Minute)}) + // router-1 has stopped reporting its target; router-3 answers. + progress := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 300, Downloads: 1} + router.statuses = []RouterTargetStatus{{Quality: Quality1080p, ConnectionID: "router-3", Status: StatusDownloading, Progress: progress}} + + for range 2 { + if _, err := service.RefreshDownloads(context.Background(), 1, 0); err != nil { + t.Fatalf("RefreshDownloads: %v", err) + } + } + if len(router.statusLog) != 2 || router.statusLog[0].conns[0].ID != "router-1" || router.statusLog[1].conns[0].ID != "router-3" { + t.Fatalf("status calls = %+v, want router-1's request and then router-3's", router.statusLog) + } + if got := onlyTarget(t, store, "req-healthy"); !sameProgress(got.Download, progress) { + t.Fatalf("healthy progress = %+v, want %+v", got.Download, progress) + } +} + +// A pass ends within its budget even while a download server has stopped +// answering and each call to it would run to the router's deadline. The call +// in flight is cut, its target goes to the back of the rotation, and the +// requests left over wait for the next pass without failing this one. +func TestRefreshDownloadsStopsWhenItsBudgetRunsOut(t *testing.T) { + store := newFakeStore() + service, router := downloadRefreshService(store) + store.integrations = append(store.integrations, routerInstOn("router-3", 3)) + router.progressCapable[3] = true + router.statusHangFor = map[int]bool{1: true} + now := time.Now().UTC() + progress := func(age time.Duration) *DownloadProgress { + return &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 900, Downloads: 1, UpdatedAt: now.Add(-age)} + } + hungA := seedDownloadTarget(t, store, "req-hung-a", "router-1", Quality1080p, StatusDownloading, progress(3*time.Minute)) + seedDownloadTarget(t, store, "req-hung-b", "router-1", Quality1080p, StatusDownloading, progress(2*time.Minute)) + seedDownloadTarget(t, store, "req-healthy", "router-3", Quality1080p, StatusDownloading, progress(time.Minute)) + + type outcome struct { + result DownloadRefreshResult + err error + } + done := make(chan outcome, 1) + go func() { + result, err := service.RefreshDownloads(context.Background(), 200, 50*time.Millisecond) + done <- outcome{result, err} + }() + var got outcome + select { + case got = <-done: + case <-time.After(10 * time.Second): + t.Fatal("RefreshDownloads outlasted its budget while a server hung") + } + if got.err != nil || got.result != (DownloadRefreshResult{Checked: 1, Errors: 1}) { + t.Fatalf("RefreshDownloads = %+v, %v; want the one cut call and no error", got.result, got.err) + } + if router.statusCalls != 1 { + t.Fatalf("status calls = %d, want only the call the budget cut", router.statusCalls) + } + if !slices.Equal(store.downloadChecks, []int64{hungA.ID}) || onlyTarget(t, store, "req-hung-a").Download == nil { + t.Fatalf("checks = %v; want the cut request's target marked asked about, its progress kept", store.downloadChecks) + } + next, err := store.ListDownloadingRequests(context.Background(), 200) + if err != nil { + t.Fatal(err) + } + if ids, want := func() []string { + ids := make([]string, 0, len(next)) + for _, req := range next { + ids = append(ids, req.ID) + } + return ids + }(), []string{"req-hung-b", "req-healthy", "req-hung-a"}; !slices.Equal(ids, want) { + t.Fatalf("next pass order = %v, want %v", ids, want) + } +} + +func TestHasDownloadsToRefresh(t *testing.T) { + store := newFakeStore() + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 10, Downloads: 1}) + if has, err := newTestService(store).HasDownloadsToRefresh(context.Background()); err != nil || has { + t.Fatalf("without a router: %v, %v; want false", has, err) + } + + store = newFakeStore() + service, _ := downloadRefreshService(store) + seedDownloadTarget(t, store, "req-idle", "router-1", Quality1080p, StatusDownloading, nil) + seedDownloadTarget(t, store, "req-queued", "router-1", Quality1080p, StatusQueued, + &DownloadProgress{Phase: DownloadPhaseQueued, Downloads: 1}) + if has, err := service.HasDownloadsToRefresh(context.Background()); err != nil || has { + t.Fatalf("with no downloading target that has progress: %v, %v; want false", has, err) + } + seedDownloadTarget(t, store, "req-live", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 10, Downloads: 1}) + if has, err := service.HasDownloadsToRefresh(context.Background()); err != nil || !has { + t.Fatalf("with a downloading target that has progress: %v, %v; want true", has, err) + } +} + +func TestRefreshDownloadsWithoutWorkMakesNoCalls(t *testing.T) { + store := newFakeStore() + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, nil) + if result, err := newTestService(store).RefreshDownloads(context.Background(), 200, 0); err != nil || result != (DownloadRefreshResult{}) { + t.Fatalf("without a router: result = %+v, err = %v", result, err) + } + + store = newFakeStore() + service, router := downloadRefreshService(store) + store.listIntegrationsCalls = 0 + if result, err := service.RefreshDownloads(context.Background(), 200, 0); err != nil || result != (DownloadRefreshResult{}) { + t.Fatalf("with nothing downloading: result = %+v, err = %v", result, err) + } + if store.listIntegrationsCalls != 0 || router.statusCalls != 0 { + t.Fatalf("integrations reads = %d, status calls = %d; want none while nothing downloads", store.listIntegrationsCalls, router.statusCalls) + } +} + +func TestGetDetailCarriesTheActiveRequestDownload(t *testing.T) { + store := newFakeStore() + eta := time.Date(2026, 5, 24, 13, 0, 0, 0, time.UTC) + seedDownloadTarget(t, store, "req-1", "router-1", Quality1080p, StatusDownloading, + &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 250, Downloads: 1, EstimatedCompletion: &eta}) + active := store.requests["req-1"] + active.RequestedByUserID, active.RequestedByProfileID = 2, "profile-2" + store.active[MediaTypeMovie][550] = active + service := newTestServiceWithTMDB(store, &fakeTMDBClient{detail: &tmdb.MediaDetail{MediaType: "movie", ID: 550, Title: "Fight Club"}}) + + detail, err := service.GetDetail(context.Background(), testViewer(1), MediaTypeMovie, 550) + if err != nil { + t.Fatalf("GetDetail: %v", err) + } + want := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 1000, BytesLeft: 250, Downloads: 1, EstimatedCompletion: &eta} + if !sameProgress(detail.Request.Download, want) { + t.Fatalf("detail request download = %+v, want %+v", detail.Request.Download, want) + } + + // A request that has not reached a download server has none. + active.Status = StatusApproved + if detail, err = service.GetDetail(context.Background(), testViewer(1), MediaTypeMovie, 550); err != nil || detail.Request.Download != nil { + t.Fatalf("approved request: download = %+v, err = %v; want none", detail.Request.Download, err) + } +} + +// The raw status write changes nothing but external_status: not the date of +// the target's last status change, the request, or its history. A target that +// is no longer queued or downloading keeps what it has. +func TestUpdateTargetExternalStatusDatabase(t *testing.T) { + repo, pool := lifecycleTestRepository(t) + ctx := t.Context() + insertLifecycleRequest(t, repo, "req-ext", 1, 802, StatusDownloading) + target, err := repo.CreateTarget(ctx, Target{RequestID: "req-ext", Quality: Quality1080p, Status: StatusDownloading, ExternalStatus: "completed/importBlocked"}) + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, `UPDATE media_request_targets SET updated_at = now() - interval '2 days' WHERE id = $1`, target.ID); err != nil { + t.Fatal(err) + } + read := func() (external string, updated time.Time, events int) { + t.Helper() + if err := pool.QueryRow(ctx, ` + SELECT t.external_status, t.updated_at, (SELECT count(*) FROM media_request_events e WHERE e.request_id = t.request_id) + FROM media_request_targets t WHERE t.id = $1`, target.ID).Scan(&external, &updated, &events); err != nil { + t.Fatal(err) + } + return external, updated, events + } + _, beforeUpdated, beforeEvents := read() + + if err := repo.UpdateTargetExternalStatus(ctx, target.ID, "warning/downloading"); err != nil { + t.Fatal(err) + } + if external, updated, events := read(); external != "warning/downloading" || !updated.Equal(beforeUpdated) || events != beforeEvents { + t.Fatalf("after write: external_status %q, updated_at %v (was %v), events %d (was %d); want the new status and nothing else changed", + external, updated, beforeUpdated, events, beforeEvents) + } + + if _, err := pool.Exec(ctx, `UPDATE media_request_targets SET status = 'completed' WHERE id = $1`, target.ID); err != nil { + t.Fatal(err) + } + if err := repo.UpdateTargetExternalStatus(ctx, target.ID, "queued"); err != nil { + t.Fatal(err) + } + if external, _, _ := read(); external != "warning/downloading" { + t.Fatalf("a completed target's external_status = %q, want it left alone", external) + } +} + +func TestUpdateTargetDownloadDatabase(t *testing.T) { + repo, pool := lifecycleTestRepository(t) + ctx := t.Context() + insertLifecycleRequest(t, repo, "req-dl", 1, 801, StatusDownloading) + target, err := repo.CreateTarget(ctx, Target{RequestID: "req-dl", Quality: Quality1080p, Status: StatusDownloading}) + if err != nil { + t.Fatal(err) + } + // Date the target's and the request's last change, and their history. + if _, err := pool.Exec(ctx, `UPDATE media_request_targets SET updated_at = now() - interval '2 days' WHERE id = $1`, target.ID); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, `UPDATE media_requests SET updated_at = now() - interval '2 days' WHERE id = 'req-dl'`); err != nil { + t.Fatal(err) + } + snapshot := func() (targetUpdated, requestUpdated time.Time, events int) { + t.Helper() + if err := pool.QueryRow(ctx, ` + SELECT t.updated_at, r.updated_at, (SELECT count(*) FROM media_request_events e WHERE e.request_id = r.id) + FROM media_request_targets t JOIN media_requests r ON r.id = t.request_id + WHERE t.id = $1`, target.ID).Scan(&targetUpdated, &requestUpdated, &events); err != nil { + t.Fatal(err) + } + return targetUpdated, requestUpdated, events + } + stored := func() *DownloadProgress { + t.Helper() + targets, err := repo.ListTargets(ctx, "req-dl") + if err != nil || len(targets) != 1 { + t.Fatalf("targets = %+v, err = %v", targets, err) + } + return targets[0].Download + } + beforeTarget, beforeRequest, beforeEvents := snapshot() + + eta := time.Now().Add(time.Hour).UTC().Truncate(time.Microsecond) + progress := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 5000, BytesLeft: 1200, Downloads: 2, EstimatedCompletion: &eta} + written := time.Now() + if err := repo.UpdateTargetDownload(ctx, target.ID, progress); err != nil { + t.Fatal(err) + } + got := stored() + if !sameProgress(got, progress) || got.UpdatedAt.Before(written.Add(-time.Minute)) { + t.Fatalf("stored progress = %+v, want %+v stamped now", got, progress) + } + // A report counts as asked about too, for the refresh rotation. + checkedAt := func() *time.Time { + t.Helper() + var at *time.Time + if err := pool.QueryRow(ctx, `SELECT download_checked_at FROM media_request_targets WHERE id = $1`, target.ID).Scan(&at); err != nil { + t.Fatal(err) + } + return at + } + if at := checkedAt(); at == nil || !at.Equal(got.UpdatedAt) { + t.Fatalf("download_checked_at = %v, want the report's %v", at, got.UpdatedAt) + } + if afterTarget, afterRequest, afterEvents := snapshot(); !afterTarget.Equal(beforeTarget) || !afterRequest.Equal(beforeRequest) || afterEvents != beforeEvents { + t.Fatalf("a progress write moved updated_at (target %v -> %v, request %v -> %v) or wrote history (%d -> %d)", + beforeTarget, afterTarget, beforeRequest, afterRequest, beforeEvents, afterEvents) + } + + // Nil clears it, again without touching the target or the request. + if err := repo.UpdateTargetDownload(ctx, target.ID, nil); err != nil { + t.Fatal(err) + } + if got := stored(); got != nil { + t.Fatalf("progress after clearing = %+v, want none", got) + } + if at := checkedAt(); at != nil { + t.Fatalf("download_checked_at after clearing = %v, want none", at) + } + if afterTarget, _, _ := snapshot(); !afterTarget.Equal(beforeTarget) { + t.Fatal("clearing progress moved the target's updated_at") + } + + // Completion clears it, and a late report cannot bring it back. + if err := repo.UpdateTargetDownload(ctx, target.ID, progress); err != nil { + t.Fatal(err) + } + if _, err := repo.UpdateTargetStatus(ctx, target.ID, StatusCompleted, "", "imported", "", Viewer{}); err != nil { + t.Fatal(err) + } + if got := stored(); got != nil { + t.Fatalf("progress after completion = %+v, want none", got) + } + if err := repo.UpdateTargetDownload(ctx, target.ID, progress); err != nil { + t.Fatal(err) + } + if got := stored(); got != nil { + t.Fatalf("a late report wrote %+v onto a completed target", got) + } +} + +func TestFailedTargetLosesDownloadProgressDatabase(t *testing.T) { + repo, _ := lifecycleTestRepository(t) + ctx := t.Context() + insertLifecycleRequest(t, repo, "req-fail", 1, 811, StatusQueued) + target, err := repo.CreateTarget(ctx, Target{RequestID: "req-fail", Quality: Quality1080p, Status: StatusQueued}) + if err != nil { + t.Fatal(err) + } + if err := repo.UpdateTargetDownload(ctx, target.ID, &DownloadProgress{Phase: DownloadPhaseStalled, BytesTotal: 10, BytesLeft: 10, Downloads: 1}); err != nil { + t.Fatal(err) + } + if _, err := repo.UpdateTargetStatus(ctx, target.ID, StatusFailed, "", "", "download failed", Viewer{}); err != nil { + t.Fatal(err) + } + targets, err := repo.ListTargets(ctx, "req-fail") + if err != nil || len(targets) != 1 || targets[0].Download != nil { + t.Fatalf("targets = %+v, err = %v; want the failed target without progress", targets, err) + } +} + +// addDownloadTestTarget stores a target for a DB test, with progress reported +// and asked about refreshedAgo ago, or without progress when refreshedAgo is +// empty. +func addDownloadTestTarget(t *testing.T, repo *Repository, pool *pgxpool.Pool, requestID string, quality Quality, status Status, refreshedAgo string) Target { + t.Helper() + ctx := t.Context() + target, err := repo.CreateTarget(ctx, Target{RequestID: requestID, Quality: quality, Status: status}) + if err != nil { + t.Fatal(err) + } + if refreshedAgo == "" { + return target + } + if _, err := pool.Exec(ctx, ` + UPDATE media_request_targets + SET download_phase = 'downloading', download_bytes_total = 1, download_bytes_left = 1, + download_count = 1, download_updated_at = now() - $2::interval, + download_checked_at = now() - $2::interval + WHERE id = $1`, target.ID, refreshedAgo); err != nil { + t.Fatal(err) + } + return target +} + +func requestIDs(reqs []*Request) []string { + ids := make([]string, 0, len(reqs)) + for _, req := range reqs { + ids = append(ids, req.ID) + } + return ids +} + +func TestListDownloadingRequestsOrderDatabase(t *testing.T) { + repo, pool := lifecycleTestRepository(t) + ctx := t.Context() + addTarget := func(requestID string, quality Quality, status Status, refreshedAgo string) { + t.Helper() + _ = addDownloadTestTarget(t, repo, pool, requestID, quality, status, refreshedAgo) + } + for i, id := range []string{"dl-a", "dl-b", "dl-c", "dl-d", "dl-e", "dl-f", "dl-g", "dl-h"} { + insertLifecycleRequest(t, repo, id, 1, 900+i, StatusDownloading) + } + addTarget("dl-a", Quality1080p, StatusDownloading, "1 minute") + addTarget("dl-b", Quality1080p, StatusDownloading, "") // no progress + addTarget("dl-c", Quality1080p, StatusQueued, "30 minutes") // nothing downloading + addTarget("dl-d", Quality1080p, StatusDownloading, "10 minutes") + // One recent target does not hide a stale sibling. + addTarget("dl-e", Quality1080p, StatusDownloading, "30 seconds") + addTarget("dl-e", Quality2160p, StatusDownloading, "5 minutes") + // A sibling without progress does not make the request look stale. + addTarget("dl-f", Quality1080p, StatusDownloading, "2 minutes") + addTarget("dl-f", Quality2160p, StatusDownloading, "") + addTarget("dl-g", Quality1080p, StatusDownloading, "20 minutes") + addTarget("dl-h", Quality1080p, StatusDownloading, "") // no progress + // A closed request is not refreshed. + if _, err := pool.Exec(ctx, `UPDATE media_requests SET outcome = 'cancelled' WHERE id = 'dl-g'`); err != nil { + t.Fatal(err) + } + + got, err := repo.ListDownloadingRequests(ctx, 10) + if err != nil { + t.Fatal(err) + } + if ids, want := requestIDs(got), []string{"dl-d", "dl-e", "dl-f", "dl-a"}; !slices.Equal(ids, want) { + t.Fatalf("downloading requests = %v, want %v", ids, want) + } + limited, err := repo.ListDownloadingRequests(ctx, 2) + if err != nil { + t.Fatal(err) + } + if ids, want := requestIDs(limited), []string{"dl-d", "dl-e"}; !slices.Equal(ids, want) { + t.Fatalf("limited = %v, want the two refreshed longest ago", ids) + } +} + +// Downloading targets that never report progress (a plugin that does not +// declare it, or one with nothing queued for them) cannot fill the batch and +// starve the targets the refresh pass exists for, however many there are. +func TestListDownloadingRequestsSkipsTargetsWithoutProgressDatabase(t *testing.T) { + repo, pool := lifecycleTestRepository(t) + ctx := t.Context() + const limit = 3 + for i := range 2 * limit { + id := fmt.Sprintf("dl-idle-%d", i) + insertLifecycleRequest(t, repo, id, 1, 950+i, StatusDownloading) + _ = addDownloadTestTarget(t, repo, pool, id, Quality1080p, StatusDownloading, "") + } + insertLifecycleRequest(t, repo, "dl-live-1", 1, 970, StatusDownloading) + _ = addDownloadTestTarget(t, repo, pool, "dl-live-1", Quality1080p, StatusDownloading, "1 minute") + insertLifecycleRequest(t, repo, "dl-live-2", 1, 971, StatusDownloading) + _ = addDownloadTestTarget(t, repo, pool, "dl-live-2", Quality1080p, StatusDownloading, "3 minutes") + + got, err := repo.ListDownloadingRequests(ctx, limit) + if err != nil { + t.Fatal(err) + } + if ids, want := requestIDs(got), []string{"dl-live-2", "dl-live-1"}; !slices.Equal(ids, want) { + t.Fatalf("downloading requests = %v, want %v", ids, want) + } +} + +// A target whose server stops answering takes its turn and goes to the back of +// the rotation, keeping its progress and when it was last heard from; it does +// not head every batch. +func TestMarkTargetDownloadCheckedRotatesDatabase(t *testing.T) { + repo, pool := lifecycleTestRepository(t) + ctx := t.Context() + insertLifecycleRequest(t, repo, "dl-silent", 1, 980, StatusDownloading) + silent := addDownloadTestTarget(t, repo, pool, "dl-silent", Quality1080p, StatusDownloading, "10 minutes") + insertLifecycleRequest(t, repo, "dl-healthy", 1, 981, StatusDownloading) + _ = addDownloadTestTarget(t, repo, pool, "dl-healthy", Quality1080p, StatusDownloading, "1 minute") + if _, err := pool.Exec(ctx, `UPDATE media_request_targets SET updated_at = now() - interval '2 days' WHERE id = $1`, silent.ID); err != nil { + t.Fatal(err) + } + read := func() (heard time.Time, updated time.Time, progress *DownloadProgress) { + t.Helper() + if err := pool.QueryRow(ctx, `SELECT download_updated_at, updated_at FROM media_request_targets WHERE id = $1`, silent.ID).Scan(&heard, &updated); err != nil { + t.Fatal(err) + } + targets, err := repo.ListTargets(ctx, "dl-silent") + if err != nil || len(targets) != 1 { + t.Fatalf("targets = %+v, err = %v", targets, err) + } + return heard, updated, targets[0].Download + } + heardBefore, updatedBefore, _ := read() + + if got, err := repo.ListDownloadingRequests(ctx, 10); err != nil || !slices.Equal(requestIDs(got), []string{"dl-silent", "dl-healthy"}) { + t.Fatalf("before = %v, %v; want the silent request first", requestIDs(got), err) + } + if err := repo.MarkTargetDownloadChecked(ctx, silent.ID); err != nil { + t.Fatal(err) + } + if got, err := repo.ListDownloadingRequests(ctx, 10); err != nil || !slices.Equal(requestIDs(got), []string{"dl-healthy", "dl-silent"}) { + t.Fatalf("after = %v, %v; want the silent request behind the healthy one", requestIDs(got), err) + } + heard, updated, progress := read() + if !heard.Equal(heardBefore) || !updated.Equal(updatedBefore) || progress == nil || progress.Phase != DownloadPhaseDownloading { + t.Fatalf("heard %v -> %v, updated_at %v -> %v, progress %+v; want all kept", heardBefore, heard, updatedBefore, updated, progress) + } + + // A target without progress, or finished, is not stamped. + bare := addDownloadTestTarget(t, repo, pool, "dl-healthy", Quality2160p, StatusDownloading, "") + if err := repo.MarkTargetDownloadChecked(ctx, bare.ID); err != nil { + t.Fatal(err) + } + if _, err := repo.UpdateTargetStatus(ctx, silent.ID, StatusCompleted, "", "imported", "", Viewer{}); err != nil { + t.Fatal(err) + } + if err := repo.MarkTargetDownloadChecked(ctx, silent.ID); err != nil { + t.Fatal(err) + } + var stamped int + if err := pool.QueryRow(ctx, `SELECT count(*) FROM media_request_targets WHERE id = ANY($1) AND download_checked_at IS NOT NULL`, []int64{bare.ID, silent.ID}).Scan(&stamped); err != nil { + t.Fatal(err) + } + if stamped != 0 { + t.Fatalf("%d targets without live progress were stamped, want none", stamped) + } +} diff --git a/internal/requests/provider.go b/internal/requests/provider.go index a554b7cc7a..d8e2b521e8 100644 --- a/internal/requests/provider.go +++ b/internal/requests/provider.go @@ -49,6 +49,9 @@ type RouterTargetStatus struct { Status Status ExternalStatus string Message string + // Progress is set by a plugin that declares reports_download_progress + // while the target has downloads in flight; nil means none. + Progress *DownloadProgress } type RouterOption struct { @@ -198,11 +201,44 @@ func (p *pluginRouterProvider) CheckStatus(ctx context.Context, installationID i out = append(out, RouterTargetStatus{ Quality: Quality(st.GetQuality()), ConnectionID: st.GetConnectionId(), Status: Status(st.GetStatus()), ExternalStatus: st.GetExternalStatus(), Message: st.GetMessage(), + Progress: downloadProgressFromProto(st.GetProgress()), }) } return out, nil } +// downloadProgressFromProto normalizes a plugin's progress report. An unknown +// or empty phase reads as downloading, but a report with neither a phase nor +// a size carries nothing and maps to nil. Byte counts are clamped so that +// 0 <= left <= total; a total of 0 means the size is unknown. +func downloadProgressFromProto(p *pluginv1.DownloadProgress) *DownloadProgress { + if p == nil { + return nil + } + phase := DownloadPhase(p.GetPhase()) + total := max(p.GetBytesTotal(), 0) + if phase == "" && total == 0 { + return nil + } + switch phase { + case DownloadPhaseQueued, DownloadPhaseDownloading, DownloadPhasePaused, + DownloadPhaseStalled, DownloadPhaseImporting, DownloadPhaseImportBlocked: + default: + phase = DownloadPhaseDownloading + } + out := &DownloadProgress{ + Phase: phase, + BytesTotal: total, + BytesLeft: min(max(p.GetBytesLeft(), 0), total), + Downloads: int(max(p.GetDownloads(), 0)), + } + if eta := p.GetEstimatedCompletion(); eta != nil && eta.IsValid() { + at := eta.AsTime().UTC() + out.EstimatedCompletion = &at + } + return out +} + func (p *pluginRouterProvider) ListConfigOptions(ctx context.Context, installationID int, capabilityID string, conn ResolvedRouterConnection) (map[string][]RouterOption, error) { client, err := p.resolver.RequestRouterClient(ctx, installationID, capabilityID) if err != nil { diff --git a/internal/requests/repository.go b/internal/requests/repository.go index 7e64e9d350..b782eaf56c 100644 --- a/internal/requests/repository.go +++ b/internal/requests/repository.go @@ -401,6 +401,47 @@ func (r *Repository) ListReconciliationCandidates(ctx context.Context, limit int return out, nil } +// ListDownloadingRequests returns active requests with a downloading target +// that has download progress, the one asked about longest ago first: a +// request is as due as its most overdue such target. A target counts as asked +// whether or not its server answered, so one that stops answering takes its +// turn and moves to the back instead of heading every batch. A downloading +// target without progress is not listed, however many there are, so targets +// whose plugin never reports any (or has nothing queued) cannot crowd the +// batch; the reconcile pass records a download's first progress. +func (r *Repository) ListDownloadingRequests(ctx context.Context, limit int) ([]*Request, error) { + if limit <= 0 || limit > 500 { + limit = 200 + } + rows, err := r.pool.Query(ctx, requestSelectSQL()+` + JOIN ( + SELECT request_id, min(download_checked_at) AS checked_at + FROM media_request_targets + WHERE status = 'downloading' AND download_phase IS NOT NULL + GROUP BY request_id + ) downloading ON downloading.request_id = media_requests.id + WHERE outcome = 'active' + ORDER BY downloading.checked_at NULLS FIRST, media_requests.id + LIMIT $1 + `, limit) + if err != nil { + return nil, fmt.Errorf("list downloading requests: %w", err) + } + defer rows.Close() + var out []*Request + for rows.Next() { + req, err := scanRequest(rows) + if err != nil { + return nil, err + } + out = append(out, req) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate downloading requests: %w", err) + } + return out, nil +} + // ListLibraryWaitCandidates returns the requests that only the library can // complete: pending ones, and ones that failed in the last 30 days without // delivering anything. Older failures are left alone so an upgrade does not diff --git a/internal/requests/router_seasons.go b/internal/requests/router_seasons.go index 9db4bff4c4..6b8d208f04 100644 --- a/internal/requests/router_seasons.go +++ b/internal/requests/router_seasons.go @@ -13,6 +13,9 @@ type RouterFeatures struct { // them to a series its download server already has, leaving the other // seasons alone. Any other plugin takes the whole series. SupportsSeasons bool + // ReportsDownloadProgress: CheckStatus fills each live target's download + // progress. The download refresh pass polls only such plugins. + ReportsDownloadProgress bool } // RouterFeatureReader reads a router capability's declared features from the @@ -28,32 +31,39 @@ type routerCapabilityKey struct { capabilityID string } -// routerSupportsSeasons reports whether the router capability takes a request -// for particular seasons. The answer is kept for the fulfill context's -// lifetime, one reconcile pass or one request. -func (s *Service) routerSupportsSeasons(ctx context.Context, fc *fulfillContext, installationID int, capabilityID string) (bool, error) { +// routerFeatures returns the features a router capability declares. The +// answer is kept for the fulfill context's lifetime, one reconcile pass or one +// request. +func (s *Service) routerFeatures(ctx context.Context, fc *fulfillContext, installationID int, capabilityID string) (RouterFeatures, error) { reader, ok := s.router.(RouterFeatureReader) if !ok { - return false, nil + return RouterFeatures{}, nil } key := routerCapabilityKey{installationID, capabilityID} fc.mu.Lock() - supported, cached := fc.seasonSupport[key] + features, cached := fc.features[key] fc.mu.Unlock() if cached { - return supported, nil + return features, nil } features, err := reader.RouterFeatures(ctx, installationID, capabilityID) if err != nil { - return false, fmt.Errorf("read request router features: %w", err) + return RouterFeatures{}, fmt.Errorf("read request router features: %w", err) } fc.mu.Lock() - if fc.seasonSupport == nil { - fc.seasonSupport = map[routerCapabilityKey]bool{} + if fc.features == nil { + fc.features = map[routerCapabilityKey]RouterFeatures{} } - fc.seasonSupport[key] = features.SupportsSeasons + fc.features[key] = features fc.mu.Unlock() - return features.SupportsSeasons, nil + return features, nil +} + +// routerSupportsSeasons reports whether the router capability takes a request +// for particular seasons. +func (s *Service) routerSupportsSeasons(ctx context.Context, fc *fulfillContext, installationID int, capabilityID string) (bool, error) { + features, err := s.routerFeatures(ctx, fc, installationID, capabilityID) + return features.SupportsSeasons, err } // allTakeSeasons reports whether every given connection is bound to a router diff --git a/internal/requests/service.go b/internal/requests/service.go index ab836fb220..2033bfc88a 100644 --- a/internal/requests/service.go +++ b/internal/requests/service.go @@ -404,10 +404,10 @@ type fulfillContext struct { // standardOn is set when Standard routing is in effect. standardOn bool - // mu guards seasonSupport, which caches whether each router capability - // takes seasons (see routerSupportsSeasons). - mu sync.Mutex - seasonSupport map[routerCapabilityKey]bool + // mu guards features, which caches the features each router capability + // declares (see routerFeatures). + mu sync.Mutex + features map[routerCapabilityKey]RouterFeatures } // routesFor returns the media type's routing rules; none means the router @@ -788,6 +788,9 @@ func (s *Service) GetDetail(ctx context.Context, viewer Viewer, mediaType MediaT } primaryState := requestStateFor(viewer, policy, available, primaryRequests[raw.ID]) primaryState.Following = primaryRequests[raw.ID] != nil && primaryFollowing[raw.ID] + if primaryState.Download, err = s.activeRequestDownload(ctx, primaryRequests[raw.ID]); err != nil { + return nil, err + } detail := &MediaDetail{ MediaType: mediaType, @@ -2864,41 +2867,78 @@ func (s *Service) reconcileRequest(ctx context.Context, req Request, fc *fulfill return reconcileUnchanged, nil } statuses, checkErr := s.checkTargetStatuses(ctx, req, targets, fc) + change, err := s.applyTargetStatuses(ctx, targets, statuses) + if err != nil { + return reconcileUnchanged, err + } + return change, checkErr +} +// applyTargetStatuses writes the statuses a router reported for the given +// targets, matched by quality and connection, and the download progress of +// each target still queued or downloading afterwards. A target that reports +// no progress has any it had cleared; one that had none is not written, so an +// idle target costs no write per pass. A target with progress that got no +// status back (its server was skipped or failed, or the call did) keeps it +// until it goes stale; see settleUnansweredDownload. Both reconcile and the +// download refresh pass apply statuses here. +func (s *Service) applyTargetStatuses(ctx context.Context, targets []Target, statuses []RouterTargetStatus) (reconcileChange, error) { change := reconcileUnchanged + answered := make([]bool, len(targets)) for _, st := range statuses { // Match the returned status to the live target by (quality, connection). var target *Target for i := range targets { if targets[i].Quality == st.Quality && targets[i].IntegrationID == st.ConnectionID { target = &targets[i] + answered[i] = true break } } if target == nil || target.Status == StatusCompleted || target.Status == StatusFailed { continue } - newStatus := st.Status - if newStatus == "" || newStatus == target.Status { + status := target.Status + if newStatus := st.Status; newStatus != "" && newStatus != target.Status { + if _, err := s.store.UpdateTargetStatus(ctx, target.ID, newStatus, "", st.ExternalStatus, st.Message, Viewer{}); err != nil { + return reconcileUnchanged, err + } + status = newStatus + switch newStatus { + case StatusCompleted: + change = reconcileCompleted + case StatusDownloading: + if change == reconcileUnchanged { + change = reconcileDownloading + } + case StatusFailed: + if change == reconcileUnchanged { + change = reconcileFailed + } + } + } else if st.ExternalStatus != "" && st.ExternalStatus != target.ExternalStatus { + // The server's own state moved without changing the target's + // status (say, a download went from importing to stalled). Keep + // the raw status in step with the progress shown beside it. + if err := s.store.UpdateTargetExternalStatus(ctx, target.ID, st.ExternalStatus); err != nil { + return reconcileUnchanged, err + } + } + if (status == StatusQueued || status == StatusDownloading) && (st.Progress != nil || target.Download != nil) { + if err := s.store.UpdateTargetDownload(ctx, target.ID, st.Progress); err != nil { + return reconcileUnchanged, err + } + } + } + for i, target := range targets { + if answered[i] { continue } - if _, err := s.store.UpdateTargetStatus(ctx, target.ID, newStatus, "", st.ExternalStatus, st.Message, Viewer{}); err != nil { + if err := s.settleUnansweredDownload(ctx, target); err != nil { return reconcileUnchanged, err } - switch newStatus { - case StatusCompleted: - change = reconcileCompleted - case StatusDownloading: - if change == reconcileUnchanged { - change = reconcileDownloading - } - case StatusFailed: - if change == reconcileUnchanged { - change = reconcileFailed - } - } } - return change, checkErr + return change, nil } // checkTargetStatuses asks each live target's plugin for its status. Targets @@ -2955,7 +2995,7 @@ func (s *Service) checkTargetStatuses(ctx context.Context, req Request, targets continue } in := integrationByID(fc, t.IntegrationID) - if in == nil || !in.Enabled || in.InstallationID == nil || in.CapabilityID == "" || strings.TrimSpace(in.APIKeyRef) == "" { + if !statusCheckable(in) { continue } g := groupFor(owner{*in.InstallationID, in.CapabilityID}) @@ -2973,6 +3013,10 @@ func (s *Service) checkTargetStatuses(ctx context.Context, req Request, targets errs = append(errs, err) continue } + keepProgress, err := s.keepsReportedProgress(ctx, fc, key.installationID, key.capabilityID, statuses) + if err != nil { + errs = append(errs, err) + } // A plugin that omits connection_id from its statuses omits it from // its targets too, and a routed target was recorded on its server // anyway. When every target in the group is on one server, a status @@ -2982,6 +3026,9 @@ func (s *Service) checkTargetStatuses(ctx context.Context, req Request, targets if st.ConnectionID == "" { st.ConnectionID = server } + if !keepProgress { + st.Progress = nil + } out = append(out, st) } } @@ -3001,6 +3048,13 @@ func soleRefConnection(refs []RouterTargetRef) string { return server } +// statusCheckable reports whether a target's server can be asked for the +// target's status: it still exists, is enabled, is bound to a router +// capability, and has an API key. +func statusCheckable(in *Integration) bool { + return in != nil && in.Enabled && in.InstallationID != nil && in.CapabilityID != "" && strings.TrimSpace(in.APIKeyRef) != "" +} + func integrationByID(fc *fulfillContext, id string) *Integration { if id == "" { return nil diff --git a/internal/requests/service_test.go b/internal/requests/service_test.go index 45d5a88831..31fbe773c1 100644 --- a/internal/requests/service_test.go +++ b/internal/requests/service_test.go @@ -2000,10 +2000,27 @@ type fakeStore struct { setExternalIDsErr error + // downloadWrites records each progress write UpdateTargetDownload + // applied; downloadErr fails it. downloadChecked holds when each target + // was last asked about, as the repository's download_checked_at, and + // downloadChecks the targets MarkTargetDownloadChecked stamped. + downloadWrites []downloadWrite + downloadErr error + downloadChecked map[int64]time.Time + downloadChecks []int64 + // externalStatusWrites records the targets UpdateTargetExternalStatus + // wrote. + externalStatusWrites []int64 + listIntegrationsCalls int getSettingsCalls int } +type downloadWrite struct { + targetID int64 + progress *DownloadProgress +} + type requestGroupProvider struct { group *access.GroupPolicy err error @@ -2148,6 +2165,57 @@ func (f *fakeStore) ListLibraryWaitCandidates(context.Context, int) ([]*Request, return f.waiting, nil } +// ListDownloadingRequests mirrors the repository: active requests with a +// downloading target that has progress, by the one asked about longest ago, +// then id. A seeded target that was never asked about counts as asked when +// its progress was reported. +func (f *fakeStore) ListDownloadingRequests(_ context.Context, limit int) ([]*Request, error) { + f.mu.Lock() + defer f.mu.Unlock() + type candidate struct { + req *Request + checked time.Time + } + var found []candidate + for id, targets := range f.targets { + req := f.requests[id] + if req == nil || req.Outcome != OutcomeActive { + continue + } + var checked *time.Time + for _, t := range targets { + if t.Status != StatusDownloading || t.Download == nil { + continue + } + at, ok := f.downloadChecked[t.ID] + if !ok { + at = t.Download.UpdatedAt + } + if checked == nil || at.Before(*checked) { + checked = &at + } + } + if checked != nil { + copy := *req + found = append(found, candidate{req: ©, checked: *checked}) + } + } + slices.SortFunc(found, func(a, b candidate) int { + if c := a.checked.Compare(b.checked); c != 0 { + return c + } + return strings.Compare(a.req.ID, b.req.ID) + }) + out := make([]*Request, 0, len(found)) + for _, c := range found { + if limit > 0 && len(out) == limit { + break + } + out = append(out, c.req) + } + return out, nil +} + func (f *fakeStore) ListFulfilledUnnotified(context.Context, int) ([]*Request, error) { f.mu.Lock() defer f.mu.Unlock() @@ -2682,6 +2750,91 @@ func (f *fakeStore) UpdateTargetStatus(_ context.Context, targetID int64, status return f.updateTargetLocked(targetID, status, externalID, externalStatus, lastErr) } +// UpdateTargetDownload mirrors the repository: it writes only while the +// target is queued or downloading, and touches nothing but the progress. +// Every write it applies is recorded in downloadWrites. +func (f *fakeStore) UpdateTargetDownload(_ context.Context, targetID int64, progress *DownloadProgress) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.downloadErr != nil { + return f.downloadErr + } + for rid, ts := range f.targets { + for i := range ts { + if ts[i].ID != targetID { + continue + } + if ts[i].Status != StatusQueued && ts[i].Status != StatusDownloading { + return nil + } + var stored *DownloadProgress + if f.downloadChecked == nil { + f.downloadChecked = map[int64]time.Time{} + } + delete(f.downloadChecked, targetID) + if progress != nil { + copy := *progress + copy.UpdatedAt = time.Now().UTC() + stored = © + f.downloadChecked[targetID] = copy.UpdatedAt + } + f.targets[rid][i].Download = stored + f.downloadWrites = append(f.downloadWrites, downloadWrite{targetID: targetID, progress: stored}) + return nil + } + } + return nil +} + +// MarkTargetDownloadChecked mirrors the repository: it stamps a queued or +// downloading target that has progress as asked about now, leaving the +// progress alone, and records the stamp in downloadChecks. +func (f *fakeStore) MarkTargetDownloadChecked(_ context.Context, targetID int64) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.downloadErr != nil { + return f.downloadErr + } + for _, ts := range f.targets { + for _, t := range ts { + if t.ID != targetID { + continue + } + if t.Download == nil || (t.Status != StatusQueued && t.Status != StatusDownloading) { + return nil + } + if f.downloadChecked == nil { + f.downloadChecked = map[int64]time.Time{} + } + f.downloadChecked[targetID] = time.Now().UTC() + f.downloadChecks = append(f.downloadChecks, targetID) + return nil + } + } + return nil +} + +// UpdateTargetExternalStatus mirrors the repository: it writes only a changed +// raw status on a queued or downloading target, and touches nothing else. +func (f *fakeStore) UpdateTargetExternalStatus(_ context.Context, targetID int64, externalStatus string) error { + f.mu.Lock() + defer f.mu.Unlock() + for rid, ts := range f.targets { + for i := range ts { + if ts[i].ID != targetID { + continue + } + if (ts[i].Status != StatusQueued && ts[i].Status != StatusDownloading) || ts[i].ExternalStatus == externalStatus { + return nil + } + f.targets[rid][i].ExternalStatus = externalStatus + f.externalStatusWrites = append(f.externalStatusWrites, targetID) + return nil + } + } + return nil +} + // RecordSubmission mirrors the repository's lease fence, then records each // target the way a create followed by a status update would. func (f *fakeStore) RecordSubmission(_ context.Context, id string, leaseUntil time.Time, targets []Target, _ Viewer) (*Request, error) { @@ -2729,6 +2882,10 @@ func (f *fakeStore) updateTargetLocked(targetID int64, status Status, externalID } f.targets[rid][i].Status = status f.targets[rid][i].LastError = lastErr + if status == StatusCompleted || status == StatusFailed { + f.targets[rid][i].Download = nil + delete(f.downloadChecked, targetID) + } requestID = rid } } @@ -3150,16 +3307,23 @@ type fakeRouterProvider struct { // seasonCapable marks the installations whose router declares // supports_seasons; RouterFeatures answers from it. seasonCapable map[int]bool - featuresErr error - gotTVDBID *int + // progressCapable marks the installations whose router declares + // reports_download_progress. + progressCapable map[int]bool + featuresErr error + gotTVDBID *int // CheckStatus behavior. statuses []RouterTargetStatus statusErr error // statusErrFor fails CheckStatus for one installation only. statusErrFor map[int]error - statusCalls int - statusLog []statusCall + // statusHangFor makes CheckStatus for an installation wait until its + // context ends, the way a call to a server that stopped answering runs to + // its deadline. + statusHangFor map[int]bool + statusCalls int + statusLog []statusCall // ListConfigOptions behavior. options map[string][]RouterOption @@ -3178,7 +3342,10 @@ type fakeRouterProvider struct { func (f *fakeRouterProvider) RouterFeatures(_ context.Context, installationID int, _ string) (RouterFeatures, error) { f.mu.Lock() defer f.mu.Unlock() - return RouterFeatures{SupportsSeasons: f.seasonCapable[installationID]}, f.featuresErr + return RouterFeatures{ + SupportsSeasons: f.seasonCapable[installationID], + ReportsDownloadProgress: f.progressCapable[installationID], + }, f.featuresErr } func (f *fakeRouterProvider) Fulfill(_ context.Context, installationID int, _ string, req Request, qualities []Quality, conns []ResolvedRouterConnection) ([]RouterTarget, string, error) { @@ -3223,11 +3390,18 @@ func (f *fakeRouterProvider) Fulfill(_ context.Context, installationID int, _ st return out, f.fulfillMsg, nil } -func (f *fakeRouterProvider) CheckStatus(_ context.Context, installationID int, capabilityID string, _ Request, refs []RouterTargetRef, conns []ResolvedRouterConnection) ([]RouterTargetStatus, error) { +func (f *fakeRouterProvider) CheckStatus(ctx context.Context, installationID int, capabilityID string, _ Request, refs []RouterTargetRef, conns []ResolvedRouterConnection) ([]RouterTargetStatus, error) { f.mu.Lock() - defer f.mu.Unlock() f.statusCalls++ f.statusLog = append(f.statusLog, statusCall{installationID: installationID, capabilityID: capabilityID, refs: slices.Clone(refs), conns: slices.Clone(conns)}) + hang := f.statusHangFor[installationID] + f.mu.Unlock() + if hang { + <-ctx.Done() + return nil, ctx.Err() + } + f.mu.Lock() + defer f.mu.Unlock() if err := f.statusErrFor[installationID]; err != nil { return nil, err } diff --git a/internal/requests/store.go b/internal/requests/store.go index fcab42db4a..395fb71abf 100644 --- a/internal/requests/store.go +++ b/internal/requests/store.go @@ -20,6 +20,10 @@ type Store interface { // last_reconciled_at. ListReconciliationCandidates(ctx context.Context, limit int) ([]*Request, error) ListLibraryWaitCandidates(ctx context.Context, limit int) ([]*Request, error) + // ListDownloadingRequests returns active requests with a downloading + // target that has download progress, for the download refresh pass, the + // least recently asked about first. Targets without progress are ignored. + ListDownloadingRequests(ctx context.Context, limit int) ([]*Request, error) // ListFulfilledUnnotified returns completed requests whose fulfillment // notification has not fired yet (presence-gated notify pass). ListFulfilledUnnotified(ctx context.Context, limit int) ([]*Request, error) @@ -99,7 +103,21 @@ type Store interface { ListTargetsForRequests(ctx context.Context, requestIDs []string) (map[string][]Target, error) CreateTarget(ctx context.Context, target Target) (Target, error) DeleteTarget(ctx context.Context, id int64) error + // UpdateTargetStatus also clears the target's download progress when it + // completes or fails. UpdateTargetStatus(ctx context.Context, targetID int64, status Status, externalID, externalStatus, lastErr string, actor Viewer) (*Request, error) + // UpdateTargetDownload stores a target's download progress, or clears it + // when progress is nil, only while the target is queued or downloading. It + // leaves updated_at, the request aggregate and the request's history alone. + UpdateTargetDownload(ctx context.Context, targetID int64, progress *DownloadProgress) error + // MarkTargetDownloadChecked records that a pass asked about a target + // with progress and got no status back, without changing the progress. + MarkTargetDownloadChecked(ctx context.Context, targetID int64) error + // UpdateTargetExternalStatus records the raw status a queued or + // downloading target's server reported when the target's own status did + // not change. Like UpdateTargetDownload, it leaves updated_at, the + // request aggregate and the request's history alone. + UpdateTargetExternalStatus(ctx context.Context, targetID int64, externalStatus string) error ListIntegrations(ctx context.Context) ([]Integration, error) GetIntegration(ctx context.Context, id string) (*Integration, error) CreateIntegration(ctx context.Context, integration Integration) (*Integration, error) diff --git a/internal/requests/targets.go b/internal/requests/targets.go index 67e60afdd5..8651c08745 100644 --- a/internal/requests/targets.go +++ b/internal/requests/targets.go @@ -11,7 +11,14 @@ import ( const targetColumns = `t.id, t.request_id, t.integration_id, t.integration_kind, COALESCE(ri.name, ''), t.quality, t.is_anime, t.external_id, t.external_status, - t.status, t.last_error, t.created_at, t.updated_at, COALESCE(t.route_id, ''), t.route_name` + t.status, t.last_error, t.created_at, t.updated_at, COALESCE(t.route_id, ''), t.route_name, + t.download_phase, t.download_bytes_total, t.download_bytes_left, t.download_eta, + t.download_count, t.download_updated_at` + +// clearTargetDownload empties a target's download progress columns. +const clearTargetDownload = `download_phase = NULL, download_bytes_total = NULL, + download_bytes_left = NULL, download_eta = NULL, download_count = NULL, + download_updated_at = NULL, download_checked_at = NULL` // aggregateStatus derives a request's status/outcome from its targets. func aggregateStatus(targets []Target) (Status, Outcome) { @@ -54,15 +61,35 @@ func aggregateStatus(targets []Target) (Status, Outcome) { func scanTarget(row requestScanner) (Target, error) { var t Target - var integrationID *string + var integrationID, downloadPhase *string + var bytesTotal, bytesLeft *int64 + var downloadCount *int + var downloadETA, downloadUpdatedAt *time.Time if err := row.Scan(&t.ID, &t.RequestID, &integrationID, &t.IntegrationKind, &t.InstanceName, &t.Quality, &t.IsAnime, &t.ExternalID, &t.ExternalStatus, - &t.Status, &t.LastError, &t.CreatedAt, &t.UpdatedAt, &t.RouteID, &t.RouteName); err != nil { + &t.Status, &t.LastError, &t.CreatedAt, &t.UpdatedAt, &t.RouteID, &t.RouteName, + &downloadPhase, &bytesTotal, &bytesLeft, &downloadETA, &downloadCount, &downloadUpdatedAt); err != nil { return Target{}, err } if integrationID != nil { t.IntegrationID = *integrationID } + if downloadPhase != nil { + d := &DownloadProgress{Phase: DownloadPhase(*downloadPhase), EstimatedCompletion: downloadETA} + if bytesTotal != nil { + d.BytesTotal = *bytesTotal + } + if bytesLeft != nil { + d.BytesLeft = *bytesLeft + } + if downloadCount != nil { + d.Downloads = *downloadCount + } + if downloadUpdatedAt != nil { + d.UpdatedAt = *downloadUpdatedAt + } + t.Download = d + } return t, nil } @@ -187,7 +214,8 @@ func (r *Repository) DeleteTarget(ctx context.Context, id int64) error { } // UpdateTargetStatus updates one target and recomputes the parent request's -// aggregate status/outcome, all in one transaction. +// aggregate status/outcome, all in one transaction. A target that completes +// or fails loses its download progress. func (r *Repository) UpdateTargetStatus(ctx context.Context, targetID int64, status Status, externalID, externalStatus, lastErr string, actor Viewer) (*Request, error) { tx, err := r.pool.Begin(ctx) @@ -210,6 +238,11 @@ func (r *Repository) UpdateTargetStatus(ctx context.Context, targetID int64, sta } return nil, fmt.Errorf("update target: %w", err) } + if status == StatusCompleted || status == StatusFailed { + if _, err := tx.Exec(ctx, `UPDATE media_request_targets SET `+clearTargetDownload+` WHERE id = $1`, targetID); err != nil { + return nil, fmt.Errorf("clear target download progress: %w", err) + } + } req, err := r.recomputeAggregate(ctx, tx, requestID, actor) if err != nil { @@ -221,6 +254,59 @@ func (r *Repository) UpdateTargetStatus(ctx context.Context, targetID int64, sta return req, nil } +// UpdateTargetDownload stores a target's download progress, stamped now as +// both heard from and asked about, or clears it when progress is nil. It +// writes only while the target is queued or downloading, so a late report +// cannot give a finished target progress again, and it touches nothing else: +// not updated_at, which dates status changes for the stalled-target backstop, +// not the request, and not its history. +func (r *Repository) UpdateTargetDownload(ctx context.Context, targetID int64, progress *DownloadProgress) error { + const live = ` WHERE id = $1 AND status IN ('queued', 'downloading')` + var err error + if progress == nil { + _, err = r.pool.Exec(ctx, `UPDATE media_request_targets SET `+clearTargetDownload+live, targetID) + } else { + _, err = r.pool.Exec(ctx, ` + UPDATE media_request_targets + SET download_phase = $2, download_bytes_total = $3, download_bytes_left = $4, + download_eta = $5, download_count = $6, download_updated_at = now(), + download_checked_at = now()`+live, + targetID, string(progress.Phase), progress.BytesTotal, progress.BytesLeft, + progress.EstimatedCompletion, progress.Downloads) + } + if err != nil { + return fmt.Errorf("update target download progress: %w", err) + } + return nil +} + +// MarkTargetDownloadChecked records that a pass asked about a target's +// download without getting its status back. It moves the target to the back +// of the download refresh rotation and leaves the progress, and when it was +// last heard from, alone. A target without progress, or no longer queued or +// downloading, is not written. +func (r *Repository) MarkTargetDownloadChecked(ctx context.Context, targetID int64) error { + if _, err := r.pool.Exec(ctx, ` + UPDATE media_request_targets SET download_checked_at = now() + WHERE id = $1 AND status IN ('queued', 'downloading') AND download_phase IS NOT NULL`, targetID); err != nil { + return fmt.Errorf("mark target download checked: %w", err) + } + return nil +} + +// UpdateTargetExternalStatus records the raw status a target's server last +// reported, while the target is queued or downloading. It leaves updated_at +// alone, since that dates the target's last status change, and writes nothing +// when the status is unchanged. +func (r *Repository) UpdateTargetExternalStatus(ctx context.Context, targetID int64, externalStatus string) error { + if _, err := r.pool.Exec(ctx, ` + UPDATE media_request_targets SET external_status = $2 + WHERE id = $1 AND status IN ('queued', 'downloading') AND external_status <> $2`, targetID, externalStatus); err != nil { + return fmt.Errorf("update target external status: %w", err) + } + return nil +} + func (r *Repository) recomputeAggregate(ctx context.Context, exec requestExecutor, requestID string, actor Viewer) (*Request, error) { // The request's history records what changed, once: a request with two // targets moving to queued is one event, not two. diff --git a/internal/requests/types.go b/internal/requests/types.go index bb1b4fd2e9..7781f3da60 100644 --- a/internal/requests/types.go +++ b/internal/requests/types.go @@ -60,6 +60,95 @@ type Target struct { // only); empty when the plugin routed it. RouteID string `json:"-"` RouteName string `json:"-"` + // Download is how far the target's downloads are, while its plugin reports + // any (v2 only). + Download *DownloadProgress `json:"-"` +} + +// DownloadPhase is where a target's downloads are, as its router plugin +// reports them. The set is open: clients read an unknown phase as downloading. +type DownloadPhase string + +const ( + DownloadPhaseQueued DownloadPhase = "queued" + DownloadPhaseDownloading DownloadPhase = "downloading" + DownloadPhasePaused DownloadPhase = "paused" + DownloadPhaseStalled DownloadPhase = "stalled" + DownloadPhaseImporting DownloadPhase = "importing" + DownloadPhaseImportBlocked DownloadPhase = "import_blocked" +) + +// downloadPhaseRank orders phases for aggregation, highest first: +// import_blocked > stalled > downloading > importing > paused > queued. A +// download that needs attention outranks the rest; otherwise downloading wins +// while anything still downloads. An unknown phase ranks as downloading. +func downloadPhaseRank(phase DownloadPhase) int { + switch phase { + case DownloadPhaseImportBlocked: + return 5 + case DownloadPhaseStalled: + return 4 + case DownloadPhaseImporting: + return 2 + case DownloadPhasePaused: + return 1 + case DownloadPhaseQueued: + return 0 + default: + return 3 + } +} + +// DownloadProgress is how far a target's downloads are, as the downstream +// service last reported them. BytesTotal is 0 while the size is unknown. +// UpdatedAt is when the server last heard from the plugin. +type DownloadProgress struct { + Phase DownloadPhase + BytesTotal int64 + BytesLeft int64 + EstimatedCompletion *time.Time + Downloads int + UpdatedAt time.Time +} + +// Download aggregates the progress of the request's live (queued or +// downloading) targets, 1080p and 4K together: bytes and downloads are +// summed, the phase is the highest ranked, the estimate is the latest, and +// UpdatedAt is the oldest report, so the figure is only as fresh as its +// stalest part. The total is unknown (0) when any target's is. It is nil when +// no live target reports progress. +func (r *Request) Download() *DownloadProgress { + var out *DownloadProgress + sizeKnown := true + for _, t := range r.Targets { + if t.Download == nil || (t.Status != StatusQueued && t.Status != StatusDownloading) { + continue + } + d := *t.Download + if d.BytesTotal <= 0 { + sizeKnown = false + } + if out == nil { + out = &d + continue + } + if downloadPhaseRank(d.Phase) > downloadPhaseRank(out.Phase) { + out.Phase = d.Phase + } + out.BytesTotal += d.BytesTotal + out.BytesLeft += d.BytesLeft + out.Downloads += d.Downloads + if d.EstimatedCompletion != nil && (out.EstimatedCompletion == nil || d.EstimatedCompletion.After(*out.EstimatedCompletion)) { + out.EstimatedCompletion = d.EstimatedCompletion + } + if d.UpdatedAt.Before(out.UpdatedAt) { + out.UpdatedAt = d.UpdatedAt + } + } + if out != nil && !sizeKnown { + out.BytesTotal, out.BytesLeft = 0, 0 + } + return out } type Availability string @@ -316,6 +405,9 @@ type RequestState struct { RequestedByViewer bool `json:"-"` // State is the active request's user-facing state (v2 only). State State `json:"-"` + // Download is the active request's download progress. Only the title + // detail fills it; search and discovery do not load targets (v2 only). + Download *DownloadProgress `json:"-"` } type MediaResult struct { @@ -499,3 +591,12 @@ type ReconcileResult struct { Deferred int `json:"deferred"` Errors int `json:"errors"` } + +// DownloadRefreshResult counts one download refresh pass. Checked counts the +// requests whose targets were asked about, Updated those where a target's +// status moved. +type DownloadRefreshResult struct { + Checked int `json:"checked"` + Updated int `json:"updated"` + Errors int `json:"errors"` +} diff --git a/internal/taskmanager/tasks/reconcile_requests.go b/internal/taskmanager/tasks/reconcile_requests.go index d239b6f195..8deb67dc98 100644 --- a/internal/taskmanager/tasks/reconcile_requests.go +++ b/internal/taskmanager/tasks/reconcile_requests.go @@ -4,15 +4,36 @@ import ( "context" "encoding/json" "fmt" + "time" "github.com/Silo-Server/silo-server/internal/requests" "github.com/Silo-Server/silo-server/internal/taskmanager" "github.com/jackc/pgx/v5/pgxpool" ) -// requestReconcileAdvisoryLock spells "SILORQRC". +// requestReconcileAdvisoryLock spells "SILORQRC". It lets one server run each +// reconcile pass. const requestReconcileAdvisoryLock int64 = 0x53494C4F52515243 +// requestTargetWriteAdvisoryLock spells "SILORQTW". It keeps the reconcile pass +// and the download refresh pass from writing request targets at the same time: +// reconcile waits for it, and the refresh skips while it is held. +const requestTargetWriteAdvisoryLock int64 = 0x53494C4F52515457 + +// requestTargetWriteWait bounds how long a reconcile pass waits for a download +// refresh pass to finish. A refresh pass asks about at most 200 requests and +// normally ends well inside a minute; requestDownloadRefreshBudget cuts it off +// before this wait runs out, even while a download server stops answering. +const requestTargetWriteWait = 2 * time.Minute + +// requestDownloadRefreshBudget bounds a download refresh pass. A plugin call +// may take the router's full 60-second deadline, and one server that stops +// answering can hang every call on it, so without a bound a pass could hold +// the target write lock past requestTargetWriteWait and fail the reconcile +// pass waiting for it. The margin covers the writes that apply the last +// answer after the budget cuts the call in flight. +const requestDownloadRefreshBudget = 90 * time.Second + type RequestReconciler interface { ReconcileRequests(ctx context.Context, limit int) (requests.ReconcileResult, error) } @@ -21,11 +42,13 @@ type RequestReconciler interface { // process runs the task manager, so an advisory lock lets one server run each // pass; the others skip. The per-request submission claim already prevents a // double submission, so the lock only saves the duplicate router status calls -// and presence lookups. +// and presence lookups. The pass then waits for the request target write lock, +// so it runs after a download refresh pass in progress instead of skipping. type ReconcileRequestsTask struct { reconciler RequestReconciler limit int lock clusterLock + writeLock waitingClusterLock } // NewReconcileRequestsTask constructs the task. A nil pool runs without the @@ -36,7 +59,8 @@ func NewReconcileRequestsTask(reconciler RequestReconciler, limit int, pool *pgx } t := &ReconcileRequestsTask{reconciler: reconciler, limit: limit} if pool != nil { - t.lock = advisoryClusterLock{pool: pool, key: requestReconcileAdvisoryLock} + t.lock = advisoryClusterLock{pool: pool, key: requestReconcileAdvisoryLock, name: "request reconcile"} + t.writeLock = advisoryClusterLock{pool: pool, key: requestTargetWriteAdvisoryLock, name: "request target writes", wait: requestTargetWriteWait} } return t } @@ -74,6 +98,16 @@ func (t *ReconcileRequestsTask) Execute(ctx context.Context, progress taskmanage } defer release() } + if t.writeLock != nil { + release, acquired, err := t.writeLock.Acquire(ctx) + if err != nil { + return fmt.Errorf("acquiring request target write lock: %w", err) + } + if !acquired { + return fmt.Errorf("request download refresh held the request target write lock for over %s", requestTargetWriteWait) + } + defer release() + } result, err := t.reconciler.ReconcileRequests(ctx, t.limit) if err != nil { return fmt.Errorf("reconcile media requests: %w", err) diff --git a/internal/taskmanager/tasks/refresh_all_library_metadata.go b/internal/taskmanager/tasks/refresh_all_library_metadata.go index 11d12d3672..01116a593b 100644 --- a/internal/taskmanager/tasks/refresh_all_library_metadata.go +++ b/internal/taskmanager/tasks/refresh_all_library_metadata.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "time" "github.com/jackc/pgx/v5/pgxpool" @@ -33,6 +34,12 @@ type clusterLock interface { TryAcquire(ctx context.Context) (release func(), acquired bool, err error) } +// waitingClusterLock takes a lock shared by every server, waiting a bounded +// time for the server holding it. acquired is false when the wait runs out. +type waitingClusterLock interface { + Acquire(ctx context.Context) (release func(), acquired bool, err error) +} + // fullMetadataRefreshAdvisoryLock spells "SILOFULR". const fullMetadataRefreshAdvisoryLock int64 = 0x53494C4F46554C52 @@ -41,6 +48,8 @@ type advisoryClusterLock struct { key int64 // name labels the guarded work in the release-failure log. name string + // wait bounds Acquire's wait; TryAcquire never waits. + wait time.Duration } func (l advisoryClusterLock) TryAcquire(ctx context.Context) (func(), bool, error) { @@ -48,11 +57,23 @@ func (l advisoryClusterLock) TryAcquire(ctx context.Context) (func(), bool, erro if err != nil || !acquired { return nil, false, err } + return l.releaser(ctx, lock), true, nil +} + +func (l advisoryClusterLock) Acquire(ctx context.Context) (func(), bool, error) { + lock, acquired, err := pglock.Acquire(ctx, l.pool, l.key, l.wait) + if err != nil || !acquired { + return nil, false, err + } + return l.releaser(ctx, lock), true, nil +} + +func (l advisoryClusterLock) releaser(ctx context.Context, lock *pglock.Lock) func() { return func() { if err := lock.Release(ctx); err != nil { slog.WarnContext(ctx, "releasing cluster task advisory lock failed", "component", "taskmanager", "work", l.name, "error", err) } - }, true, nil + } } // RefreshAllLibraryMetadataTask re-fetches provider metadata for every item in diff --git a/internal/taskmanager/tasks/refresh_request_downloads.go b/internal/taskmanager/tasks/refresh_request_downloads.go new file mode 100644 index 0000000000..87a115e1c7 --- /dev/null +++ b/internal/taskmanager/tasks/refresh_request_downloads.go @@ -0,0 +1,97 @@ +package tasks + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/Silo-Server/silo-server/internal/requests" + "github.com/Silo-Server/silo-server/internal/taskmanager" + "github.com/jackc/pgx/v5/pgxpool" +) + +type RequestDownloadRefresher interface { + RefreshDownloads(ctx context.Context, limit int, budget time.Duration) (requests.DownloadRefreshResult, error) + HasDownloadsToRefresh(ctx context.Context) (bool, error) +} + +// RefreshRequestDownloadsTask refreshes the status and download progress of +// downloading request targets whose router plugin reports progress, between +// reconcile passes. It tries the request target write lock without waiting, +// so one server runs each pass, and it skips while a reconcile pass holds the +// lock anywhere in the cluster: the two never write the same target at once, +// and a reconcile pass waits for a refresh pass instead of skipping. Each pass +// runs within requestDownloadRefreshBudget, so that wait always ends. +type RefreshRequestDownloadsTask struct { + refresher RequestDownloadRefresher + limit int + lock clusterLock +} + +// NewRefreshRequestDownloadsTask constructs the task. A nil pool runs without +// the cluster lock. +func NewRefreshRequestDownloadsTask(refresher RequestDownloadRefresher, limit int, pool *pgxpool.Pool) *RefreshRequestDownloadsTask { + if limit <= 0 { + limit = 200 + } + t := &RefreshRequestDownloadsTask{refresher: refresher, limit: limit} + if pool != nil { + t.lock = advisoryClusterLock{pool: pool, key: requestTargetWriteAdvisoryLock, name: "request download refresh"} + } + return t +} + +func (t *RefreshRequestDownloadsTask) Key() string { return "refresh_request_downloads" } +func (t *RefreshRequestDownloadsTask) Name() string { return "Refresh Request Downloads" } +func (t *RefreshRequestDownloadsTask) Description() string { + return "Refreshes the download progress of media requests that Radarr, Sonarr, or another request plugin is downloading" +} +func (t *RefreshRequestDownloadsTask) Category() taskmanager.TaskCategory { + return taskmanager.TaskCategoryLibrary +} +func (t *RefreshRequestDownloadsTask) IsHidden() bool { return true } + +func (t *RefreshRequestDownloadsTask) DefaultTriggers() []taskmanager.TriggerConfig { + return []taskmanager.TriggerConfig{ + {Type: taskmanager.TriggerTypeInterval, IntervalMs: 60 * 1000}, + } +} + +// ShouldRun skips a scheduled run while no downloading target has progress to +// refresh, so an idle server records no run every minute. The reconcile pass +// finds a download's first progress. +func (t *RefreshRequestDownloadsTask) ShouldRun(ctx context.Context) (bool, error) { + if t == nil || t.refresher == nil { + return false, nil + } + return t.refresher.HasDownloadsToRefresh(ctx) +} + +func (t *RefreshRequestDownloadsTask) Execute(ctx context.Context, progress taskmanager.ProgressReporter) error { + progress.Report(0, "Refreshing request downloads") + if t.refresher == nil { + progress.Report(100, "Request download refresh unavailable") + return nil + } + if t.lock != nil { + release, acquired, err := t.lock.TryAcquire(ctx) + if err != nil { + return fmt.Errorf("acquiring request target write lock: %w", err) + } + if !acquired { + progress.Report(100, "A request reconcile or download refresh pass is already running") + return nil + } + defer release() + } + result, err := t.refresher.RefreshDownloads(ctx, t.limit, requestDownloadRefreshBudget) + if err != nil { + return fmt.Errorf("refresh request downloads: %w", err) + } + if data, err := json.Marshal(result); err == nil { + progress.SetResultData(data) + } + progress.Report(100, "Request download refresh complete") + return nil +} diff --git a/internal/taskmanager/tasks/refresh_request_downloads_test.go b/internal/taskmanager/tasks/refresh_request_downloads_test.go new file mode 100644 index 0000000000..cc0996e513 --- /dev/null +++ b/internal/taskmanager/tasks/refresh_request_downloads_test.go @@ -0,0 +1,339 @@ +package tasks + +import ( + "context" + "encoding/json" + "errors" + "os" + "sync/atomic" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/Silo-Server/silo-server/internal/database/pglock" + "github.com/Silo-Server/silo-server/internal/requests" +) + +type downloadRefresherStub struct { + result requests.DownloadRefreshResult + err error + runs atomic.Int32 + limit int + budget time.Duration + + hasWork bool + hasWorkErr error +} + +func (s *downloadRefresherStub) RefreshDownloads(_ context.Context, limit int, budget time.Duration) (requests.DownloadRefreshResult, error) { + s.runs.Add(1) + s.limit, s.budget = limit, budget + return s.result, s.err +} + +func (s *downloadRefresherStub) HasDownloadsToRefresh(context.Context) (bool, error) { + return s.hasWork, s.hasWorkErr +} + +// reconcilerStub counts reconcile passes. With proceed set, a pass reports +// on entered and then blocks until proceed closes. +type reconcilerStub struct { + runs atomic.Int32 + entered chan struct{} + proceed chan struct{} +} + +func (s *reconcilerStub) ReconcileRequests(ctx context.Context, _ int) (requests.ReconcileResult, error) { + s.runs.Add(1) + if s.proceed != nil { + s.entered <- struct{}{} + select { + case <-s.proceed: + case <-ctx.Done(): + return requests.ReconcileResult{}, ctx.Err() + } + } + return requests.ReconcileResult{}, nil +} + +// fakeWaitingLock stands in for a lock taken with a bounded wait. +type fakeWaitingLock struct { + acquired bool + err error + waits int + released int +} + +func (f *fakeWaitingLock) Acquire(context.Context) (func(), bool, error) { + f.waits++ + if f.err != nil || !f.acquired { + return nil, false, f.err + } + return func() { f.released++ }, true, nil +} + +func TestRefreshRequestDownloadsTaskRunsUnderTheLock(t *testing.T) { + refresher := &downloadRefresherStub{result: requests.DownloadRefreshResult{Checked: 3, Updated: 1}} + lock := &fakeClusterLock{acquired: true} + task := NewRefreshRequestDownloadsTask(refresher, 200, nil) + task.lock = lock + progress := &bulkEnrichmentTaskProgress{} + + if err := task.Execute(context.Background(), progress); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if refresher.runs.Load() != 1 || refresher.limit != 200 || lock.released != 1 { + t.Fatalf("runs = %d, limit = %d, lock releases = %d; want 1, 200 and 1", refresher.runs.Load(), refresher.limit, lock.released) + } + if refresher.budget != requestDownloadRefreshBudget { + t.Fatalf("budget = %s, want %s", refresher.budget, requestDownloadRefreshBudget) + } + var result requests.DownloadRefreshResult + if err := json.Unmarshal(progress.result, &result); err != nil || result.Checked != 3 || result.Updated != 1 { + t.Fatalf("result data %s: %v", progress.result, err) + } +} + +func TestRefreshRequestDownloadsTaskSkipsWhileTheWriteLockIsHeld(t *testing.T) { + refresher := &downloadRefresherStub{} + task := NewRefreshRequestDownloadsTask(refresher, 200, nil) + task.lock = &fakeClusterLock{acquired: false} + + if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if refresher.runs.Load() != 0 { + t.Fatalf("runs = %d, want 0 while the lock is held", refresher.runs.Load()) + } +} + +func TestRefreshRequestDownloadsTaskReportsFailure(t *testing.T) { + task := NewRefreshRequestDownloadsTask(&downloadRefresherStub{err: errors.New("database unavailable")}, 0, nil) + if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err == nil { + t.Fatal("Execute() error = nil, want the pass's error") + } +} + +// Scheduled runs happen only while some downloading target has progress, so +// an idle server records no run every minute. +func TestRefreshRequestDownloadsTaskShouldRun(t *testing.T) { + ctx := context.Background() + if run, err := NewRefreshRequestDownloadsTask(&downloadRefresherStub{}, 0, nil).ShouldRun(ctx); err != nil || run { + t.Fatalf("ShouldRun() without work = %v, %v; want false", run, err) + } + if run, err := NewRefreshRequestDownloadsTask(&downloadRefresherStub{hasWork: true}, 0, nil).ShouldRun(ctx); err != nil || !run { + t.Fatalf("ShouldRun() with work = %v, %v; want true", run, err) + } + failure := errors.New("database unavailable") + if _, err := NewRefreshRequestDownloadsTask(&downloadRefresherStub{hasWorkErr: failure}, 0, nil).ShouldRun(ctx); !errors.Is(err, failure) { + t.Fatalf("ShouldRun() err = %v, want the check's", err) + } + if run, err := NewRefreshRequestDownloadsTask(nil, 0, nil).ShouldRun(ctx); err != nil || run { + t.Fatalf("ShouldRun() without a refresher = %v, %v; want false", run, err) + } +} + +// Reconcile keeps its own lock as the one-pass-at-a-time guard and waits for +// the target write lock; the refresh tries only the write lock. The pool never +// connects here. +func TestRequestTaskLockKeys(t *testing.T) { + pool, err := pgxpool.New(context.Background(), "postgres://silo@127.0.0.1:1/silo?sslmode=disable") + if err != nil { + t.Fatal(err) + } + t.Cleanup(pool.Close) + refresh, _ := NewRefreshRequestDownloadsTask(&downloadRefresherStub{}, 0, pool).lock.(advisoryClusterLock) + reconcileTask := NewReconcileRequestsTask(nil, 0, pool) + reconcile, _ := reconcileTask.lock.(advisoryClusterLock) + writes, _ := reconcileTask.writeLock.(advisoryClusterLock) + if refresh.key != requestTargetWriteAdvisoryLock { + t.Fatalf("refresh lock key = %#x, want the target write lock %#x", refresh.key, requestTargetWriteAdvisoryLock) + } + if reconcile.key != requestReconcileAdvisoryLock { + t.Fatalf("reconcile lock key = %#x, want %#x", reconcile.key, requestReconcileAdvisoryLock) + } + if writes.key != requestTargetWriteAdvisoryLock || writes.wait != requestTargetWriteWait { + t.Fatalf("reconcile write lock = %#x waiting %s, want %#x waiting %s", writes.key, writes.wait, requestTargetWriteAdvisoryLock, requestTargetWriteWait) + } +} + +// A refresh pass must end inside the reconcile pass's wait for the write lock +// even when a download server stops answering, or a reconcile tick that lands +// during it fails. The budget cuts the call in flight, so the pass ends at the +// budget plus the writes that apply the last answer; the margin covers those. +func TestRequestDownloadRefreshBudgetEndsInsideTheReconcileWait(t *testing.T) { + const writeMargin = 30 * time.Second + if requestDownloadRefreshBudget <= 0 || requestDownloadRefreshBudget+writeMargin > requestTargetWriteWait { + t.Fatalf("refresh budget %s leaves under %s of the reconcile's %s wait", requestDownloadRefreshBudget, writeMargin, requestTargetWriteWait) + } +} + +func TestReconcileRequestsTaskWaitsForTheWriteLock(t *testing.T) { + reconciler := &reconcilerStub{} + task := NewReconcileRequestsTask(reconciler, 0, nil) + lock := &fakeClusterLock{acquired: true} + writes := &fakeWaitingLock{acquired: true} + task.lock, task.writeLock = lock, writes + + if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if reconciler.runs.Load() != 1 || writes.waits != 1 || writes.released != 1 || lock.released != 1 { + t.Fatalf("runs = %d, write lock waits = %d and releases = %d, reconcile lock releases = %d; want 1 each", + reconciler.runs.Load(), writes.waits, writes.released, lock.released) + } +} + +func TestReconcileRequestsTaskSkipsWhileAnotherReconcileRuns(t *testing.T) { + reconciler := &reconcilerStub{} + task := NewReconcileRequestsTask(reconciler, 0, nil) + writes := &fakeWaitingLock{acquired: true} + task.lock, task.writeLock = &fakeClusterLock{acquired: false}, writes + + if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if reconciler.runs.Load() != 0 || writes.waits != 0 { + t.Fatalf("runs = %d, write lock waits = %d; want neither while another reconcile runs", reconciler.runs.Load(), writes.waits) + } +} + +// A refresh that holds the write lock past the wait fails the pass loudly +// instead of skipping it quietly. +func TestReconcileRequestsTaskFailsWhenTheWaitRunsOut(t *testing.T) { + reconciler := &reconcilerStub{} + task := NewReconcileRequestsTask(reconciler, 0, nil) + lock := &fakeClusterLock{acquired: true} + task.lock, task.writeLock = lock, &fakeWaitingLock{acquired: false} + + if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err == nil { + t.Fatal("Execute() error = nil, want the wait to fail the pass") + } + if reconciler.runs.Load() != 0 || lock.released != 1 { + t.Fatalf("runs = %d, reconcile lock releases = %d; want 0 and 1", reconciler.runs.Load(), lock.released) + } +} + +func requestLockTestPool(t *testing.T) *pgxpool.Pool { + t.Helper() + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + pool, err := pgxpool.New(t.Context(), dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(pool.Close) + return pool +} + +// waitForLockWaiter returns once a session is queued for advisory lock key, +// failing the test if done fires first. +func waitForLockWaiter(t *testing.T, pool *pgxpool.Pool, key int64, done <-chan error) { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + for { + var waiting bool + if err := pool.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM pg_locks + WHERE locktype = 'advisory' AND NOT granted + AND ((classid::bigint << 32) | objid::bigint) = $1 + )`, key).Scan(&waiting); err != nil { + t.Fatalf("inspect pg_locks: %v", err) + } + if waiting { + return + } + select { + case err := <-done: + t.Fatalf("Execute() returned %v before queueing for the lock", err) + case <-ctx.Done(): + t.Fatal("nothing queued for the lock") + case <-time.After(10 * time.Millisecond): + } + } +} + +// A reconcile tick that lands while a download refresh holds the target write +// lock waits for it and then runs. +func TestReconcileWaitsForADownloadRefreshPostgres(t *testing.T) { + pool := requestLockTestPool(t) + reconciler := &reconcilerStub{} + task := NewReconcileRequestsTask(reconciler, 0, pool) + + refresh, acquired, err := pglock.TryAcquire(t.Context(), pool, requestTargetWriteAdvisoryLock) + if err != nil || !acquired { + t.Fatalf("take the target write lock: acquired = %v, err = %v", acquired, err) + } + done := make(chan error, 1) + go func() { done <- task.Execute(t.Context(), &bulkEnrichmentTaskProgress{}) }() + waitForLockWaiter(t, pool, requestTargetWriteAdvisoryLock, done) + if runs := reconciler.runs.Load(); runs != 0 { + t.Fatalf("runs = %d while the refresh held the write lock, want 0", runs) + } + if err := refresh.Release(t.Context()); err != nil { + t.Fatal(err) + } + if err := <-done; err != nil { + t.Fatalf("Execute() after the refresh released the lock: %v", err) + } + if runs := reconciler.runs.Load(); runs != 1 { + t.Fatalf("runs = %d, want 1 once the refresh finished", runs) + } +} + +// Reconcile stays one pass at a time across the cluster. +func TestReconcileSkipsWhileAnotherReconcileRunsPostgres(t *testing.T) { + pool := requestLockTestPool(t) + reconciler := &reconcilerStub{} + task := NewReconcileRequestsTask(reconciler, 0, pool) + + other, acquired, err := pglock.TryAcquire(t.Context(), pool, requestReconcileAdvisoryLock) + if err != nil || !acquired { + t.Fatalf("take the reconcile lock: acquired = %v, err = %v", acquired, err) + } + defer func() { _ = other.Release(t.Context()) }() + if err := task.Execute(t.Context(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() while another reconcile runs: %v", err) + } + if runs := reconciler.runs.Load(); runs != 0 { + t.Fatalf("runs = %d while another reconcile ran, want 0", runs) + } +} + +// A refresh tick skips while a reconcile pass runs, and runs once it ends. +func TestRefreshRequestDownloadsSkipsDuringReconcilePostgres(t *testing.T) { + pool := requestLockTestPool(t) + reconciler := &reconcilerStub{entered: make(chan struct{}, 1), proceed: make(chan struct{})} + reconcile := NewReconcileRequestsTask(reconciler, 0, pool) + refresher := &downloadRefresherStub{} + refresh := NewRefreshRequestDownloadsTask(refresher, 200, pool) + + done := make(chan error, 1) + go func() { done <- reconcile.Execute(t.Context(), &bulkEnrichmentTaskProgress{}) }() + select { + case <-reconciler.entered: + case err := <-done: + t.Fatalf("reconcile returned %v before its pass started", err) + } + if err := refresh.Execute(t.Context(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() while reconcile runs: %v", err) + } + if runs := refresher.runs.Load(); runs != 0 { + t.Fatalf("refresh runs = %d while reconcile ran, want 0", runs) + } + close(reconciler.proceed) + if err := <-done; err != nil { + t.Fatalf("reconcile: %v", err) + } + if err := refresh.Execute(t.Context(), &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() after reconcile: %v", err) + } + if runs := refresher.runs.Load(); runs != 1 { + t.Fatalf("refresh runs = %d after reconcile ended, want 1", runs) + } +} diff --git a/internal/workmetrics/work.go b/internal/workmetrics/work.go index d7b62ce3d9..5dcaad0c65 100644 --- a/internal/workmetrics/work.go +++ b/internal/workmetrics/work.go @@ -70,7 +70,7 @@ func Category(key string) string { return "recommendations" case "seed_content_availability", "rebuild_release_interest": return workloadNotifications - case "reconcile_requests", "reconcile_watch_history", "database_maintenance", "cleanup_client_diagnostics", "cleanup_operational_log", "cleanup_orphaned_media_items", "setting_mutations_retention", "backfill_media_item_aliases", "repair_provider_id_integrity", "sync_collections", "sync_user_collections", "check_plugin_updates": + case "reconcile_requests", "refresh_request_downloads", "reconcile_watch_history", "database_maintenance", "cleanup_client_diagnostics", "cleanup_operational_log", "cleanup_orphaned_media_items", "setting_mutations_retention", "backfill_media_item_aliases", "repair_provider_id_integrity", "sync_collections", "sync_user_collections", "check_plugin_updates": return "maintenance" default: if strings.HasPrefix(key, "plugin:") || strings.HasPrefix(key, "plugin_") { diff --git a/migrations/sql/20260928143912_request_target_download_progress.sql b/migrations/sql/20260928143912_request_target_download_progress.sql new file mode 100644 index 0000000000..35a4f34c6a --- /dev/null +++ b/migrations/sql/20260928143912_request_target_download_progress.sql @@ -0,0 +1,29 @@ +-- +goose Up +-- How far a request target's downloads are, as its router plugin last reported +-- them (request_router.reports_download_progress). Set only while the target is +-- queued or downloading, and cleared when it completes or fails. +-- download_updated_at is when the server last heard from the plugin, and +-- download_checked_at when a pass last asked about the target, answered or +-- not; the download refresh pass takes targets in that order. A progress write +-- leaves updated_at alone: that column dates the target's last status change, +-- and the stalled-target backstop reads it. +ALTER TABLE media_request_targets + ADD COLUMN download_phase text, + ADD COLUMN download_bytes_total bigint + CONSTRAINT media_request_targets_download_bytes_total_check CHECK (download_bytes_total >= 0), + ADD COLUMN download_bytes_left bigint + CONSTRAINT media_request_targets_download_bytes_left_check CHECK (download_bytes_left >= 0), + ADD COLUMN download_eta timestamptz, + ADD COLUMN download_count integer, + ADD COLUMN download_updated_at timestamptz, + ADD COLUMN download_checked_at timestamptz; + +-- +goose Down +ALTER TABLE media_request_targets + DROP COLUMN download_checked_at, + DROP COLUMN download_updated_at, + DROP COLUMN download_count, + DROP COLUMN download_eta, + DROP COLUMN download_bytes_left, + DROP COLUMN download_bytes_total, + DROP COLUMN download_phase; diff --git a/web/src/api/types.ts b/web/src/api/types.ts index f06e6a58c3..a6c777d6c8 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -1848,6 +1848,30 @@ export interface RequestState { requested_by_viewer?: boolean; /** User-facing state of the active request. */ state?: RequestUserState; + /** How far the active request's downloads are. Only the title detail carries it. */ + download?: RequestDownload; +} + +/** + * How far a request's downloads are, while its download server reports them: + * for one server on a target, summed over its servers on a request. + */ +export interface RequestDownload { + /** + * queued, downloading, paused, stalled, importing or import_blocked. The + * server may add phases; read one this client does not know as downloading. + */ + phase: string; + /** Rounded down; absent while the size is unknown. */ + percent?: number; + bytes_total?: number; + bytes_left?: number; + /** Absent when the download server cannot tell. */ + estimated_completion_at?: string; + /** Distinct downloads in flight; a season pack counts once. */ + downloads: number; + /** When the server last heard from the download server. */ + updated_at: string; } export interface RequestMediaResult { @@ -2019,6 +2043,7 @@ export interface RequestTarget { external_status?: string; status: MediaRequestStatus | "failed"; last_error?: string; + download?: RequestDownload; created_at: string; updated_at: string; } @@ -2049,6 +2074,8 @@ export interface MediaRequest { /** Series season requests: each requested season's episodes, once the series is in the library. */ season_progress?: RequestSeasonProgress[]; targets?: RequestTarget[]; + /** Over every server of the request: the phase that needs the most attention, the latest estimate. */ + download?: RequestDownload; integration_kind?: string; external_id?: string; external_status?: string; diff --git a/web/src/api/v2/requests.test.ts b/web/src/api/v2/requests.test.ts new file mode 100644 index 0000000000..fc5ccd25cb --- /dev/null +++ b/web/src/api/v2/requests.test.ts @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import listMyRequestsOk from "../../../../contracts/api/v2/fixtures/list_my_requests_ok.json"; +import { v2 } from "./request"; +import { getRequestMediaDetailV2, listMyMediaRequestsV2 } from "./requests"; +import type { components } from "./schema"; + +vi.mock("./request", async (importOriginal) => ({ + ...(await importOriginal()), + v2: vi.fn(), +})); + +type Schemas = components["schemas"]; + +afterEach(() => vi.resetAllMocks()); + +// The adapters rebuild each body, so a field they leave out never reaches a +// page. Download progress must survive every one of them. +describe("request v2 adapters keep download progress", () => { + const download: Schemas["RequestDownload"] = { + phase: "downloading", + percent: 43, + bytes_total: 4294967296, + bytes_left: 2448131358, + estimated_completion_at: "2026-01-02T03:16:05.678Z", + downloads: 1, + updated_at: "2026-01-02T03:04:05.678Z", + }; + + it("on a request and on each of its servers", async () => { + vi.mocked(v2).mockResolvedValue(listMyRequestsOk as never); + + const [waiting, downloading] = await listMyMediaRequestsV2(); + + expect(downloading!.download).toEqual(download); + expect(downloading!.targets![0]!.download).toEqual(download); + expect(waiting!.download).toBeUndefined(); + expect(waiting!.targets![0]).not.toHaveProperty("download"); + }); + + it("on the title detail's request state", async () => { + const detail: Schemas["RequestMediaDetail"] = { + media_type: "movie", + tmdb_id: 949, + title: "Heat", + availability: "missing", + cast: [], + creators: [], + genres: [], + networks: [], + production_companies: [], + recommendations: [], + seasons: [], + request: { + requestable: false, + following: true, + requested_by_viewer: true, + status: "downloading", + state: "processing", + request_id: "r-1", + download, + }, + }; + vi.mocked(v2).mockResolvedValue(detail as never); + + const mapped = await getRequestMediaDetailV2("movie", 949); + + expect(mapped.request.download).toEqual(download); + }); +}); diff --git a/web/src/api/v2/schema.ts b/web/src/api/v2/schema.ts index ab01af006e..5cc6ebb63e 100644 --- a/web/src/api/v2/schema.ts +++ b/web/src/api/v2/schema.ts @@ -19547,6 +19547,8 @@ export interface components { FeatureStatus: { /** @description Whether the current principal may use the capability */ allowed: boolean; + /** @description Whether the server reports download progress (download on requests, their targets, and the title detail's request state). Whether a given request has any depends on its download server's request plugin. */ + download_progress_supported: boolean; follow_supported: boolean; /** @description Whether a series already in the library can be requested for the seasons it is missing. False while a download server that takes series uses a request plugin that cannot fetch individual seasons, so such a series stays already_available. */ missing_seasons_requestable: boolean; @@ -21196,6 +21198,8 @@ export interface components { * @example 2026-01-02T03:04:05.000Z */ created_at: string; + /** @description How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time */ + download?: components["schemas"]["RequestDownload"]; external_id?: string; external_status?: string; /** @@ -24378,6 +24382,49 @@ export interface components { RequestCancelInputBody: { reason?: string; }; + RequestDownload: { + /** + * Format: int64 + * @description Bytes still to download; present whenever bytes_total is + * @example 2448131358 + */ + bytes_left?: number; + /** + * Format: int64 + * @description Size of the downloads in bytes; absent while unknown + * @example 4294967296 + */ + bytes_total?: number; + /** + * Format: int64 + * @description Distinct downloads in flight; a season pack counts once + * @example 1 + */ + downloads: number; + /** + * Format: date-time + * @description When the download server expects the downloads to finish; absent when it cannot tell + * @example 2026-01-02T03:16:05.000Z + */ + estimated_completion_at?: string; + /** + * Format: int64 + * @description How much has downloaded, rounded down; absent while the size is unknown + * @example 43 + */ + percent?: number; + /** + * @description queued, downloading, paused, stalled, importing or import_blocked. More values may be added: read an unknown one as downloading, without a percentage + * @example downloading + */ + phase: string; + /** + * Format: date-time + * @description When the server last heard from the download server. A client may hide figures older than about ten minutes + * @example 2026-01-02T03:04:05.000Z + */ + updated_at: string; + }; RequestMediaCastMember: { /** @example Vincent Hanna */ character?: string; @@ -24574,6 +24621,8 @@ export interface components { season_number: number; }; RequestMediaState: { + /** @description How far the active request's downloads are, while its download server reports them. Only the title detail (getRequestMediaDetail) carries it */ + download?: components["schemas"]["RequestDownload"]; /** * @description Whether the viewer will be notified when the media becomes available: they requested it or follow it * @example false @@ -24636,6 +24685,8 @@ export interface components { * @example 2026-01-02T03:04:05.000Z */ created_at: string; + /** @description How far this target's downloads are, while its download server reports them */ + download?: components["schemas"]["RequestDownload"]; /** @description The integration's own identifier */ external_id?: string; external_status?: string; diff --git a/web/src/components/RequestDownloadProgress.test.tsx b/web/src/components/RequestDownloadProgress.test.tsx new file mode 100644 index 0000000000..656ae64dc1 --- /dev/null +++ b/web/src/components/RequestDownloadProgress.test.tsx @@ -0,0 +1,86 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, render, screen } from "@testing-library/react"; +import type { RequestDownload } from "@/api/types"; +import { RequestDownloadProgress } from "./RequestDownloadProgress"; + +const download = (overrides: Partial = {}): RequestDownload => ({ + phase: "downloading", + percent: 43, + bytes_total: 4294967296, + bytes_left: 2448131358, + estimated_completion_at: "2026-01-02T03:16:05Z", + downloads: 1, + updated_at: "2026-01-02T03:03:05Z", + ...overrides, +}); + +describe("RequestDownloadProgress", () => { + beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2026-01-02T03:04:05Z")); + }); + afterEach(() => vi.useRealTimers()); + + it("draws a bar at the percentage over the label", () => { + render(); + + expect(screen.getByRole("progressbar", { name: "Download progress" })).toHaveAttribute( + "aria-valuenow", + "43", + ); + expect(screen.getByText("Downloading · 43% · about 12 min left")).toBeInTheDocument(); + }); + + it("shows only the label while the size is unknown, or the phase is new", () => { + const { rerender } = render( + , + ); + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + expect(screen.getByText("Waiting to download")).toBeInTheDocument(); + + rerender(); + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + expect(screen.getByText("Downloading")).toBeInTheDocument(); + }); + + // Polls that return the same figures do not render the card again, so the + // estimate keeps time on its own: it counts down, and it goes once the + // figures are more than ten minutes old. + it("keeps the estimate true while the figures stay the same", () => { + vi.useRealTimers(); + vi.useFakeTimers({ toFake: ["Date", "setInterval", "clearInterval"] }); + vi.setSystemTime(new Date("2026-01-02T03:04:05Z")); + render(); + expect(screen.getByText("Downloading · 43% · about 12 min left")).toBeInTheDocument(); + + act(() => vi.advanceTimersByTime(5 * 60_000)); + expect(screen.getByText("Downloading · 43% · about 7 min left")).toBeInTheDocument(); + + act(() => vi.advanceTimersByTime(6 * 60_000)); + expect(screen.getByText("Downloading · 43%")).toBeInTheDocument(); + }); + + it("keeps no clock without an estimate", () => { + vi.useRealTimers(); + vi.useFakeTimers({ toFake: ["Date", "setInterval", "clearInterval"] }); + vi.setSystemTime(new Date("2026-01-02T03:04:05Z")); + const { unmount } = render( + , + ); + expect(vi.getTimerCount()).toBe(0); + unmount(); + + render(); + expect(vi.getTimerCount()).toBe(1); + }); + + it("names a blocked import for admins and a wait for requesters", () => { + const blocked = download({ phase: "import_blocked", percent: 100 }); + const { rerender } = render(); + expect(screen.getByText("Waiting for import")).toBeInTheDocument(); + + rerender(); + expect(screen.getByText("Import blocked")).toBeInTheDocument(); + expect(screen.getByRole("progressbar")).toHaveAttribute("aria-valuenow", "100"); + }); +}); diff --git a/web/src/components/RequestDownloadProgress.tsx b/web/src/components/RequestDownloadProgress.tsx new file mode 100644 index 0000000000..9d8cc22a7e --- /dev/null +++ b/web/src/components/RequestDownloadProgress.tsx @@ -0,0 +1,49 @@ +import { useEffect, useState } from "react"; +import type { RequestDownload } from "@/api/types"; +import { Progress } from "@/components/ui/progress"; +import { formatRequestDownload, requestDownloadPercent } from "@/lib/requestDownload"; +import { cn } from "@/lib/utils"; + +/** How often a shown estimate is checked against the clock again. */ +const ESTIMATE_CLOCK_INTERVAL_MS = 30_000; + +/** + * Renders again every 30 seconds while enabled. The estimate is worked out + * against the clock, and a poll that returns the same figures does not + * render its readers again, so without this an estimate would neither count + * down nor disappear once it has passed or its figures have gone stale. + */ +function useEstimateClock(enabled: boolean) { + const [, setTick] = useState(0); + useEffect(() => { + if (!enabled) return; + const id = window.setInterval(() => setTick((tick) => tick + 1), ESTIMATE_CLOCK_INTERVAL_MS); + return () => window.clearInterval(id); + }, [enabled]); +} + +/** + * How far a request's or one server's downloads are: a bar once the size is + * known, over the phase with its percentage and estimate. Admin views name a + * blocked import as such. + */ +export function RequestDownloadProgress({ + download, + admin = false, + className, +}: { + download: RequestDownload; + admin?: boolean; + className?: string; +}) { + useEstimateClock(Boolean(download.estimated_completion_at)); + const percent = requestDownloadPercent(download); + return ( +
+ {percent !== undefined ? : null} +

+ {formatRequestDownload(download, { admin })} +

+
+ ); +} diff --git a/web/src/hooks/queries/admin/requests.ts b/web/src/hooks/queries/admin/requests.ts index f4d316bf07..d92ddb69e2 100644 --- a/web/src/hooks/queries/admin/requests.ts +++ b/web/src/hooks/queries/admin/requests.ts @@ -58,6 +58,7 @@ import { REQUESTS_STALE_TIME, invalidateRequestSurfaces, isValidationFailure, + requestDownloadRefetchInterval, } from "../useRequests"; const ADMIN_QUEUE_STALE_TIME = 10_000; @@ -79,7 +80,8 @@ function requestQueueKey(filter: AdminRequestQueueFilter) { * One queue view, a page at a time. A new search or type filter keeps the * rows on screen until its first page arrives; a new view does not, since its * rows take different actions. With `enabled: false` it only reads the rows - * another reader of the same view loads, and follows their refetches. + * another reader of the same view loads, and follows their refetches. While + * a loaded row downloads, the view is read again every 30 seconds. */ export function useAdminRequestQueue( filter: AdminRequestQueueFilter, @@ -103,6 +105,8 @@ export function useAdminRequestQueue( ? previous : undefined, staleTime: ADMIN_QUEUE_STALE_TIME, + refetchInterval: (query) => + requestDownloadRefetchInterval(query.state.data?.pages.flatMap((page) => page.items)), }); } diff --git a/web/src/hooks/queries/useRequests.test.tsx b/web/src/hooks/queries/useRequests.test.tsx index 51063e5157..ef83727b49 100644 --- a/web/src/hooks/queries/useRequests.test.tsx +++ b/web/src/hooks/queries/useRequests.test.tsx @@ -10,6 +10,7 @@ import { adminKeys, requestKeys } from "./keys"; const mocks = vi.hoisted(() => ({ useQuery: vi.fn(), + useInfiniteQuery: vi.fn(), useCurrentProfile: vi.fn(), api: vi.fn(), })); @@ -20,6 +21,7 @@ vi.mock("@tanstack/react-query", async () => { return { ...actual, useQuery: (...args: unknown[]) => mocks.useQuery(...args), + useInfiniteQuery: (...args: unknown[]) => mocks.useInfiniteQuery(...args), }; }); @@ -37,12 +39,15 @@ import { nextDiscoverySectionPage, useCancelMediaRequest, useCreateMediaRequest, + useMyMediaRequests, useRequestFeatureStatus, + useRequestMediaDetail, useRequestSearch, useToggleRequestFollow, } from "./useRequests"; import { useAdminCancelMediaRequest, + useAdminRequestQueue, useRequestGroupLimit, useUpdateRequestGroupLimit, } from "./admin/requests"; @@ -459,3 +464,82 @@ describe("useToggleRequestFollow", () => { expect(invalidations).toHaveBeenCalledWith({ queryKey: ["requests"] }); }); }); + +describe("polling while something downloads", () => { + const download = { phase: "downloading", downloads: 1, updated_at: "2026-01-02T03:04:05Z" }; + type RefetchInterval = (query: { state: { data: unknown } }) => number | false; + + function refetchInterval(hook: typeof mocks.useQuery): RefetchInterval { + return (hook.mock.calls.at(-1)![0] as { refetchInterval: RefetchInterval }).refetchInterval; + } + + beforeEach(() => { + mocks.useQuery.mockReset(); + mocks.useInfiniteQuery.mockReset(); + }); + + it("reads the viewer's requests every 30 seconds while one of them downloads", () => { + function CallMine() { + useMyMediaRequests({ limit: 100 }); + return null; + } + render(); + const interval = refetchInterval(mocks.useQuery); + + expect(interval({ state: { data: undefined } })).toBe(false); + expect(interval({ state: { data: [{ id: "a" }, { id: "b", targets: [{ id: 1 }] }] } })).toBe( + false, + ); + expect(interval({ state: { data: [{ id: "a" }, { id: "b", download }] } })).toBe(30_000); + expect(interval({ state: { data: [{ id: "a", targets: [{ id: 1, download }] }] } })).toBe( + 30_000, + ); + }); + + it("leaves the viewer's requests alone on a surface that opts out", () => { + function CallMine() { + useMyMediaRequests({ outcome: "active" }, { pollDownloads: false }); + return null; + } + render(); + const interval = refetchInterval(mocks.useQuery); + + expect(interval({ state: { data: [{ id: "a" }, { id: "b", download }] } })).toBe(false); + expect(interval({ state: { data: [{ id: "a", targets: [{ id: 1, download }] }] } })).toBe( + false, + ); + }); + + it("reads a title's request every 30 seconds while it downloads", () => { + function CallDetail() { + useRequestMediaDetail("movie", 949); + return null; + } + render(); + const interval = refetchInterval(mocks.useQuery); + + expect(interval({ state: { data: undefined } })).toBe(false); + expect(interval({ state: { data: { request: { requestable: false } } } })).toBe(false); + expect(interval({ state: { data: { request: { requestable: false, download } } } })).toBe( + 30_000, + ); + }); + + it("reads the admin queue every 30 seconds while a row on any loaded page downloads", () => { + function CallQueue() { + useAdminRequestQueue({ view: "in_progress" }); + return null; + } + render(); + const interval = refetchInterval(mocks.useInfiniteQuery); + const pages = (...items: unknown[][]) => ({ pages: items.map((rows) => ({ items: rows })) }); + + expect(interval({ state: { data: undefined } })).toBe(false); + expect(interval({ state: { data: pages([{ id: "a", targets: [] }]) } })).toBe(false); + expect( + interval({ + state: { data: pages([{ id: "a" }], [{ id: "b", targets: [{ id: 1, download }] }]) }, + }), + ).toBe(30_000); + }); +}); diff --git a/web/src/hooks/queries/useRequests.ts b/web/src/hooks/queries/useRequests.ts index 995dca159c..67b0ce35c7 100644 --- a/web/src/hooks/queries/useRequests.ts +++ b/web/src/hooks/queries/useRequests.ts @@ -28,6 +28,7 @@ import type { CreateMediaRequestInput, DiscoverBrowseKind, DiscoverBrowseResponse, + MediaRequest, RequestDiscoverySection, RequestListParams, RequestMediaPage, @@ -40,6 +41,22 @@ import { adminKeys, requestKeys } from "./keys"; export const REQUESTS_STALE_TIME = 30_000; const DISCOVER_BRAND_STALE_TIME = 24 * 60 * 60 * 1000; const BROWSE_STALE_TIME = 60 * 1000; +/** How often a request surface is read again while something on it downloads. */ +export const REQUEST_DOWNLOAD_REFETCH_INTERVAL = 30_000; + +/** + * Polls while a loaded request, or one of its servers, reports download + * progress, and stops once none does. The server refreshes progress about + * once a minute. + */ +export function requestDownloadRefetchInterval( + requests: readonly Pick[] | undefined, +): number | false { + const downloading = requests?.some( + (request) => request.download || request.targets?.some((target) => target.download), + ); + return downloading ? REQUEST_DOWNLOAD_REFETCH_INTERVAL : false; +} function listParamsKey(params: RequestListParams) { return { @@ -191,6 +208,8 @@ export function useRequestMediaDetail( queryFn: () => getRequestMediaDetailV2(mediaType, tmdbID), enabled: tmdbID > 0 && (options.enabled ?? true), staleTime: REQUESTS_STALE_TIME, + refetchInterval: (query) => + query.state.data?.request.download ? REQUEST_DOWNLOAD_REFETCH_INTERVAL : false, }); } @@ -312,15 +331,22 @@ export function useCancelMediaRequest() { }); } +/** + * The viewer's own requests. A surface that shows no download progress passes + * pollDownloads: false so a download does not make it read the list again. + */ export function useMyMediaRequests( params: RequestListParams = {}, - options: { enabled?: boolean } = {}, + options: { enabled?: boolean; pollDownloads?: boolean } = {}, ) { const key = listParamsKey(params); + const pollDownloads = options.pollDownloads ?? true; return useQuery({ queryKey: requestKeys.mine(key), queryFn: () => listMyMediaRequestsV2(params), enabled: options.enabled ?? true, staleTime: REQUESTS_STALE_TIME, + refetchInterval: (query) => + pollDownloads ? requestDownloadRefetchInterval(query.state.data) : false, }); } diff --git a/web/src/lib/requestDownload.test.ts b/web/src/lib/requestDownload.test.ts new file mode 100644 index 0000000000..1328dae22b --- /dev/null +++ b/web/src/lib/requestDownload.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; +import type { RequestDownload } from "@/api/types"; +import { formatRequestDownload, requestDownloadPercent } from "./requestDownload"; + +describe("request download progress", () => { + const now = new Date("2026-01-02T03:04:05Z"); + const download = (overrides: Partial = {}): RequestDownload => ({ + phase: "downloading", + percent: 43, + bytes_total: 4294967296, + bytes_left: 2448131358, + estimated_completion_at: "2026-01-02T03:16:05Z", + downloads: 1, + updated_at: "2026-01-02T03:03:05Z", + ...overrides, + }); + + it("names the phase, the percentage and the time left while downloading", () => { + expect(formatRequestDownload(download(), { now })).toBe( + "Downloading · 43% · about 12 min left", + ); + expect( + formatRequestDownload(download({ estimated_completion_at: "2026-01-02T05:34:05Z" }), { + now, + }), + ).toBe("Downloading · 43% · about 2 hr 30 min left"); + }); + + it("leaves out what the server does not know yet", () => { + expect( + formatRequestDownload( + download({ + percent: undefined, + bytes_total: undefined, + bytes_left: undefined, + estimated_completion_at: undefined, + }), + { now }, + ), + ).toBe("Downloading"); + }); + + it("drops an estimate that has passed or comes from figures over ten minutes old", () => { + expect( + formatRequestDownload(download({ estimated_completion_at: "2026-01-02T03:04:00Z" }), { + now, + }), + ).toBe("Downloading · 43%"); + expect(formatRequestDownload(download({ updated_at: "2026-01-02T02:54:04Z" }), { now })).toBe( + "Downloading · 43%", + ); + // Exactly ten minutes old is still recent enough. + expect(formatRequestDownload(download({ updated_at: "2026-01-02T02:54:05Z" }), { now })).toBe( + "Downloading · 43% · about 12 min left", + ); + }); + + it.each([ + ["queued", "Waiting to download"], + ["paused", "Download paused"], + ["stalled", "Download stalled"], + ["importing", "Importing"], + ["import_blocked", "Waiting for import"], + ])("labels %s as %s", (phase, label) => { + expect(formatRequestDownload(download({ phase }), { now })).toBe(label); + }); + + it("tells an admin that an import is blocked", () => { + expect(formatRequestDownload(download({ phase: "import_blocked" }), { now, admin: true })).toBe( + "Import blocked", + ); + expect(formatRequestDownload(download(), { now, admin: true })).toBe( + "Downloading · 43% · about 12 min left", + ); + }); + + it("reads a phase it does not know as downloading, without figures", () => { + const unknown = download({ phase: "seeding" }); + expect(formatRequestDownload(unknown, { now })).toBe("Downloading"); + expect(requestDownloadPercent(unknown)).toBeUndefined(); + }); + + it("gives a bar percentage once the size is known, clamped to 0–100", () => { + expect(requestDownloadPercent(download())).toBe(43); + expect(requestDownloadPercent(download({ phase: "paused" }))).toBe(43); + expect(requestDownloadPercent(download({ percent: undefined }))).toBeUndefined(); + expect(requestDownloadPercent(download({ percent: 140 }))).toBe(100); + expect(requestDownloadPercent(download({ percent: -3 }))).toBe(0); + }); +}); diff --git a/web/src/lib/requestDownload.ts b/web/src/lib/requestDownload.ts new file mode 100644 index 0000000000..477c672c8f --- /dev/null +++ b/web/src/lib/requestDownload.ts @@ -0,0 +1,72 @@ +import type { RequestDownload } from "@/api/types"; +import { formatHoursMinutes } from "@/lib/audiobooks/duration"; + +// Download progress labels live apart from mediaRequests: that module is on +// the launch path (search and status badges import it), and only the lazily +// loaded request pages show progress. + +const DOWNLOAD_PHASES = new Set([ + "queued", + "downloading", + "paused", + "stalled", + "importing", + "import_blocked", +]); + +/** Figures the server has not refreshed for this long no longer support an estimate. */ +const DOWNLOAD_ESTIMATE_MAX_AGE_MS = 10 * 60_000; + +/** + * How much has downloaded, for a progress bar; undefined while the size is + * unknown, and for a phase this client does not know. + */ +export function requestDownloadPercent(download: RequestDownload): number | undefined { + if (download.percent == null || !DOWNLOAD_PHASES.has(download.phase)) return undefined; + return Math.min(100, Math.max(0, download.percent)); +} + +/** + * "Downloading · 43% · about 12 min left", or the phase alone. The estimate + * is left out once it has passed, or when the server last heard from the + * download server more than ten minutes ago. A phase this client does not + * know reads as Downloading. Admin views call a blocked import what it is; + * a requester only needs to know it is waiting. + */ +export function formatRequestDownload( + download: RequestDownload, + { now = new Date(), admin = false }: { now?: Date; admin?: boolean } = {}, +): string { + switch (download.phase) { + case "queued": + return "Waiting to download"; + case "paused": + return "Download paused"; + case "stalled": + return "Download stalled"; + case "importing": + return "Importing"; + case "import_blocked": + return admin ? "Import blocked" : "Waiting for import"; + case "downloading": + break; + default: + return "Downloading"; + } + const parts = ["Downloading"]; + const percent = requestDownloadPercent(download); + if (percent !== undefined) parts.push(`${percent}%`); + const left = downloadTimeLeft(download, now); + if (left) parts.push(`about ${left} left`); + return parts.join(" · "); +} + +function downloadTimeLeft(download: RequestDownload, now: Date): string | null { + if (!download.estimated_completion_at) return null; + const eta = Date.parse(download.estimated_completion_at); + const heard = Date.parse(download.updated_at); + const at = now.getTime(); + if (!Number.isFinite(eta) || eta <= at) return null; + if (!Number.isFinite(heard) || at - heard > DOWNLOAD_ESTIMATE_MAX_AGE_MS) return null; + return formatHoursMinutes((eta - at) / 1000); +} diff --git a/web/src/pages/AdminRequests.v2.test.tsx b/web/src/pages/AdminRequests.v2.test.tsx index fce077d760..f87129beaa 100644 --- a/web/src/pages/AdminRequests.v2.test.tsx +++ b/web/src/pages/AdminRequests.v2.test.tsx @@ -632,6 +632,54 @@ describe("request administration", () => { expect(screen.queryByRole("link", { name: "Waiting Title" })).not.toBeInTheDocument(); }); + it("shows each server's download progress and names a blocked import", async () => { + const heard = "2026-09-01T00:05:00Z"; + serve({ + ...queue({ + in_progress: [ + request("r3", "Downloading Title", { + status: "downloading", + state: "processing", + targets: [ + target("t1", { + status: "downloading", + download: { + phase: "import_blocked", + percent: 100, + downloads: 1, + updated_at: heard, + }, + }), + target("t2", { + quality: "2160p", + instance_name: "Radarr 4K", + status: "downloading", + download: { phase: "downloading", percent: 43, downloads: 1, updated_at: heard }, + }), + ], + download: { phase: "import_blocked", percent: 71, downloads: 2, updated_at: heard }, + }), + ], + }), + "GET /api/v2/admin/requests/{id}/events": () => ({ items: [] }), + }); + mount("/admin/requests?view=in_progress"); + const row = await rowOf("Downloading Title"); + expect(within(row).getByText("Import blocked")).toBeInTheDocument(); + expect(within(row).getByText("Downloading · 43%")).toBeInTheDocument(); + expect( + within(row) + .getAllByRole("progressbar") + .map((bar) => bar.getAttribute("aria-valuenow")), + ).toEqual(["100", "43"]); + expect(within(row).queryByText("Waiting for import")).not.toBeInTheDocument(); + + fireEvent.click(within(row).getByRole("button", { name: "Details: Downloading Title" })); + const servers = within(await screen.findByRole("dialog")).getByRole("table"); + expect(within(servers).getByText("Import blocked")).toBeInTheDocument(); + expect(within(servers).getByText("Downloading · 43%")).toBeInTheDocument(); + }); + it("loads the next page from the cursor the last one returned", async () => { serve({ ...queue({}), diff --git a/web/src/pages/ItemDetail/components/RequestActionBar.tsx b/web/src/pages/ItemDetail/components/RequestActionBar.tsx index 21c8c9b7df..c6be827bb7 100644 --- a/web/src/pages/ItemDetail/components/RequestActionBar.tsx +++ b/web/src/pages/ItemDetail/components/RequestActionBar.tsx @@ -13,6 +13,7 @@ import { } from "lucide-react"; import type { MediaRequest, RequestMediaDetail } from "@/api/types"; import { CancelRequestDialog } from "@/components/CancelRequestDialog"; +import { RequestDownloadProgress } from "@/components/RequestDownloadProgress"; import { RequestSeasonsDialog } from "@/components/RequestSeasonsDialog"; import { useCancelMediaRequest, @@ -58,9 +59,10 @@ interface RequestActionBarProps { /** * The action row for a title outside the library: Request in the Play pill's - * place, the request's state once there is one, and cancelling or following - * that request. Owns the request mutations so their pending state re-renders - * only this row, as MediaUserActionBar does for library items. + * place, the request's state once there is one, cancelling or following that + * request, and how far its download is. Owns the request mutations so their + * pending state re-renders only this row, as MediaUserActionBar does for + * library items. */ export default function RequestActionBar({ item, libraryHref }: RequestActionBarProps) { const navigate = useViewTransitionNavigate(); @@ -148,6 +150,9 @@ export default function RequestActionBar({ item, libraryHref }: RequestActionBar return ( <> + {item.request.download ? ( + + ) : null} {ownRequest ? ( { ); }); + it("shows how far a request's download is, without naming a blocked import", () => { + mocks.mine = [ + request("r13", "Heat", { + status: "downloading", + state: "processing", + download: { + phase: "downloading", + percent: 43, + downloads: 1, + updated_at: "2026-01-01T00:00:00Z", + }, + }), + request("r14", "Ronin", { + status: "downloading", + state: "processing", + download: { + phase: "import_blocked", + percent: 100, + downloads: 1, + updated_at: "2026-01-01T00:00:00Z", + }, + }), + ]; + renderYours(); + + const heat = row("Heat"); + expect(within(heat).getByRole("progressbar")).toHaveAttribute("aria-valuenow", "43"); + expect(within(heat).getByText("Downloading · 43%")).toBeInTheDocument(); + expect(within(row("Ronin")).getByText("Waiting for import")).toBeInTheDocument(); + expect(screen.queryByText("Import blocked")).not.toBeInTheDocument(); + }); + it("shows when a request was made and when it last changed", () => { mocks.mine = [ request("r11", "Old Movie", { updated_at: "2026-01-01T00:00:30Z" }), diff --git a/web/src/pages/Requests.tsx b/web/src/pages/Requests.tsx index f3f22696ec..0e9a82f550 100644 --- a/web/src/pages/Requests.tsx +++ b/web/src/pages/Requests.tsx @@ -4,6 +4,7 @@ import { Film, Info, Library, RefreshCw, Tv } from "lucide-react"; import BrandCarousel from "@/components/BrandCarousel"; import { CancelRequestDialog } from "@/components/CancelRequestDialog"; import MediaCarousel from "@/components/MediaCarousel"; +import { RequestDownloadProgress } from "@/components/RequestDownloadProgress"; import RequestPosterCard from "@/components/RequestPosterCard"; import { RequestStatusBadge } from "@/components/RequestStatusBadge"; import SearchBar from "@/components/SearchBar"; @@ -428,6 +429,12 @@ function RequestRow({ {state ? : null}

{details.join(" · ")}

+ {request.download ? ( + + ) : null}

Requested{" "}

by {target.route_name}

) : null} + {target.download ? ( + + ) : null} {target.last_error ? (

{target.last_error}

) : null} diff --git a/web/src/pages/admin-requests/RequestQueueDialog.tsx b/web/src/pages/admin-requests/RequestQueueDialog.tsx index dbaa4abb0f..2d87066f4a 100644 --- a/web/src/pages/admin-requests/RequestQueueDialog.tsx +++ b/web/src/pages/admin-requests/RequestQueueDialog.tsx @@ -3,6 +3,7 @@ import type { ReactNode } from "react"; import { Link } from "react-router"; import { AlertTriangle, ExternalLink, Library, X } from "lucide-react"; import type { MediaRequest } from "@/api/types"; +import { RequestDownloadProgress } from "@/components/RequestDownloadProgress"; import { Button } from "@/components/ui/button"; import { Dialog, @@ -272,6 +273,13 @@ function TargetRows({ target }: { target: NonNullable[n {target.external_status ? ( {target.external_status} ) : null} + {target.download ? ( + + ) : null} From b0ebcda598be4db27ca1b56e39833029fb8280f0 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:43:43 +0000 Subject: [PATCH 2/4] fix(requests): hold both reconcile locks on one session and size requests from every live target The reconcile pass took its own advisory lock and the request target write lock on two pooled connections, so with database.max_connections at 2 its first query waited forever for a third. pglock gains Lock.AcquireAlso, which takes a second key on the session already holding a lock, and Release frees every key the session holds; the reconcile pass takes both locks through it. pglock.Acquire, which only this pass used, is gone. A request's combined download progress skipped a live target that had not reported progress yet, so a 1080p and 4K request could show 90% from the 1080p copy alone. Such a target now leaves the combined size unknown, as the docs already said. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/architecture/media-requests.md | 10 +- internal/database/pglock/pglock.go | 110 +++++++++------ internal/database/pglock/pglock_test.go | 133 ++++++++++++------ internal/requests/downloads_test.go | 19 +++ internal/requests/types.go | 8 +- .../taskmanager/tasks/reconcile_requests.go | 78 +++++++--- .../tasks/refresh_all_library_metadata.go | 23 +-- .../tasks/refresh_request_downloads_test.go | 99 ++++++++----- 8 files changed, 311 insertions(+), 169 deletions(-) diff --git a/docs/architecture/media-requests.md b/docs/architecture/media-requests.md index fa9f0efac0..2796b51285 100644 --- a/docs/architecture/media-requests.md +++ b/docs/architecture/media-requests.md @@ -368,9 +368,10 @@ downloading target has progress. Two advisory locks keep the passes apart. The reconcile lock lets one server run each reconcile pass; the others skip. The request target write lock keeps the two passes from writing the same target at once. A reconcile pass takes it -after its own lock and waits up to two minutes for a refresh pass that holds -it, so a refresh never makes a reconcile pass skip; a wait that runs out fails -the pass. A refresh pass only tries the write lock, so it runs on one server at +after its own lock, on the same database session so the pass keeps the rest of +the connection pool for its own work, and waits up to two minutes for a +refresh pass that holds it, so a refresh never makes a reconcile pass skip; a +wait that runs out fails the pass. A refresh pass only tries the write lock, so it runs on one server at a time and skips while a reconcile pass runs anywhere in the cluster. A refresh pass also stops after 90 seconds, cutting the plugin call in flight and leaving the requests it has not reached for the next pass, so it always ends inside the @@ -437,7 +438,8 @@ and on the title detail's request state. The request's figure combines its live targets (1080p and 4K together): bytes and downloads summed, the phase by the order above, the latest estimate, and the oldest report's time, so a client that hides figures older than about ten minutes hides a partly stale one too. -Its size is unknown while any target's is. `percent` is +Its size is unknown while any live target's is, including a live target that +has reported no progress yet. `percent` is `floor((total - left) * 100 / total)`, and the byte counts and percent are absent while the size is unknown. Only the title detail (`GET /api/v2/requests/detail/{media_type}/{tmdb_id}`) fills the request diff --git a/internal/database/pglock/pglock.go b/internal/database/pglock/pglock.go index 5c36e1cc59..56608c1e2d 100644 --- a/internal/database/pglock/pglock.go +++ b/internal/database/pglock/pglock.go @@ -13,6 +13,7 @@ import ( "context" "errors" "fmt" + "slices" "strconv" "time" @@ -33,6 +34,8 @@ const ArtworkReconcileLockKey int64 = 0x53494c4f535452 type Lock struct { conn *pgxpool.Conn key int64 + // also holds the keys AcquireAlso took on the same session. + also []int64 } // TryAcquire takes advisory lock key without blocking. It reports acquired @@ -65,49 +68,59 @@ func TryAcquire(ctx context.Context, pool *pgxpool.Pool, key int64) (*Lock, bool // lock wait. const lockNotAvailable = "55P03" -// Acquire takes advisory lock key, waiting up to wait for the session holding -// it to release it. It reports acquired false (with a nil Lock and no error) -// when the wait runs out, or when pool is nil. A wait of zero or less does not -// wait at all. ctx still bounds the call. +// AcquireAlso takes advisory lock key on the session that already holds l, +// waiting up to wait for another session holding key to release it, or not +// at all when wait is zero or less. Work guarded by two locks then holds one +// pooled connection instead of two. It reports false with no error when the +// wait runs out; l keeps its own lock. Release unlocks every key l holds. // // PostgreSQL enforces the wait with a transaction-local lock_timeout, so a -// wait that runs out leaves the lock queue and returns a clean connection to -// the pool. The lock itself is session-level and outlives that transaction. -// Any other failure closes the connection instead of returning it, because -// PostgreSQL may have granted the lock just before reporting the error. -// -// The caller owns the returned Lock and must call Release exactly once. -func Acquire(ctx context.Context, pool *pgxpool.Pool, key int64, wait time.Duration) (*Lock, bool, error) { - if wait <= 0 { - return TryAcquire(ctx, pool, key) +// wait that runs out leaves the lock queue and the session clean. The lock +// itself is session-level and outlives that transaction. Any other failure +// closes the session, since PostgreSQL may have granted the lock just before +// reporting the error; that drops l's lock too, and Release becomes a no-op. +func (l *Lock) AcquireAlso(ctx context.Context, key int64, wait time.Duration) (bool, error) { + if l == nil || l.conn == nil { + return false, fmt.Errorf("acquiring advisory lock %d: no session holds a lock", key) } - if pool == nil { - return nil, false, nil + var locked bool + var err error + if wait <= 0 { + err = l.conn.QueryRow(ctx, `SELECT pg_try_advisory_lock($1)`, key).Scan(&locked) + } else { + locked, err = lockWithin(ctx, l.conn, key, wait) } - conn, err := pool.Acquire(ctx) if err != nil { - return nil, false, fmt.Errorf("acquiring connection for advisory lock %d: %w", key, err) + conn := l.conn + l.conn = nil + _ = conn.Hijack().Close(context.WithoutCancel(ctx)) + return false, fmt.Errorf("acquiring advisory lock %d: %w", key, err) + } + if locked { + l.also = append(l.also, key) } + return locked, nil +} + +// lockWithin takes advisory lock key on conn, waiting up to wait. It reports +// false with no error when the wait runs out. After any error the session's +// lock state is unknown and conn must be closed, not reused. +func lockWithin(ctx context.Context, conn *pgxpool.Conn, key int64, wait time.Duration) (bool, error) { tx, err := conn.Begin(ctx) - if err == nil { - _, err = tx.Exec(ctx, `SELECT set_config('lock_timeout', $1, true)`, strconv.FormatInt(max(wait.Milliseconds(), 1), 10)) + if err != nil { + return false, err } - if err == nil { - _, err = tx.Exec(ctx, `SELECT pg_advisory_lock($1)`, key) - if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == lockNotAvailable { - _ = tx.Rollback(ctx) - conn.Release() - return nil, false, nil - } + if _, err := tx.Exec(ctx, `SELECT set_config('lock_timeout', $1, true)`, strconv.FormatInt(max(wait.Milliseconds(), 1), 10)); err != nil { + return false, err } - if err == nil { - err = tx.Commit(ctx) + _, err = tx.Exec(ctx, `SELECT pg_advisory_lock($1)`, key) + if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok && pgErr.Code == lockNotAvailable { + return false, tx.Rollback(ctx) } if err != nil { - _ = conn.Hijack().Close(context.WithoutCancel(ctx)) - return nil, false, fmt.Errorf("acquiring advisory lock %d: %w", key, err) + return false, err } - return &Lock{conn: conn, key: key}, true, nil + return true, tx.Commit(ctx) } // Conn exposes the connection holding the lock. Work that must be serialized @@ -120,9 +133,10 @@ func (l *Lock) Conn() *pgxpool.Conn { return l.conn } -// Release unlocks and returns the connection to the pool. If the unlock fails +// Release unlocks every key the session holds (the lock's own and any +// AcquireAlso took) and returns the connection to the pool. If an unlock fails // or reports that the lock was not held, the connection is hijacked out of the -// pool and closed so the stranded lock dies with it. +// pool and closed so the stranded locks die with it. // // Release is idempotent and safe on a nil Lock. func (l *Lock) Release(ctx context.Context) error { @@ -135,17 +149,23 @@ func (l *Lock) Release(ctx context.Context) error { releaseCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), releaseTimeout) defer cancel() - var unlocked bool - err := conn.QueryRow(releaseCtx, `SELECT pg_advisory_unlock($1)`, l.key).Scan(&unlocked) - if err == nil && unlocked { - conn.Release() - return nil - } - - rawConn := conn.Hijack() - _ = rawConn.Close(context.WithoutCancel(ctx)) - if err != nil { - return fmt.Errorf("releasing advisory lock %d: %w", l.key, err) + // The keys AcquireAlso took, newest first, then the lock's own key. + keys := append(slices.Clone(l.also), l.key) + slices.Reverse(keys[:len(l.also)]) + l.also = nil + for _, key := range keys { + var unlocked bool + err := conn.QueryRow(releaseCtx, `SELECT pg_advisory_unlock($1)`, key).Scan(&unlocked) + if err == nil && unlocked { + continue + } + rawConn := conn.Hijack() + _ = rawConn.Close(context.WithoutCancel(ctx)) + if err != nil { + return fmt.Errorf("releasing advisory lock %d: %w", key, err) + } + return fmt.Errorf("releasing advisory lock %d: lock was not held", key) } - return fmt.Errorf("releasing advisory lock %d: lock was not held", l.key) + conn.Release() + return nil } diff --git a/internal/database/pglock/pglock_test.go b/internal/database/pglock/pglock_test.go index 445366308b..59aed1d1c0 100644 --- a/internal/database/pglock/pglock_test.go +++ b/internal/database/pglock/pglock_test.go @@ -138,96 +138,139 @@ func lockWaiters(t *testing.T, pool *pgxpool.Pool, key int64) int { return waiters } -func TestAcquireWaitsForTheHolder(t *testing.T) { +// pglockOtherTestKey is a second key for the tests that hold two locks. +const pglockOtherTestKey int64 = 0x70676C6F636B02 + +// AcquireAlso waits for another session's hold on the second key, then holds +// both keys on one session, and Release frees both. +func TestAcquireAlsoWaitsForTheHolder(t *testing.T) { pool := testPool(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - held, acquired, err := TryAcquire(ctx, pool, pglockTestKey) + other, acquired, err := TryAcquire(ctx, pool, pglockOtherTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire(other) = (%v, %v), want acquired", acquired, err) + } + lock, acquired, err := TryAcquire(ctx, pool, pglockTestKey) if err != nil || !acquired { t.Fatalf("TryAcquire = (%v, %v), want acquired", acquired, err) } type result struct { - lock *Lock acquired bool err error } done := make(chan result, 1) go func() { - lock, acquired, err := Acquire(ctx, pool, pglockTestKey, 20*time.Second) - done <- result{lock, acquired, err} + acquired, err := lock.AcquireAlso(ctx, pglockOtherTestKey, 20*time.Second) + done <- result{acquired, err} }() - for lockWaiters(t, pool, pglockTestKey) == 0 { + for lockWaiters(t, pool, pglockOtherTestKey) == 0 { select { case got := <-done: - t.Fatalf("Acquire returned (%v, %v) while the lock was held", got.acquired, got.err) + t.Fatalf("AcquireAlso returned (%v, %v) while the key was held", got.acquired, got.err) case <-ctx.Done(): - t.Fatal("Acquire never queued for the lock") + t.Fatal("AcquireAlso never queued for the key") case <-time.After(10 * time.Millisecond): } } - if err := held.Release(ctx); err != nil { - t.Fatalf("Release: %v", err) + if err := other.Release(ctx); err != nil { + t.Fatalf("Release(other): %v", err) } - got := <-done - if got.err != nil || !got.acquired { - t.Fatalf("Acquire = (%v, %v), want acquired once the holder released", got.acquired, got.err) + if got := <-done; got.err != nil || !got.acquired { + t.Fatalf("AcquireAlso = (%v, %v), want acquired once the holder released", got.acquired, got.err) } - // The lock outlives the transaction that set the wait. - if !lockHeld(t, pool, pglockTestKey) { - t.Fatal("advisory lock not held after Acquire") + // Both keys outlive the transaction that set the wait, on one session. + var sessions int + if err := pool.QueryRow(ctx, ` + SELECT count(DISTINCT pid) FROM pg_locks + WHERE locktype = 'advisory' AND granted + AND ((classid::bigint << 32) | objid::bigint) = ANY($1)`, []int64{pglockTestKey, pglockOtherTestKey}).Scan(&sessions); err != nil { + t.Fatalf("inspect pg_locks: %v", err) } - if _, again, err := TryAcquire(ctx, pool, pglockTestKey); err != nil || again { - t.Fatalf("TryAcquire beside Acquire's lock = (%v, %v), want not acquired", again, err) + if !lockHeld(t, pool, pglockTestKey) || !lockHeld(t, pool, pglockOtherTestKey) || sessions != 1 { + t.Fatalf("held = %v and %v on %d sessions, want both keys on one session", + lockHeld(t, pool, pglockTestKey), lockHeld(t, pool, pglockOtherTestKey), sessions) } - if err := got.lock.Release(ctx); err != nil { + if err := lock.Release(ctx); err != nil { t.Fatalf("Release: %v", err) } - if lockHeld(t, pool, pglockTestKey) { - t.Fatal("advisory lock still held after Release") + if lockHeld(t, pool, pglockTestKey) || lockHeld(t, pool, pglockOtherTestKey) { + t.Fatal("a key is still held after Release") } } -// A wait that runs out reports not acquired and hands the connection back -// without the lock_timeout it set. The single-connection pool makes the next -// borrower get the same session. -func TestAcquireGivesUpAfterTheWait(t *testing.T) { +// A wait that runs out reports not acquired, keeps the session's own lock, and +// leaves the session without the lock_timeout it set. +func TestAcquireAlsoGivesUpAfterTheWait(t *testing.T) { pool := testPool(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - config := pool.Config().Copy() - config.MaxConns = 1 - single, err := pgxpool.NewWithConfig(ctx, config) - if err != nil { - t.Fatalf("connect single-connection pool: %v", err) - } - t.Cleanup(single.Close) - held, acquired, err := TryAcquire(ctx, pool, pglockTestKey) + other, acquired, err := TryAcquire(ctx, pool, pglockOtherTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire(other) = (%v, %v), want acquired", acquired, err) + } + defer func() { _ = other.Release(ctx) }() + lock, acquired, err := TryAcquire(ctx, pool, pglockTestKey) if err != nil || !acquired { t.Fatalf("TryAcquire = (%v, %v), want acquired", acquired, err) } - defer func() { _ = held.Release(ctx) }() + defer func() { _ = lock.Release(ctx) }() - lock, acquired, err := Acquire(ctx, single, pglockTestKey, 100*time.Millisecond) - if err != nil || acquired || lock != nil { - t.Fatalf("Acquire = (%v, %v, %v), want (nil, false, nil) after the wait", lock, acquired, err) + if acquired, err := lock.AcquireAlso(ctx, pglockOtherTestKey, 100*time.Millisecond); err != nil || acquired { + t.Fatalf("AcquireAlso = (%v, %v), want (false, nil) after the wait", acquired, err) + } + if waiters := lockWaiters(t, pool, pglockOtherTestKey); waiters != 0 { + t.Fatalf("%d sessions still queued for the key, want none", waiters) } - if waiters := lockWaiters(t, pool, pglockTestKey); waiters != 0 { - t.Fatalf("%d sessions still queued for the lock, want none", waiters) + if !lockHeld(t, pool, pglockTestKey) { + t.Fatal("the session lost its own lock when the wait ran out") } var timeout string - if err := single.QueryRow(ctx, `SHOW lock_timeout`).Scan(&timeout); err != nil { + if err := lock.Conn().QueryRow(ctx, `SHOW lock_timeout`).Scan(&timeout); err != nil { t.Fatalf("read lock_timeout: %v", err) } if timeout != "0" { - t.Fatalf("pooled session lock_timeout = %q, want the default 0", timeout) + t.Fatalf("session lock_timeout = %q, want the default 0", timeout) + } + if err := lock.Release(ctx); err != nil { + t.Fatalf("Release: %v", err) + } + if lockHeld(t, pool, pglockTestKey) { + t.Fatal("advisory lock still held after Release") } } -func TestAcquireNilPoolReportsNotAcquired(t *testing.T) { - lock, acquired, err := Acquire(context.Background(), nil, pglockTestKey, time.Minute) - if err != nil || acquired || lock != nil { - t.Fatalf("Acquire(nil pool) = (%v, %v, %v), want (nil, false, nil)", lock, acquired, err) +// Without a wait, AcquireAlso only tries. +func TestAcquireAlsoWithoutAWaitOnlyTries(t *testing.T) { + pool := testPool(t) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + lock, acquired, err := TryAcquire(ctx, pool, pglockTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire = (%v, %v), want acquired", acquired, err) + } + defer func() { _ = lock.Release(ctx) }() + other, acquired, err := TryAcquire(ctx, pool, pglockOtherTestKey) + if err != nil || !acquired { + t.Fatalf("TryAcquire(other) = (%v, %v), want acquired", acquired, err) + } + if acquired, err := lock.AcquireAlso(ctx, pglockOtherTestKey, 0); err != nil || acquired { + t.Fatalf("AcquireAlso while held = (%v, %v), want (false, nil)", acquired, err) + } + if err := other.Release(ctx); err != nil { + t.Fatalf("Release(other): %v", err) + } + if acquired, err := lock.AcquireAlso(ctx, pglockOtherTestKey, 0); err != nil || !acquired { + t.Fatalf("AcquireAlso once free = (%v, %v), want acquired", acquired, err) + } +} + +func TestAcquireAlsoWithoutALockFails(t *testing.T) { + var lock *Lock + if acquired, err := lock.AcquireAlso(context.Background(), pglockOtherTestKey, time.Minute); err == nil || acquired { + t.Fatalf("AcquireAlso on a nil Lock = (%v, %v), want an error", acquired, err) } } diff --git a/internal/requests/downloads_test.go b/internal/requests/downloads_test.go index 914a253ad2..1aea1ac935 100644 --- a/internal/requests/downloads_test.go +++ b/internal/requests/downloads_test.go @@ -92,6 +92,25 @@ func TestRequestDownloadWithAnUnknownSizeHasNoTotal(t *testing.T) { } } +// A live target that reports no progress yet (a 4K copy still waiting for a +// release, say) leaves the request's size unknown, so the request shows no +// percentage for its 1080p copy alone. +func TestRequestDownloadWithASilentLiveTargetHasNoTotal(t *testing.T) { + req := Request{Targets: []Target{ + {Quality: Quality1080p, Status: StatusDownloading, Download: &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 4000, BytesLeft: 400, Downloads: 1}}, + {Quality: Quality2160p, Status: StatusQueued}, + }} + got := req.Download() + if got == nil || got.Phase != DownloadPhaseDownloading || got.BytesTotal != 0 || got.BytesLeft != 0 || got.Downloads != 1 { + t.Fatalf("aggregate = %+v, want downloading with no total while the 4K copy reports nothing", got) + } + // A finished target is not live and does not hide the total. + req.Targets[1].Status = StatusCompleted + if got := req.Download(); got == nil || got.BytesTotal != 4000 || got.BytesLeft != 400 { + t.Fatalf("aggregate = %+v, want the 1080p figures once the 4K copy is done", got) + } +} + func TestRequestDownloadIgnoresFinishedTargetsAndTargetsWithoutProgress(t *testing.T) { stale := &DownloadProgress{Phase: DownloadPhaseDownloading, BytesTotal: 100} req := Request{Targets: []Target{ diff --git a/internal/requests/types.go b/internal/requests/types.go index 7781f3da60..2ba5223941 100644 --- a/internal/requests/types.go +++ b/internal/requests/types.go @@ -121,7 +121,13 @@ func (r *Request) Download() *DownloadProgress { var out *DownloadProgress sizeKnown := true for _, t := range r.Targets { - if t.Download == nil || (t.Status != StatusQueued && t.Status != StatusDownloading) { + if t.Status != StatusQueued && t.Status != StatusDownloading { + continue + } + if t.Download == nil { + // A live target reporting nothing yet still has a size to come; a + // percentage without it would cover only part of the request. + sizeKnown = false continue } d := *t.Download diff --git a/internal/taskmanager/tasks/reconcile_requests.go b/internal/taskmanager/tasks/reconcile_requests.go index 8deb67dc98..702483e82d 100644 --- a/internal/taskmanager/tasks/reconcile_requests.go +++ b/internal/taskmanager/tasks/reconcile_requests.go @@ -4,8 +4,10 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "time" + "github.com/Silo-Server/silo-server/internal/database/pglock" "github.com/Silo-Server/silo-server/internal/requests" "github.com/Silo-Server/silo-server/internal/taskmanager" "github.com/jackc/pgx/v5/pgxpool" @@ -47,8 +49,58 @@ type RequestReconciler interface { type ReconcileRequestsTask struct { reconciler RequestReconciler limit int - lock clusterLock - writeLock waitingClusterLock + locks reconcileLocker +} + +// reconcileLockOutcome is how taking the reconcile pass's locks went. +type reconcileLockOutcome int + +const ( + reconcileLocksHeld reconcileLockOutcome = iota + // reconcileLocksBusy: another server's reconcile pass is running. + reconcileLocksBusy + // reconcileLocksWaitedOut: a download refresh pass held the request target + // write lock past requestTargetWriteWait. + reconcileLocksWaitedOut +) + +// reconcileLocker takes both of the reconcile pass's locks. release is set +// only when outcome is reconcileLocksHeld. +type reconcileLocker interface { + Acquire(ctx context.Context) (release func(), outcome reconcileLockOutcome, err error) +} + +// requestReconcileLocks takes requestReconcileAdvisoryLock without waiting, +// then requestTargetWriteAdvisoryLock with a wait of requestTargetWriteWait, +// both on one database session. One session leaves the rest of the pool to +// the pass itself: a session per lock would take both connections of a +// two-connection pool and leave the pass none. +type requestReconcileLocks struct { + pool *pgxpool.Pool +} + +func (l requestReconcileLocks) Acquire(ctx context.Context) (func(), reconcileLockOutcome, error) { + lock, acquired, err := pglock.TryAcquire(ctx, l.pool, requestReconcileAdvisoryLock) + if err != nil { + return nil, reconcileLocksBusy, fmt.Errorf("acquiring request reconcile lock: %w", err) + } + if !acquired { + return nil, reconcileLocksBusy, nil + } + release := func() { + if err := lock.Release(ctx); err != nil { + slog.WarnContext(ctx, "releasing request reconcile locks failed", "component", "taskmanager", "error", err) + } + } + held, err := lock.AcquireAlso(ctx, requestTargetWriteAdvisoryLock, requestTargetWriteWait) + if err != nil || !held { + release() + if err != nil { + return nil, reconcileLocksWaitedOut, fmt.Errorf("acquiring request target write lock: %w", err) + } + return nil, reconcileLocksWaitedOut, nil + } + return release, reconcileLocksHeld, nil } // NewReconcileRequestsTask constructs the task. A nil pool runs without the @@ -59,8 +111,7 @@ func NewReconcileRequestsTask(reconciler RequestReconciler, limit int, pool *pgx } t := &ReconcileRequestsTask{reconciler: reconciler, limit: limit} if pool != nil { - t.lock = advisoryClusterLock{pool: pool, key: requestReconcileAdvisoryLock, name: "request reconcile"} - t.writeLock = advisoryClusterLock{pool: pool, key: requestTargetWriteAdvisoryLock, name: "request target writes", wait: requestTargetWriteWait} + t.locks = requestReconcileLocks{pool: pool} } return t } @@ -87,23 +138,16 @@ func (t *ReconcileRequestsTask) Execute(ctx context.Context, progress taskmanage progress.Report(100, "Request reconciliation unavailable") return nil } - if t.lock != nil { - release, acquired, err := t.lock.TryAcquire(ctx) + if t.locks != nil { + release, outcome, err := t.locks.Acquire(ctx) if err != nil { - return fmt.Errorf("acquiring request reconcile lock: %w", err) + return err } - if !acquired { + switch outcome { + case reconcileLocksBusy: progress.Report(100, "Another server is reconciling media requests") return nil - } - defer release() - } - if t.writeLock != nil { - release, acquired, err := t.writeLock.Acquire(ctx) - if err != nil { - return fmt.Errorf("acquiring request target write lock: %w", err) - } - if !acquired { + case reconcileLocksWaitedOut: return fmt.Errorf("request download refresh held the request target write lock for over %s", requestTargetWriteWait) } defer release() diff --git a/internal/taskmanager/tasks/refresh_all_library_metadata.go b/internal/taskmanager/tasks/refresh_all_library_metadata.go index 01116a593b..11d12d3672 100644 --- a/internal/taskmanager/tasks/refresh_all_library_metadata.go +++ b/internal/taskmanager/tasks/refresh_all_library_metadata.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "log/slog" - "time" "github.com/jackc/pgx/v5/pgxpool" @@ -34,12 +33,6 @@ type clusterLock interface { TryAcquire(ctx context.Context) (release func(), acquired bool, err error) } -// waitingClusterLock takes a lock shared by every server, waiting a bounded -// time for the server holding it. acquired is false when the wait runs out. -type waitingClusterLock interface { - Acquire(ctx context.Context) (release func(), acquired bool, err error) -} - // fullMetadataRefreshAdvisoryLock spells "SILOFULR". const fullMetadataRefreshAdvisoryLock int64 = 0x53494C4F46554C52 @@ -48,8 +41,6 @@ type advisoryClusterLock struct { key int64 // name labels the guarded work in the release-failure log. name string - // wait bounds Acquire's wait; TryAcquire never waits. - wait time.Duration } func (l advisoryClusterLock) TryAcquire(ctx context.Context) (func(), bool, error) { @@ -57,23 +48,11 @@ func (l advisoryClusterLock) TryAcquire(ctx context.Context) (func(), bool, erro if err != nil || !acquired { return nil, false, err } - return l.releaser(ctx, lock), true, nil -} - -func (l advisoryClusterLock) Acquire(ctx context.Context) (func(), bool, error) { - lock, acquired, err := pglock.Acquire(ctx, l.pool, l.key, l.wait) - if err != nil || !acquired { - return nil, false, err - } - return l.releaser(ctx, lock), true, nil -} - -func (l advisoryClusterLock) releaser(ctx context.Context, lock *pglock.Lock) func() { return func() { if err := lock.Release(ctx); err != nil { slog.WarnContext(ctx, "releasing cluster task advisory lock failed", "component", "taskmanager", "work", l.name, "error", err) } - } + }, true, nil } // RefreshAllLibraryMetadataTask re-fetches provider metadata for every item in diff --git a/internal/taskmanager/tasks/refresh_request_downloads_test.go b/internal/taskmanager/tasks/refresh_request_downloads_test.go index cc0996e513..7bdca82744 100644 --- a/internal/taskmanager/tasks/refresh_request_downloads_test.go +++ b/internal/taskmanager/tasks/refresh_request_downloads_test.go @@ -57,20 +57,20 @@ func (s *reconcilerStub) ReconcileRequests(ctx context.Context, _ int) (requests return requests.ReconcileResult{}, nil } -// fakeWaitingLock stands in for a lock taken with a bounded wait. -type fakeWaitingLock struct { - acquired bool +// fakeReconcileLocks stands in for the reconcile pass's two locks. +type fakeReconcileLocks struct { + outcome reconcileLockOutcome err error - waits int + takes int released int } -func (f *fakeWaitingLock) Acquire(context.Context) (func(), bool, error) { - f.waits++ - if f.err != nil || !f.acquired { - return nil, false, f.err +func (f *fakeReconcileLocks) Acquire(context.Context) (func(), reconcileLockOutcome, error) { + f.takes++ + if f.err != nil || f.outcome != reconcileLocksHeld { + return nil, f.outcome, f.err } - return func() { f.released++ }, true, nil + return func() { f.released++ }, reconcileLocksHeld, nil } func TestRefreshRequestDownloadsTaskRunsUnderTheLock(t *testing.T) { @@ -134,9 +134,8 @@ func TestRefreshRequestDownloadsTaskShouldRun(t *testing.T) { } } -// Reconcile keeps its own lock as the one-pass-at-a-time guard and waits for -// the target write lock; the refresh tries only the write lock. The pool never -// connects here. +// The refresh tries only the target write lock; reconcile takes its own lock +// and the write lock together on one session. The pool never connects here. func TestRequestTaskLockKeys(t *testing.T) { pool, err := pgxpool.New(context.Background(), "postgres://silo@127.0.0.1:1/silo?sslmode=disable") if err != nil { @@ -144,17 +143,11 @@ func TestRequestTaskLockKeys(t *testing.T) { } t.Cleanup(pool.Close) refresh, _ := NewRefreshRequestDownloadsTask(&downloadRefresherStub{}, 0, pool).lock.(advisoryClusterLock) - reconcileTask := NewReconcileRequestsTask(nil, 0, pool) - reconcile, _ := reconcileTask.lock.(advisoryClusterLock) - writes, _ := reconcileTask.writeLock.(advisoryClusterLock) if refresh.key != requestTargetWriteAdvisoryLock { t.Fatalf("refresh lock key = %#x, want the target write lock %#x", refresh.key, requestTargetWriteAdvisoryLock) } - if reconcile.key != requestReconcileAdvisoryLock { - t.Fatalf("reconcile lock key = %#x, want %#x", reconcile.key, requestReconcileAdvisoryLock) - } - if writes.key != requestTargetWriteAdvisoryLock || writes.wait != requestTargetWriteWait { - t.Fatalf("reconcile write lock = %#x waiting %s, want %#x waiting %s", writes.key, writes.wait, requestTargetWriteAdvisoryLock, requestTargetWriteWait) + if locks, ok := NewReconcileRequestsTask(nil, 0, pool).locks.(requestReconcileLocks); !ok || locks.pool != pool { + t.Fatalf("reconcile locks = %#v, want requestReconcileLocks on the pool", locks) } } @@ -169,33 +162,30 @@ func TestRequestDownloadRefreshBudgetEndsInsideTheReconcileWait(t *testing.T) { } } -func TestReconcileRequestsTaskWaitsForTheWriteLock(t *testing.T) { +func TestReconcileRequestsTaskRunsUnderItsLocks(t *testing.T) { reconciler := &reconcilerStub{} task := NewReconcileRequestsTask(reconciler, 0, nil) - lock := &fakeClusterLock{acquired: true} - writes := &fakeWaitingLock{acquired: true} - task.lock, task.writeLock = lock, writes + locks := &fakeReconcileLocks{outcome: reconcileLocksHeld} + task.locks = locks if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err != nil { t.Fatalf("Execute() error = %v", err) } - if reconciler.runs.Load() != 1 || writes.waits != 1 || writes.released != 1 || lock.released != 1 { - t.Fatalf("runs = %d, write lock waits = %d and releases = %d, reconcile lock releases = %d; want 1 each", - reconciler.runs.Load(), writes.waits, writes.released, lock.released) + if reconciler.runs.Load() != 1 || locks.takes != 1 || locks.released != 1 { + t.Fatalf("runs = %d, lock takes = %d, releases = %d; want 1 each", reconciler.runs.Load(), locks.takes, locks.released) } } func TestReconcileRequestsTaskSkipsWhileAnotherReconcileRuns(t *testing.T) { reconciler := &reconcilerStub{} task := NewReconcileRequestsTask(reconciler, 0, nil) - writes := &fakeWaitingLock{acquired: true} - task.lock, task.writeLock = &fakeClusterLock{acquired: false}, writes + task.locks = &fakeReconcileLocks{outcome: reconcileLocksBusy} if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err != nil { t.Fatalf("Execute() error = %v", err) } - if reconciler.runs.Load() != 0 || writes.waits != 0 { - t.Fatalf("runs = %d, write lock waits = %d; want neither while another reconcile runs", reconciler.runs.Load(), writes.waits) + if runs := reconciler.runs.Load(); runs != 0 { + t.Fatalf("runs = %d while another reconcile runs, want 0", runs) } } @@ -204,14 +194,13 @@ func TestReconcileRequestsTaskSkipsWhileAnotherReconcileRuns(t *testing.T) { func TestReconcileRequestsTaskFailsWhenTheWaitRunsOut(t *testing.T) { reconciler := &reconcilerStub{} task := NewReconcileRequestsTask(reconciler, 0, nil) - lock := &fakeClusterLock{acquired: true} - task.lock, task.writeLock = lock, &fakeWaitingLock{acquired: false} + task.locks = &fakeReconcileLocks{outcome: reconcileLocksWaitedOut} if err := task.Execute(context.Background(), &bulkEnrichmentTaskProgress{}); err == nil { t.Fatal("Execute() error = nil, want the wait to fail the pass") } - if reconciler.runs.Load() != 0 || lock.released != 1 { - t.Fatalf("runs = %d, reconcile lock releases = %d; want 0 and 1", reconciler.runs.Load(), lock.released) + if runs := reconciler.runs.Load(); runs != 0 { + t.Fatalf("runs = %d, want 0", runs) } } @@ -286,6 +275,46 @@ func TestReconcileWaitsForADownloadRefreshPostgres(t *testing.T) { } } +// The reconcile pass holds both of its locks on one session, so a pool of two +// connections still has one for the pass itself. With a session per lock, the +// pass's first query would wait forever for a connection. +func TestReconcileLeavesAConnectionForItsPassPostgres(t *testing.T) { + base := requestLockTestPool(t) + config := base.Config().Copy() + config.MaxConns = 2 + pool, err := pgxpool.NewWithConfig(t.Context(), config) + if err != nil { + t.Fatal(err) + } + t.Cleanup(pool.Close) + reconciler := &queryingReconcilerStub{pool: pool} + task := NewReconcileRequestsTask(reconciler, 0, pool) + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + if err := task.Execute(ctx, &bulkEnrichmentTaskProgress{}); err != nil { + t.Fatalf("Execute() with a two-connection pool: %v", err) + } + if reconciler.queries.Load() != 1 { + t.Fatalf("queries = %d, want the pass to have run one", reconciler.queries.Load()) + } +} + +// queryingReconcilerStub runs one query on pool, as a real pass would. +type queryingReconcilerStub struct { + pool *pgxpool.Pool + queries atomic.Int32 +} + +func (s *queryingReconcilerStub) ReconcileRequests(ctx context.Context, _ int) (requests.ReconcileResult, error) { + var one int + if err := s.pool.QueryRow(ctx, `SELECT 1`).Scan(&one); err != nil { + return requests.ReconcileResult{}, err + } + s.queries.Add(1) + return requests.ReconcileResult{}, nil +} + // Reconcile stays one pass at a time across the cluster. func TestReconcileSkipsWhileAnotherReconcileRunsPostgres(t *testing.T) { pool := requestLockTestPool(t) From 2d7372042901194ecff1144bcde38311018befb8 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:23:28 +0000 Subject: [PATCH 3/4] perf(requests): index the targets the download refresh pass looks for The download refresh pass, and its idle check on every API node, select downloading targets that have progress once a minute. Without an index on that predicate each run scanned all of media_request_targets, which grows with request history. A partial index on (request_id, download_checked_at) where status = 'downloading' and download_phase is set covers only those few rows, built concurrently. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...212208_request_target_download_refresh_index.sql | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 migrations/sql/20260928212208_request_target_download_refresh_index.sql diff --git a/migrations/sql/20260928212208_request_target_download_refresh_index.sql b/migrations/sql/20260928212208_request_target_download_refresh_index.sql new file mode 100644 index 0000000000..ec8209fa00 --- /dev/null +++ b/migrations/sql/20260928212208_request_target_download_refresh_index.sql @@ -0,0 +1,13 @@ +-- +goose NO TRANSACTION +-- +goose Up +-- The download refresh pass, and its idle check on every API node, look for +-- downloading targets that have progress. Only those few rows are indexed, so +-- neither has to scan a request history. A failed concurrent build leaves an +-- invalid index, so drop before building. +DROP INDEX CONCURRENTLY IF EXISTS public.idx_media_request_targets_download_refresh; +CREATE INDEX CONCURRENTLY idx_media_request_targets_download_refresh +ON public.media_request_targets USING btree (request_id, download_checked_at) +WHERE status = 'downloading' AND download_phase IS NOT NULL; + +-- +goose Down +DROP INDEX CONCURRENTLY IF EXISTS public.idx_media_request_targets_download_refresh; From 8a50d47ea5f4d61dda155b76f2913ed90f4c74aa Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:58:49 -0400 Subject: [PATCH 4/4] fix(requests): keep download server details out of requesters' requests (#1651) The profile-scoped v2 request operations (createRequest, listMyRequests, getRequest, cancelRequest) gave a requester every target's download server id, kind and name, the server's own id and raw status, the routing rule and the target error, plus the request's integration kind, external fields and submission error. These are admin details. They show how the admin named their servers and routing rules, and the errors can carry a plugin's raw text, such as a server URL or a release title. mediaRequestOf now takes the viewer and fills those fields for an admin only. A requester keeps the request's state and outcome_reason and each target's quality, status and download progress. An admin still sees everything, on the profile-scoped operations and on the admin request operations. /api/v1 is frozen and unchanged. The fields were already optional, so the contract diff reports no change; their descriptions now say admins only. Refs #1646 Co-authored-by: Claude Opus 5.5 (1M context) --- .../api/v2/fixtures/admin_requests_ok.json | 18 +++ .../api/v2/fixtures/cancel_request_ok.json | 1 - .../api/v2/fixtures/create_request_ok.json | 1 - .../api/v2/fixtures/get_system_info_ok.json | 2 +- .../api/v2/fixtures/list_my_requests_ok.json | 2 - contracts/api/v2/openapi.json | 13 +- docs/architecture/media-requests.md | 17 ++ internal/apiv2/admin_requests.go | 4 +- internal/apiv2/admin_requests_test.go | 39 ++++- internal/apiv2/request_lifecycle.go | 5 +- internal/apiv2/requests.go | 60 +++---- internal/apiv2/requests_test.go | 146 +++++++++++++++++- web/src/api/types.ts | 2 + web/src/api/v2/schema.ts | 21 ++- 14 files changed, 287 insertions(+), 44 deletions(-) diff --git a/contracts/api/v2/fixtures/admin_requests_ok.json b/contracts/api/v2/fixtures/admin_requests_ok.json index 6d2e7fb825..551cd3f907 100644 --- a/contracts/api/v2/fixtures/admin_requests_ok.json +++ b/contracts/api/v2/fixtures/admin_requests_ok.json @@ -20,9 +20,15 @@ { "id": "42", "request_id": "r-3", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } @@ -51,9 +57,15 @@ { "id": "42", "request_id": "r-2", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } @@ -82,9 +94,15 @@ { "id": "42", "request_id": "r-1", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } diff --git a/contracts/api/v2/fixtures/cancel_request_ok.json b/contracts/api/v2/fixtures/cancel_request_ok.json index d1c73648b3..212803d679 100644 --- a/contracts/api/v2/fixtures/cancel_request_ok.json +++ b/contracts/api/v2/fixtures/cancel_request_ok.json @@ -12,7 +12,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { diff --git a/contracts/api/v2/fixtures/create_request_ok.json b/contracts/api/v2/fixtures/create_request_ok.json index 6f706cbf8a..9f05dc9465 100644 --- a/contracts/api/v2/fixtures/create_request_ok.json +++ b/contracts/api/v2/fixtures/create_request_ok.json @@ -12,7 +12,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [], "created_at": "2026-01-02T03:04:05.678Z", diff --git a/contracts/api/v2/fixtures/get_system_info_ok.json b/contracts/api/v2/fixtures/get_system_info_ok.json index 1d645f16f0..e94c54a5f9 100644 --- a/contracts/api/v2/fixtures/get_system_info_ok.json +++ b/contracts/api/v2/fixtures/get_system_info_ok.json @@ -1,7 +1,7 @@ { "server_version": "unavailable", "api_major": 2, - "contract_digest": "8348e2ab6b2f48273ea3734b2c955923f5a8cb5e218c1be06af7c465b294c575", + "contract_digest": "2d062f1cc2421ddb2af274137f1c243c82eedfc881dce028c2f2f93def30a6ad", "links": { "openapi": "/api/v2/openapi.json", "capabilities": "/api/v2/capabilities", diff --git a/contracts/api/v2/fixtures/list_my_requests_ok.json b/contracts/api/v2/fixtures/list_my_requests_ok.json index 01624f3db0..67a73c0b0a 100644 --- a/contracts/api/v2/fixtures/list_my_requests_ok.json +++ b/contracts/api/v2/fixtures/list_my_requests_ok.json @@ -14,7 +14,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { @@ -45,7 +44,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { diff --git a/contracts/api/v2/openapi.json b/contracts/api/v2/openapi.json index 03e7db0d33..41d2d90e99 100644 --- a/contracts/api/v2/openapi.json +++ b/contracts/api/v2/openapi.json @@ -28544,9 +28544,11 @@ "description": "How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time" }, "external_id": { + "description": "Admins only: the integration's own identifier", "type": "string" }, "external_status": { + "description": "Admins only: the status as the download server reports it", "type": "string" }, "id": { @@ -28564,6 +28566,7 @@ "type": "string" }, "integration_kind": { + "description": "Admins only: the download server's kind", "examples": [ "radarr" ], @@ -28573,6 +28576,7 @@ "type": "boolean" }, "last_error": { + "description": "Admins only: why the last submission to a download server failed. It can name servers and routing rules", "type": "string" }, "library_content_id": { @@ -36939,10 +36943,11 @@ "description": "How far this target's downloads are, while its download server reports them" }, "external_id": { - "description": "The integration's own identifier", + "description": "Admins only: the integration's own identifier", "type": "string" }, "external_status": { + "description": "Admins only: the status as the download server reports it", "type": "string" }, "id": { @@ -36954,12 +36959,15 @@ "type": "string" }, "instance_name": { + "description": "Admins only: the download server's name", "type": "string" }, "integration_id": { + "description": "Admins only: the download server holding this target", "type": "string" }, "integration_kind": { + "description": "Admins only: the download server's kind", "examples": [ "radarr" ], @@ -36969,6 +36977,7 @@ "type": "boolean" }, "last_error": { + "description": "Admins only: why the download server failed this target", "type": "string" }, "quality": { @@ -36986,7 +36995,7 @@ "type": "string" }, "route_name": { - "description": "The routing rule that sent this target to its server, as named when it was sent", + "description": "Admins only: the routing rule that sent this target to its server, as named when it was sent", "type": "string" }, "status": { diff --git a/docs/architecture/media-requests.md b/docs/architecture/media-requests.md index fa61253337..e2fd20cf11 100644 --- a/docs/architecture/media-requests.md +++ b/docs/architecture/media-requests.md @@ -465,6 +465,23 @@ accounts' failed requests are left alone as those users' history. Retrying one of them after someone else has requested the title answers `ErrAlreadyRequested`, since only one active request per title may exist. +## What a requester sees + +The v2 request operations a profile calls (`createRequest`, `listMyRequests`, +`getRequest` and `cancelRequest`) give a viewer who is not an admin the request +without its download server details: the request's `integration_kind`, +`external_id`, `external_status` and `last_error`, and each target's +`integration_id`, `integration_kind`, `instance_name`, `external_id`, +`external_status`, `route_name` and `last_error`. These name the admin's +download servers and routing rules and carry the servers' raw statuses and +errors, none of which a requester can act on. The request's `last_error` is +written for the admin who fixes the submission: it can name a server or a +routing rule, or pass on a plugin's own error text. A requester still sees the +request's state and `outcome_reason`, and each target's quality, status and +`download`. An admin sees every field, on those operations and on the +`/api/v2/admin/requests` operations. The frozen `/api/v1` request routes still +return them to everyone. + ## Admin queue The admin queue groups requests by what an admin does next, from status and diff --git a/internal/apiv2/admin_requests.go b/internal/apiv2/admin_requests.go index d5434156a9..eebffd2266 100644 --- a/internal/apiv2/admin_requests.go +++ b/internal/apiv2/admin_requests.go @@ -250,7 +250,7 @@ func registerAdminRequests(reg *Registry) { if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(r)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(r, v)}, nil }) } Register(reg, op(http.MethodGet, "/admin/request-settings", opGetAdminRequestSettings, false), reg.getAdminRequestSettings) @@ -306,7 +306,7 @@ func (reg *Registry) listAdminRequests(ctx context.Context, cursors *Cursors, in } items := make([]MediaRequest, 0, len(rows)) for _, r := range rows { - items = append(items, mediaRequestOf(r)) + items = append(items, mediaRequestOf(r, v)) } return &MediaRequestCollectionOutput{Body: MediaRequestCollection{Collection: Paginated(items, next)}}, nil } diff --git a/internal/apiv2/admin_requests_test.go b/internal/apiv2/admin_requests_test.go index dc91b3013e..adbf765c49 100644 --- a/internal/apiv2/admin_requests_test.go +++ b/internal/apiv2/admin_requests_test.go @@ -22,6 +22,9 @@ type fakeAdminRequests struct { filter mediarequests.ListFilter action, reason, requestID string probedBaseURL string + // failed gives every returned request the errors a failed submission + // leaves (withSubmissionErrors). + failed bool } func fixtureAdminRequests() *fakeAdminRequests { @@ -130,7 +133,7 @@ func TestAdminRequestOptionsUnreachableIntegration(t *testing.T) { func (f *fakeAdminRequests) ListAdmin(_ context.Context, v mediarequests.Viewer, filter mediarequests.ListFilter) ([]*mediarequests.Request, error) { f.viewer = v f.filter = filter - rows := []*mediarequests.Request{fixtureMediaRequest("r-3", 3), fixtureMediaRequest("r-2", 2), fixtureMediaRequest("r-1", 1)} + rows := []*mediarequests.Request{f.request("r-3", 3), f.request("r-2", 2), f.request("r-1", 1)} out := []*mediarequests.Request{} for _, r := range rows { if filter.Before != nil && r.ID >= filter.Before.ID { @@ -147,7 +150,14 @@ func (f *fakeAdminRequests) moderate(v mediarequests.Viewer, action, id, reason f.viewer = v f.action, f.requestID, f.reason = action, id, reason f.writes++ - return fixtureMediaRequest(id, 1), nil + return f.request(id, 1), nil +} +func (f *fakeAdminRequests) request(id string, tmdbID int) *mediarequests.Request { + r := fixtureMediaRequest(id, tmdbID) + if f.failed { + withSubmissionErrors(r) + } + return r } func (f *fakeAdminRequests) Approve(_ context.Context, v mediarequests.Viewer, id string) (*mediarequests.Request, error) { return f.moderate(v, "approve", id, "") @@ -319,6 +329,31 @@ func TestAdminRequestLimitsModerationAndOptions(t *testing.T) { t.Fatalf("validation %+v", p) } } + +// The admin request operations carry every download server detail. +func TestAdminRequestsCarryDownloadServerDetails(t *testing.T) { + f := fixtureAdminRequests() + f.failed = true + h := adminRequestsHandler(f) + rec := do(t, h, http.MethodGet, Prefix+"/admin/requests", "", actingRequestAdmin) + var page struct { + Items []map[string]any `json:"items"` + } + decodeBody(t, rec.Body, &page) + if rec.Code != http.StatusOK || len(page.Items) != 3 { + t.Fatalf("listAdminRequests: %d %s", rec.Code, rec.Body.String()) + } + for _, item := range page.Items { + if targets, _ := item["targets"].([]any); len(targets) == 0 { + t.Fatalf("listAdminRequests: no targets in %v", item) + } + requireAdminMembers(t, "listAdminRequests", item, adminRequestMembers, adminTargetMembers) + } + for _, action := range []string{"approve", "decline", "cancel", "retry"} { + got := requireTargets(t, action, do(t, h, http.MethodPost, Prefix+"/admin/requests/r-1/"+action, `{}`, actingRequestAdmin)) + requireAdminMembers(t, action, got, adminRequestMembers, adminTargetMembers) + } +} func TestAdminRequestCursorBoundaries(t *testing.T) { f := fixtureAdminRequests() h := adminRequestsHandler(f) diff --git a/internal/apiv2/request_lifecycle.go b/internal/apiv2/request_lifecycle.go index 21fad4600c..6ba0f69473 100644 --- a/internal/apiv2/request_lifecycle.go +++ b/internal/apiv2/request_lifecycle.go @@ -194,11 +194,12 @@ func registerRequestLifecycle(reg *Registry, requests RequestLifecycleService, p if requests == nil { return nil, unavailable("requests") } - result, err := requests.Cancel(ctx, lifecycleViewer(ctx), string(in.ID), in.Body.Reason) + viewer := lifecycleViewer(ctx) + result, err := requests.Cancel(ctx, viewer, string(in.ID), in.Body.Reason) if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(result)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(result, viewer)}, nil }) scope := func(ctx context.Context) (int, string, error) { if providers == nil { diff --git a/internal/apiv2/requests.go b/internal/apiv2/requests.go index 7b5554050b..39e4151e7e 100644 --- a/internal/apiv2/requests.go +++ b/internal/apiv2/requests.go @@ -184,20 +184,21 @@ type DiscoverBrowsePage struct { } // RequestTarget is one fulfillment of a request against one integration -// instance at one quality. +// instance at one quality. The download server details are for admins only: +// see mediaRequestOf. type RequestTarget struct { ID ID `json:"id" example:"42"` RequestID ID `json:"request_id" example:"1834729"` - IntegrationID string `json:"integration_id,omitempty"` - IntegrationKind string `json:"integration_kind,omitempty" example:"radarr"` - InstanceName string `json:"instance_name,omitempty"` + IntegrationID string `json:"integration_id,omitempty" doc:"Admins only: the download server holding this target"` + IntegrationKind string `json:"integration_kind,omitempty" doc:"Admins only: the download server's kind" example:"radarr"` + InstanceName string `json:"instance_name,omitempty" doc:"Admins only: the download server's name"` Quality string `json:"quality" example:"1080p"` IsAnime bool `json:"is_anime"` - ExternalID string `json:"external_id,omitempty" doc:"The integration's own identifier"` - ExternalStatus string `json:"external_status,omitempty"` + ExternalID string `json:"external_id,omitempty" doc:"Admins only: the integration's own identifier"` + ExternalStatus string `json:"external_status,omitempty" doc:"Admins only: the status as the download server reports it"` Status string `json:"status" example:"queued"` - LastError string `json:"last_error,omitempty"` - RouteName string `json:"route_name,omitempty" doc:"The routing rule that sent this target to its server, as named when it was sent"` + LastError string `json:"last_error,omitempty" doc:"Admins only: why the download server failed this target"` + RouteName string `json:"route_name,omitempty" doc:"Admins only: the routing rule that sent this target to its server, as named when it was sent"` CreatedAt Instant `json:"created_at" example:"2026-01-02T03:04:05.000Z"` UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` // Download is set while the target's router plugin reports progress. @@ -225,13 +226,13 @@ type MediaRequest struct { OutcomeReason string `json:"outcome_reason,omitempty" doc:"Why the request was declined or withdrawn, when a reason was given"` RequestedByUserID ID `json:"requested_by_user_id,omitempty" example:"1"` RequestedByProfileID ID `json:"requested_by_profile_id,omitempty" example:"p-owner"` - IntegrationKind string `json:"integration_kind,omitempty" example:"radarr"` + IntegrationKind string `json:"integration_kind,omitempty" doc:"Admins only: the download server's kind" example:"radarr"` IsAnime bool `json:"is_anime"` Targets []RequestTarget `json:"targets" doc:"Empty, never null"` - ExternalID string `json:"external_id,omitempty"` - ExternalStatus string `json:"external_status,omitempty"` + ExternalID string `json:"external_id,omitempty" doc:"Admins only: the integration's own identifier"` + ExternalStatus string `json:"external_status,omitempty" doc:"Admins only: the status as the download server reports it"` LibraryContentID string `json:"library_content_id,omitempty" doc:"The catalog item once the media is in the library"` - LastError string `json:"last_error,omitempty"` + LastError string `json:"last_error,omitempty" doc:"Admins only: why the last submission to a download server failed. It can name servers and routing rules"` CreatedAt Instant `json:"created_at" example:"2026-01-02T03:04:05.000Z"` UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` ApprovedAt *Instant `json:"approved_at,omitempty"` @@ -522,7 +523,7 @@ func (reg *Registry) createRequest(ctx context.Context, in *MediaRequestCreateIn if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(req)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(req, viewer)}, nil } // listMyRequests pages by the last emitted creation time and unique request ID. @@ -568,7 +569,7 @@ func (reg *Registry) listMyRequests(ctx context.Context, cursors *Cursors, in *M } items := make([]MediaRequest, 0, len(rows)) for _, r := range rows { - items = append(items, mediaRequestOf(r)) + items = append(items, mediaRequestOf(r, viewer)) } return &MediaRequestCollectionOutput{Body: MediaRequestCollection{Collection: Paginated(items, next)}}, nil } @@ -583,7 +584,7 @@ func (reg *Registry) getRequest(ctx context.Context, in *MediaRequestGetInput) ( if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(req)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(req, viewer)}, nil } // searchRequestMedia is v1 GET /requests/search. @@ -799,7 +800,12 @@ func requestProblem(err error) *Problem { return NewProblem(TypeInternalError, "An unexpected error occurred.") } -func mediaRequestOf(r *mediarequests.Request) MediaRequest { +// mediaRequestOf maps a request for the viewer. The download server details +// (which server and routing rule took each target, the server's own ids and +// raw statuses, and the submission and target errors, which can name servers +// and routing rules) go to an admin only. A requester keeps each target's +// quality, status and download progress. +func mediaRequestOf(r *mediarequests.Request, viewer mediarequests.Viewer) MediaRequest { out := MediaRequest{ ID: ID(r.ID), Provider: r.Provider, @@ -818,31 +824,33 @@ func mediaRequestOf(r *mediarequests.Request) MediaRequest { Seasons: NonNil(r.Seasons), SeasonProgress: requestSeasonProgressOf(r.SeasonProgress), OutcomeReason: r.OutcomeReason, - IntegrationKind: r.IntegrationKind, IsAnime: r.IsAnime, Targets: make([]RequestTarget, 0, len(r.Targets)), - ExternalID: r.ExternalID, - ExternalStatus: r.ExternalStatus, LibraryContentID: r.LibraryContentID, - LastError: r.LastError, CreatedAt: NewInstant(r.CreatedAt), UpdatedAt: NewInstant(r.UpdatedAt), ApprovedAt: instantPtr(r.ApprovedAt), CompletedAt: instantPtr(r.CompletedAt), Download: requestDownloadOf(r.Download()), } + if viewer.IsAdmin { + out.IntegrationKind, out.ExternalID, out.ExternalStatus, out.LastError = r.IntegrationKind, r.ExternalID, r.ExternalStatus, r.LastError + } if r.RequestedByUserID != 0 { out.RequestedByUserID = IDFromInt(int64(r.RequestedByUserID)) } out.RequestedByProfileID = ID(r.RequestedByProfileID) for _, t := range r.Targets { - out.Targets = append(out.Targets, RequestTarget{ - ID: IDFromInt(t.ID), RequestID: ID(t.RequestID), IntegrationID: t.IntegrationID, - IntegrationKind: t.IntegrationKind, InstanceName: t.InstanceName, Quality: string(t.Quality), - IsAnime: t.IsAnime, ExternalID: t.ExternalID, ExternalStatus: t.ExternalStatus, Status: string(t.Status), - LastError: t.LastError, RouteName: t.RouteName, CreatedAt: NewInstant(t.CreatedAt), UpdatedAt: NewInstant(t.UpdatedAt), + target := RequestTarget{ + ID: IDFromInt(t.ID), RequestID: ID(t.RequestID), Quality: string(t.Quality), IsAnime: t.IsAnime, + Status: string(t.Status), CreatedAt: NewInstant(t.CreatedAt), UpdatedAt: NewInstant(t.UpdatedAt), Download: requestDownloadOf(t.Download), - }) + } + if viewer.IsAdmin { + target.IntegrationID, target.IntegrationKind, target.InstanceName = t.IntegrationID, t.IntegrationKind, t.InstanceName + target.ExternalID, target.ExternalStatus, target.LastError, target.RouteName = t.ExternalID, t.ExternalStatus, t.LastError, t.RouteName + } + out.Targets = append(out.Targets, target) } return out } diff --git a/internal/apiv2/requests_test.go b/internal/apiv2/requests_test.go index e2a4eb93ac..697dec25b0 100644 --- a/internal/apiv2/requests_test.go +++ b/internal/apiv2/requests_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "net/http" + "net/http/httptest" "slices" "strings" "testing" @@ -28,13 +29,20 @@ type fakeRequests struct { lastArgs []any } +// fixtureMediaRequest is an approved movie request with one queued target. +// The target names its download server and routing rule, which only an +// admin sees. func fixtureMediaRequest(id string, tmdbID int) *mediarequests.Request { year := 1995 approved := fixedTime() return &mediarequests.Request{ ID: id, Provider: "tmdb", MediaType: mediarequests.MediaTypeMovie, TMDBID: tmdbID, Title: "Heat", Year: &year, Status: mediarequests.StatusApproved, Outcome: mediarequests.OutcomeActive, RequestedByUserID: 1, RequestedByProfileID: "p-owner", - IntegrationKind: "radarr", Targets: []mediarequests.Target{{ID: 42, RequestID: id, Quality: mediarequests.Quality1080p, Status: mediarequests.StatusQueued, CreatedAt: fixedTime(), UpdatedAt: fixedTime()}}, + IntegrationKind: "radarr", Targets: []mediarequests.Target{{ + ID: 42, RequestID: id, IntegrationID: "integration-1", IntegrationKind: "radarr", InstanceName: "Radarr", + Quality: mediarequests.Quality1080p, ExternalID: "7", ExternalStatus: "queued", Status: mediarequests.StatusQueued, + RouteName: "Movies", CreatedAt: fixedTime(), UpdatedAt: fixedTime(), + }}, CreatedAt: fixedTime(), UpdatedAt: fixedTime(), ApprovedAt: &approved, } } @@ -375,6 +383,142 @@ func TestGetRequest(t *testing.T) { } } +// A request's download server details are for admins only: the servers and +// routing rules a request went to, the servers' own ids and raw statuses, and +// errors that can name them. +var ( + adminRequestMembers = []string{"integration_kind", "external_id", "external_status", "last_error"} + adminTargetMembers = []string{"integration_id", "integration_kind", "instance_name", "external_id", "external_status", "last_error", "route_name"} + // serverRequestMembers and serverTargetMembers are the ones + // fixtureMediaRequest fills; withSubmissionErrors fills the rest. + serverRequestMembers = []string{"integration_kind"} + serverTargetMembers = []string{"integration_id", "integration_kind", "instance_name", "external_id", "external_status", "route_name"} + // requesterTargetMembers are what every viewer gets on a target. + requesterTargetMembers = []string{"id", "request_id", "quality", "is_anime", "status", "created_at", "updated_at"} +) + +// withSubmissionErrors adds the rest of the admin details: the errors a failed +// submission leaves, which name a server and a routing rule, and the request's +// own server fields. +func withSubmissionErrors(r *mediarequests.Request) { + r.ExternalID, r.ExternalStatus = "3", "5" + r.LastError = `route "Movies" sends to "Radarr", which is disabled` + for i := range r.Targets { + r.Targets[i].LastError = `Post "http://radarr.lan:7878/api/v3/movie": connection refused` + } +} + +// requireAdminMembers checks that a request body carries exactly the wanted +// admin-only members, on the request and on each of its targets, and that its +// targets keep what a requester sees. +func requireAdminMembers(t *testing.T, label string, req map[string]any, wantRequest, wantTarget []string) { + t.Helper() + for _, m := range adminRequestMembers { + if _, got := req[m]; got != slices.Contains(wantRequest, m) { + t.Errorf("%s: request %s present = %v", label, m, got) + } + } + targets, _ := req["targets"].([]any) + for _, raw := range targets { + target, _ := raw.(map[string]any) + for _, m := range adminTargetMembers { + if _, got := target[m]; got != slices.Contains(wantTarget, m) { + t.Errorf("%s: target %s present = %v", label, m, got) + } + } + for _, m := range requesterTargetMembers { + if _, ok := target[m]; !ok { + t.Errorf("%s: target lost %s", label, m) + } + } + } +} + +// requireTargets decodes one request body and checks it has targets. +func requireTargets(t *testing.T, label string, rec *httptest.ResponseRecorder) map[string]any { + t.Helper() + if rec.Code != http.StatusOK { + t.Fatalf("%s: %d %s", label, rec.Code, rec.Body.String()) + } + var req map[string]any + decodeBody(t, rec.Body, &req) + if targets, _ := req["targets"].([]any); len(targets) == 0 { + t.Fatalf("%s: no targets in %s", label, rec.Body.String()) + } + return req +} + +// The profile-scoped request operations leave the download server details +// out for a requester and keep them for an admin. +func TestRequestDownloadServerDetailsAreForAdmins(t *testing.T) { + svc := fixtureRequests() + for _, r := range svc.requests { + withSubmissionErrors(r) + } + deps := requestDeps(svc) + deps.RequestLifecycle = &fakeLifecycle{} + h := newTestHandler(t, deps) + admin := with(bearer(adminToken), "X-Profile-Id", "p-primary") + + // A requester sees none of them, on any operation that answers with a + // request. + created := do(t, h, http.MethodPost, "/api/v2/requests", `{"media_type":"movie","tmdb_id":7,"title":"Heat"}`, requestOwner) + if created.Code != http.StatusCreated { + t.Fatalf("createRequest: %d %s", created.Code, created.Body.String()) + } + var body map[string]any + decodeBody(t, created.Body, &body) + requireAdminMembers(t, "createRequest", body, nil, nil) + + var mine struct { + Items []map[string]any `json:"items"` + } + rec := do(t, h, http.MethodGet, "/api/v2/requests/mine", "", requestOwner) + decodeBody(t, rec.Body, &mine) + if rec.Code != http.StatusOK || len(mine.Items) != 2 { + t.Fatalf("listMyRequests: %d %s", rec.Code, rec.Body.String()) + } + for _, item := range mine.Items { + requireAdminMembers(t, "listMyRequests", item, nil, nil) + } + got := requireTargets(t, "getRequest", do(t, h, http.MethodGet, "/api/v2/requests/r-1", "", requestOwner)) + requireAdminMembers(t, "getRequest", got, nil, nil) + target := got["targets"].([]any)[0].(map[string]any) + if target["quality"] != "1080p" || target["status"] != "downloading" || target["download"] == nil { + t.Fatalf("getRequest: target = %v, want its quality, status and download", target) + } + got = requireTargets(t, "cancelRequest", do(t, h, http.MethodPost, "/api/v2/requests/r-1/cancel", `{}`, requestOwner)) + requireAdminMembers(t, "cancelRequest", got, nil, nil) + + // An admin sees every one the request has on the same operations. The + // create and cancel fakes answer with a fresh fixture, which carries no + // errors and, once created, no targets. + created = do(t, h, http.MethodPost, "/api/v2/requests", `{"media_type":"movie","tmdb_id":8,"title":"Heat"}`, admin) + if created.Code != http.StatusCreated { + t.Fatalf("admin createRequest: %d %s", created.Code, created.Body.String()) + } + var adminBody map[string]any + decodeBody(t, created.Body, &adminBody) + requireAdminMembers(t, "admin createRequest", adminBody, serverRequestMembers, nil) + var adminMine struct { + Items []map[string]any `json:"items"` + } + rec = do(t, h, http.MethodGet, "/api/v2/requests/mine", "", admin) + decodeBody(t, rec.Body, &adminMine) + if rec.Code != http.StatusOK || len(adminMine.Items) != 1 { + t.Fatalf("admin listMyRequests: %d %s", rec.Code, rec.Body.String()) + } + requireAdminMembers(t, "admin listMyRequests", adminMine.Items[0], adminRequestMembers, adminTargetMembers) + got = requireTargets(t, "admin getRequest", do(t, h, http.MethodGet, "/api/v2/requests/r-1", "", admin)) + requireAdminMembers(t, "admin getRequest", got, adminRequestMembers, adminTargetMembers) + target = got["targets"].([]any)[0].(map[string]any) + if target["instance_name"] != "Radarr" || target["route_name"] != "Movies" || target["last_error"] != `Post "http://radarr.lan:7878/api/v3/movie": connection refused` { + t.Fatalf("admin getRequest: target = %v", target) + } + got = requireTargets(t, "admin cancelRequest", do(t, h, http.MethodPost, "/api/v2/requests/r-1/cancel", `{}`, admin)) + requireAdminMembers(t, "admin cancelRequest", got, serverRequestMembers, serverTargetMembers) +} + func TestSearchRequestMedia(t *testing.T) { svc := fixtureRequests() h := newTestHandler(t, requestDeps(svc)) diff --git a/web/src/api/types.ts b/web/src/api/types.ts index a6c777d6c8..ec27f00776 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -2029,6 +2029,7 @@ export interface CreateMediaRequestInput { seasons?: number[]; } +/** The download server details (integration_*, instance_name, route_name, external_*, last_error) reach admins only. */ export interface RequestTarget { id: number; request_id: string; @@ -2048,6 +2049,7 @@ export interface RequestTarget { updated_at: string; } +/** integration_kind, external_id, external_status and last_error reach admins only. */ export interface MediaRequest { id: string; provider: string; diff --git a/web/src/api/v2/schema.ts b/web/src/api/v2/schema.ts index 9b34031718..b85b1bdbea 100644 --- a/web/src/api/v2/schema.ts +++ b/web/src/api/v2/schema.ts @@ -21200,7 +21200,9 @@ export interface components { created_at: string; /** @description How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time */ download?: components["schemas"]["RequestDownload"]; + /** @description Admins only: the integration's own identifier */ external_id?: string; + /** @description Admins only: the status as the download server reports it */ external_status?: string; /** * @description Opaque identifier @@ -21209,9 +21211,13 @@ export interface components { id: string; /** @example tt0113277 */ imdb_id?: string; - /** @example radarr */ + /** + * @description Admins only: the download server's kind + * @example radarr + */ integration_kind?: string; is_anime: boolean; + /** @description Admins only: why the last submission to a download server failed. It can name servers and routing rules */ last_error?: string; /** @description The catalog item once the media is in the library */ library_content_id?: string; @@ -24687,19 +24693,26 @@ export interface components { created_at: string; /** @description How far this target's downloads are, while its download server reports them */ download?: components["schemas"]["RequestDownload"]; - /** @description The integration's own identifier */ + /** @description Admins only: the integration's own identifier */ external_id?: string; + /** @description Admins only: the status as the download server reports it */ external_status?: string; /** * @description Opaque identifier * @example 42 */ id: string; + /** @description Admins only: the download server's name */ instance_name?: string; + /** @description Admins only: the download server holding this target */ integration_id?: string; - /** @example radarr */ + /** + * @description Admins only: the download server's kind + * @example radarr + */ integration_kind?: string; is_anime: boolean; + /** @description Admins only: why the download server failed this target */ last_error?: string; /** @example 1080p */ quality: string; @@ -24708,7 +24721,7 @@ export interface components { * @example 1834729 */ request_id: string; - /** @description The routing rule that sent this target to its server, as named when it was sent */ + /** @description Admins only: the routing rule that sent this target to its server, as named when it was sent */ route_name?: string; /** @example queued */ status: string;