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;