From 042a93e39cc05718d746cd9de6e247cfa52e3fac Mon Sep 17 00:00:00 2001 From: fallenbagel <98979876+Fallenbagel@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:21:39 +0800 Subject: [PATCH] fix(requests): serialize requests for the same title Duplicates and overlapping seasons are already rejected, but both checks run well before the insert, so two users asking for the same title at the same moment both got through. Creation now takes a second lock keyed on the title inside the per-user one, always in that order so the two cannot deadlock. is4k is normalized at the same time. It is optional, and an undefined one binds as null in the duplicate query, so an API caller that omitted it could request the same title repeatedly. --- server/entity/MediaRequest.test.ts | 79 ++++++++++++++++++++++++++++-- server/entity/MediaRequest.ts | 11 +++-- server/utils/requestLock.ts | 3 ++ 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/server/entity/MediaRequest.test.ts b/server/entity/MediaRequest.test.ts index 9ff5067f80..3f819c9581 100644 --- a/server/entity/MediaRequest.test.ts +++ b/server/entity/MediaRequest.test.ts @@ -9,7 +9,9 @@ import { MediaRequest, QuotaRestrictedError, } from '@server/entity/MediaRequest'; +import SeasonRequest from '@server/entity/SeasonRequest'; import { User } from '@server/entity/User'; +import { Permission } from '@server/lib/permissions'; import { setupTestDb } from '@server/test/db'; // get is a prototype method unlike getMovie, and replaces the cache lookup too @@ -19,15 +21,16 @@ const externalApiGetMock = mock.method( }, 'get', async (endpoint: string) => { - const movieId = Number(endpoint.replace('/movie/', '')); + const tmdbId = Number(endpoint.replace(/^\/(movie|tv)\//, '')); - if (!movieId) { + if (!tmdbId) { throw new Error(`Unstubbed external endpoint: ${endpoint}`); } return { - id: movieId, + id: tmdbId, external_ids: {}, + seasons: [1, 2, 3].map((season_number) => ({ season_number })), // Skips getMovie's localized fallback call videos: { results: [{ type: 'Trailer', key: 'trailer' }] }, }; @@ -53,6 +56,12 @@ async function seedRequester(movieQuotaLimit: number): Promise { return userRepository.save(requester); } +async function createRequester(email: string): Promise { + return getRepository(User).save( + new User({ email, permissions: Permission.REQUEST, avatar: '' }) + ); +} + function requestMovies(mediaIds: number[], requester: User) { return Promise.allSettled( mediaIds.map((mediaId) => @@ -96,4 +105,68 @@ describe('MediaRequest.request', () => { assert.strictEqual(await requestRepository.count(), 1); assert.strictEqual(externalApiGetMock.callCount(), 2); }); + + it('rejects a duplicate request that omits is4k', async () => { + const requestRepository = getRepository(MediaRequest); + const requester = await seedRequester(5); + const body = { mediaId: 66666, mediaType: MediaType.MOVIE }; + + await MediaRequest.request(body, requester); + + await assert.rejects( + () => MediaRequest.request(body, requester), + DuplicateMediaRequestError + ); + assert.strictEqual(await requestRepository.count(), 1); + }); + + it('rejects a concurrent duplicate request from a different user', async () => { + const requestRepository = getRepository(MediaRequest); + const requester = await seedRequester(5); + const otherRequester = await createRequester('second@seerr.dev'); + + const results = await Promise.allSettled( + [requester, otherRequester].map((user) => + MediaRequest.request( + { mediaId: 44444, mediaType: MediaType.MOVIE, is4k: false }, + user + ) + ) + ); + const rejected = rejections(results); + + assert.strictEqual(rejected.length, 1); + assert.ok(rejected[0].reason instanceof DuplicateMediaRequestError); + assert.strictEqual(await requestRepository.count(), 1); + }); + + it('gives an overlapping season to only one of two concurrent users', async () => { + const seasonRequestRepository = getRepository(SeasonRequest); + const requester = await seedRequester(5); + const otherRequester = await createRequester('second@seerr.dev'); + + const results = await Promise.allSettled( + [ + [requester, [1, 2]], + [otherRequester, [2, 3]], + ].map(([user, seasons]) => + MediaRequest.request( + { + mediaId: 55555, + mediaType: MediaType.TV, + seasons: seasons as number[], + is4k: false, + }, + user as User + ) + ) + ); + + assert.strictEqual(rejections(results).length, 0); + assert.strictEqual( + await seasonRequestRepository.count({ where: { seasonNumber: 2 } }), + 1 + ); + assert.strictEqual(await seasonRequestRepository.count(), 3); + }); }); diff --git a/server/entity/MediaRequest.ts b/server/entity/MediaRequest.ts index 0c3a4f0fb6..ea516ef9b3 100644 --- a/server/entity/MediaRequest.ts +++ b/server/entity/MediaRequest.ts @@ -14,7 +14,7 @@ import { Permission } from '@server/lib/permissions'; import { getSettings } from '@server/lib/settings'; import logger from '@server/logger'; import { DbAwareColumn, resolveDbType } from '@server/utils/DbColumnHelper'; -import requestLock from '@server/utils/requestLock'; +import requestLock, { mediaLock } from '@server/utils/requestLock'; import { truncate } from 'lodash'; import { AfterInsert, @@ -50,8 +50,13 @@ export class MediaRequest { user: User, options: MediaRequestOptions = {} ): Promise { - return requestLock.dispatch(requestBody.userId || user.id, () => - MediaRequest.createRequest(requestBody, user, options) + // is4k is optional, and an undefined one binds as null in the duplicate query + const body = { ...requestBody, is4k: !!requestBody.is4k }; + + return requestLock.dispatch(body.userId || user.id, () => + mediaLock.dispatch(`${body.mediaType}:${body.mediaId}:${body.is4k}`, () => + MediaRequest.createRequest(body, user, options) + ) ); } diff --git a/server/utils/requestLock.ts b/server/utils/requestLock.ts index ca980ad47e..8b6747e1b6 100644 --- a/server/utils/requestLock.ts +++ b/server/utils/requestLock.ts @@ -4,4 +4,7 @@ import AsyncLock from '@server/utils/asyncLock'; // would block while holding the save's connection. const requestLock = new AsyncLock(); +// keyed on media. always taken inside requestLock, never around it. +export const mediaLock = new AsyncLock(); + export default requestLock;