From 3d25d91e777fd071a527c163c364de372802800b Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 29 Feb 2024 15:10:08 -0500 Subject: [PATCH 1/3] refactor: library e2e (#7538) * refactor: library e2e * refactor: remove before each usages --- e2e/docker-compose.yml | 1 + e2e/src/api/specs/library.e2e-spec.ts | 456 +++++++++++++++++ e2e/src/utils.ts | 10 + e2e/vitest.config.ts | 10 +- open-api/immich-openapi-specs.json | 6 +- server/e2e/api/specs/library.e2e-spec.ts | 475 ------------------ server/e2e/client/library-api.ts | 4 +- server/e2e/jobs/specs/library.e2e-spec.ts | 9 +- .../immich/controllers/library.controller.ts | 5 +- 9 files changed, 489 insertions(+), 487 deletions(-) create mode 100644 e2e/src/api/specs/library.e2e-spec.ts delete mode 100644 server/e2e/api/specs/library.e2e-spec.ts diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml index 44f79b55ac..6e2d4b78dd 100644 --- a/e2e/docker-compose.yml +++ b/e2e/docker-compose.yml @@ -16,6 +16,7 @@ x-server-build: &server-common - IMMICH_MACHINE_LEARNING_ENABLED=false volumes: - upload:/usr/src/app/upload + - ../server/test/assets:/data/assets depends_on: - redis - database diff --git a/e2e/src/api/specs/library.e2e-spec.ts b/e2e/src/api/specs/library.e2e-spec.ts new file mode 100644 index 0000000000..8213cc86ea --- /dev/null +++ b/e2e/src/api/specs/library.e2e-spec.ts @@ -0,0 +1,456 @@ +import { LibraryResponseDto, LibraryType, LoginResponseDto, getAllLibraries } from '@immich/sdk'; +import { userDto, uuidDto } from 'src/fixtures'; +import { errorDto } from 'src/responses'; +import { apiUtils, app, asBearerAuth, dbUtils, testAssetDirInternal } from 'src/utils'; +import request from 'supertest'; +import { beforeAll, describe, expect, it } from 'vitest'; + +describe('/library', () => { + let admin: LoginResponseDto; + let user: LoginResponseDto; + let library: LibraryResponseDto; + + beforeAll(async () => { + apiUtils.setup(); + await dbUtils.reset(); + admin = await apiUtils.adminSetup(); + user = await apiUtils.userSetup(admin.accessToken, userDto.user1); + library = await apiUtils.createLibrary(admin.accessToken, { type: LibraryType.External }); + }); + + describe('GET /library', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).get('/library'); + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should start with a default upload library', async () => { + const { status, body } = await request(app).get('/library').set('Authorization', `Bearer ${admin.accessToken}`); + expect(status).toBe(200); + expect(body).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + ownerId: admin.userId, + type: LibraryType.Upload, + name: 'Default Library', + refreshedAt: null, + assetCount: 0, + importPaths: [], + exclusionPatterns: [], + }), + ]), + ); + }); + }); + + describe('POST /library', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).post('/library').send({}); + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should require admin authentication', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${user.accessToken}`) + .send({ type: LibraryType.External }); + + expect(status).toBe(403); + expect(body).toEqual(errorDto.forbidden); + }); + + it('should create an external library with defaults', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ type: LibraryType.External }); + + expect(status).toBe(201); + expect(body).toEqual( + expect.objectContaining({ + ownerId: admin.userId, + type: LibraryType.External, + name: 'New External Library', + refreshedAt: null, + assetCount: 0, + importPaths: [], + exclusionPatterns: [], + }), + ); + }); + + it('should create an external library with options', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ + type: LibraryType.External, + name: 'My Awesome Library', + importPaths: ['/path/to/import'], + exclusionPatterns: ['**/Raw/**'], + }); + + expect(status).toBe(201); + expect(body).toEqual( + expect.objectContaining({ + name: 'My Awesome Library', + importPaths: ['/path/to/import'], + }), + ); + }); + + it('should not create an external library with duplicate import paths', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ + type: LibraryType.External, + name: 'My Awesome Library', + importPaths: ['/path', '/path'], + exclusionPatterns: ['**/Raw/**'], + }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(["All importPaths's elements must be unique"])); + }); + + it('should not create an external library with duplicate exclusion patterns', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ + type: LibraryType.External, + name: 'My Awesome Library', + importPaths: ['/path/to/import'], + exclusionPatterns: ['**/Raw/**', '**/Raw/**'], + }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(["All exclusionPatterns's elements must be unique"])); + }); + + it('should create an upload library with defaults', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ type: LibraryType.Upload }); + + expect(status).toBe(201); + expect(body).toEqual( + expect.objectContaining({ + ownerId: admin.userId, + type: LibraryType.Upload, + name: 'New Upload Library', + refreshedAt: null, + assetCount: 0, + importPaths: [], + exclusionPatterns: [], + }), + ); + }); + + it('should create an upload library with options', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ type: LibraryType.Upload, name: 'My Awesome Library' }); + + expect(status).toBe(201); + expect(body).toEqual( + expect.objectContaining({ + name: 'My Awesome Library', + }), + ); + }); + + it('should not allow upload libraries to have import paths', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ type: LibraryType.Upload, importPaths: ['/path/to/import'] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest('Upload libraries cannot have import paths')); + }); + + it('should not allow upload libraries to have exclusion patterns', async () => { + const { status, body } = await request(app) + .post('/library') + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ type: LibraryType.Upload, exclusionPatterns: ['**/Raw/**'] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest('Upload libraries cannot have exclusion patterns')); + }); + }); + + describe('PUT /library/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).put(`/library/${uuidDto.notFound}`).send({}); + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should change the library name', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ name: 'New Library Name' }); + + expect(status).toBe(200); + expect(body).toEqual( + expect.objectContaining({ + name: 'New Library Name', + }), + ); + }); + + it('should not set an empty name', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ name: '' }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['name should not be empty'])); + }); + + it('should change the import paths', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ importPaths: [testAssetDirInternal] }); + + expect(status).toBe(200); + expect(body).toEqual( + expect.objectContaining({ + importPaths: [testAssetDirInternal], + }), + ); + }); + + it('should reject an empty import path', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ importPaths: [''] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['each value in importPaths should not be empty'])); + }); + + it('should reject duplicate import paths', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ importPaths: ['/path', '/path'] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(["All importPaths's elements must be unique"])); + }); + + it('should change the exclusion pattern', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ exclusionPatterns: ['**/Raw/**'] }); + + expect(status).toBe(200); + expect(body).toEqual( + expect.objectContaining({ + exclusionPatterns: ['**/Raw/**'], + }), + ); + }); + + it('should reject duplicate exclusion patterns', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ exclusionPatterns: ['**/*.jpg', '**/*.jpg'] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(["All exclusionPatterns's elements must be unique"])); + }); + + it('should reject an empty exclusion pattern', async () => { + const { status, body } = await request(app) + .put(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ exclusionPatterns: [''] }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['each value in exclusionPatterns should not be empty'])); + }); + }); + + describe('GET /library/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).get(`/library/${uuidDto.notFound}`); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should require admin access', async () => { + const { status, body } = await request(app) + .get(`/library/${uuidDto.notFound}`) + .set('Authorization', `Bearer ${user.accessToken}`); + expect(status).toBe(403); + expect(body).toEqual(errorDto.forbidden); + }); + + it('should get library by id', async () => { + const library = await apiUtils.createLibrary(admin.accessToken, { type: LibraryType.External }); + + const { status, body } = await request(app) + .get(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(200); + expect(body).toEqual( + expect.objectContaining({ + ownerId: admin.userId, + type: LibraryType.External, + name: 'New External Library', + refreshedAt: null, + assetCount: 0, + importPaths: [], + exclusionPatterns: [], + }), + ); + }); + }); + + describe('DELETE /library/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).delete(`/library/${uuidDto.notFound}`); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should not delete the last upload library', async () => { + const libraries = await getAllLibraries( + { $type: LibraryType.Upload }, + { headers: asBearerAuth(admin.accessToken) }, + ); + + const adminLibraries = libraries.filter((library) => library.ownerId === admin.userId); + expect(adminLibraries.length).toBeGreaterThanOrEqual(1); + const lastLibrary = adminLibraries.pop() as LibraryResponseDto; + + // delete all but the last upload library + for (const library of adminLibraries) { + const { status } = await request(app) + .delete(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`); + expect(status).toBe(204); + } + + const { status, body } = await request(app) + .delete(`/library/${lastLibrary.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`); + + expect(body).toEqual(errorDto.noDeleteUploadLibrary); + expect(status).toBe(400); + }); + + it('should delete an external library', async () => { + const library = await apiUtils.createLibrary(admin.accessToken, { type: LibraryType.External }); + + const { status, body } = await request(app) + .delete(`/library/${library.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(204); + expect(body).toEqual({}); + + const libraries = await getAllLibraries({}, { headers: asBearerAuth(admin.accessToken) }); + expect(libraries).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: library.id, + }), + ]), + ); + }); + }); + + describe('GET /library/:id/statistics', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).get(`/library/${uuidDto.notFound}/statistics`); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + }); + + describe('POST /library/:id/scan', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).post(`/library/${uuidDto.notFound}/scan`).send({}); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + }); + + describe('POST /library/:id/removeOffline', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).post(`/library/${uuidDto.notFound}/removeOffline`).send({}); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + }); + + describe('POST /library/:id/validate', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).post(`/library/${uuidDto.notFound}/validate`).send({}); + + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should pass with no import paths', async () => { + const response = await apiUtils.validateLibrary(admin.accessToken, library.id, { importPaths: [] }); + expect(response.importPaths).toEqual([]); + }); + + it('should fail if path does not exist', async () => { + const pathToTest = `${testAssetDirInternal}/does/not/exist`; + + const response = await apiUtils.validateLibrary(admin.accessToken, library.id, { + importPaths: [pathToTest], + }); + + expect(response.importPaths?.length).toEqual(1); + const pathResponse = response?.importPaths?.at(0); + + expect(pathResponse).toEqual({ + importPath: pathToTest, + isValid: false, + message: `Path does not exist (ENOENT)`, + }); + }); + + it('should fail if path is a file', async () => { + const pathToTest = `${testAssetDirInternal}/albums/nature/el_torcal_rocks.jpg`; + + const response = await apiUtils.validateLibrary(admin.accessToken, library.id, { + importPaths: [pathToTest], + }); + + expect(response.importPaths?.length).toEqual(1); + const pathResponse = response?.importPaths?.at(0); + + expect(pathResponse).toEqual({ + importPath: pathToTest, + isValid: false, + message: `Not a directory`, + }); + }); + }); +}); diff --git a/e2e/src/utils.ts b/e2e/src/utils.ts index b02e0053f3..34f25e396d 100644 --- a/e2e/src/utils.ts +++ b/e2e/src/utils.ts @@ -3,11 +3,14 @@ import { AssetResponseDto, CreateAlbumDto, CreateAssetDto, + CreateLibraryDto, CreateUserDto, PersonUpdateDto, SharedLinkCreateDto, + ValidateLibraryDto, createAlbum, createApiKey, + createLibrary, createPerson, createSharedLink, createUser, @@ -18,6 +21,7 @@ import { setAdminOnboarding, signUpAdmin, updatePerson, + validate, } from '@immich/sdk'; import { BrowserContext } from '@playwright/test'; import { exec, spawn } from 'node:child_process'; @@ -42,6 +46,7 @@ const directoryExists = (directory: string) => // TODO move test assets into e2e/assets export const testAssetDir = path.resolve(`./../server/test/assets/`); +export const testAssetDirInternal = '/data/assets'; export const tempDir = tmpdir(); const serverContainerName = 'immich-e2e-server'; @@ -103,6 +108,7 @@ export const dbUtils = { } tables = tables || [ + 'libraries', 'shared_links', 'person', 'albums', @@ -313,6 +319,10 @@ export const apiUtils = { }, createSharedLink: (accessToken: string, dto: SharedLinkCreateDto) => createSharedLink({ sharedLinkCreateDto: dto }, { headers: asBearerAuth(accessToken) }), + createLibrary: (accessToken: string, dto: CreateLibraryDto) => + createLibrary({ createLibraryDto: dto }, { headers: asBearerAuth(accessToken) }), + validateLibrary: (accessToken: string, id: string, dto: ValidateLibraryDto) => + validate({ id, validateLibraryDto: dto }, { headers: asBearerAuth(accessToken) }), }; export const cliUtils = { diff --git a/e2e/vitest.config.ts b/e2e/vitest.config.ts index 72c126a899..b8cc098ddd 100644 --- a/e2e/vitest.config.ts +++ b/e2e/vitest.config.ts @@ -1,9 +1,17 @@ import { defineConfig } from 'vitest/config'; +// skip `docker compose up` if `make e2e` was already run +const globalSetup: string[] = []; +try { + await fetch('http://127.0.0.1:2283/api/server-info/ping'); +} catch { + globalSetup.push('src/setup.ts'); +} + export default defineConfig({ test: { include: ['src/{api,cli}/specs/*.e2e-spec.ts'], - globalSetup: ['src/setup.ts'], + globalSetup, poolOptions: { threads: { singleThread: true, diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 3092c6cc63..f38780f294 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -3396,7 +3396,7 @@ } ], "responses": { - "200": { + "204": { "description": "" } }, @@ -3521,7 +3521,7 @@ } ], "responses": { - "201": { + "204": { "description": "" } }, @@ -3566,7 +3566,7 @@ "required": true }, "responses": { - "201": { + "204": { "description": "" } }, diff --git a/server/e2e/api/specs/library.e2e-spec.ts b/server/e2e/api/specs/library.e2e-spec.ts deleted file mode 100644 index edb0a9feb7..0000000000 --- a/server/e2e/api/specs/library.e2e-spec.ts +++ /dev/null @@ -1,475 +0,0 @@ -import { LibraryResponseDto, LoginResponseDto } from '@app/domain'; -import { LibraryController } from '@app/immich'; -import { LibraryType } from '@app/infra/entities'; -import { errorStub, userDto, uuidStub } from '@test/fixtures'; -import { IMMICH_TEST_ASSET_TEMP_PATH, restoreTempFolder } from 'src/test-utils/utils'; -import request from 'supertest'; -import { api } from '../../client'; -import { testApp } from '../utils'; - -describe(`${LibraryController.name} (e2e)`, () => { - let server: any; - let admin: LoginResponseDto; - let user: LoginResponseDto; - - beforeAll(async () => { - const app = await testApp.create(); - server = app.getHttpServer(); - }); - - afterAll(async () => { - await testApp.teardown(); - }); - - beforeEach(async () => { - await restoreTempFolder(); - await testApp.reset(); - await api.authApi.adminSignUp(server); - admin = await api.authApi.adminLogin(server); - - await api.userApi.create(server, admin.accessToken, userDto.user1); - user = await api.authApi.login(server, userDto.user1); - }); - - describe('GET /library', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).get('/library'); - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - it('should start with a default upload library', async () => { - const { status, body } = await request(server) - .get('/library') - .set('Authorization', `Bearer ${admin.accessToken}`); - expect(status).toBe(200); - expect(body).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - ownerId: admin.userId, - type: LibraryType.UPLOAD, - name: 'Default Library', - refreshedAt: null, - assetCount: 0, - importPaths: [], - exclusionPatterns: [], - }), - ]), - ); - }); - }); - - describe('POST /library', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).post('/library').send({}); - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - it('should require admin authentication', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${user.accessToken}`) - .send({ type: LibraryType.EXTERNAL }); - - expect(status).toBe(403); - expect(body).toEqual(errorStub.forbidden); - }); - - it('should create an external library with defaults', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ type: LibraryType.EXTERNAL }); - - expect(status).toBe(201); - expect(body).toEqual( - expect.objectContaining({ - ownerId: admin.userId, - type: LibraryType.EXTERNAL, - name: 'New External Library', - refreshedAt: null, - assetCount: 0, - importPaths: [], - exclusionPatterns: [], - }), - ); - }); - - it('should create an external library with options', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ - type: LibraryType.EXTERNAL, - name: 'My Awesome Library', - importPaths: ['/path/to/import'], - exclusionPatterns: ['**/Raw/**'], - }); - - expect(status).toBe(201); - expect(body).toEqual( - expect.objectContaining({ - name: 'My Awesome Library', - importPaths: ['/path/to/import'], - }), - ); - }); - - it('should not create an external library with duplicate import paths', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ - type: LibraryType.EXTERNAL, - name: 'My Awesome Library', - importPaths: ['/path', '/path'], - exclusionPatterns: ['**/Raw/**'], - }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(["All importPaths's elements must be unique"])); - }); - - it('should not create an external library with duplicate exclusion patterns', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ - type: LibraryType.EXTERNAL, - name: 'My Awesome Library', - importPaths: ['/path/to/import'], - exclusionPatterns: ['**/Raw/**', '**/Raw/**'], - }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(["All exclusionPatterns's elements must be unique"])); - }); - - it('should create an upload library with defaults', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ type: LibraryType.UPLOAD }); - - expect(status).toBe(201); - expect(body).toEqual( - expect.objectContaining({ - ownerId: admin.userId, - type: LibraryType.UPLOAD, - name: 'New Upload Library', - refreshedAt: null, - assetCount: 0, - importPaths: [], - exclusionPatterns: [], - }), - ); - }); - - it('should create an upload library with options', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ type: LibraryType.UPLOAD, name: 'My Awesome Library' }); - - expect(status).toBe(201); - expect(body).toEqual( - expect.objectContaining({ - name: 'My Awesome Library', - }), - ); - }); - - it('should not allow upload libraries to have import paths', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ type: LibraryType.UPLOAD, importPaths: ['/path/to/import'] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest('Upload libraries cannot have import paths')); - }); - - it('should not allow upload libraries to have exclusion patterns', async () => { - const { status, body } = await request(server) - .post('/library') - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ type: LibraryType.UPLOAD, exclusionPatterns: ['**/Raw/**'] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest('Upload libraries cannot have exclusion patterns')); - }); - }); - - describe('PUT /library/:id', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).put(`/library/${uuidStub.notFound}`).send({}); - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - describe('external library', () => { - let library: LibraryResponseDto; - - beforeEach(async () => { - // Create an external library with default settings - library = await api.libraryApi.create(server, admin.accessToken, { type: LibraryType.EXTERNAL }); - }); - - it('should change the library name', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ name: 'New Library Name' }); - - expect(status).toBe(200); - expect(body).toEqual( - expect.objectContaining({ - name: 'New Library Name', - }), - ); - }); - - it('should not set an empty name', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ name: '' }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(['name should not be empty'])); - }); - - it('should change the import paths', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ importPaths: [IMMICH_TEST_ASSET_TEMP_PATH] }); - - expect(status).toBe(200); - expect(body).toEqual( - expect.objectContaining({ - importPaths: [IMMICH_TEST_ASSET_TEMP_PATH], - }), - ); - }); - - it('should reject an empty import path', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ importPaths: [''] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(['each value in importPaths should not be empty'])); - }); - - it('should reject duplicate import paths', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ importPaths: ['/path', '/path'] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(["All importPaths's elements must be unique"])); - }); - - it('should change the exclusion pattern', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ exclusionPatterns: ['**/Raw/**'] }); - - expect(status).toBe(200); - expect(body).toEqual( - expect.objectContaining({ - exclusionPatterns: ['**/Raw/**'], - }), - ); - }); - - it('should reject duplicate exclusion patterns', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ exclusionPatterns: ['**/*.jpg', '**/*.jpg'] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(["All exclusionPatterns's elements must be unique"])); - }); - - it('should reject an empty exclusion pattern', async () => { - const { status, body } = await request(server) - .put(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`) - .send({ exclusionPatterns: [''] }); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.badRequest(['each value in exclusionPatterns should not be empty'])); - }); - }); - }); - - describe('GET /library/:id', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).get(`/library/${uuidStub.notFound}`); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - it('should require admin access', async () => { - const { status, body } = await request(server) - .get(`/library/${uuidStub.notFound}`) - .set('Authorization', `Bearer ${user.accessToken}`); - expect(status).toBe(403); - expect(body).toEqual(errorStub.forbidden); - }); - - it('should get library by id', async () => { - const library = await api.libraryApi.create(server, admin.accessToken, { type: LibraryType.EXTERNAL }); - - const { status, body } = await request(server) - .get(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`); - - expect(status).toBe(200); - expect(body).toEqual( - expect.objectContaining({ - ownerId: admin.userId, - type: LibraryType.EXTERNAL, - name: 'New External Library', - refreshedAt: null, - assetCount: 0, - importPaths: [], - exclusionPatterns: [], - }), - ); - }); - }); - - describe('DELETE /library/:id', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).delete(`/library/${uuidStub.notFound}`); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - it('should not delete the last upload library', async () => { - const [defaultLibrary] = await api.libraryApi.getAll(server, admin.accessToken); - expect(defaultLibrary).toBeDefined(); - - const { status, body } = await request(server) - .delete(`/library/${defaultLibrary.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`); - - expect(status).toBe(400); - expect(body).toEqual(errorStub.noDeleteUploadLibrary); - }); - - it('should delete an external library', async () => { - const library = await api.libraryApi.create(server, admin.accessToken, { type: LibraryType.EXTERNAL }); - - const { status, body } = await request(server) - .delete(`/library/${library.id}`) - .set('Authorization', `Bearer ${admin.accessToken}`); - - expect(status).toBe(200); - expect(body).toEqual({}); - - const libraries = await api.libraryApi.getAll(server, admin.accessToken); - expect(libraries).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ - id: library.id, - }), - ]), - ); - }); - }); - - describe('GET /library/:id/statistics', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).get(`/library/${uuidStub.notFound}/statistics`); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - }); - - describe('POST /library/:id/scan', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).post(`/library/${uuidStub.notFound}/scan`).send({}); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - }); - - describe('POST /library/:id/removeOffline', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).post(`/library/${uuidStub.notFound}/removeOffline`).send({}); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - }); - - describe('POST /library/:id/validate', () => { - it('should require authentication', async () => { - const { status, body } = await request(server).post(`/library/${uuidStub.notFound}/validate`).send({}); - - expect(status).toBe(401); - expect(body).toEqual(errorStub.unauthorized); - }); - - describe('Validate import path', () => { - let library: LibraryResponseDto; - - beforeEach(async () => { - // Create an external library with default settings - library = await api.libraryApi.create(server, admin.accessToken, { type: LibraryType.EXTERNAL }); - }); - - it('should pass with no import paths', async () => { - const response = await api.libraryApi.validate(server, admin.accessToken, library.id, { importPaths: [] }); - expect(response.importPaths).toEqual([]); - }); - - it('should fail if path does not exist', async () => { - const pathToTest = `${IMMICH_TEST_ASSET_TEMP_PATH}/does/not/exist`; - - const response = await api.libraryApi.validate(server, admin.accessToken, library.id, { - importPaths: [pathToTest], - }); - - expect(response.importPaths?.length).toEqual(1); - const pathResponse = response?.importPaths?.at(0); - - expect(pathResponse).toEqual({ - importPath: pathToTest, - isValid: false, - message: `Path does not exist (ENOENT)`, - }); - }); - - it('should fail if path is a file', async () => { - const pathToTest = `${IMMICH_TEST_ASSET_TEMP_PATH}/does/not/exist`; - - const response = await api.libraryApi.validate(server, admin.accessToken, library.id, { - importPaths: [pathToTest], - }); - - expect(response.importPaths?.length).toEqual(1); - const pathResponse = response?.importPaths?.at(0); - - expect(pathResponse).toEqual({ - importPath: pathToTest, - isValid: false, - message: `Path does not exist (ENOENT)`, - }); - }); - }); - }); -}); diff --git a/server/e2e/client/library-api.ts b/server/e2e/client/library-api.ts index 9f2d0b77ef..e0b1331267 100644 --- a/server/e2e/client/library-api.ts +++ b/server/e2e/client/library-api.ts @@ -36,14 +36,14 @@ export const libraryApi = { .post(`/library/${id}/scan`) .set('Authorization', `Bearer ${accessToken}`) .send(dto); - expect(status).toBe(201); + expect(status).toBe(204); }, removeOfflineFiles: async (server: any, accessToken: string, id: string) => { const { status } = await request(server) .post(`/library/${id}/removeOffline`) .set('Authorization', `Bearer ${accessToken}`) .send(); - expect(status).toBe(201); + expect(status).toBe(204); }, getLibraryStatistics: async (server: any, accessToken: string, id: string): Promise => { const { body, status } = await request(server) diff --git a/server/e2e/jobs/specs/library.e2e-spec.ts b/server/e2e/jobs/specs/library.e2e-spec.ts index 33208fde29..0657227f8d 100644 --- a/server/e2e/jobs/specs/library.e2e-spec.ts +++ b/server/e2e/jobs/specs/library.e2e-spec.ts @@ -45,12 +45,11 @@ describe(`${LibraryController.name} (e2e)`, () => { const assets = await api.assetApi.getAllAssets(server, admin.accessToken); expect(assets.length).toBeGreaterThan(2); - const { status, body } = await request(server) + const { status } = await request(server) .delete(`/library/${library.id}`) .set('Authorization', `Bearer ${admin.accessToken}`); - expect(status).toBe(200); - expect(body).toEqual({}); + expect(status).toBe(204); const libraries = await api.libraryApi.getAll(server, admin.accessToken); expect(libraries).toHaveLength(1); @@ -392,7 +391,7 @@ describe(`${LibraryController.name} (e2e)`, () => { .post(`/library/${library.id}/removeOffline`) .set('Authorization', `Bearer ${admin.accessToken}`) .send(); - expect(status).toBe(201); + expect(status).toBe(204); const assets = await api.assetApi.getAllAssets(server, admin.accessToken); @@ -416,7 +415,7 @@ describe(`${LibraryController.name} (e2e)`, () => { .post(`/library/${library.id}/removeOffline`) .set('Authorization', `Bearer ${admin.accessToken}`) .send(); - expect(status).toBe(201); + expect(status).toBe(204); const assetsAfter = await api.assetApi.getAllAssets(server, admin.accessToken); diff --git a/server/src/immich/controllers/library.controller.ts b/server/src/immich/controllers/library.controller.ts index fb68b2626f..9ad7119799 100644 --- a/server/src/immich/controllers/library.controller.ts +++ b/server/src/immich/controllers/library.controller.ts @@ -10,7 +10,7 @@ import { ValidateLibraryDto, ValidateLibraryResponseDto, } from '@app/domain'; -import { Body, Controller, Delete, Get, HttpCode, Param, Post, Put, Query } from '@nestjs/common'; +import { Body, Controller, Delete, Get, HttpCode, HttpStatus, Param, Post, Put, Query } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { AdminRoute, Auth, Authenticated } from '../app.guard'; import { UseValidation } from '../app.utils'; @@ -55,6 +55,7 @@ export class LibraryController { } @Delete(':id') + @HttpCode(HttpStatus.NO_CONTENT) deleteLibrary(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise { return this.service.delete(auth, id); } @@ -65,11 +66,13 @@ export class LibraryController { } @Post(':id/scan') + @HttpCode(HttpStatus.NO_CONTENT) scanLibrary(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto, @Body() dto: ScanLibraryDto) { return this.service.queueScan(auth, id, dto); } @Post(':id/removeOffline') + @HttpCode(HttpStatus.NO_CONTENT) removeOfflineFiles(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto) { return this.service.queueRemoveOffline(auth, id); } From 15a4a4aaaa2a2a5a89f68848622df7d630286ad8 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 29 Feb 2024 16:02:08 -0500 Subject: [PATCH 2/3] chore: remove unused upload property (#7535) * chore: remove isExternal * chore: open-api --- mobile/openapi/doc/AssetApi.md | 6 ++--- mobile/openapi/lib/api/asset_api.dart | 14 +++-------- mobile/openapi/test/asset_api_test.dart | 2 +- open-api/immich-openapi-specs.json | 3 --- open-api/typescript-sdk/src/fetch-client.ts | 1 - .../src/immich/api-v1/asset/asset.service.ts | 9 -------- .../api-v1/asset/dto/create-asset.dto.ts | 23 ++++++++----------- 7 files changed, 15 insertions(+), 43 deletions(-) diff --git a/mobile/openapi/doc/AssetApi.md b/mobile/openapi/doc/AssetApi.md index 93b758a595..c65e6a605f 100644 --- a/mobile/openapi/doc/AssetApi.md +++ b/mobile/openapi/doc/AssetApi.md @@ -1401,7 +1401,7 @@ void (empty response body) [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) # **uploadFile** -> AssetFileUploadResponseDto uploadFile(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key, duration, isArchived, isExternal, isFavorite, isOffline, isReadOnly, isVisible, libraryId, livePhotoData, sidecarData) +> AssetFileUploadResponseDto uploadFile(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key, duration, isArchived, isFavorite, isOffline, isReadOnly, isVisible, libraryId, livePhotoData, sidecarData) @@ -1432,7 +1432,6 @@ final fileModifiedAt = 2013-10-20T19:20:30+01:00; // DateTime | final key = key_example; // String | final duration = duration_example; // String | final isArchived = true; // bool | -final isExternal = true; // bool | final isFavorite = true; // bool | final isOffline = true; // bool | final isReadOnly = true; // bool | @@ -1442,7 +1441,7 @@ final livePhotoData = BINARY_DATA_HERE; // MultipartFile | final sidecarData = BINARY_DATA_HERE; // MultipartFile | try { - final result = api_instance.uploadFile(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key, duration, isArchived, isExternal, isFavorite, isOffline, isReadOnly, isVisible, libraryId, livePhotoData, sidecarData); + final result = api_instance.uploadFile(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key, duration, isArchived, isFavorite, isOffline, isReadOnly, isVisible, libraryId, livePhotoData, sidecarData); print(result); } catch (e) { print('Exception when calling AssetApi->uploadFile: $e\n'); @@ -1461,7 +1460,6 @@ Name | Type | Description | Notes **key** | **String**| | [optional] **duration** | **String**| | [optional] **isArchived** | **bool**| | [optional] - **isExternal** | **bool**| | [optional] **isFavorite** | **bool**| | [optional] **isOffline** | **bool**| | [optional] **isReadOnly** | **bool**| | [optional] diff --git a/mobile/openapi/lib/api/asset_api.dart b/mobile/openapi/lib/api/asset_api.dart index bdcb874092..786129b457 100644 --- a/mobile/openapi/lib/api/asset_api.dart +++ b/mobile/openapi/lib/api/asset_api.dart @@ -1676,8 +1676,6 @@ class AssetApi { /// /// * [bool] isArchived: /// - /// * [bool] isExternal: - /// /// * [bool] isFavorite: /// /// * [bool] isOffline: @@ -1691,7 +1689,7 @@ class AssetApi { /// * [MultipartFile] livePhotoData: /// /// * [MultipartFile] sidecarData: - Future uploadFileWithHttpInfo(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String? key, String? duration, bool? isArchived, bool? isExternal, bool? isFavorite, bool? isOffline, bool? isReadOnly, bool? isVisible, String? libraryId, MultipartFile? livePhotoData, MultipartFile? sidecarData, }) async { + Future uploadFileWithHttpInfo(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String? key, String? duration, bool? isArchived, bool? isFavorite, bool? isOffline, bool? isReadOnly, bool? isVisible, String? libraryId, MultipartFile? livePhotoData, MultipartFile? sidecarData, }) async { // ignore: prefer_const_declarations final path = r'/asset/upload'; @@ -1739,10 +1737,6 @@ class AssetApi { hasFields = true; mp.fields[r'isArchived'] = parameterToString(isArchived); } - if (isExternal != null) { - hasFields = true; - mp.fields[r'isExternal'] = parameterToString(isExternal); - } if (isFavorite != null) { hasFields = true; mp.fields[r'isFavorite'] = parameterToString(isFavorite); @@ -1806,8 +1800,6 @@ class AssetApi { /// /// * [bool] isArchived: /// - /// * [bool] isExternal: - /// /// * [bool] isFavorite: /// /// * [bool] isOffline: @@ -1821,8 +1813,8 @@ class AssetApi { /// * [MultipartFile] livePhotoData: /// /// * [MultipartFile] sidecarData: - Future uploadFile(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String? key, String? duration, bool? isArchived, bool? isExternal, bool? isFavorite, bool? isOffline, bool? isReadOnly, bool? isVisible, String? libraryId, MultipartFile? livePhotoData, MultipartFile? sidecarData, }) async { - final response = await uploadFileWithHttpInfo(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key: key, duration: duration, isArchived: isArchived, isExternal: isExternal, isFavorite: isFavorite, isOffline: isOffline, isReadOnly: isReadOnly, isVisible: isVisible, libraryId: libraryId, livePhotoData: livePhotoData, sidecarData: sidecarData, ); + Future uploadFile(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String? key, String? duration, bool? isArchived, bool? isFavorite, bool? isOffline, bool? isReadOnly, bool? isVisible, String? libraryId, MultipartFile? livePhotoData, MultipartFile? sidecarData, }) async { + final response = await uploadFileWithHttpInfo(assetData, deviceAssetId, deviceId, fileCreatedAt, fileModifiedAt, key: key, duration: duration, isArchived: isArchived, isFavorite: isFavorite, isOffline: isOffline, isReadOnly: isReadOnly, isVisible: isVisible, libraryId: libraryId, livePhotoData: livePhotoData, sidecarData: sidecarData, ); if (response.statusCode >= HttpStatus.badRequest) { throw ApiException(response.statusCode, await _decodeBodyBytes(response)); } diff --git a/mobile/openapi/test/asset_api_test.dart b/mobile/openapi/test/asset_api_test.dart index 06375c6b49..846a5998cc 100644 --- a/mobile/openapi/test/asset_api_test.dart +++ b/mobile/openapi/test/asset_api_test.dart @@ -135,7 +135,7 @@ void main() { // TODO }); - //Future uploadFile(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String key, String duration, bool isArchived, bool isExternal, bool isFavorite, bool isOffline, bool isReadOnly, bool isVisible, String libraryId, MultipartFile livePhotoData, MultipartFile sidecarData }) async + //Future uploadFile(MultipartFile assetData, String deviceAssetId, String deviceId, DateTime fileCreatedAt, DateTime fileModifiedAt, { String key, String duration, bool isArchived, bool isFavorite, bool isOffline, bool isReadOnly, bool isVisible, String libraryId, MultipartFile livePhotoData, MultipartFile sidecarData }) async test('test uploadFile', () async { // TODO }); diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index f38780f294..c0e8689850 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -7541,9 +7541,6 @@ "isArchived": { "type": "boolean" }, - "isExternal": { - "type": "boolean" - }, "isFavorite": { "type": "boolean" }, diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index ce768d518b..b512bce296 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -289,7 +289,6 @@ export type CreateAssetDto = { fileCreatedAt: string; fileModifiedAt: string; isArchived?: boolean; - isExternal?: boolean; isFavorite?: boolean; isOffline?: boolean; isReadOnly?: boolean; diff --git a/server/src/immich/api-v1/asset/asset.service.ts b/server/src/immich/api-v1/asset/asset.service.ts index 17488016c9..5dcc487be4 100644 --- a/server/src/immich/api-v1/asset/asset.service.ts +++ b/server/src/immich/api-v1/asset/asset.service.ts @@ -341,7 +341,6 @@ export class AssetService { fileCreatedAt: dto.fileCreatedAt, fileModifiedAt: dto.fileModifiedAt, localDateTime: dto.fileCreatedAt, - deletedAt: null, type: mimeTypes.assetType(file.originalPath), isFavorite: dto.isFavorite, @@ -349,17 +348,9 @@ export class AssetService { duration: dto.duration || null, isVisible: dto.isVisible ?? true, livePhotoVideo: livePhotoAssetId === null ? null : ({ id: livePhotoAssetId } as AssetEntity), - resizePath: null, - webpPath: null, - thumbhash: null, - encodedVideoPath: null, - tags: [], - sharedLinks: [], originalFileName: parse(file.originalName).name, - faces: [], sidecarPath: sidecarPath || null, isReadOnly: dto.isReadOnly ?? false, - isExternal: dto.isExternal ?? false, isOffline: dto.isOffline ?? false, }); diff --git a/server/src/immich/api-v1/asset/dto/create-asset.dto.ts b/server/src/immich/api-v1/asset/dto/create-asset.dto.ts index ae347e61b6..9850384d96 100644 --- a/server/src/immich/api-v1/asset/dto/create-asset.dto.ts +++ b/server/src/immich/api-v1/asset/dto/create-asset.dto.ts @@ -3,7 +3,10 @@ import { ApiProperty } from '@nestjs/swagger'; import { Transform, Type } from 'class-transformer'; import { IsBoolean, IsDate, IsNotEmpty, IsString } from 'class-validator'; -export class CreateAssetBase { +export class CreateAssetDto { + @ValidateUUID({ optional: true }) + libraryId?: string; + @IsNotEmpty() @IsString() deviceAssetId!: string; @@ -22,6 +25,10 @@ export class CreateAssetBase { @Type(() => Date) fileModifiedAt!: Date; + @Optional() + @IsString() + duration?: string; + @Optional() @IsBoolean() @Transform(toBoolean) @@ -37,28 +44,16 @@ export class CreateAssetBase { @Transform(toBoolean) isVisible?: boolean; - @Optional() - @IsString() - duration?: string; - - @Optional() - @IsBoolean() - isExternal?: boolean; - @Optional() @IsBoolean() + @Transform(toBoolean) isOffline?: boolean; -} -export class CreateAssetDto extends CreateAssetBase { @Optional() @IsBoolean() @Transform(toBoolean) isReadOnly?: boolean; - @ValidateUUID({ optional: true }) - libraryId?: string; - // The properties below are added to correctly generate the API docs // and client SDKs. Validation should be handled in the controller. @ApiProperty({ type: 'string', format: 'binary' }) From c89d91e006474dcabc5379d0935ef4e9b6d37306 Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Thu, 29 Feb 2024 22:14:48 +0100 Subject: [PATCH 3/3] feat: filter people when using smart search (#7521) --- mobile/openapi/doc/SmartSearchDto.md | 1 + .../openapi/lib/model/smart_search_dto.dart | 11 +++++++++- .../openapi/test/smart_search_dto_test.dart | 5 +++++ open-api/immich-openapi-specs.json | 6 ++++++ open-api/typescript-sdk/src/fetch-client.ts | 1 + server/src/domain/search/dto/search.dto.ts | 6 +++--- .../infra/repositories/search.repository.ts | 21 +++++++++++++++++-- .../search-bar/search-filter-box.svelte | 9 -------- 8 files changed, 45 insertions(+), 15 deletions(-) diff --git a/mobile/openapi/doc/SmartSearchDto.md b/mobile/openapi/doc/SmartSearchDto.md index d4ec1a70f6..d5f4c40256 100644 --- a/mobile/openapi/doc/SmartSearchDto.md +++ b/mobile/openapi/doc/SmartSearchDto.md @@ -27,6 +27,7 @@ Name | Type | Description | Notes **make** | **String** | | [optional] **model** | **String** | | [optional] **page** | **num** | | [optional] +**personIds** | **List** | | [optional] [default to const []] **query** | **String** | | **size** | **num** | | [optional] **state** | **String** | | [optional] diff --git a/mobile/openapi/lib/model/smart_search_dto.dart b/mobile/openapi/lib/model/smart_search_dto.dart index 664850db82..0b99acdd66 100644 --- a/mobile/openapi/lib/model/smart_search_dto.dart +++ b/mobile/openapi/lib/model/smart_search_dto.dart @@ -32,6 +32,7 @@ class SmartSearchDto { this.make, this.model, this.page, + this.personIds = const [], required this.query, this.size, this.state, @@ -199,6 +200,8 @@ class SmartSearchDto { /// num? page; + List personIds; + String query; /// @@ -312,6 +315,7 @@ class SmartSearchDto { other.make == make && other.model == model && other.page == page && + _deepEquality.equals(other.personIds, personIds) && other.query == query && other.size == size && other.state == state && @@ -348,6 +352,7 @@ class SmartSearchDto { (make == null ? 0 : make!.hashCode) + (model == null ? 0 : model!.hashCode) + (page == null ? 0 : page!.hashCode) + + (personIds.hashCode) + (query.hashCode) + (size == null ? 0 : size!.hashCode) + (state == null ? 0 : state!.hashCode) + @@ -363,7 +368,7 @@ class SmartSearchDto { (withExif == null ? 0 : withExif!.hashCode); @override - String toString() => 'SmartSearchDto[city=$city, country=$country, createdAfter=$createdAfter, createdBefore=$createdBefore, deviceId=$deviceId, isArchived=$isArchived, isEncoded=$isEncoded, isExternal=$isExternal, isFavorite=$isFavorite, isMotion=$isMotion, isNotInAlbum=$isNotInAlbum, isOffline=$isOffline, isReadOnly=$isReadOnly, isVisible=$isVisible, lensModel=$lensModel, libraryId=$libraryId, make=$make, model=$model, page=$page, query=$query, size=$size, state=$state, takenAfter=$takenAfter, takenBefore=$takenBefore, trashedAfter=$trashedAfter, trashedBefore=$trashedBefore, type=$type, updatedAfter=$updatedAfter, updatedBefore=$updatedBefore, withArchived=$withArchived, withDeleted=$withDeleted, withExif=$withExif]'; + String toString() => 'SmartSearchDto[city=$city, country=$country, createdAfter=$createdAfter, createdBefore=$createdBefore, deviceId=$deviceId, isArchived=$isArchived, isEncoded=$isEncoded, isExternal=$isExternal, isFavorite=$isFavorite, isMotion=$isMotion, isNotInAlbum=$isNotInAlbum, isOffline=$isOffline, isReadOnly=$isReadOnly, isVisible=$isVisible, lensModel=$lensModel, libraryId=$libraryId, make=$make, model=$model, page=$page, personIds=$personIds, query=$query, size=$size, state=$state, takenAfter=$takenAfter, takenBefore=$takenBefore, trashedAfter=$trashedAfter, trashedBefore=$trashedBefore, type=$type, updatedAfter=$updatedAfter, updatedBefore=$updatedBefore, withArchived=$withArchived, withDeleted=$withDeleted, withExif=$withExif]'; Map toJson() { final json = {}; @@ -462,6 +467,7 @@ class SmartSearchDto { } else { // json[r'page'] = null; } + json[r'personIds'] = this.personIds; json[r'query'] = this.query; if (this.size != null) { json[r'size'] = this.size; @@ -549,6 +555,9 @@ class SmartSearchDto { make: mapValueOfType(json, r'make'), model: mapValueOfType(json, r'model'), page: num.parse('${json[r'page']}'), + personIds: json[r'personIds'] is Iterable + ? (json[r'personIds'] as Iterable).cast().toList(growable: false) + : const [], query: mapValueOfType(json, r'query')!, size: num.parse('${json[r'size']}'), state: mapValueOfType(json, r'state'), diff --git a/mobile/openapi/test/smart_search_dto_test.dart b/mobile/openapi/test/smart_search_dto_test.dart index 4db3ac0808..5263f7bb6a 100644 --- a/mobile/openapi/test/smart_search_dto_test.dart +++ b/mobile/openapi/test/smart_search_dto_test.dart @@ -111,6 +111,11 @@ void main() { // TODO }); + // List personIds (default value: const []) + test('to test the property `personIds`', () async { + // TODO + }); + // String query test('to test the property `query`', () async { // TODO diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index c0e8689850..d5bffc887a 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -9539,6 +9539,12 @@ "page": { "type": "number" }, + "personIds": { + "items": { + "type": "string" + }, + "type": "array" + }, "query": { "type": "string" }, diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index b512bce296..12238e40bc 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -671,6 +671,7 @@ export type SmartSearchDto = { make?: string; model?: string; page?: number; + personIds?: string[]; query: string; size?: number; state?: string; diff --git a/server/src/domain/search/dto/search.dto.ts b/server/src/domain/search/dto/search.dto.ts index 877a494e4d..c529f6887b 100644 --- a/server/src/domain/search/dto/search.dto.ts +++ b/server/src/domain/search/dto/search.dto.ts @@ -122,6 +122,9 @@ class BaseSearchDto { @QueryBoolean({ optional: true }) isNotInAlbum?: boolean; + + @Optional() + personIds?: string[]; } export class MetadataSearchDto extends BaseSearchDto { @@ -173,9 +176,6 @@ export class MetadataSearchDto extends BaseSearchDto { @Optional() @ApiProperty({ enumName: 'AssetOrder', enum: AssetOrder }) order?: AssetOrder; - - @Optional() - personIds?: string[]; } export class SmartSearchDto extends BaseSearchDto { diff --git a/server/src/infra/repositories/search.repository.ts b/server/src/infra/repositories/search.repository.ts index c8dc5070f7..0ff26a4f5f 100644 --- a/server/src/infra/repositories/search.repository.ts +++ b/server/src/infra/repositories/search.repository.ts @@ -22,7 +22,7 @@ import { import { ImmichLogger } from '@app/infra/logger'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { Repository, SelectQueryBuilder } from 'typeorm'; import { vectorExt } from '../database.config'; import { DummyValue, GenerateSql } from '../infra.util'; import { asVector, isValidInteger, paginatedBuilder, searchAssetBuilder } from '../infra.utils'; @@ -81,6 +81,14 @@ export class SearchRepository implements ISearchRepository { }); } + private createPersonFilter(builder: SelectQueryBuilder, personIds: string[]) { + return builder + .select(`${builder.alias}."assetId"`) + .where(`${builder.alias}."personId" IN (:...personIds)`, { personIds }) + .groupBy(`${builder.alias}."assetId"`) + .having(`COUNT(DISTINCT ${builder.alias}."personId") = :personCount`, { personCount: personIds.length }); + } + @GenerateSql({ params: [ { page: 1, size: 100 }, @@ -96,12 +104,21 @@ export class SearchRepository implements ISearchRepository { }) async searchSmart( pagination: SearchPaginationOptions, - { embedding, userIds, ...options }: SmartSearchOptions, + { embedding, userIds, personIds, ...options }: SmartSearchOptions, ): Paginated { let results: PaginationResult = { items: [], hasNextPage: false }; await this.assetRepository.manager.transaction(async (manager) => { let builder = manager.createQueryBuilder(AssetEntity, 'asset'); + + if (personIds?.length) { + const assetFaceBuilder = manager.createQueryBuilder(AssetFaceEntity, 'asset_face'); + const cte = this.createPersonFilter(assetFaceBuilder, personIds); + builder + .addCommonTableExpression(cte, 'asset_face_ids') + .innerJoin('asset_face_ids', 'a', 'a."assetId" = asset.id'); + } + builder = searchAssetBuilder(builder, options); builder .innerJoin('asset.smartSearch', 'search') diff --git a/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte b/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte index 8d060f4d0d..b05e2d5a3b 100644 --- a/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte +++ b/web/src/lib/components/shared-components/search-bar/search-filter-box.svelte @@ -22,7 +22,6 @@