From fd0619ae525f6076f90b80636adc092326e95d04 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 14:34:56 -0400 Subject: [PATCH 01/11] feat(discover): add TMDB list custom slider Adds a TMDB_LIST custom Discover slider so admins can surface any public TMDB list as a row of posters, with the usual request buttons and a browsable "show more" page. Closes #1837. v3 vs v4 investigation ---------------------- The lists people want to display are created through the TMDB v4 API. Verified against the live API that the v3 endpoint `GET /3/list/{id}` reads those v4 lists just fine, with the same status codes as the v4 endpoint (200 for public, 401 for private, 404 for unknown), the same per-item `media_type` discriminator, and 20-items-per-page pagination: GET /3/list/8542986?api_key= -> 200, media_type: tv GET /4/list/8542986 (v4 read bearer) -> 200, same item So no v4 read token and no new admin configuration are needed; the existing TheMovieDb client and its shipped v3 key are used as-is. Server ------ The internals are built as a small list-provider abstraction (server/lib/medialists) so other sources such as Trakt or MDBList can be added later: a provider resolves a list id to an ordered set of {mediaType, tmdbId} items, hydration fills in any missing TMDB payload (a no-op for TMDB's own lists, which already carry it), and the service returns a paginated, MediaSlider-compatible response. Arbitrary-URL fetching is deliberately not supported; validateListId is the single choke point where the admin-supplied slider data is checked, and TMDB list ids must be plain integers. The same check rejects bad data at slider create/update time. Caching reuses the existing cacheManager: a new medialist cache holds the fully hydrated list per provider/list/language, fresh for 6 hours and resident for 48 so a stale copy can still be served when TMDB is unreachable. Concurrent requests for the same list are collapsed into a single upstream fetch. Deleted or private lists are recorded as empty, logged once, and render an empty slider rather than failing the page. Local availability is resolved per request since it is user-specific. Frontend -------- New slider type in the Discover switch and in the Create Slider form (with the live preview the other types have), a browsable /discover/list/[listId] page mirroring the network and studio pages, and English strings via the repo's message system. Mixed movie/series lists work and list order is preserved throughout. The enum member is appended at the end of DiscoverSliderType since the values are persisted; a comment now records that constraint. --- seerr-api.yml | 67 ++++ server/api/themoviedb/index.ts | 35 ++ server/api/themoviedb/interfaces.ts | 17 + server/constants/discover.ts | 5 + server/lib/cache.ts | 10 +- server/lib/medialists/index.ts | 259 ++++++++++++ server/lib/medialists/providers/tmdb.ts | 91 +++++ server/lib/medialists/types.ts | 71 ++++ server/routes/discover.test.ts | 369 ++++++++++++++++++ server/routes/discover.ts | 41 ++ server/routes/settings/discover.ts | 45 +++ .../Discover/CreateSlider/index.tsx | 23 +- .../Discover/DiscoverList/index.tsx | 61 +++ .../Discover/DiscoverSliderEdit/index.tsx | 10 +- src/components/Discover/constants.ts | 1 + src/components/Discover/index.tsx | 10 + src/i18n/locale/en.json | 4 + src/pages/discover/list/[listId]/index.tsx | 8 + 18 files changed, 1122 insertions(+), 5 deletions(-) create mode 100644 server/lib/medialists/index.ts create mode 100644 server/lib/medialists/providers/tmdb.ts create mode 100644 server/lib/medialists/types.ts create mode 100644 server/routes/discover.test.ts create mode 100644 src/components/Discover/DiscoverList/index.tsx create mode 100644 src/pages/discover/list/[listId]/index.tsx diff --git a/seerr-api.yml b/seerr-api.yml index 4b33bf7892..e052d55c15 100644 --- a/seerr-api.yml +++ b/seerr-api.yml @@ -6257,6 +6257,73 @@ paths: - $ref: '#/components/schemas/MovieResult' - $ref: '#/components/schemas/TvResult' - $ref: '#/components/schemas/PersonResult' + /discover/list/{listId}: + get: + summary: Get the contents of a TMDB list + description: >- + Returns a page of the movies and series contained in a public TMDB list, + in the order defined by the list owner, in a JSON object. + tags: + - search + parameters: + - in: path + name: listId + required: true + description: Numeric ID of a public TMDB list. + schema: + type: string + example: '8542986' + - in: query + name: page + schema: + type: number + example: 1 + default: 1 + - in: query + name: language + schema: + type: string + example: en + responses: + '200': + description: Results + content: + application/json: + schema: + type: object + properties: + page: + type: number + example: 1 + totalPages: + type: number + example: 20 + totalResults: + type: number + example: 200 + list: + type: object + properties: + providerId: + type: string + example: tmdb + listId: + type: string + example: '8542986' + name: + type: string + example: My Watchlist + description: + type: string + example: Movies and series I want to watch + results: + type: array + items: + anyOf: + - $ref: '#/components/schemas/MovieResult' + - $ref: '#/components/schemas/TvResult' + '400': + description: Invalid list ID /discover/keyword/{keywordId}/movies: get: summary: Get movies from keyword diff --git a/server/api/themoviedb/index.ts b/server/api/themoviedb/index.ts index 305d6048d7..df10e669d8 100644 --- a/server/api/themoviedb/index.ts +++ b/server/api/themoviedb/index.ts @@ -12,6 +12,7 @@ import type { TmdbKeyword, TmdbKeywordSearchResponse, TmdbLanguage, + TmdbListResponse, TmdbMovieDetails, TmdbNetwork, TmdbPersonCombinedCredits, @@ -1063,6 +1064,40 @@ class TheMovieDb extends ExternalAPI implements TvShowProvider { } } + /** + * Fetches a single page of a TMDB list. + * + * The v3 `/list/{listId}` endpoint reads both classic v3 lists and lists + * created through the v4 API (as long as they are public), so no v4 read + * token is required. Returns `null` when the list does not exist or is not + * publicly readable. + */ + public async getList({ + listId, + page = 1, + language = this.locale, + }: { + listId: number; + page?: number; + language?: string; + }): Promise { + try { + const data = await this.get(`/list/${listId}`, { + params: { page, language }, + }); + + return data; + } catch (e) { + // 401 is returned for private lists, 404 for lists that do not exist + if (e.response?.status === 401 || e.response?.status === 404) { + return null; + } + throw new Error(`[TMDB] Failed to fetch list: ${e.message}`, { + cause: e, + }); + } + } + public async getMovieGenres({ language = this.locale, }: { diff --git a/server/api/themoviedb/interfaces.ts b/server/api/themoviedb/interfaces.ts index a4c49e9ab0..03e8162301 100644 --- a/server/api/themoviedb/interfaces.ts +++ b/server/api/themoviedb/interfaces.ts @@ -87,6 +87,23 @@ export interface TmdbExternalIdResponse { person_results: TmdbPersonResult[]; } +/** + * Response shape of `GET /3/list/{listId}`. + * + * The v3 endpoint serves both classic v3 lists and the newer v4 lists. v4 lists + * can hold both movies and series, so every item carries its own `media_type`. + */ +export interface TmdbListResponse extends TmdbPaginatedResponse { + id: number; + name: string; + description?: string; + created_by?: string; + iso_639_1?: string; + poster_path?: string; + item_count: number; + items: (TmdbMovieResult | TmdbTvResult)[]; +} + export interface TmdbCreditCast { cast_id: number; character: string; diff --git a/server/constants/discover.ts b/server/constants/discover.ts index fda0682243..8a14c1677f 100644 --- a/server/constants/discover.ts +++ b/server/constants/discover.ts @@ -1,5 +1,9 @@ import type DiscoverSlider from '@server/entity/DiscoverSlider'; +/** + * These values are persisted in the database. Only ever append new members to + * this enum; never reorder or remove existing ones. + */ export enum DiscoverSliderType { RECENTLY_ADDED = 1, RECENT_REQUESTS, @@ -22,6 +26,7 @@ export enum DiscoverSliderType { TMDB_NETWORK, TMDB_MOVIE_STREAMING_SERVICES, TMDB_TV_STREAMING_SERVICES, + TMDB_LIST, } export const defaultSliders: Partial[] = [ diff --git a/server/lib/cache.ts b/server/lib/cache.ts index 64b5c79ee8..afaabb4717 100644 --- a/server/lib/cache.ts +++ b/server/lib/cache.ts @@ -10,7 +10,8 @@ export type AvailableCacheIds = | 'plexguid' | 'plextv' | 'plexwatchlist' - | 'tvdb'; + | 'tvdb' + | 'medialist'; const DEFAULT_TTL = 300; const DEFAULT_CHECK_PERIOD = 120; @@ -75,6 +76,13 @@ class CacheManager { stdTtl: 21600, checkPeriod: 60 * 30, }), + // Entries stay resident for 48 hours so that a stale list can still be + // served when the upstream provider is unreachable. Freshness is tracked + // separately by the media list service. + medialist: new Cache('medialist', 'Media Lists', { + stdTtl: 86400 * 2, + checkPeriod: 60 * 30, + }), }; public getCache(id: AvailableCacheIds): Cache { diff --git a/server/lib/medialists/index.ts b/server/lib/medialists/index.ts new file mode 100644 index 0000000000..8792260690 --- /dev/null +++ b/server/lib/medialists/index.ts @@ -0,0 +1,259 @@ +import TheMovieDb from '@server/api/themoviedb'; +import Media from '@server/entity/Media'; +import type { User } from '@server/entity/User'; +import cacheManager from '@server/lib/cache'; +import { TmdbListProvider } from '@server/lib/medialists/providers/tmdb'; +import type { + HydratedMediaList, + HydratedMediaListItem, + MediaListItem, + MediaListProvider, + MediaListResponse, +} from '@server/lib/medialists/types'; +import logger from '@server/logger'; +import { + mapMovieDetailsToResult, + mapMovieResult, + mapTvDetailsToResult, + mapTvResult, +} from '@server/models/Search'; + +/** Serve the cached list without refetching for this long. */ +const FRESH_TTL_MS = 6 * 60 * 60 * 1000; + +/** Number of items returned per page, matching TMDB's discover endpoints. */ +export const MEDIA_LIST_PAGE_SIZE = 20; + +/** How many missing TMDB payloads we resolve concurrently during hydration. */ +const HYDRATION_CONCURRENCY = 10; + +const providers: Record = { + tmdb: new TmdbListProvider(), +}; + +export const getMediaListProvider = ( + providerId: string +): MediaListProvider | undefined => providers[providerId]; + +export const getMediaListCacheKey = ( + providerId: string, + listId: string, + language?: string +): string => `${providerId}:${listId}:${language ?? ''}`; + +interface CachedMediaList { + /** `null` records a list that is missing or private, so we stop hammering. */ + list: HydratedMediaList | null; + fetchedAt: number; + /** Set once we have logged the unavailability, to keep the log to one line. */ + loggedUnavailable?: boolean; +} + +const inFlight = new Map>(); + +/** + * Fills in the TMDB payload for any item a provider could only resolve to an + * id. Providers backed by TMDB's own lists return fully populated items, so + * this is a no-op for them. Items that cannot be resolved are dropped rather + * than rendered as blanks. + */ +const hydrateItems = async ( + items: MediaListItem[], + language?: string +): Promise => { + const missing = items.filter((item) => !item.tmdbResult); + + if (missing.length === 0) { + return items as HydratedMediaListItem[]; + } + + const tmdb = new TheMovieDb(); + const resolved = new Map(); + + for (let i = 0; i < missing.length; i += HYDRATION_CONCURRENCY) { + const chunk = missing.slice(i, i + HYDRATION_CONCURRENCY); + + await Promise.all( + chunk.map(async (item) => { + try { + const hydrated: HydratedMediaListItem = + item.mediaType === 'movie' + ? { + mediaType: 'movie', + tmdbId: item.tmdbId, + tmdbResult: mapMovieDetailsToResult( + await tmdb.getMovie({ movieId: item.tmdbId, language }) + ), + } + : { + mediaType: 'tv', + tmdbId: item.tmdbId, + tmdbResult: mapTvDetailsToResult( + await tmdb.getTvShow({ tvId: item.tmdbId, language }) + ), + }; + + resolved.set(`${item.mediaType}:${item.tmdbId}`, hydrated); + } catch (e) { + logger.debug('Failed to hydrate media list item', { + label: 'Media List', + mediaType: item.mediaType, + tmdbId: item.tmdbId, + errorMessage: e.message, + }); + } + }) + ); + } + + return items.reduce((acc, item) => { + if (item.tmdbResult) { + acc.push(item as HydratedMediaListItem); + return acc; + } + + const hydrated = resolved.get(`${item.mediaType}:${item.tmdbId}`); + if (hydrated) { + acc.push(hydrated); + } + + return acc; + }, []); +}; + +const fetchAndCache = async ( + provider: MediaListProvider, + listId: string, + language: string | undefined, + cacheKey: string, + stale: CachedMediaList | undefined +): Promise => { + const cache = cacheManager.getCache('medialist').data; + + try { + const list = await provider.fetchList(listId, { language }); + + const entry: CachedMediaList = { + list: list + ? { ...list, items: await hydrateItems(list.items, language) } + : null, + fetchedAt: Date.now(), + loggedUnavailable: stale?.loggedUnavailable, + }; + + if (!list && !entry.loggedUnavailable) { + logger.warn( + 'Media list is unavailable; it may have been deleted or made private. The slider will render empty.', + { label: 'Media List', provider: provider.id, listId } + ); + entry.loggedUnavailable = true; + } + + cache.set(cacheKey, entry); + return entry; + } catch (e) { + // Serve the stale copy rather than failing the whole discover page. + if (stale) { + logger.debug('Failed to refresh media list; serving stale data', { + label: 'Media List', + provider: provider.id, + listId, + errorMessage: e.message, + }); + return stale; + } + + throw e; + } +}; + +const getMediaList = async ( + provider: MediaListProvider, + listId: string, + language?: string +): Promise => { + const cacheKey = getMediaListCacheKey(provider.id, listId, language); + const cache = cacheManager.getCache('medialist').data; + const cached = cache.get(cacheKey); + + if (cached && Date.now() - cached.fetchedAt < FRESH_TTL_MS) { + return cached.list; + } + + // Single-flight: concurrent requests for the same list share one fetch. + const existing = inFlight.get(cacheKey); + if (existing) { + return (await existing).list; + } + + const pending = fetchAndCache( + provider, + listId, + language, + cacheKey, + cached + ).finally(() => { + inFlight.delete(cacheKey); + }); + + inFlight.set(cacheKey, pending); + + return (await pending).list; +}; + +/** + * Resolves a single page of a provider list, mapped into the same shape the + * discover endpoints return so that `MediaSlider` can consume it directly. + * + * The list itself is cached per provider/list/language; the local availability + * of each title is resolved per request because it depends on the user. + */ +export const getMediaListPage = async ({ + provider, + listId, + page = 1, + language, + user, +}: { + provider: MediaListProvider; + listId: string; + page?: number; + language?: string; + user?: User; +}): Promise => { + const list = await getMediaList(provider, listId, language); + const items = list?.items ?? []; + + const currentPage = Math.max(1, page); + const offset = (currentPage - 1) * MEDIA_LIST_PAGE_SIZE; + const pageItems = items.slice(offset, offset + MEDIA_LIST_PAGE_SIZE); + + const media = await Media.getRelatedMedia( + user, + pageItems.map((item) => ({ + tmdbId: item.tmdbId, + mediaType: item.mediaType, + })) + ); + + return { + page: currentPage, + totalPages: Math.max(1, Math.ceil(items.length / MEDIA_LIST_PAGE_SIZE)), + totalResults: items.length, + list: { + providerId: provider.id, + listId, + name: list?.name, + description: list?.description, + }, + results: pageItems.map((item) => { + const relatedMedia = media.find( + (m) => m.tmdbId === item.tmdbId && m.mediaType === item.mediaType + ); + + return item.mediaType === 'movie' + ? mapMovieResult(item.tmdbResult, relatedMedia) + : mapTvResult(item.tmdbResult, relatedMedia); + }), + }; +}; diff --git a/server/lib/medialists/providers/tmdb.ts b/server/lib/medialists/providers/tmdb.ts new file mode 100644 index 0000000000..3f7cd0c35d --- /dev/null +++ b/server/lib/medialists/providers/tmdb.ts @@ -0,0 +1,91 @@ +import TheMovieDb from '@server/api/themoviedb'; +import type { + MediaList, + MediaListItem, + MediaListProvider, +} from '@server/lib/medialists/types'; + +/** + * TMDB list ids are plain integers. Anything else is rejected before we make an + * outbound request, which keeps admin-supplied slider data from being used to + * build arbitrary URLs. + */ +const TMDB_LIST_ID_REGEX = /^[1-9]\d{0,11}$/; + +/** + * TMDB serves lists 20 items at a time. We cap how far we page to keep a single + * cold cache fill bounded; sliders only ever surface the first handful of pages. + */ +const MAX_PAGES = 25; + +export class TmdbListProvider implements MediaListProvider { + public readonly id = 'tmdb'; + + public validateListId(listId: string): boolean { + return TMDB_LIST_ID_REGEX.test(listId); + } + + public async fetchList( + listId: string, + { language }: { language?: string } = {} + ): Promise { + if (!this.validateListId(listId)) { + return null; + } + + const tmdb = new TheMovieDb(); + const numericListId = Number(listId); + + const firstPage = await tmdb.getList({ + listId: numericListId, + page: 1, + language, + }); + + if (!firstPage) { + return null; + } + + const pages = [firstPage]; + const totalPages = Math.min(firstPage.total_pages ?? 1, MAX_PAGES); + + for (let page = 2; page <= totalPages; page++) { + const nextPage = await tmdb.getList({ + listId: numericListId, + page, + language, + }); + + if (!nextPage) { + break; + } + + pages.push(nextPage); + } + + return { + providerId: this.id, + listId, + name: firstPage.name, + description: firstPage.description || undefined, + // TMDB returns the items in the order defined by the list owner; we keep it. + items: pages + .flatMap((page) => page.items ?? []) + .reduce((acc, item) => { + if (item.media_type === 'movie') { + acc.push({ + mediaType: 'movie', + tmdbId: item.id, + tmdbResult: item, + }); + } else if (item.media_type === 'tv') { + acc.push({ mediaType: 'tv', tmdbId: item.id, tmdbResult: item }); + } + + return acc; + }, []), + }; + } +} + +export default TmdbListProvider; diff --git a/server/lib/medialists/types.ts b/server/lib/medialists/types.ts new file mode 100644 index 0000000000..96d3b81899 --- /dev/null +++ b/server/lib/medialists/types.ts @@ -0,0 +1,71 @@ +import type { + TmdbMovieResult, + TmdbTvResult, +} from '@server/api/themoviedb/interfaces'; +import type { MovieResult, TvResult } from '@server/models/Search'; + +export type MediaListMediaType = 'movie' | 'tv'; + +/** + * A single entry of a media list. + * + * Providers are only required to resolve `mediaType` and `tmdbId`. When a + * provider already receives the full TMDB payload (as is the case for TMDB's + * own lists), it should pass it along as `tmdbResult` so that hydration does + * not have to issue an extra request per item. + */ +export type MediaListItem = + | { mediaType: 'movie'; tmdbId: number; tmdbResult?: TmdbMovieResult } + | { mediaType: 'tv'; tmdbId: number; tmdbResult?: TmdbTvResult }; + +/** A media list item whose TMDB payload has been resolved. */ +export type HydratedMediaListItem = + | { mediaType: 'movie'; tmdbId: number; tmdbResult: TmdbMovieResult } + | { mediaType: 'tv'; tmdbId: number; tmdbResult: TmdbTvResult }; + +/** A provider list, in the order defined by the list owner. */ +export interface MediaList { + providerId: string; + listId: string; + name?: string; + description?: string; + items: TItem[]; +} + +export type HydratedMediaList = MediaList; + +/** + * A source of media lists. Implement this interface to add support for other + * list sources (Trakt, MDBList, …). Providers must never accept arbitrary URLs: + * `validateListId` is the single place where the untrusted, admin-supplied + * slider data is checked before any outbound request is made. + */ +export interface MediaListProvider { + readonly id: string; + + /** Returns true when `listId` is a well-formed identifier for this provider. */ + validateListId(listId: string): boolean; + + /** + * Resolves the complete, ordered list. Returns `null` when the list does not + * exist or is not publicly readable. + */ + fetchList( + listId: string, + options?: { language?: string } + ): Promise; +} + +/** A page of a media list, shaped for consumption by `MediaSlider`. */ +export interface MediaListResponse { + page: number; + totalPages: number; + totalResults: number; + list: { + providerId: string; + listId: string; + name?: string; + description?: string; + }; + results: (MovieResult | TvResult)[]; +} diff --git a/server/routes/discover.test.ts b/server/routes/discover.test.ts new file mode 100644 index 0000000000..0b488497ae --- /dev/null +++ b/server/routes/discover.test.ts @@ -0,0 +1,369 @@ +import assert from 'node:assert/strict'; +import { before, beforeEach, describe, it, mock } from 'node:test'; + +import TheMovieDb from '@server/api/themoviedb'; +import type { TmdbListResponse } from '@server/api/themoviedb/interfaces'; +import { DiscoverSliderType } from '@server/constants/discover'; +import { MediaStatus, MediaType } from '@server/constants/media'; +import { getRepository } from '@server/datasource'; +import Media from '@server/entity/Media'; +import cacheManager from '@server/lib/cache'; +import { setupTestDb } from '@server/test/db'; +import type { Express } from 'express'; +import express from 'express'; +import session from 'express-session'; +import request from 'supertest'; +import authRoutes from './auth'; +import discoverRoutes from './discover'; +import discoverSettingRoutes from './settings/discover'; + +import { getSettings } from '@server/lib/settings'; +import { checkUser } from '@server/middleware/auth'; + +const MOVIE_ITEM = { + id: 603, + media_type: 'movie' as const, + title: 'The Matrix', + original_title: 'The Matrix', + release_date: '1999-03-30', + adult: false, + video: false, + popularity: 50, + poster_path: '/poster-movie.jpg', + backdrop_path: '/backdrop-movie.jpg', + vote_count: 100, + vote_average: 8.2, + genre_ids: [28], + overview: 'A hacker learns the truth.', + original_language: 'en', +}; + +const TV_ITEM = { + id: 14929, + media_type: 'tv' as const, + name: 'Heartland', + original_name: 'Heartland', + first_air_date: '2007-10-14', + origin_country: ['CA'], + popularity: 34, + poster_path: '/poster-tv.jpg', + backdrop_path: '/backdrop-tv.jpg', + vote_count: 580, + vote_average: 8.3, + genre_ids: [18], + overview: 'Life on a ranch.', + original_language: 'en', +}; + +const listResponse = ( + overrides: Partial = {} +): TmdbListResponse => ({ + id: 8542986, + name: 'My Mixed List', + description: 'A list with both movies and series', + item_count: 2, + page: 1, + total_pages: 1, + total_results: 2, + // TV first, so we can assert that the list order is preserved + items: [TV_ITEM, MOVIE_ITEM], + ...overrides, +}); + +let getListMock: ReturnType['mock']; +let app: Express; + +function createApp() { + const app = express(); + app.use(express.json()); + app.use( + session({ + secret: 'test-secret', + resave: false, + saveUninitialized: false, + }) + ); + app.use(checkUser); + app.use('/auth', authRoutes); + app.use('/discover', discoverRoutes); + app.use('/settings/discover', discoverSettingRoutes); + app.use( + ( + err: { status?: number; message?: string }, + _req: express.Request, + res: express.Response, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _next: express.NextFunction + ) => { + res + .status(err.status ?? 500) + .json({ status: err.status ?? 500, message: err.message }); + } + ); + return app; +} + +before(async () => { + app = createApp(); + getListMock = mock.method(TheMovieDb.prototype, 'getList', async () => + listResponse() + ).mock; +}); + +beforeEach(() => { + getListMock.resetCalls(); + getListMock.mockImplementation(async () => listResponse()); + cacheManager.getCache('medialist').flush(); +}); + +setupTestDb(); + +async function loginAsAdmin() { + const settings = getSettings(); + const priorLocalLogin = settings.main.localLogin; + settings.main.localLogin = true; + + try { + const agent = request.agent(app); + const res = await agent + .post('/auth/local') + .send({ email: 'admin@seerr.dev', password: 'test1234' }); + assert.strictEqual(res.status, 200); + return agent; + } finally { + settings.main.localLogin = priorLocalLogin; + } +} + +describe('GET /discover/list/:listId', () => { + it('returns mixed movie/tv results in list order', async () => { + const agent = await loginAsAdmin(); + const res = await agent.get('/discover/list/8542986'); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.totalResults, 2); + assert.strictEqual(res.body.totalPages, 1); + assert.strictEqual(res.body.list.providerId, 'tmdb'); + assert.strictEqual(res.body.list.listId, '8542986'); + assert.strictEqual(res.body.list.name, 'My Mixed List'); + + assert.deepStrictEqual( + res.body.results.map((r: { id: number; mediaType: string }) => [ + r.mediaType, + r.id, + ]), + [ + ['tv', 14929], + ['movie', 603], + ] + ); + }); + + it('attaches local media info to results', async () => { + await getRepository(Media).save( + new Media({ + mediaType: MediaType.MOVIE, + tmdbId: 603, + status: MediaStatus.AVAILABLE, + status4k: MediaStatus.UNKNOWN, + }) + ); + + const agent = await loginAsAdmin(); + const res = await agent.get('/discover/list/8542986'); + + assert.strictEqual(res.status, 200); + const movie = res.body.results.find( + (r: { mediaType: string }) => r.mediaType === 'movie' + ); + assert.strictEqual(movie.mediaInfo.status, MediaStatus.AVAILABLE); + }); + + it('rejects a non-numeric list id without contacting the provider', async () => { + const agent = await loginAsAdmin(); + + for (const listId of [ + 'abc', + '..%2F..%2Fetc', + 'http:%2F%2Fevil.test', + '0', + '1a', + ]) { + const res = await agent.get(`/discover/list/${listId}`); + assert.strictEqual(res.status, 400, `expected 400 for "${listId}"`); + } + + assert.strictEqual(getListMock.callCount(), 0); + }); + + it('paginates a long list without refetching it', async () => { + const items = Array.from({ length: 45 }, (_, i) => ({ + ...MOVIE_ITEM, + id: 1000 + i, + })); + + getListMock.mockImplementation(async ({ page }: { page: number }) => { + const start = (page - 1) * 20; + return listResponse({ + item_count: items.length, + total_pages: 3, + total_results: items.length, + page, + items: items.slice(start, start + 20), + }); + }); + + const agent = await loginAsAdmin(); + + const first = await agent.get('/discover/list/8542986?page=1'); + assert.strictEqual(first.status, 200); + assert.strictEqual(first.body.totalResults, 45); + assert.strictEqual(first.body.totalPages, 3); + assert.strictEqual(first.body.results.length, 20); + assert.strictEqual(first.body.results[0].id, 1000); + + // The provider is paged through once to build the cached list + const callsAfterFirstRequest = getListMock.callCount(); + assert.strictEqual(callsAfterFirstRequest, 3); + + const third = await agent.get('/discover/list/8542986?page=3'); + assert.strictEqual(third.status, 200); + assert.strictEqual(third.body.results.length, 5); + assert.strictEqual(third.body.results[0].id, 1040); + + // Subsequent pages are served from the cache + assert.strictEqual(getListMock.callCount(), callsAfterFirstRequest); + }); + + it('serves repeat requests from the cache', async () => { + const agent = await loginAsAdmin(); + + await agent.get('/discover/list/8542986'); + assert.strictEqual(getListMock.callCount(), 1); + + await agent.get('/discover/list/8542986'); + assert.strictEqual(getListMock.callCount(), 1); + }); + + it('collapses concurrent requests into a single upstream fetch', async () => { + const agent = await loginAsAdmin(); + + const [a, b, c] = await Promise.all([ + agent.get('/discover/list/8542986'), + agent.get('/discover/list/8542986'), + agent.get('/discover/list/8542986'), + ]); + + assert.strictEqual(a.status, 200); + assert.strictEqual(b.status, 200); + assert.strictEqual(c.status, 200); + assert.strictEqual(getListMock.callCount(), 1); + }); + + it('renders empty and does not crash when the list is missing or private', async () => { + getListMock.mockImplementation(async () => null); + + const agent = await loginAsAdmin(); + const res = await agent.get('/discover/list/8542986'); + + assert.strictEqual(res.status, 200); + assert.deepStrictEqual(res.body.results, []); + assert.strictEqual(res.body.totalResults, 0); + assert.strictEqual(res.body.totalPages, 1); + }); + + it('serves stale data when the provider fails after the entry goes stale', async () => { + const agent = await loginAsAdmin(); + + const fresh = await agent.get('/discover/list/8542986'); + assert.strictEqual(fresh.body.totalResults, 2); + + // Age the cache entry past the 6 hour freshness window + const cache = cacheManager.getCache('medialist').data; + const [cacheKey] = cache.keys(); + const entry = cache.get<{ fetchedAt: number }>(cacheKey); + assert.ok(entry); + cache.set(cacheKey, { + ...entry, + fetchedAt: Date.now() - 7 * 60 * 60 * 1000, + }); + + getListMock.mockImplementation(async () => { + throw new Error('TMDB is down'); + }); + + const stale = await agent.get('/discover/list/8542986'); + assert.strictEqual(stale.status, 200); + assert.strictEqual(stale.body.totalResults, 2); + assert.strictEqual(getListMock.callCount(), 2); + }); + + it('returns 500 when the provider fails and nothing is cached', async () => { + getListMock.mockImplementation(async () => { + throw new Error('TMDB is down'); + }); + + const agent = await loginAsAdmin(); + const res = await agent.get('/discover/list/8542986'); + + assert.strictEqual(res.status, 500); + }); +}); + +describe('TMDB list slider validation', () => { + it('rejects a slider whose data is not a numeric list id', async () => { + const agent = await loginAsAdmin(); + + const res = await agent.post('/settings/discover/add').send({ + type: DiscoverSliderType.TMDB_LIST, + title: 'Evil list', + data: 'https://evil.test/list', + }); + + assert.strictEqual(res.status, 400); + }); + + it('accepts a slider with a numeric list id', async () => { + const agent = await loginAsAdmin(); + + const res = await agent.post('/settings/discover/add').send({ + type: DiscoverSliderType.TMDB_LIST, + title: 'My list', + data: '8542986', + }); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.data, '8542986'); + }); + + it('rejects an invalid list id when updating an existing slider', async () => { + const agent = await loginAsAdmin(); + + const created = await agent.post('/settings/discover/add').send({ + type: DiscoverSliderType.TMDB_LIST, + title: 'My list', + data: '8542986', + }); + assert.strictEqual(created.status, 200); + + const res = await agent.put(`/settings/discover/${created.body.id}`).send({ + type: DiscoverSliderType.TMDB_LIST, + title: 'My list', + data: 'not-a-list', + }); + + assert.strictEqual(res.status, 400); + }); + + it('leaves other slider types untouched', async () => { + const agent = await loginAsAdmin(); + + const res = await agent.post('/settings/discover/add').send({ + type: DiscoverSliderType.TMDB_SEARCH, + title: 'Search', + data: 'the matrix', + }); + + assert.strictEqual(res.status, 200); + }); +}); diff --git a/server/routes/discover.ts b/server/routes/discover.ts index 7f250e6a6e..65b126d150 100644 --- a/server/routes/discover.ts +++ b/server/routes/discover.ts @@ -11,6 +11,8 @@ import type { GenreSliderItem, WatchlistResponse, } from '@server/interfaces/api/discoverInterfaces'; +import { getMediaListPage, getMediaListProvider } from '@server/lib/medialists'; +import type { MediaListResponse } from '@server/lib/medialists/types'; import { getSettings } from '@server/lib/settings'; import logger from '@server/logger'; import { mapProductionCompany } from '@server/models/Movie'; @@ -783,6 +785,45 @@ discoverRoutes.get('/trending', async (req, res, next) => { } }); +discoverRoutes.get<{ listId: string }, MediaListResponse>( + '/list/:listId', + async (req, res, next) => { + // TMDB is currently the only supported list source. Additional providers + // can be registered in `server/lib/medialists`. + const provider = getMediaListProvider('tmdb'); + + if (!provider) { + return next({ status: 404, message: 'List provider not found.' }); + } + + if (!provider.validateListId(req.params.listId)) { + return next({ status: 400, message: 'Invalid list ID.' }); + } + + try { + const data = await getMediaListPage({ + provider, + listId: req.params.listId, + page: Number(req.query.page) || 1, + language: (req.query.language as string) ?? req.locale, + user: req.user, + }); + + return res.status(200).json(data); + } catch (e) { + logger.debug('Something went wrong retrieving a media list', { + label: 'API', + errorMessage: e.message, + listId: req.params.listId, + }); + return next({ + status: 500, + message: 'Unable to retrieve media list.', + }); + } + } +); + discoverRoutes.get<{ keywordId: string }>( '/keyword/:keywordId/movies', async (req, res, next) => { diff --git a/server/routes/settings/discover.ts b/server/routes/settings/discover.ts index 344ce72bea..2484c850cc 100644 --- a/server/routes/settings/discover.ts +++ b/server/routes/settings/discover.ts @@ -1,10 +1,35 @@ +import { DiscoverSliderType } from '@server/constants/discover'; import { getRepository } from '@server/datasource'; import DiscoverSlider from '@server/entity/DiscoverSlider'; +import { getMediaListProvider } from '@server/lib/medialists'; import logger from '@server/logger'; import { Router } from 'express'; const discoverSettingRoutes = Router(); +/** + * Validates the free-form `data` field of a slider. Returns an error message + * when the value is not usable, otherwise `null`. + * + * List sliders are the only type whose data is used to build an outbound + * request path, so it must be a well-formed provider list id and nothing else. + */ +export const validateSliderData = ( + slider: Pick +): string | null => { + if (Number(slider.type) !== DiscoverSliderType.TMDB_LIST) { + return null; + } + + const provider = getMediaListProvider('tmdb'); + + if (!slider.data || !provider?.validateListId(slider.data)) { + return 'Invalid TMDB list ID. Provide the numeric ID of a public TMDB list.'; + } + + return null; +}; + discoverSettingRoutes.post('/', async (req, res) => { const sliderRepository = getRepository(DiscoverSlider); @@ -14,6 +39,14 @@ discoverSettingRoutes.post('/', async (req, res) => { return res.status(400).json({ message: 'Invalid request body.' }); } + for (const slider of sliders) { + const validationError = validateSliderData(slider); + + if (validationError) { + return res.status(400).json({ message: validationError }); + } + } + for (let x = 0; x < sliders.length; x++) { const slider = sliders[x]; const existingSlider = await sliderRepository.findOne({ @@ -55,6 +88,12 @@ discoverSettingRoutes.post('/add', async (req, res) => { const slider = req.body as DiscoverSlider; + const validationError = validateSliderData(slider); + + if (validationError) { + return res.status(400).json({ message: validationError }); + } + const newSlider = new DiscoverSlider({ isBuiltIn: false, data: slider.data, @@ -82,6 +121,12 @@ discoverSettingRoutes.put('/:sliderId', async (req, res, next) => { const slider = req.body as DiscoverSlider; + const validationError = validateSliderData(slider); + + if (validationError) { + return res.status(400).json({ message: validationError }); + } + try { const existingSlider = await sliderRepository.findOneOrFail({ where: { diff --git a/src/components/Discover/CreateSlider/index.tsx b/src/components/Discover/CreateSlider/index.tsx index db803518fa..cb3dad3eeb 100644 --- a/src/components/Discover/CreateSlider/index.tsx +++ b/src/components/Discover/CreateSlider/index.tsx @@ -31,6 +31,7 @@ const messages = defineMessages('components.Discover.CreateSlider', { providetmdbsearch: 'Provide a search query', providetmdbstudio: 'Provide TMDB Studio ID', providetmdbnetwork: 'Provide TMDB Network ID', + providetmdblist: 'Provide TMDB List ID', addsuccess: 'Created new slider and saved discover customization settings.', addfail: 'Failed to create new slider.', editsuccess: 'Edited slider and saved discover customization settings.', @@ -38,6 +39,7 @@ const messages = defineMessages('components.Discover.CreateSlider', { needresults: 'You need to have at least 1 result.', validationDatarequired: 'You must provide a data value.', validationTitlerequired: 'You must provide a title.', + validationListidnumeric: 'The TMDB list ID must be a number.', addcustomslider: 'Create Custom Slider', searchKeywords: 'Search keywords…', searchGenres: 'Search genres…', @@ -158,9 +160,17 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => { title: Yup.string().required( intl.formatMessage(messages.validationTitlerequired) ), - data: Yup.string().required( - intl.formatMessage(messages.validationDatarequired) - ), + data: Yup.string() + .required(intl.formatMessage(messages.validationDatarequired)) + .when('sliderType', { + is: (sliderType: DiscoverSliderType | string) => + Number(sliderType) === DiscoverSliderType.TMDB_LIST, + then: (schema) => + schema.matches( + /^[1-9]\d*$/, + intl.formatMessage(messages.validationListidnumeric) + ), + }), }); const updateResultCount = useCallback( @@ -273,6 +283,13 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => { titlePlaceholderText: intl.formatMessage(messages.slidernameplaceholder), dataPlaceholderText: intl.formatMessage(messages.providetmdbnetwork), }, + { + type: DiscoverSliderType.TMDB_LIST, + title: intl.formatMessage(sliderTitles.tmdblist), + dataUrl: '/api/v1/discover/list/$value', + titlePlaceholderText: intl.formatMessage(messages.slidernameplaceholder), + dataPlaceholderText: intl.formatMessage(messages.providetmdblist), + }, { type: DiscoverSliderType.TMDB_SEARCH, title: intl.formatMessage(sliderTitles.tmdbsearch), diff --git a/src/components/Discover/DiscoverList/index.tsx b/src/components/Discover/DiscoverList/index.tsx new file mode 100644 index 0000000000..05970d41a2 --- /dev/null +++ b/src/components/Discover/DiscoverList/index.tsx @@ -0,0 +1,61 @@ +import Header from '@app/components/Common/Header'; +import ListView from '@app/components/Common/ListView'; +import PageTitle from '@app/components/Common/PageTitle'; +import useDiscover from '@app/hooks/useDiscover'; +import globalMessages from '@app/i18n/globalMessages'; +import ErrorPage from '@app/pages/_error'; +import defineMessages from '@app/utils/defineMessages'; +import type { MediaListResponse } from '@server/lib/medialists/types'; +import type { MovieResult, TvResult } from '@server/models/Search'; +import { useRouter } from 'next/router'; +import { useIntl } from 'react-intl'; + +const messages = defineMessages('components.Discover.DiscoverList', { + listTitle: 'TMDB List', +}); + +const DiscoverList = () => { + const router = useRouter(); + const intl = useIntl(); + + const { + isLoadingInitialData, + isEmpty, + isLoadingMore, + isReachingEnd, + titles, + fetchMore, + error, + firstResultData, + } = useDiscover>( + `/api/v1/discover/list/${router.query.listId}` + ); + + if (error) { + return ; + } + + const title = isLoadingInitialData + ? intl.formatMessage(globalMessages.loading) + : (firstResultData?.list.name ?? intl.formatMessage(messages.listTitle)); + + return ( + <> + +
+
{title}
+
+ 0) + } + isReachingEnd={isReachingEnd} + onScrollBottom={fetchMore} + /> + + ); +}; + +export default DiscoverList; diff --git a/src/components/Discover/DiscoverSliderEdit/index.tsx b/src/components/Discover/DiscoverSliderEdit/index.tsx index c24a0591ad..6d54b24cd4 100644 --- a/src/components/Discover/DiscoverSliderEdit/index.tsx +++ b/src/components/Discover/DiscoverSliderEdit/index.tsx @@ -10,7 +10,10 @@ import KeywordTag from '@app/components/KeywordTag'; import useToasts from '@app/hooks/useToasts'; import globalMessages from '@app/i18n/globalMessages'; import defineMessages from '@app/utils/defineMessages'; -import { MagnifyingGlassIcon } from '@heroicons/react/24/outline'; +import { + MagnifyingGlassIcon, + QueueListIcon, +} from '@heroicons/react/24/outline'; import { ArrowUturnLeftIcon, Bars3Icon, @@ -169,6 +172,8 @@ const DiscoverSliderEdit = ({ return intl.formatMessage(sliderTitles.tmdbmoviestreamingservices); case DiscoverSliderType.TMDB_TV_STREAMING_SERVICES: return intl.formatMessage(sliderTitles.tmdbtvstreamingservices); + case DiscoverSliderType.TMDB_LIST: + return intl.formatMessage(sliderTitles.tmdblist); default: return 'Unknown Slider'; } @@ -245,6 +250,9 @@ const DiscoverSliderEdit = ({ {slider.type === DiscoverSliderType.TMDB_SEARCH && ( }>{slider.data} )} + {slider.type === DiscoverSliderType.TMDB_LIST && ( + }>{slider.data} + )}
{!slider.isBuiltIn && ( diff --git a/src/components/Discover/constants.ts b/src/components/Discover/constants.ts index 4ce5e34f66..8bfdd8c5b7 100644 --- a/src/components/Discover/constants.ts +++ b/src/components/Discover/constants.ts @@ -88,6 +88,7 @@ export const sliderTitles = defineMessages('components.Discover', { tmdbsearch: 'TMDB Search', tmdbmoviestreamingservices: 'TMDB Movie Streaming Services', tmdbtvstreamingservices: 'TMDB TV Streaming Services', + tmdblist: 'TMDB List', }); export const QueryFilterOptions = z.object({ diff --git a/src/components/Discover/index.tsx b/src/components/Discover/index.tsx index 5638d6fb34..76efd7cfe7 100644 --- a/src/components/Discover/index.tsx +++ b/src/components/Discover/index.tsx @@ -355,6 +355,16 @@ const Discover = () => { /> ); break; + case DiscoverSliderType.TMDB_LIST: + sliderComponent = ( + + ); + break; case DiscoverSliderType.TMDB_SEARCH: sliderComponent = ( { + return ; +}; + +export default DiscoverListPage; From 840fc0b6f12da0b4f6781fd7a285da9ad7b164a0 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:49:05 -0400 Subject: [PATCH 02/11] refactor(discover): fetch and cache TMDB lists page by page The list provider accumulated up to 25 upstream pages into a single cache entry, which both truncated long lists (the endpoint claimed to paginate the whole list but never served past item 500) and turned one cold slider render into 25 outbound requests. Providers now resolve exactly one page: `fetchListPage(listId, { page })` requests only that page from TMDB and reports `totalPages`/`totalResults` straight from the upstream response, so the pagination contract is honest for lists of any size and a cold fill costs one request. The cache is keyed per (provider, listId, language, page) and keeps the same 6h fresh / 48h stale-on-error / single-flight semantics. Also bounds what can reach the cache: - the language dimension is no longer taken from the query string; the route pins it to the locale resolved server-side and the service canonicalises it, so the key can only take a handful of values - the cache is capped at 500 entries, evicting least-recently-fetched - `page` is parsed once and must be a safe integer >= 1, else HTTP 400 (rejects 0, -1, 1.5, Infinity, oversized values, arrays and objects) - the TMDB payload is validated at the boundary: pagination counters must be numbers, `items` an array of objects, ids safe positive integers and `media_type` one of movie/tv. A malformed payload is treated as an upstream failure, so a stale copy is served instead of being replaced with garbage, and nothing malformed is ever cached. The response now distinguishes a list that could not be read at all (`list.unavailable: true`) from one that exists and is empty, so the frontend can react differently to each. Tests go from 13 to 28: single-flight is proven with a deferred barrier (all callers pending before the upstream resolves), a rejected in-flight fetch is shown to be evicted so the next request retries, concurrent stale callers are shown to share one failed refresh, an entry past its residency window is shown not to be served, and the page/list-id validation and malformed-payload cases are enumerated. The discover router is mounted behind `isAuthenticated()` as it is in production, with an unauthenticated request asserted to be rejected. Co-Authored-By: Claude Fable 5 --- server/lib/cache.ts | 7 +- server/lib/medialists/index.ts | 150 ++++++--- server/lib/medialists/providers/tmdb.ts | 125 +++++--- server/lib/medialists/types.ts | 36 ++- server/routes/discover.test.ts | 391 +++++++++++++++++++----- server/routes/discover.ts | 19 +- 6 files changed, 558 insertions(+), 170 deletions(-) diff --git a/server/lib/cache.ts b/server/lib/cache.ts index afaabb4717..ee555e9b11 100644 --- a/server/lib/cache.ts +++ b/server/lib/cache.ts @@ -76,9 +76,10 @@ class CacheManager { stdTtl: 21600, checkPeriod: 60 * 30, }), - // Entries stay resident for 48 hours so that a stale list can still be - // served when the upstream provider is unreachable. Freshness is tracked - // separately by the media list service. + // Holds one entry per list page. Entries stay resident for 48 hours so that + // a stale page can still be served when the upstream provider is + // unreachable. Freshness, single-flight and the entry ceiling are handled + // by the media list service. medialist: new Cache('medialist', 'Media Lists', { stdTtl: 86400 * 2, checkPeriod: 60 * 30, diff --git a/server/lib/medialists/index.ts b/server/lib/medialists/index.ts index 8792260690..725f0fd479 100644 --- a/server/lib/medialists/index.ts +++ b/server/lib/medialists/index.ts @@ -4,8 +4,8 @@ import type { User } from '@server/entity/User'; import cacheManager from '@server/lib/cache'; import { TmdbListProvider } from '@server/lib/medialists/providers/tmdb'; import type { - HydratedMediaList, HydratedMediaListItem, + HydratedMediaListPage, MediaListItem, MediaListProvider, MediaListResponse, @@ -18,15 +18,22 @@ import { mapTvResult, } from '@server/models/Search'; -/** Serve the cached list without refetching for this long. */ +/** Serve the cached page without refetching for this long. */ const FRESH_TTL_MS = 6 * 60 * 60 * 1000; -/** Number of items returned per page, matching TMDB's discover endpoints. */ -export const MEDIA_LIST_PAGE_SIZE = 20; - /** How many missing TMDB payloads we resolve concurrently during hydration. */ const HYDRATION_CONCURRENCY = 10; +/** + * Upper bound on the number of cached list pages. The cache is keyed partly by + * request-controlled values (list id, page), so it needs a ceiling; the oldest + * entries are dropped first. + */ +const MAX_CACHE_ENTRIES = 500; + +/** Well-formed BCP-47-ish tags, which is all TMDB accepts. */ +const LANGUAGE_REGEX = /^([a-z]{2,3})(?:-([a-z]{2}))?$/i; + const providers: Record = { tmdb: new TmdbListProvider(), }; @@ -35,21 +42,39 @@ export const getMediaListProvider = ( providerId: string ): MediaListProvider | undefined => providers[providerId]; +/** + * Reduces a locale to a canonical form so that the cache key can only ever take + * one of a small number of values. Anything unrecognised collapses to the empty + * string, which lets the TMDB client fall back to its configured locale. + */ +export const normalizeMediaListLanguage = (language?: string): string => { + const match = LANGUAGE_REGEX.exec((language ?? '').trim()); + + if (!match) { + return ''; + } + + return match[2] + ? `${match[1].toLowerCase()}-${match[2].toUpperCase()}` + : match[1].toLowerCase(); +}; + export const getMediaListCacheKey = ( providerId: string, listId: string, - language?: string -): string => `${providerId}:${listId}:${language ?? ''}`; + language: string, + page: number +): string => `${providerId}:${listId}:${language}:${page}`; -interface CachedMediaList { +interface CachedMediaListPage { /** `null` records a list that is missing or private, so we stop hammering. */ - list: HydratedMediaList | null; + page: HydratedMediaListPage | null; fetchedAt: number; /** Set once we have logged the unavailability, to keep the log to one line. */ loggedUnavailable?: boolean; } -const inFlight = new Map>(); +const inFlight = new Map>(); /** * Fills in the TMDB payload for any item a provider could only resolve to an @@ -121,34 +146,64 @@ const hydrateItems = async ( }, []); }; +/** + * Keeps the cache under `MAX_CACHE_ENTRIES` by dropping the least recently + * fetched pages, so that a burst of distinct list/page combinations cannot grow + * it without bound. + */ +const evictOverflow = (reservedKey: string): void => { + const cache = cacheManager.getCache('medialist').data; + const keys = cache.keys().filter((key) => key !== reservedKey); + + const overflow = keys.length - (MAX_CACHE_ENTRIES - 1); + + if (overflow <= 0) { + return; + } + + keys + .map((key) => ({ + key, + fetchedAt: cache.get(key)?.fetchedAt ?? 0, + })) + .sort((a, b) => a.fetchedAt - b.fetchedAt) + .slice(0, overflow) + .forEach(({ key }) => cache.del(key)); +}; + const fetchAndCache = async ( provider: MediaListProvider, listId: string, - language: string | undefined, + page: number, + language: string, cacheKey: string, - stale: CachedMediaList | undefined -): Promise => { + stale: CachedMediaListPage | undefined +): Promise => { const cache = cacheManager.getCache('medialist').data; try { - const list = await provider.fetchList(listId, { language }); - - const entry: CachedMediaList = { - list: list - ? { ...list, items: await hydrateItems(list.items, language) } + const listPage = await provider.fetchListPage(listId, { + page, + language: language || undefined, + }); + + const entry: CachedMediaListPage = { + page: listPage + ? { ...listPage, items: await hydrateItems(listPage.items, language) } : null, fetchedAt: Date.now(), loggedUnavailable: stale?.loggedUnavailable, }; - if (!list && !entry.loggedUnavailable) { + if (!listPage && !entry.loggedUnavailable) { logger.warn( - 'Media list is unavailable; it may have been deleted or made private. The slider will render empty.', + 'Media list is unavailable; it may have been deleted or made private. The slider will be hidden.', { label: 'Media List', provider: provider.id, listId } ); entry.loggedUnavailable = true; } + evictOverflow(cacheKey); cache.set(cacheKey, entry); return entry; } catch (e) { @@ -158,6 +213,7 @@ const fetchAndCache = async ( label: 'Media List', provider: provider.id, listId, + page, errorMessage: e.message, }); return stale; @@ -167,28 +223,30 @@ const fetchAndCache = async ( } }; -const getMediaList = async ( +const getCachedListPage = async ( provider: MediaListProvider, listId: string, - language?: string -): Promise => { - const cacheKey = getMediaListCacheKey(provider.id, listId, language); + page: number, + language: string +): Promise => { + const cacheKey = getMediaListCacheKey(provider.id, listId, language, page); const cache = cacheManager.getCache('medialist').data; - const cached = cache.get(cacheKey); + const cached = cache.get(cacheKey); if (cached && Date.now() - cached.fetchedAt < FRESH_TTL_MS) { - return cached.list; + return cached.page; } - // Single-flight: concurrent requests for the same list share one fetch. + // Single-flight: concurrent requests for the same page share one fetch. const existing = inFlight.get(cacheKey); if (existing) { - return (await existing).list; + return (await existing).page; } const pending = fetchAndCache( provider, listId, + page, language, cacheKey, cached @@ -198,15 +256,16 @@ const getMediaList = async ( inFlight.set(cacheKey, pending); - return (await pending).list; + return (await pending).page; }; /** * Resolves a single page of a provider list, mapped into the same shape the * discover endpoints return so that `MediaSlider` can consume it directly. * - * The list itself is cached per provider/list/language; the local availability - * of each title is resolved per request because it depends on the user. + * Only the requested page is fetched from, and cached for, the provider; the + * local availability of each title is resolved per request because it depends + * on the user. */ export const getMediaListPage = async ({ provider, @@ -221,32 +280,35 @@ export const getMediaListPage = async ({ language?: string; user?: User; }): Promise => { - const list = await getMediaList(provider, listId, language); - const items = list?.items ?? []; - - const currentPage = Math.max(1, page); - const offset = (currentPage - 1) * MEDIA_LIST_PAGE_SIZE; - const pageItems = items.slice(offset, offset + MEDIA_LIST_PAGE_SIZE); + const normalizedLanguage = normalizeMediaListLanguage(language); + const listPage = await getCachedListPage( + provider, + listId, + page, + normalizedLanguage + ); + const items = listPage?.items ?? []; const media = await Media.getRelatedMedia( user, - pageItems.map((item) => ({ + items.map((item) => ({ tmdbId: item.tmdbId, mediaType: item.mediaType, })) ); return { - page: currentPage, - totalPages: Math.max(1, Math.ceil(items.length / MEDIA_LIST_PAGE_SIZE)), - totalResults: items.length, + page: listPage?.page ?? page, + totalPages: Math.max(1, listPage?.totalPages ?? 1), + totalResults: listPage?.totalResults ?? 0, list: { providerId: provider.id, listId, - name: list?.name, - description: list?.description, + name: listPage?.name, + description: listPage?.description, + ...(listPage ? {} : { unavailable: true }), }, - results: pageItems.map((item) => { + results: items.map((item) => { const relatedMedia = media.find( (m) => m.tmdbId === item.tmdbId && m.mediaType === item.mediaType ); diff --git a/server/lib/medialists/providers/tmdb.ts b/server/lib/medialists/providers/tmdb.ts index 3f7cd0c35d..811a869a64 100644 --- a/server/lib/medialists/providers/tmdb.ts +++ b/server/lib/medialists/providers/tmdb.ts @@ -1,7 +1,8 @@ import TheMovieDb from '@server/api/themoviedb'; +import type { TmdbListResponse } from '@server/api/themoviedb/interfaces'; import type { - MediaList, MediaListItem, + MediaListPage, MediaListProvider, } from '@server/lib/medialists/types'; @@ -12,11 +13,18 @@ import type { */ const TMDB_LIST_ID_REGEX = /^[1-9]\d{0,11}$/; -/** - * TMDB serves lists 20 items at a time. We cap how far we page to keep a single - * cold cache fill bounded; sliders only ever surface the first handful of pages. - */ -const MAX_PAGES = 25; +const isCount = (value: unknown): value is number => + typeof value === 'number' && Number.isInteger(value) && value >= 0; + +const isTmdbId = (value: unknown): value is number => + typeof value === 'number' && Number.isSafeInteger(value) && value > 0; + +class MalformedListResponseError extends Error { + constructor(reason: string) { + super(`[TMDB] Malformed list response: ${reason}`); + this.name = 'MalformedListResponseError'; + } +} export class TmdbListProvider implements MediaListProvider { public readonly id = 'tmdb'; @@ -25,65 +33,96 @@ export class TmdbListProvider implements MediaListProvider { return TMDB_LIST_ID_REGEX.test(listId); } - public async fetchList( + public async fetchListPage( listId: string, - { language }: { language?: string } = {} - ): Promise { + { page, language }: { page: number; language?: string } + ): Promise { if (!this.validateListId(listId)) { return null; } const tmdb = new TheMovieDb(); - const numericListId = Number(listId); - const firstPage = await tmdb.getList({ - listId: numericListId, - page: 1, + const response = await tmdb.getList({ + listId: Number(listId), + page, language, }); - if (!firstPage) { + if (!response) { return null; } - const pages = [firstPage]; - const totalPages = Math.min(firstPage.total_pages ?? 1, MAX_PAGES); + return this.mapResponse(listId, response); + } - for (let page = 2; page <= totalPages; page++) { - const nextPage = await tmdb.getList({ - listId: numericListId, - page, - language, - }); + /** + * Validates the upstream payload before it is allowed anywhere near the cache. + * A response that does not match the documented shape is treated as an + * upstream failure (thrown), never as an empty or partially usable list. + */ + private mapResponse( + listId: string, + response: TmdbListResponse + ): MediaListPage { + if (typeof response !== 'object') { + throw new MalformedListResponseError('response is not an object'); + } - if (!nextPage) { - break; - } + // `page` is 1-based; `total_pages`/`total_results` are 0 for an empty list. + if (!isTmdbId(response.page)) { + throw new MalformedListResponseError('page is not a positive integer'); + } + + if (!isCount(response.total_pages) || !isCount(response.total_results)) { + throw new MalformedListResponseError( + 'total_pages/total_results are not counts' + ); + } - pages.push(nextPage); + if (!Array.isArray(response.items)) { + throw new MalformedListResponseError('items is not an array'); } + const items = response.items.reduce((acc, item) => { + if (typeof item !== 'object' || item === null) { + throw new MalformedListResponseError('item is not an object'); + } + + if (!isTmdbId(item.id)) { + throw new MalformedListResponseError( + 'item id is not a positive integer' + ); + } + + if (item.media_type === 'movie') { + acc.push({ mediaType: 'movie', tmdbId: item.id, tmdbResult: item }); + } else if (item.media_type === 'tv') { + acc.push({ mediaType: 'tv', tmdbId: item.id, tmdbResult: item }); + } else { + throw new MalformedListResponseError( + `unsupported item media_type "${ + (item as { media_type?: unknown }).media_type + }"` + ); + } + + return acc; + }, []); + return { providerId: this.id, listId, - name: firstPage.name, - description: firstPage.description || undefined, + name: typeof response.name === 'string' ? response.name : undefined, + description: + typeof response.description === 'string' && response.description + ? response.description + : undefined, + page: response.page, + totalPages: response.total_pages, + totalResults: response.total_results, // TMDB returns the items in the order defined by the list owner; we keep it. - items: pages - .flatMap((page) => page.items ?? []) - .reduce((acc, item) => { - if (item.media_type === 'movie') { - acc.push({ - mediaType: 'movie', - tmdbId: item.id, - tmdbResult: item, - }); - } else if (item.media_type === 'tv') { - acc.push({ mediaType: 'tv', tmdbId: item.id, tmdbResult: item }); - } - - return acc; - }, []), + items, }; } } diff --git a/server/lib/medialists/types.ts b/server/lib/medialists/types.ts index 96d3b81899..d4c41984d8 100644 --- a/server/lib/medialists/types.ts +++ b/server/lib/medialists/types.ts @@ -23,16 +23,25 @@ export type HydratedMediaListItem = | { mediaType: 'movie'; tmdbId: number; tmdbResult: TmdbMovieResult } | { mediaType: 'tv'; tmdbId: number; tmdbResult: TmdbTvResult }; -/** A provider list, in the order defined by the list owner. */ -export interface MediaList { +/** + * One page of a provider list, in the order defined by the list owner. + * + * `totalPages` and `totalResults` describe the whole list and are reported by + * the provider; `items` only ever holds the entries of `page`. + */ +export interface MediaListPage { providerId: string; listId: string; name?: string; description?: string; + /** 1-based index of the page `items` was taken from. */ + page: number; + totalPages: number; + totalResults: number; items: TItem[]; } -export type HydratedMediaList = MediaList; +export type HydratedMediaListPage = MediaListPage; /** * A source of media lists. Implement this interface to add support for other @@ -47,13 +56,19 @@ export interface MediaListProvider { validateListId(listId: string): boolean; /** - * Resolves the complete, ordered list. Returns `null` when the list does not - * exist or is not publicly readable. + * Resolves a *single page* of the list, never the whole list: implementations + * must request only `page` upstream and report the list-wide `totalPages` and + * `totalResults` from the upstream response. + * + * Returns `null` when the list does not exist or is not publicly readable. + * Throws when the upstream request fails or answers with a payload that does + * not match the documented shape, so that callers can fall back to a stale + * copy instead of caching garbage. */ - fetchList( + fetchListPage( listId: string, - options?: { language?: string } - ): Promise; + options: { page: number; language?: string } + ): Promise; } /** A page of a media list, shaped for consumption by `MediaSlider`. */ @@ -66,6 +81,11 @@ export interface MediaListResponse { listId: string; name?: string; description?: string; + /** + * Set when the list itself could not be read (deleted, or no longer + * public). Distinguishes that case from a list that simply has no entries. + */ + unavailable?: boolean; }; results: (MovieResult | TvResult)[]; } diff --git a/server/routes/discover.test.ts b/server/routes/discover.test.ts index 0b488497ae..c46d1c1f75 100644 --- a/server/routes/discover.test.ts +++ b/server/routes/discover.test.ts @@ -8,6 +8,8 @@ import { MediaStatus, MediaType } from '@server/constants/media'; import { getRepository } from '@server/datasource'; import Media from '@server/entity/Media'; import cacheManager from '@server/lib/cache'; +import { getMediaListPage, getMediaListProvider } from '@server/lib/medialists'; +import type { MediaListProvider } from '@server/lib/medialists/types'; import { setupTestDb } from '@server/test/db'; import type { Express } from 'express'; import express from 'express'; @@ -18,7 +20,7 @@ import discoverRoutes from './discover'; import discoverSettingRoutes from './settings/discover'; import { getSettings } from '@server/lib/settings'; -import { checkUser } from '@server/middleware/auth'; +import { checkUser, isAuthenticated } from '@server/middleware/auth'; const MOVIE_ITEM = { id: 603, @@ -70,11 +72,49 @@ const listResponse = ( ...overrides, }); +const LIST_ID = '8542986'; + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (reason: unknown) => void; +} + +const createDeferred = (): Deferred => { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + + // Keep an unhandled rejection from taking the test process down before the + // assertion that awaits it runs. + promise.catch(() => undefined); + + return { promise, resolve, reject }; +}; + +const flushMicrotasks = () => new Promise((resolve) => setImmediate(resolve)); + +const tmdbProvider = (): MediaListProvider => { + const provider = getMediaListProvider('tmdb'); + assert.ok(provider); + return provider; +}; + +const cachedListKeys = () => cacheManager.getCache('medialist').data.keys(); + let getListMock: ReturnType['mock']; let app: Express; -function createApp() { +function createApp(queryParser?: 'simple' | 'extended') { const app = express(); + + if (queryParser) { + app.set('query parser', queryParser); + } + app.use(express.json()); app.use( session({ @@ -85,7 +125,8 @@ function createApp() { ); app.use(checkUser); app.use('/auth', authRoutes); - app.use('/discover', discoverRoutes); + // Mirrors how the router mounts discover in production. + app.use('/discover', isAuthenticated(), discoverRoutes); app.use('/settings/discover', discoverSettingRoutes); app.use( ( @@ -118,13 +159,13 @@ beforeEach(() => { setupTestDb(); -async function loginAsAdmin() { +async function loginAsAdmin(target: Express = app) { const settings = getSettings(); const priorLocalLogin = settings.main.localLogin; settings.main.localLogin = true; try { - const agent = request.agent(app); + const agent = request.agent(target); const res = await agent .post('/auth/local') .send({ email: 'admin@seerr.dev', password: 'test1234' }); @@ -136,16 +177,24 @@ async function loginAsAdmin() { } describe('GET /discover/list/:listId', () => { + it('rejects an unauthenticated request', async () => { + const res = await request(app).get(`/discover/list/${LIST_ID}`); + + assert.strictEqual(res.status, 403); + assert.strictEqual(getListMock.callCount(), 0); + }); + it('returns mixed movie/tv results in list order', async () => { const agent = await loginAsAdmin(); - const res = await agent.get('/discover/list/8542986'); + const res = await agent.get(`/discover/list/${LIST_ID}`); assert.strictEqual(res.status, 200); assert.strictEqual(res.body.totalResults, 2); assert.strictEqual(res.body.totalPages, 1); assert.strictEqual(res.body.list.providerId, 'tmdb'); - assert.strictEqual(res.body.list.listId, '8542986'); + assert.strictEqual(res.body.list.listId, LIST_ID); assert.strictEqual(res.body.list.name, 'My Mixed List'); + assert.strictEqual(res.body.list.unavailable, undefined); assert.deepStrictEqual( res.body.results.map((r: { id: number; mediaType: string }) => [ @@ -170,7 +219,7 @@ describe('GET /discover/list/:listId', () => { ); const agent = await loginAsAdmin(); - const res = await agent.get('/discover/list/8542986'); + const res = await agent.get(`/discover/list/${LIST_ID}`); assert.strictEqual(res.status, 200); const movie = res.body.results.find( @@ -179,7 +228,7 @@ describe('GET /discover/list/:listId', () => { assert.strictEqual(movie.mediaInfo.status, MediaStatus.AVAILABLE); }); - it('rejects a non-numeric list id without contacting the provider', async () => { + it('rejects a malformed list id without contacting the provider', async () => { const agent = await loginAsAdmin(); for (const listId of [ @@ -188,6 +237,7 @@ describe('GET /discover/list/:listId', () => { 'http:%2F%2Fevil.test', '0', '1a', + '1234567890123', ]) { const res = await agent.get(`/discover/list/${listId}`); assert.strictEqual(res.status, 400, `expected 400 for "${listId}"`); @@ -196,117 +246,318 @@ describe('GET /discover/list/:listId', () => { assert.strictEqual(getListMock.callCount(), 0); }); - it('paginates a long list without refetching it', async () => { - const items = Array.from({ length: 45 }, (_, i) => ({ - ...MOVIE_ITEM, - id: 1000 + i, - })); - - getListMock.mockImplementation(async ({ page }: { page: number }) => { - const start = (page - 1) * 20; - return listResponse({ - item_count: items.length, - total_pages: 3, - total_results: items.length, - page, - items: items.slice(start, start + 20), - }); - }); - + it('rejects a malformed page without contacting the provider', async () => { const agent = await loginAsAdmin(); - const first = await agent.get('/discover/list/8542986?page=1'); - assert.strictEqual(first.status, 200); - assert.strictEqual(first.body.totalResults, 45); - assert.strictEqual(first.body.totalPages, 3); - assert.strictEqual(first.body.results.length, 20); - assert.strictEqual(first.body.results[0].id, 1000); + const queries = [ + 'page=0', + 'page=-1', + 'page=1.5', + 'page=Infinity', + 'page=abc', + 'page=', + 'page=99999999999999999999', + // Repeated keys make `req.query.page` an array under every query parser. + 'page=1&page=2', + ]; + + for (const query of queries) { + const res = await agent.get(`/discover/list/${LIST_ID}?${query}`); + assert.strictEqual(res.status, 400, `expected 400 for "${query}"`); + } - // The provider is paged through once to build the cached list - const callsAfterFirstRequest = getListMock.callCount(); - assert.strictEqual(callsAfterFirstRequest, 3); + assert.strictEqual(getListMock.callCount(), 0); + }); - const third = await agent.get('/discover/list/8542986?page=3'); - assert.strictEqual(third.status, 200); - assert.strictEqual(third.body.results.length, 5); - assert.strictEqual(third.body.results[0].id, 1040); + it('rejects array and object page values under an extended query parser', async () => { + const agent = await loginAsAdmin(createApp('extended')); - // Subsequent pages are served from the cache - assert.strictEqual(getListMock.callCount(), callsAfterFirstRequest); + for (const query of ['page[]=1', 'page[foo]=1']) { + const res = await agent.get(`/discover/list/${LIST_ID}?${query}`); + assert.strictEqual(res.status, 400, `expected 400 for "${query}"`); + } + + assert.strictEqual(getListMock.callCount(), 0); }); - it('serves repeat requests from the cache', async () => { + it('fetches only the requested page and reports upstream totals', async () => { + getListMock.mockImplementation(async ({ page }: { page: number }) => + listResponse({ + page, + item_count: 812, + total_pages: 41, + total_results: 812, + items: [MOVIE_ITEM], + }) + ); + const agent = await loginAsAdmin(); + const res = await agent.get(`/discover/list/${LIST_ID}?page=7`); - await agent.get('/discover/list/8542986'); - assert.strictEqual(getListMock.callCount(), 1); + assert.strictEqual(res.status, 200); + // Totals come from the upstream response, not from a truncated array. + assert.strictEqual(res.body.totalResults, 812); + assert.strictEqual(res.body.totalPages, 41); + assert.strictEqual(res.body.page, 7); - await agent.get('/discover/list/8542986'); assert.strictEqual(getListMock.callCount(), 1); + assert.strictEqual( + (getListMock.calls[0].arguments[0] as { page: number }).page, + 7 + ); }); - it('collapses concurrent requests into a single upstream fetch', async () => { + it('caches each page separately and serves repeats from the cache', async () => { + getListMock.mockImplementation(async ({ page }: { page: number }) => + listResponse({ page, total_pages: 3, total_results: 45 }) + ); + const agent = await loginAsAdmin(); - const [a, b, c] = await Promise.all([ - agent.get('/discover/list/8542986'), - agent.get('/discover/list/8542986'), - agent.get('/discover/list/8542986'), - ]); + await agent.get(`/discover/list/${LIST_ID}?page=1`); + await agent.get(`/discover/list/${LIST_ID}?page=2`); + assert.strictEqual(getListMock.callCount(), 2); + + await agent.get(`/discover/list/${LIST_ID}?page=1`); + await agent.get(`/discover/list/${LIST_ID}?page=2`); + assert.strictEqual(getListMock.callCount(), 2); + assert.strictEqual(cachedListKeys().length, 2); + }); + + it('does not fetch the whole list to serve one page', async () => { + getListMock.mockImplementation(async ({ page }: { page: number }) => + listResponse({ page, total_pages: 100, total_results: 2000 }) + ); + + const agent = await loginAsAdmin(); + await agent.get(`/discover/list/${LIST_ID}?page=1`); - assert.strictEqual(a.status, 200); - assert.strictEqual(b.status, 200); - assert.strictEqual(c.status, 200); assert.strictEqual(getListMock.callCount(), 1); }); - it('renders empty and does not crash when the list is missing or private', async () => { + it('reports an unavailable list without failing the page', async () => { getListMock.mockImplementation(async () => null); const agent = await loginAsAdmin(); - const res = await agent.get('/discover/list/8542986'); + const res = await agent.get(`/discover/list/${LIST_ID}`); assert.strictEqual(res.status, 200); assert.deepStrictEqual(res.body.results, []); assert.strictEqual(res.body.totalResults, 0); assert.strictEqual(res.body.totalPages, 1); + assert.strictEqual(res.body.list.unavailable, true); }); - it('serves stale data when the provider fails after the entry goes stale', async () => { + it('returns 500 when the provider fails and nothing is cached', async () => { + getListMock.mockImplementation(async () => { + throw new Error('TMDB is down'); + }); + const agent = await loginAsAdmin(); + const res = await agent.get(`/discover/list/${LIST_ID}`); - const fresh = await agent.get('/discover/list/8542986'); - assert.strictEqual(fresh.body.totalResults, 2); + assert.strictEqual(res.status, 500); + }); +}); - // Age the cache entry past the 6 hour freshness window +describe('media list caching', () => { + const ageCacheEntry = (ms: number) => { const cache = cacheManager.getCache('medialist').data; const [cacheKey] = cache.keys(); const entry = cache.get<{ fetchedAt: number }>(cacheKey); assert.ok(entry); - cache.set(cacheKey, { - ...entry, - fetchedAt: Date.now() - 7 * 60 * 60 * 1000, + cache.set(cacheKey, { ...entry, fetchedAt: Date.now() - ms }); + return cacheKey; + }; + + it('collapses concurrent requests into a single upstream fetch', async () => { + const gate = createDeferred(); + getListMock.mockImplementation(async () => gate.promise); + + // Every caller is started before anything resolves, so the assertion below + // can only hold if they genuinely share one in-flight fetch. + const pending = [1, 2, 3].map(() => + getMediaListPage({ provider: tmdbProvider(), listId: LIST_ID }) + ); + + await flushMicrotasks(); + assert.strictEqual(getListMock.callCount(), 1); + + gate.resolve(listResponse()); + const results = await Promise.all(pending); + + assert.strictEqual(getListMock.callCount(), 1); + results.forEach((result) => assert.strictEqual(result.totalResults, 2)); + }); + + it('drops a rejected in-flight fetch so the next request retries', async () => { + const gate = createDeferred(); + getListMock.mockImplementation(async () => gate.promise); + + const failing = getMediaListPage({ + provider: tmdbProvider(), + listId: LIST_ID, }); + await flushMicrotasks(); + gate.reject(new Error('TMDB is down')); + await assert.rejects(failing); + + getListMock.mockImplementation(async () => listResponse()); + const retried = await getMediaListPage({ + provider: tmdbProvider(), + listId: LIST_ID, + }); + + assert.strictEqual(retried.totalResults, 2); + assert.strictEqual(getListMock.callCount(), 2); + }); + + it('refreshes and replaces the content once the entry is no longer fresh', async () => { + const agent = await loginAsAdmin(); + + const fresh = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(fresh.body.list.name, 'My Mixed List'); + + ageCacheEntry(7 * 60 * 60 * 1000); + + getListMock.mockImplementation(async () => + listResponse({ + name: 'Renamed List', + item_count: 1, + total_results: 1, + items: [MOVIE_ITEM], + }) + ); + + const refreshed = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(refreshed.body.list.name, 'Renamed List'); + assert.strictEqual(refreshed.body.totalResults, 1); + assert.strictEqual(getListMock.callCount(), 2); + }); + + it('serves stale data when the provider fails after the entry goes stale', async () => { + const agent = await loginAsAdmin(); + + const fresh = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(fresh.body.totalResults, 2); + + ageCacheEntry(7 * 60 * 60 * 1000); + getListMock.mockImplementation(async () => { throw new Error('TMDB is down'); }); - const stale = await agent.get('/discover/list/8542986'); + const stale = await agent.get(`/discover/list/${LIST_ID}`); assert.strictEqual(stale.status, 200); assert.strictEqual(stale.body.totalResults, 2); assert.strictEqual(getListMock.callCount(), 2); }); - it('returns 500 when the provider fails and nothing is cached', async () => { + it('does not serve an entry that has outlived its residency window', async () => { + const agent = await loginAsAdmin(); + await agent.get(`/discover/list/${LIST_ID}`); + + // Expire the entry the same way node-cache would once the 48h TTL elapses. + const cache = cacheManager.getCache('medialist').data; + const [cacheKey] = cache.keys(); + cache.set(cacheKey, cache.get(cacheKey), -1); + assert.strictEqual(cache.get(cacheKey), undefined); + getListMock.mockImplementation(async () => { throw new Error('TMDB is down'); }); + const res = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(res.status, 500); + }); + + it('shares one failed refresh between concurrent stale callers', async () => { + await getMediaListPage({ provider: tmdbProvider(), listId: LIST_ID }); + assert.strictEqual(getListMock.callCount(), 1); + + ageCacheEntry(7 * 60 * 60 * 1000); + + const gate = createDeferred(); + getListMock.mockImplementation(async () => gate.promise); + + const pending = [1, 2, 3].map(() => + getMediaListPage({ provider: tmdbProvider(), listId: LIST_ID }) + ); + + await flushMicrotasks(); + assert.strictEqual(getListMock.callCount(), 2); + + gate.reject(new Error('TMDB is down')); + const results = await Promise.all(pending); + + // One shared refresh, and every caller gets the stale copy. + assert.strictEqual(getListMock.callCount(), 2); + results.forEach((result) => assert.strictEqual(result.totalResults, 2)); + }); +}); + +describe('media list payload validation', () => { + const malformedPayloads: [string, Partial][] = [ + ['a null item', { items: [null] } as unknown as Partial], + [ + 'a non-array items field', + { items: 'nope' } as unknown as Partial, + ], + [ + 'an unsupported media_type', + { + items: [{ ...MOVIE_ITEM, media_type: 'person' }], + } as unknown as Partial, + ], + [ + 'a non-numeric item id', + { + items: [{ ...MOVIE_ITEM, id: '603' }], + } as unknown as Partial, + ], + ['a missing page', { page: undefined }], + [ + 'non-numeric pagination fields', + { total_pages: 'many' } as unknown as Partial, + ], + ]; + + for (const [label, overrides] of malformedPayloads) { + it(`returns 500 and caches nothing for ${label}`, async () => { + getListMock.mockImplementation(async () => listResponse(overrides)); + + const agent = await loginAsAdmin(); + const res = await agent.get(`/discover/list/${LIST_ID}`); + + assert.strictEqual(res.status, 500); + assert.strictEqual(cachedListKeys().length, 0); + }); + } + + it('serves the stale copy rather than a malformed refresh', async () => { const agent = await loginAsAdmin(); - const res = await agent.get('/discover/list/8542986'); - assert.strictEqual(res.status, 500); + const fresh = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(fresh.body.totalResults, 2); + + const cache = cacheManager.getCache('medialist').data; + const [cacheKey] = cache.keys(); + const entry = cache.get<{ fetchedAt: number }>(cacheKey); + assert.ok(entry); + cache.set(cacheKey, { + ...entry, + fetchedAt: Date.now() - 7 * 60 * 60 * 1000, + }); + + getListMock.mockImplementation(async () => + listResponse({ items: [null] } as unknown as Partial) + ); + + const stale = await agent.get(`/discover/list/${LIST_ID}`); + assert.strictEqual(stale.status, 200); + assert.strictEqual(stale.body.totalResults, 2); + assert.strictEqual(stale.body.results.length, 2); }); }); @@ -329,11 +580,11 @@ describe('TMDB list slider validation', () => { const res = await agent.post('/settings/discover/add').send({ type: DiscoverSliderType.TMDB_LIST, title: 'My list', - data: '8542986', + data: LIST_ID, }); assert.strictEqual(res.status, 200); - assert.strictEqual(res.body.data, '8542986'); + assert.strictEqual(res.body.data, LIST_ID); }); it('rejects an invalid list id when updating an existing slider', async () => { @@ -342,7 +593,7 @@ describe('TMDB list slider validation', () => { const created = await agent.post('/settings/discover/add').send({ type: DiscoverSliderType.TMDB_LIST, title: 'My list', - data: '8542986', + data: LIST_ID, }); assert.strictEqual(created.status, 200); diff --git a/server/routes/discover.ts b/server/routes/discover.ts index 65b126d150..816a9338fd 100644 --- a/server/routes/discover.ts +++ b/server/routes/discover.ts @@ -800,12 +800,27 @@ discoverRoutes.get<{ listId: string }, MediaListResponse>( return next({ status: 400, message: 'Invalid list ID.' }); } + // Parsed once, and strictly: the page number is part of the cache key. + const rawPage = req.query.page; + + if (rawPage !== undefined && typeof rawPage !== 'string') { + return next({ status: 400, message: 'Invalid page.' }); + } + + const page = rawPage === undefined ? 1 : Number(rawPage); + + if (!Number.isSafeInteger(page) || page < 1) { + return next({ status: 400, message: 'Invalid page.' }); + } + try { const data = await getMediaListPage({ provider, listId: req.params.listId, - page: Number(req.query.page) || 1, - language: (req.query.language as string) ?? req.locale, + page, + // Deliberately not read from the query string: the language is part of + // the cache key, so it is pinned to the locale resolved server-side. + language: req.locale, user: req.user, }); From 93debf8d776233fb357c9645f0c36ea54ba4d987 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:49:44 -0400 Subject: [PATCH 03/11] docs(api): tighten the TMDB list endpoint schema Constrains what the endpoint actually accepts and documents what it actually answers: page is an integer with a minimum of 1, listId carries the same pattern the server validates against, the language query parameter is gone (it is resolved server-side), the stable response fields are marked required, list.unavailable is described, and the 403 and 500 responses are documented alongside the existing 400. Co-Authored-By: Claude Fable 5 --- seerr-api.yml | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/seerr-api.yml b/seerr-api.yml index e052d55c15..89058d0ca7 100644 --- a/seerr-api.yml +++ b/seerr-api.yml @@ -6262,7 +6262,12 @@ paths: summary: Get the contents of a TMDB list description: >- Returns a page of the movies and series contained in a public TMDB list, - in the order defined by the list owner, in a JSON object. + in the order defined by the list owner, in a JSON object. Only the + requested page is fetched upstream; totalPages and totalResults describe + the whole list. The language is resolved server-side from the requesting + user's locale and cannot be overridden. When the list has been deleted or + is no longer public, the response is a 200 with an empty results array + and list.unavailable set to true. tags: - search parameters: @@ -6272,18 +6277,15 @@ paths: description: Numeric ID of a public TMDB list. schema: type: string + pattern: '^[1-9]\d{0,11}$' example: '8542986' - in: query name: page schema: - type: number + type: integer + minimum: 1 example: 1 default: 1 - - in: query - name: language - schema: - type: string - example: en responses: '200': description: Results @@ -6291,6 +6293,12 @@ paths: application/json: schema: type: object + required: + - page + - totalPages + - totalResults + - list + - results properties: page: type: number @@ -6303,6 +6311,9 @@ paths: example: 200 list: type: object + required: + - providerId + - listId properties: providerId: type: string @@ -6316,6 +6327,12 @@ paths: description: type: string example: Movies and series I want to watch + unavailable: + type: boolean + description: >- + Present and true when the list itself could not be + read, which is distinct from a list with no entries. + example: true results: type: array items: @@ -6323,7 +6340,11 @@ paths: - $ref: '#/components/schemas/MovieResult' - $ref: '#/components/schemas/TvResult' '400': - description: Invalid list ID + description: Invalid list ID or page + '403': + description: Unauthenticated + '500': + description: Unable to retrieve the list from the upstream provider /discover/keyword/{keywordId}/movies: get: summary: Get movies from keyword From a45d96a2e61e136f3b9b2caadc40afda791ded09 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:51:42 -0400 Subject: [PATCH 04/11] fix(discover): stop rendering a bare row for an unavailable list A TMDB list that has been deleted or made private used to render as a slider heading over an empty strip on Discover, and as the generic "No results." grid on its own page, neither of which tells anyone what happened. The list slider now sets `hideWhenEmpty`, so the row disappears instead of advertising an empty one, and the list page renders a dedicated, localized "no longer available" state, keeping the list name as the heading when it is still known. Two supporting fixes in the shared slider: - `hideWhenEmpty` was evaluated before the row had answered, because the optional chain collapsed "not fetched yet" and "fetched, empty" into the same value; it now waits for data, which also removes the crash on an empty `data` array - `isEmpty` was hardcoded to false, so an empty slider drew nothing at all rather than its empty message; it is now derived from the rendered items Co-Authored-By: Claude Fable 5 --- .../Discover/DiscoverList/index.tsx | 36 ++++++++++++++----- src/components/Discover/index.tsx | 3 ++ src/components/MediaSlider/index.tsx | 6 ++-- src/i18n/locale/en.json | 2 ++ 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/components/Discover/DiscoverList/index.tsx b/src/components/Discover/DiscoverList/index.tsx index 05970d41a2..bf2fa73310 100644 --- a/src/components/Discover/DiscoverList/index.tsx +++ b/src/components/Discover/DiscoverList/index.tsx @@ -12,6 +12,9 @@ import { useIntl } from 'react-intl'; const messages = defineMessages('components.Discover.DiscoverList', { listTitle: 'TMDB List', + listunavailable: 'This list is no longer available', + listunavailabledescription: + 'It may have been deleted, or it may no longer be public on TMDB.', }); const DiscoverList = () => { @@ -35,6 +38,10 @@ const DiscoverList = () => { return ; } + // A list that could not be read at all is a different story from a list that + // is simply empty, and deserves more than the generic "No results." grid. + const isUnavailable = !!firstResultData?.list.unavailable; + const title = isLoadingInitialData ? intl.formatMessage(globalMessages.loading) : (firstResultData?.list.name ?? intl.formatMessage(messages.listTitle)); @@ -45,15 +52,26 @@ const DiscoverList = () => {
{title}
- 0) - } - isReachingEnd={isReachingEnd} - onScrollBottom={fetchMore} - /> + {isUnavailable ? ( +
+

+ {intl.formatMessage(messages.listunavailable)} +

+

+ {intl.formatMessage(messages.listunavailabledescription)} +

+
+ ) : ( + 0) + } + isReachingEnd={isReachingEnd} + onScrollBottom={fetchMore} + /> + )} ); }; diff --git a/src/components/Discover/index.tsx b/src/components/Discover/index.tsx index 76efd7cfe7..593b3b31e7 100644 --- a/src/components/Discover/index.tsx +++ b/src/components/Discover/index.tsx @@ -362,6 +362,9 @@ const Discover = () => { title={slider.title ?? ''} url={`/api/v1/discover/list/${slider.data}`} linkUrl={`/discover/list/${slider.data}`} + // A list that was deleted or made private would otherwise leave + // a bare heading over an empty strip on everyone's Discover. + hideWhenEmpty /> ); break; diff --git a/src/components/MediaSlider/index.tsx b/src/components/MediaSlider/index.tsx index 4418c12314..abfafed8e0 100644 --- a/src/components/MediaSlider/index.tsx +++ b/src/components/MediaSlider/index.tsx @@ -98,7 +98,9 @@ const MediaSlider = ({ } }, [titles, setSize, size, data, onNewTitles]); - if (hideWhenEmpty && (data?.[0].results ?? []).length === 0) { + // Only once we know what the row holds: while `data` is undefined the row has + // not answered yet, and hiding it early would keep it from ever loading. + if (hideWhenEmpty && data && (data[0]?.results ?? []).length === 0) { return null; } @@ -192,7 +194,7 @@ const MediaSlider = ({ diff --git a/src/i18n/locale/en.json b/src/i18n/locale/en.json index dadd25bdaf..f6cf0652fd 100644 --- a/src/i18n/locale/en.json +++ b/src/i18n/locale/en.json @@ -50,6 +50,8 @@ "components.Discover.CreateSlider.validationListidnumeric": "The TMDB list ID must be a number.", "components.Discover.CreateSlider.validationTitlerequired": "You must provide a title.", "components.Discover.DiscoverList.listTitle": "TMDB List", + "components.Discover.DiscoverList.listunavailable": "This list is no longer available", + "components.Discover.DiscoverList.listunavailabledescription": "It may have been deleted, or it may no longer be public on TMDB.", "components.Discover.DiscoverMovieGenre.genreMovies": "{genre} Movies", "components.Discover.DiscoverMovieKeyword.keywordMovies": "{keywordTitle} Movies", "components.Discover.DiscoverMovieLanguage.languageMovies": "{language} Movies", From 0c6bd110249c98e2c66b9dc7fe265cc45d451283 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:53:40 -0400 Subject: [PATCH 05/11] feat(discover): make the TMDB list id field self-explanatory The list id field was a bare text input: nothing said where the number comes from, a wrong id produced a silent, permanently disabled Add button, and every keystroke fired a preview request for an id that was still being typed. - persistent help text under the field points at the number in the list URL - client-side validation now mirrors the server pattern, including the 12 digit cap, instead of accepting any positive integer - the preview waits 400ms after the last keystroke and only fires for a well-formed id - a preview that cannot be read (unknown id, private list, upstream error) renders an explicit message saying so Co-Authored-By: Claude Fable 5 --- .../Discover/CreateSlider/index.tsx | 162 ++++++++++++++---- src/i18n/locale/en.json | 4 +- 2 files changed, 132 insertions(+), 34 deletions(-) diff --git a/src/components/Discover/CreateSlider/index.tsx b/src/components/Discover/CreateSlider/index.tsx index cb3dad3eeb..a014637a90 100644 --- a/src/components/Discover/CreateSlider/index.tsx +++ b/src/components/Discover/CreateSlider/index.tsx @@ -3,6 +3,7 @@ import Tooltip from '@app/components/Common/Tooltip'; import { sliderTitles } from '@app/components/Discover/constants'; import MediaSlider from '@app/components/MediaSlider'; import { WatchProviderSelector } from '@app/components/Selector'; +import useDebouncedState from '@app/hooks/useDebouncedState'; import { encodeURIExtraParams } from '@app/hooks/useDiscover'; import useToasts from '@app/hooks/useToasts'; import defineMessages from '@app/utils/defineMessages'; @@ -14,12 +15,14 @@ import type { import { DiscoverSliderType } from '@server/constants/discover'; import type DiscoverSlider from '@server/entity/DiscoverSlider'; import type { GenreSliderItem } from '@server/interfaces/api/discoverInterfaces'; +import type { MediaListResponse } from '@server/lib/medialists/types'; import type { Keyword, ProductionCompany } from '@server/models/common'; import axios from 'axios'; import { Field, Form, Formik } from 'formik'; import { useCallback, useEffect, useState } from 'react'; import { useIntl } from 'react-intl'; import AsyncSelect from 'react-select/async'; +import useSWR from 'swr'; import * as Yup from 'yup'; const messages = defineMessages('components.Discover.CreateSlider', { @@ -39,15 +42,88 @@ const messages = defineMessages('components.Discover.CreateSlider', { needresults: 'You need to have at least 1 result.', validationDatarequired: 'You must provide a data value.', validationTitlerequired: 'You must provide a title.', - validationListidnumeric: 'The TMDB list ID must be a number.', + validationListidnumeric: + 'The TMDB list ID must be a number of at most 12 digits.', addcustomslider: 'Create Custom Slider', searchKeywords: 'Search keywords…', searchGenres: 'Search genres…', searchStudios: 'Search studios…', starttyping: 'Starting typing to search.', nooptions: 'No results.', + helptmdblist: + 'The number at the end of the list URL, e.g. 8677608 for themoviedb.org/list/8677608.', + listpreviewfailed: + 'This list could not be loaded. Check that the ID is correct and that the list is public on TMDB.', }); +/** Mirrors the pattern the server validates list ids against. */ +const TMDB_LIST_ID_REGEX = /^[1-9]\d{0,11}$/; + +/** Long enough that typing an 8 digit id does not fire eight requests. */ +const LIST_PREVIEW_DEBOUNCE_MS = 400; + +/** + * Preview row for the TMDB list slider. Unlike the other slider types, an id + * typed by hand is regularly wrong, so the preview waits for the typing to + * settle, only asks for well-formed ids, and says so when the list cannot be + * read instead of silently leaving the Add button disabled. + */ +const TmdbListPreview = ({ + title, + listId, + onNewTitles, +}: { + title: string; + listId: string; + onNewTitles: (count: number) => void; +}) => { + const intl = useIntl(); + const [, debouncedListId, setListId] = useDebouncedState( + listId, + LIST_PREVIEW_DEBOUNCE_MS + ); + + useEffect(() => { + setListId(listId); + }, [listId, setListId]); + + const isValidListId = TMDB_LIST_ID_REGEX.test(debouncedListId); + + // Same key as the slider below, so SWR serves both from one request. + const { data, error } = useSWR( + isValidListId ? `/api/v1/discover/list/${debouncedListId}?page=1` : null + ); + + const hasFailed = !!error || !!data?.list.unavailable; + + useEffect(() => { + if (!isValidListId || hasFailed) { + onNewTitles(0); + } + }, [isValidListId, hasFailed, onNewTitles]); + + if (!isValidListId) { + return null; + } + + if (hasFailed) { + return ( +
+ {intl.formatMessage(messages.listpreviewfailed)} +
+ ); + } + + return ( + + ); +}; + type CreateSliderProps = { onCreate: () => void; slider?: Partial; @@ -167,7 +243,7 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => { Number(sliderType) === DiscoverSliderType.TMDB_LIST, then: (schema) => schema.matches( - /^[1-9]\d*$/, + TMDB_LIST_ID_REGEX, intl.formatMessage(messages.validationListidnumeric) ), }), @@ -520,6 +596,11 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => {
{errors.title}
)} {dataInput} + {activeOption?.type === DiscoverSliderType.TMDB_LIST && ( +
+ {intl.formatMessage(messages.helptmdblist)} +
+ )} {errors.data && touched.data && typeof errors.data === 'string' && ( @@ -549,38 +630,53 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => { )}
- {activeOption && values.title && values.data && ( -
- + +
+ )} + + {activeOption && + activeOption.type !== DiscoverSliderType.TMDB_LIST && + values.title && + values.data && ( +
+ -
- )} + } + onNewTitles={updateResultCount} + /> + + )} ); }} diff --git a/src/i18n/locale/en.json b/src/i18n/locale/en.json index f6cf0652fd..7150def437 100644 --- a/src/i18n/locale/en.json +++ b/src/i18n/locale/en.json @@ -33,6 +33,8 @@ "components.Discover.CreateSlider.editSlider": "Edit Slider", "components.Discover.CreateSlider.editfail": "Failed to edit slider.", "components.Discover.CreateSlider.editsuccess": "Edited slider and saved discover customization settings.", + "components.Discover.CreateSlider.helptmdblist": "The number at the end of the list URL, e.g. 8677608 for themoviedb.org/list/8677608.", + "components.Discover.CreateSlider.listpreviewfailed": "This list could not be loaded. Check that the ID is correct and that the list is public on TMDB.", "components.Discover.CreateSlider.needresults": "You need to have at least 1 result.", "components.Discover.CreateSlider.nooptions": "No results.", "components.Discover.CreateSlider.providetmdbgenreid": "Provide a TMDB Genre ID", @@ -47,7 +49,7 @@ "components.Discover.CreateSlider.slidernameplaceholder": "Slider Name", "components.Discover.CreateSlider.starttyping": "Starting typing to search.", "components.Discover.CreateSlider.validationDatarequired": "You must provide a data value.", - "components.Discover.CreateSlider.validationListidnumeric": "The TMDB list ID must be a number.", + "components.Discover.CreateSlider.validationListidnumeric": "The TMDB list ID must be a number of at most 12 digits.", "components.Discover.CreateSlider.validationTitlerequired": "You must provide a title.", "components.Discover.DiscoverList.listTitle": "TMDB List", "components.Discover.DiscoverList.listunavailable": "This list is no longer available", From 85ab9ef7b003e77504212ef980d5c89fe7e4f9c8 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:55:18 -0400 Subject: [PATCH 06/11] perf(discover): only fetch a slider once it nears the viewport Every MediaSlider on Discover issued its two initial page requests on mount, so a customised Discover with a dozen rows fired two dozen requests up front for posters most of which are never scrolled to. TMDB list rows make that worse, since each one is a distinct upstream list. Rows now hold their SWR key back until an IntersectionObserver reports the row heading within 600px of the viewport, which keeps above-the-fold rows loading immediately and defers the rest. Environments without IntersectionObserver, and rows opting out with `lazy={false}` (the slider previews, whose result count gates the Add button), fetch as before. Co-Authored-By: Claude Fable 5 --- .../Discover/CreateSlider/index.tsx | 4 ++ src/components/MediaSlider/index.tsx | 48 ++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/components/Discover/CreateSlider/index.tsx b/src/components/Discover/CreateSlider/index.tsx index a014637a90..9d06ed4fcb 100644 --- a/src/components/Discover/CreateSlider/index.tsx +++ b/src/components/Discover/CreateSlider/index.tsx @@ -120,6 +120,8 @@ const TmdbListPreview = ({ title={title} url={`/api/v1/discover/list/${debouncedListId}`} onNewTitles={onNewTitles} + // The Add button is gated on this row's result count, so it must load. + lazy={false} /> ); }; @@ -674,6 +676,8 @@ const CreateSlider = ({ onCreate, slider }: CreateSliderProps) => { ) } onNewTitles={updateResultCount} + // The Add button is gated on this row's result count. + lazy={false} /> )} diff --git a/src/components/MediaSlider/index.tsx b/src/components/MediaSlider/index.tsx index abfafed8e0..c5d15c7934 100644 --- a/src/components/MediaSlider/index.tsx +++ b/src/components/MediaSlider/index.tsx @@ -13,9 +13,12 @@ import type { TvResult, } from '@server/models/Search'; import Link from 'next/link'; -import { useEffect } from 'react'; +import { useEffect, useRef, useState } from 'react'; import useSWRInfinite from 'swr/infinite'; +/** How early a row starts loading, relative to the viewport. */ +const LAZY_MOUNT_ROOT_MARGIN = '600px 0px'; + interface MixedResult { page: number; totalResults: number; @@ -31,6 +34,12 @@ interface MediaSliderProps { hideWhenEmpty?: boolean; extraParams?: string; onNewTitles?: (titleCount: number) => void; + /** + * Rows hold off fetching until they are near the viewport. Set to false for a + * row that must load even if it is never scrolled to, such as a preview whose + * result count gates a form. + */ + lazy?: boolean; } const MediaSlider = ({ @@ -41,11 +50,46 @@ const MediaSlider = ({ sliderKey, hideWhenEmpty = false, onNewTitles, + lazy = true, }: MediaSliderProps) => { const settings = useSettings(); const { hasPermission } = useUser(); + const headerRef = useRef(null); + const [shouldFetch, setShouldFetch] = useState(!lazy); + + // A Discover page can hold a dozen rows; without this they would all fetch at + // once on mount, most of them for posters nobody scrolls down to. + useEffect(() => { + if (shouldFetch) { + return; + } + + if (!headerRef.current || typeof IntersectionObserver === 'undefined') { + setShouldFetch(true); + return; + } + + const observer = new IntersectionObserver( + (entries) => { + if (entries.some((entry) => entry.isIntersecting)) { + setShouldFetch(true); + observer.disconnect(); + } + }, + { rootMargin: LAZY_MOUNT_ROOT_MARGIN } + ); + + observer.observe(headerRef.current); + + return () => observer.disconnect(); + }, [shouldFetch]); + const { data, error, setSize, size } = useSWRInfinite( (pageIndex: number, previousPageData: MixedResult | null) => { + if (!shouldFetch) { + return null; + } + if (previousPageData && pageIndex + 1 > previousPageData.totalPages) { return null; } @@ -179,7 +223,7 @@ const MediaSlider = ({ return ( <> -
+
{linkUrl ? ( {title} From f5dfc4449b03e1a7e15f4aa74216adcadc2729ad Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 16:56:06 -0400 Subject: [PATCH 07/11] fix(a11y): label the slider scroll buttons Both slider navigation buttons were icon-only with no accessible name, so screen readers announced two unlabelled buttons on every row of every discover page. They now carry localized aria-labels. Co-Authored-By: Claude Fable 5 --- src/components/Slider/index.tsx | 8 ++++++++ src/i18n/locale/en.json | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/components/Slider/index.tsx b/src/components/Slider/index.tsx index 3be9c383a6..000d447ea1 100644 --- a/src/components/Slider/index.tsx +++ b/src/components/Slider/index.tsx @@ -1,11 +1,17 @@ import TitleCard from '@app/components/TitleCard'; import globalMessages from '@app/i18n/globalMessages'; +import defineMessages from '@app/utils/defineMessages'; import { ChevronLeftIcon, ChevronRightIcon } from '@heroicons/react/24/outline'; import { useSpring } from '@react-spring/web'; import { debounce } from 'lodash'; import { useCallback, useEffect, useRef, useState, type JSX } from 'react'; import { useIntl } from 'react-intl'; +const messages = defineMessages('components.Slider', { + previous: 'Scroll backward', + next: 'Scroll forward', +}); + interface SliderProps { sliderKey: string; items?: JSX.Element[]; @@ -153,6 +159,7 @@ const Slider = ({ onClick={() => slide(Direction.LEFT)} disabled={scrollPos.isStart} type="button" + aria-label={intl.formatMessage(messages.previous)} > @@ -163,6 +170,7 @@ const Slider = ({ onClick={() => slide(Direction.RIGHT)} disabled={scrollPos.isEnd} type="button" + aria-label={intl.formatMessage(messages.next)} > diff --git a/src/i18n/locale/en.json b/src/i18n/locale/en.json index 7150def437..42421eb796 100644 --- a/src/i18n/locale/en.json +++ b/src/i18n/locale/en.json @@ -1345,6 +1345,8 @@ "components.Setup.signinWithPlex": "Enter your Plex details", "components.Setup.subtitle": "Get started by choosing your media server", "components.Setup.welcome": "Welcome to Seerr", + "components.Slider.next": "Scroll forward", + "components.Slider.previous": "Scroll backward", "components.StatusBadge.managemedia": "Manage {mediaType}", "components.StatusBadge.openinarr": "Open in {arr}", "components.StatusBadge.playonplex": "Play on {mediaServerName}", From 15beb1a7af32c1cb7ee0238b24f5bcd0459169f5 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 18:20:21 -0400 Subject: [PATCH 08/11] fix(discover): accept a validator-coerced numeric page parameter The OpenAPI request validator coerces the documented integer page query parameter to a number before the route handler runs, so rejecting every non-string value returned 400 for all requests, including the default page. Accept both representations; the safe-integer check still rejects malformed input. --- server/routes/discover.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/routes/discover.ts b/server/routes/discover.ts index 816a9338fd..13c04917e2 100644 --- a/server/routes/discover.ts +++ b/server/routes/discover.ts @@ -801,9 +801,15 @@ discoverRoutes.get<{ listId: string }, MediaListResponse>( } // Parsed once, and strictly: the page number is part of the cache key. + // The OpenAPI request validator may have already coerced the value to a + // number, so both string and number inputs are accepted here. const rawPage = req.query.page; - if (rawPage !== undefined && typeof rawPage !== 'string') { + if ( + rawPage !== undefined && + typeof rawPage !== 'string' && + typeof rawPage !== 'number' + ) { return next({ status: 400, message: 'Invalid page.' }); } From eea8cdd29f8ed6f82e30d686d16d24e04dab8fc4 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 19:15:43 -0400 Subject: [PATCH 09/11] fix(discover): treat a list item without media_type as a movie Classic v3 lists are movie-only and their items omit the `media_type` discriminator entirely; only v4 lists, which can mix movies and series, carry one. The boundary validator rejected the missing field as a malformed payload, so every classic list answered HTTP 500 instead of rendering. `TmdbListItem` now models the field as optional on the movie arm, and the provider reads an absent discriminator as a movie, normalising it on the way through. A `media_type` that is present but unsupported is still treated as malformed. Reported by CodeRabbit on #3344. Co-Authored-By: Claude Fable 5 --- server/api/themoviedb/interfaces.ts | 16 +++++++++++++--- server/lib/medialists/providers/tmdb.ts | 12 +++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/server/api/themoviedb/interfaces.ts b/server/api/themoviedb/interfaces.ts index 03e8162301..eb0b445077 100644 --- a/server/api/themoviedb/interfaces.ts +++ b/server/api/themoviedb/interfaces.ts @@ -87,11 +87,21 @@ export interface TmdbExternalIdResponse { person_results: TmdbPersonResult[]; } +/** + * A single entry of a TMDB list. + * + * v4 lists can hold both movies and series, so their items carry their own + * `media_type`. Classic v3 lists are movie-only and omit the field entirely, + * which is why it is optional here: a missing `media_type` means a movie. + */ +export type TmdbListItem = + | TmdbTvResult + | (Omit & { media_type?: 'movie' }); + /** * Response shape of `GET /3/list/{listId}`. * - * The v3 endpoint serves both classic v3 lists and the newer v4 lists. v4 lists - * can hold both movies and series, so every item carries its own `media_type`. + * The v3 endpoint serves both classic v3 lists and the newer v4 lists. */ export interface TmdbListResponse extends TmdbPaginatedResponse { id: number; @@ -101,7 +111,7 @@ export interface TmdbListResponse extends TmdbPaginatedResponse { iso_639_1?: string; poster_path?: string; item_count: number; - items: (TmdbMovieResult | TmdbTvResult)[]; + items: TmdbListItem[]; } export interface TmdbCreditCast { diff --git a/server/lib/medialists/providers/tmdb.ts b/server/lib/medialists/providers/tmdb.ts index 811a869a64..dc43e47f9e 100644 --- a/server/lib/medialists/providers/tmdb.ts +++ b/server/lib/medialists/providers/tmdb.ts @@ -95,10 +95,16 @@ export class TmdbListProvider implements MediaListProvider { ); } - if (item.media_type === 'movie') { - acc.push({ mediaType: 'movie', tmdbId: item.id, tmdbResult: item }); - } else if (item.media_type === 'tv') { + if (item.media_type === 'tv') { acc.push({ mediaType: 'tv', tmdbId: item.id, tmdbResult: item }); + } else if (item.media_type === 'movie' || item.media_type === undefined) { + // Classic v3 lists hold movies only and leave media_type out entirely, + // so an absent discriminator is a movie rather than a broken payload. + acc.push({ + mediaType: 'movie', + tmdbId: item.id, + tmdbResult: { ...item, media_type: 'movie' }, + }); } else { throw new MalformedListResponseError( `unsupported item media_type "${ From 6f77c921536c7b92f2a3f6ee7c522ec8f1f31a66 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 19:15:44 -0400 Subject: [PATCH 10/11] fix(discover): clear the list preview count when the id changes The preview debounces for 400ms, during which the row below still shows the previous list and its reported result count still stands. Replacing a working id with another well-formed one therefore left the Add button enabled on the strength of a preview belonging to a different list. The raw id changing now zeroes the count immediately, so the button can only re-enable once the new preview has actually returned titles. Reported by CodeRabbit on #3344. Co-Authored-By: Claude Fable 5 --- src/components/Discover/CreateSlider/index.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/Discover/CreateSlider/index.tsx b/src/components/Discover/CreateSlider/index.tsx index 9d06ed4fcb..15efc1a576 100644 --- a/src/components/Discover/CreateSlider/index.tsx +++ b/src/components/Discover/CreateSlider/index.tsx @@ -84,8 +84,13 @@ const TmdbListPreview = ({ ); useEffect(() => { + // Until the debounce settles the row below still shows the previous list, + // so the count it reported has to be dropped straight away: another + // well-formed id would otherwise keep the Add button enabled on the + // strength of a preview belonging to a different list. + onNewTitles(0); setListId(listId); - }, [listId, setListId]); + }, [listId, onNewTitles, setListId]); const isValidListId = TMDB_LIST_ID_REGEX.test(debouncedListId); From a75550c676fd8cd6e6382feafba1f8cb2a817033 Mon Sep 17 00:00:00 2001 From: Jean-David Rioux Date: Mon, 3 Aug 2026 19:15:45 -0400 Subject: [PATCH 11/11] test(discover): cover the media list cache bounds The two guards that keep the media list cache from being driven by request input had no tests. Adds coverage for both, exporting the entry ceiling so the test cannot drift from the implementation: - locales differing only in case or padding collapse to one cache key, a genuinely different locale gets its own, and anything unusable falls back to the single empty-language key - filling the cache past its ceiling holds the entry count and evicts the least recently fetched page Also covers a classic v3 list whose items carry no media_type. Co-Authored-By: Claude Fable 5 --- server/lib/medialists/index.ts | 6 +- server/routes/discover.test.ts | 133 ++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 4 deletions(-) diff --git a/server/lib/medialists/index.ts b/server/lib/medialists/index.ts index 725f0fd479..adf6d6feda 100644 --- a/server/lib/medialists/index.ts +++ b/server/lib/medialists/index.ts @@ -29,7 +29,7 @@ const HYDRATION_CONCURRENCY = 10; * request-controlled values (list id, page), so it needs a ceiling; the oldest * entries are dropped first. */ -const MAX_CACHE_ENTRIES = 500; +export const MAX_MEDIA_LIST_CACHE_ENTRIES = 500; /** Well-formed BCP-47-ish tags, which is all TMDB accepts. */ const LANGUAGE_REGEX = /^([a-z]{2,3})(?:-([a-z]{2}))?$/i; @@ -147,7 +147,7 @@ const hydrateItems = async ( }; /** - * Keeps the cache under `MAX_CACHE_ENTRIES` by dropping the least recently + * Keeps the cache under `MAX_MEDIA_LIST_CACHE_ENTRIES` by dropping the least recently * fetched pages, so that a burst of distinct list/page combinations cannot grow * it without bound. */ @@ -155,7 +155,7 @@ const evictOverflow = (reservedKey: string): void => { const cache = cacheManager.getCache('medialist').data; const keys = cache.keys().filter((key) => key !== reservedKey); - const overflow = keys.length - (MAX_CACHE_ENTRIES - 1); + const overflow = keys.length - (MAX_MEDIA_LIST_CACHE_ENTRIES - 1); if (overflow <= 0) { return; diff --git a/server/routes/discover.test.ts b/server/routes/discover.test.ts index c46d1c1f75..b3274cac20 100644 --- a/server/routes/discover.test.ts +++ b/server/routes/discover.test.ts @@ -8,7 +8,13 @@ import { MediaStatus, MediaType } from '@server/constants/media'; import { getRepository } from '@server/datasource'; import Media from '@server/entity/Media'; import cacheManager from '@server/lib/cache'; -import { getMediaListPage, getMediaListProvider } from '@server/lib/medialists'; +import { + MAX_MEDIA_LIST_CACHE_ENTRIES, + getMediaListCacheKey, + getMediaListPage, + getMediaListProvider, + normalizeMediaListLanguage, +} from '@server/lib/medialists'; import type { MediaListProvider } from '@server/lib/medialists/types'; import { setupTestDb } from '@server/test/db'; import type { Express } from 'express'; @@ -208,6 +214,27 @@ describe('GET /discover/list/:listId', () => { ); }); + it('reads a classic v3 list whose items carry no media_type', async () => { + const movieWithoutMediaType: Record = { ...MOVIE_ITEM }; + delete movieWithoutMediaType.media_type; + + getListMock.mockImplementation(async () => + listResponse({ + item_count: 1, + total_results: 1, + items: [movieWithoutMediaType], + } as unknown as Partial) + ); + + const agent = await loginAsAdmin(); + const res = await agent.get(`/discover/list/${LIST_ID}`); + + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body.results.length, 1); + assert.strictEqual(res.body.results[0].mediaType, 'movie'); + assert.strictEqual(res.body.results[0].id, 603); + }); + it('attaches local media info to results', async () => { await getRepository(Media).save( new Media({ @@ -497,6 +524,110 @@ describe('media list caching', () => { }); }); +describe('media list cache bounds', () => { + it('canonicalises the language before it reaches the cache key', () => { + for (const [input, expected] of [ + ['en', 'en'], + ['EN', 'en'], + [' en ', 'en'], + ['fil', 'fil'], + ['en-us', 'en-US'], + ['EN-US', 'en-US'], + ['pt-BR', 'pt-BR'], + ] as [string, string][]) { + assert.strictEqual( + normalizeMediaListLanguage(input), + expected, + `expected "${input}" to normalize to "${expected}"` + ); + } + + // Anything unrecognised collapses to one key rather than minting its own. + for (const junk of [ + undefined, + '', + 'not-a-locale', + '../../etc', + 'en_US', + 'en-USA', + 'e'.repeat(200), + ]) { + assert.strictEqual( + normalizeMediaListLanguage(junk), + '', + `expected "${junk}" to normalize to ""` + ); + } + }); + + it('gives locales that differ only in case or padding one cache entry', async () => { + const provider = tmdbProvider(); + + for (const language of ['en', 'EN', ' en ']) { + await getMediaListPage({ provider, listId: LIST_ID, language }); + } + + assert.strictEqual(getListMock.callCount(), 1); + assert.deepStrictEqual(cachedListKeys(), [ + getMediaListCacheKey('tmdb', LIST_ID, 'en', 1), + ]); + + // A genuinely different locale is a different list, and an unusable one + // falls back to the single empty-language key. + await getMediaListPage({ provider, listId: LIST_ID, language: 'en-us' }); + await getMediaListPage({ provider, listId: LIST_ID, language: 'nonsense' }); + await getMediaListPage({ provider, listId: LIST_ID, language: '!!!' }); + + assert.strictEqual(getListMock.callCount(), 3); + assert.deepStrictEqual( + cachedListKeys().sort(), + [ + getMediaListCacheKey('tmdb', LIST_ID, '', 1), + getMediaListCacheKey('tmdb', LIST_ID, 'en', 1), + getMediaListCacheKey('tmdb', LIST_ID, 'en-US', 1), + ].sort() + ); + }); + + it('caps the cache and evicts the least recently fetched page', async () => { + const provider = tmdbProvider(); + getListMock.mockImplementation(async ({ page }: { page: number }) => + listResponse({ page, total_pages: 100000, total_results: 2000000 }) + ); + + for (let page = 1; page <= MAX_MEDIA_LIST_CACHE_ENTRIES; page++) { + await getMediaListPage({ provider, listId: LIST_ID, page }); + } + + const oldestKey = getMediaListCacheKey('tmdb', LIST_ID, '', 1); + assert.strictEqual( + cachedListKeys().length, + MAX_MEDIA_LIST_CACHE_ENTRIES, + 'the cache should have filled to exactly the cap' + ); + assert.ok(cachedListKeys().includes(oldestKey)); + + const overflowPage = MAX_MEDIA_LIST_CACHE_ENTRIES + 1; + await getMediaListPage({ provider, listId: LIST_ID, page: overflowPage }); + + assert.strictEqual( + cachedListKeys().length, + MAX_MEDIA_LIST_CACHE_ENTRIES, + 'the cap should hold once it is exceeded' + ); + assert.ok( + !cachedListKeys().includes(oldestKey), + 'the least recently fetched page should have been evicted' + ); + assert.ok( + cachedListKeys().includes( + getMediaListCacheKey('tmdb', LIST_ID, '', overflowPage) + ), + 'the page that overflowed the cache should be cached' + ); + }); +}); + describe('media list payload validation', () => { const malformedPayloads: [string, Partial][] = [ ['a null item', { items: [null] } as unknown as Partial],