fix(requests): serialize requests for the same title

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<User> {
   return userRepository.save(requester);
 }
 
+async function createRequester(email: string): Promise<User> {
+  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);
+  });
 });

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<MediaRequest> {
-    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)
+      )
     );
   }
 

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;