Prechádzať zdrojové kódy

fix(server): Offset of random endpoint could be higher than user's asset count (#4342)

* fix offset of all assets with correct ownerId

* (e2e): test if user does not have all assets
Daniel Dietzler 1 rok pred
rodič
commit
ff331ffad9

+ 1 - 1
server/src/infra/repositories/asset.repository.ts

@@ -435,7 +435,7 @@ export class AssetRepository implements IAssetRepository {
       `SELECT *
        FROM assets
        WHERE "ownerId" = $1
-       OFFSET FLOOR(RANDOM() * (SELECT GREATEST(COUNT(*) - $2, 0) FROM ASSETS)) LIMIT $2`,
+       OFFSET FLOOR(RANDOM() * (SELECT GREATEST(COUNT(*) - $2, 0) FROM ASSETS WHERE "ownerId" = $1)) LIMIT $2`,
       [ownerId, count],
     );
   }

+ 32 - 2
server/test/e2e/asset.e2e-spec.ts

@@ -1,4 +1,11 @@
-import { AssetResponseDto, IAssetRepository, IPersonRepository, LoginResponseDto, TimeBucketSize } from '@app/domain';
+import {
+  AssetResponseDto,
+  IAssetRepository,
+  IPersonRepository,
+  LibraryResponseDto,
+  LoginResponseDto,
+  TimeBucketSize,
+} from '@app/domain';
 import { AppModule, AssetController } from '@app/immich';
 import { AssetEntity, AssetType } from '@app/infra/entities';
 import { INestApplication } from '@nestjs/common';
@@ -68,6 +75,7 @@ describe(`${AssetController.name} (e2e)`, () => {
   let app: INestApplication;
   let server: any;
   let assetRepository: IAssetRepository;
+  let defaultLibrary: LibraryResponseDto;
   let user1: LoginResponseDto;
   let user2: LoginResponseDto;
   let asset1: AssetEntity;
@@ -96,7 +104,7 @@ describe(`${AssetController.name} (e2e)`, () => {
       api.userApi.create(server, admin.accessToken, user2Dto),
     ]);
 
-    const defaultLibrary = libraries[0];
+    defaultLibrary = libraries[0];
 
     [user1, user2] = await Promise.all([
       api.authApi.login(server, { email: user1Dto.email, password: user1Dto.password }),
@@ -385,6 +393,16 @@ describe(`${AssetController.name} (e2e)`, () => {
   });
 
   describe('GET /asset/random', () => {
+    beforeAll(async () => {
+      await Promise.all([
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+        createAsset(assetRepository, user1, defaultLibrary.id, new Date('1970-02-01')),
+      ]);
+    });
     it('should require authentication', async () => {
       const { status, body } = await request(server).get('/asset/random');
 
@@ -427,6 +445,18 @@ describe(`${AssetController.name} (e2e)`, () => {
       }
     });
 
+    it.each(Array(10))(
+      'should return 1 asset if there are 10 assets in the database but user 2 only has 1',
+      async () => {
+        const { status, body } = await request(server)
+          .get('/[]asset/random')
+          .set('Authorization', `Bearer ${user2.accessToken}`);
+
+        expect(status).toBe(200);
+        expect(body).toEqual([expect.objectContaining({ id: asset4.id })]);
+      },
+    );
+
     it('should return error', async () => {
       const { status } = await request(server)
         .get('/asset/random?count=ABC')