From b306cf564e26c13905ddcbf609254699bab47340 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 25 Jan 2024 12:52:21 -0500 Subject: [PATCH] refactor(server): move asset detail endpoint to new controller (#6636) * refactor(server): move asset by id to new controller * chore: open api * refactor: more consolidation * refactor: asset service --- mobile/openapi/README.md | Bin 24114 -> 24202 bytes mobile/openapi/doc/AssetApi.md | Bin 69392 -> 71469 bytes mobile/openapi/lib/api/asset_api.dart | Bin 67112 -> 68827 bytes mobile/openapi/test/asset_api_test.dart | Bin 6519 -> 6660 bytes open-api/immich-openapi-specs.json | 49 +++++++++ open-api/typescript-sdk/client/api.ts | 103 ++++++++++++++++++ server/e2e/api/specs/asset.e2e-spec.ts | 103 ++++++++++++++++++ server/src/domain/asset/asset.service.spec.ts | 58 +++++++++- server/src/domain/asset/asset.service.ts | 40 ++++++- .../immich/api-v1/asset/asset-repository.ts | 35 +----- .../immich/api-v1/asset/asset.controller.ts | 33 +++--- server/src/immich/api-v1/asset/asset.core.ts | 4 +- .../immich/api-v1/asset/asset.service.spec.ts | 90 +++------------ .../src/immich/api-v1/asset/asset.service.ts | 56 +++------- server/src/immich/app.module.ts | 6 +- .../immich/controllers/asset.controller.ts | 6 + .../asset-viewer/detail-panel.svelte | 6 +- .../sharedlinks-page/shared-link-card.svelte | 2 +- web/src/lib/stores/asset-viewing.store.ts | 2 +- .../share/[key]/photos/[assetId]/+page.ts | 2 +- 20 files changed, 421 insertions(+), 174 deletions(-) diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md index 0f27662b813a4d6e2c8a8da960109a0bf597b1da..65ec44b4e50faf66117ea45598bc8f0cf75f7b45 100644 GIT binary patch delta 45 zcmdnAhp}rfR7@+V(lmSD*QF&J5eH*aDt3xEh4yc82jPc3mQE>0~0snf_z z(NV}wt=wF&NttCbpMLA)xe*)?rStV8CeKfRayBNkOQj2TbN>;oTe*b9)y6ZhHwN diff --git a/mobile/openapi/lib/api/asset_api.dart b/mobile/openapi/lib/api/asset_api.dart index 2d7f17c1618457700dc9ab2ef9d3aa321befa786..878fa7734b528e8523cddc7be26f7ab566dfc3bf 100644 GIT binary patch delta 94 zcmZ3{!*Y8j%LXf_$$9>h-W_w1Q0 vpH{GW!7U+1uu2(w>B)SudXp7G<-w{q=PS4}O}^mII(gsI*vh29S3#* diff --git a/mobile/openapi/test/asset_api_test.dart b/mobile/openapi/test/asset_api_test.dart index 27bb2545289531073005619f4042f9f49f952571..e17104fd70b68cefe28ab5acfe7890811a5775ae 100644 GIT binary patch delta 42 scmexv)MB#XB*$bvF`>zR92_j3d1?8R6 => { @@ -7180,6 +7181,53 @@ export const AssetApiAxiosParamCreator = function (configuration?: Configuration + setSearchParams(localVarUrlObj, localVarQueryParameter); + let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, + /** + * + * @param {string} id + * @param {string} [key] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + getAssetInfo: async (id: string, key?: string, options: AxiosRequestConfig = {}): Promise => { + // verify required parameter 'id' is not null or undefined + assertParamExists('getAssetInfo', 'id', id) + const localVarPath = `/asset/{id}` + .replace(`{${"id"}}`, encodeURIComponent(String(id))); + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { method: 'GET', ...baseOptions, ...options}; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + // authentication cookie required + + // authentication api_key required + await setApiKeyToObject(localVarHeaderParameter, "x-api-key", configuration) + + // authentication bearer required + // http bearer authentication required + await setBearerAuthToObject(localVarHeaderParameter, configuration) + + if (key !== undefined) { + localVarQueryParameter['key'] = key; + } + + + setSearchParams(localVarUrlObj, localVarQueryParameter); let headersFromBaseOptions = baseOptions && baseOptions.headers ? baseOptions.headers : {}; localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers}; @@ -8609,12 +8657,24 @@ export const AssetApiFp = function(configuration?: Configuration) { * @param {string} id * @param {string} [key] * @param {*} [options] Override http request option. + * @deprecated * @throws {RequiredError} */ async getAssetById(id: string, key?: string, options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { const localVarAxiosArgs = await localVarAxiosParamCreator.getAssetById(id, key, options); return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); }, + /** + * + * @param {string} id + * @param {string} [key] + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async getAssetInfo(id: string, key?: string, options?: AxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.getAssetInfo(id, key, options); + return createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration); + }, /** * * @param {*} [options] Override http request option. @@ -8982,11 +9042,21 @@ export const AssetApiFactory = function (configuration?: Configuration, basePath * Get a single asset\'s information * @param {AssetApiGetAssetByIdRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. + * @deprecated * @throws {RequiredError} */ getAssetById(requestParameters: AssetApiGetAssetByIdRequest, options?: AxiosRequestConfig): AxiosPromise { return localVarFp.getAssetById(requestParameters.id, requestParameters.key, options).then((request) => request(axios, basePath)); }, + /** + * + * @param {AssetApiGetAssetInfoRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + getAssetInfo(requestParameters: AssetApiGetAssetInfoRequest, options?: AxiosRequestConfig): AxiosPromise { + return localVarFp.getAssetInfo(requestParameters.id, requestParameters.key, options).then((request) => request(axios, basePath)); + }, /** * * @param {*} [options] Override http request option. @@ -9348,6 +9418,27 @@ export interface AssetApiGetAssetByIdRequest { readonly key?: string } +/** + * Request parameters for getAssetInfo operation in AssetApi. + * @export + * @interface AssetApiGetAssetInfoRequest + */ +export interface AssetApiGetAssetInfoRequest { + /** + * + * @type {string} + * @memberof AssetApiGetAssetInfo + */ + readonly id: string + + /** + * + * @type {string} + * @memberof AssetApiGetAssetInfo + */ + readonly key?: string +} + /** * Request parameters for getAssetStatistics operation in AssetApi. * @export @@ -10272,6 +10363,7 @@ export class AssetApi extends BaseAPI { * Get a single asset\'s information * @param {AssetApiGetAssetByIdRequest} requestParameters Request parameters. * @param {*} [options] Override http request option. + * @deprecated * @throws {RequiredError} * @memberof AssetApi */ @@ -10279,6 +10371,17 @@ export class AssetApi extends BaseAPI { return AssetApiFp(this.configuration).getAssetById(requestParameters.id, requestParameters.key, options).then((request) => request(this.axios, this.basePath)); } + /** + * + * @param {AssetApiGetAssetInfoRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof AssetApi + */ + public getAssetInfo(requestParameters: AssetApiGetAssetInfoRequest, options?: AxiosRequestConfig) { + return AssetApiFp(this.configuration).getAssetInfo(requestParameters.id, requestParameters.key, options).then((request) => request(this.axios, this.basePath)); + } + /** * * @param {*} [options] Override http request option. diff --git a/server/e2e/api/specs/asset.e2e-spec.ts b/server/e2e/api/specs/asset.e2e-spec.ts index 5f39059fdb..4e7dc03b08 100644 --- a/server/e2e/api/specs/asset.e2e-spec.ts +++ b/server/e2e/api/specs/asset.e2e-spec.ts @@ -542,6 +542,109 @@ describe(`${AssetController.name} (e2e)`, () => { } }); + // TODO remove with deprecated endpoint + describe('GET /asset/assetById/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(server).get(`/asset/assetById/${uuidStub.notFound}`); + expect(body).toEqual(errorStub.unauthorized); + expect(status).toBe(401); + }); + + it('should require a valid id', async () => { + const { status, body } = await request(server) + .get(`/asset/assetById/${uuidStub.invalid}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.badRequest(['id must be a UUID'])); + }); + + it('should require access', async () => { + const { status, body } = await request(server) + .get(`/asset/assetById/${asset4.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.noPermission); + }); + + it('should get the asset info', async () => { + const { status, body } = await request(server) + .get(`/asset/assetById/${asset1.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toMatchObject({ + id: asset1.id, + stack: [], + stackCount: 0, + }); + }); + + it('should work with a shared link', async () => { + const sharedLink = await api.sharedLinkApi.create(server, user1.accessToken, { + type: SharedLinkType.INDIVIDUAL, + assetIds: [asset1.id], + }); + + const { status, body } = await request(server).get(`/asset/assetById/${asset1.id}?key=${sharedLink.key}`); + expect(status).toBe(200); + expect(body).toMatchObject({ + id: asset1.id, + stack: [], + stackCount: 0, + }); + }); + }); + + describe('GET /asset/:id', () => { + it('should require authentication', async () => { + const { status, body } = await request(server).get(`/asset/${uuidStub.notFound}`); + expect(body).toEqual(errorStub.unauthorized); + expect(status).toBe(401); + }); + + it('should require a valid id', async () => { + const { status, body } = await request(server) + .get(`/asset/${uuidStub.invalid}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.badRequest(['id must be a UUID'])); + }); + + it('should require access', async () => { + const { status, body } = await request(server) + .get(`/asset/${asset4.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(400); + expect(body).toEqual(errorStub.noPermission); + }); + + it('should get the asset info', async () => { + const { status, body } = await request(server) + .get(`/asset/${asset1.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`); + expect(status).toBe(200); + expect(body).toMatchObject({ + id: asset1.id, + stack: [], + stackCount: 0, + }); + }); + + it('should work with a shared link', async () => { + const sharedLink = await api.sharedLinkApi.create(server, user1.accessToken, { + type: SharedLinkType.INDIVIDUAL, + assetIds: [asset1.id], + }); + + const { status, body } = await request(server).get(`/asset/${asset1.id}?key=${sharedLink.key}`); + expect(status).toBe(200); + expect(body).toMatchObject({ + id: asset1.id, + stack: [], + stackCount: 0, + }); + }); + }); + describe('POST /asset/upload', () => { it('should require authentication', async () => { const { status, body } = await request(server) diff --git a/server/src/domain/asset/asset.service.spec.ts b/server/src/domain/asset/asset.service.spec.ts index 3bc39379ba..100dc9a4ef 100644 --- a/server/src/domain/asset/asset.service.spec.ts +++ b/server/src/domain/asset/asset.service.spec.ts @@ -8,7 +8,6 @@ import { newAccessRepositoryMock, newAssetRepositoryMock, newCommunicationRepositoryMock, - newCryptoRepositoryMock, newJobRepositoryMock, newPartnerRepositoryMock, newStorageRepositoryMock, @@ -24,7 +23,6 @@ import { ClientEvent, IAssetRepository, ICommunicationRepository, - ICryptoRepository, IJobRepository, IPartnerRepository, IStorageRepository, @@ -168,7 +166,6 @@ describe(AssetService.name, () => { let sut: AssetService; let accessMock: IAccessRepositoryMock; let assetMock: jest.Mocked; - let cryptoMock: jest.Mocked; let jobMock: jest.Mocked; let storageMock: jest.Mocked; let userMock: jest.Mocked; @@ -184,7 +181,6 @@ describe(AssetService.name, () => { accessMock = newAccessRepositoryMock(); assetMock = newAssetRepositoryMock(); communicationMock = newCommunicationRepositoryMock(); - cryptoMock = newCryptoRepositoryMock(); jobMock = newJobRepositoryMock(); storageMock = newStorageRepositoryMock(); userMock = newUserRepositoryMock(); @@ -194,7 +190,6 @@ describe(AssetService.name, () => { sut = new AssetService( accessMock, assetMock, - cryptoMock, jobMock, configMock, storageMock, @@ -657,6 +652,59 @@ describe(AssetService.name, () => { }); }); + describe('get', () => { + it('should allow owner access', async () => { + accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + assetMock.getById.mockResolvedValue(assetStub.image); + await sut.get(authStub.admin, assetStub.image.id); + expect(accessMock.asset.checkOwnerAccess).toHaveBeenCalledWith( + authStub.admin.user.id, + new Set([assetStub.image.id]), + ); + }); + + it('should allow shared link access', async () => { + accessMock.asset.checkSharedLinkAccess.mockResolvedValue(new Set([assetStub.image.id])); + assetMock.getById.mockResolvedValue(assetStub.image); + await sut.get(authStub.adminSharedLink, assetStub.image.id); + expect(accessMock.asset.checkSharedLinkAccess).toHaveBeenCalledWith( + authStub.adminSharedLink.sharedLink?.id, + new Set([assetStub.image.id]), + ); + }); + + it('should allow partner sharing access', async () => { + accessMock.asset.checkPartnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + assetMock.getById.mockResolvedValue(assetStub.image); + await sut.get(authStub.admin, assetStub.image.id); + expect(accessMock.asset.checkPartnerAccess).toHaveBeenCalledWith( + authStub.admin.user.id, + new Set([assetStub.image.id]), + ); + }); + + it('should allow shared album access', async () => { + accessMock.asset.checkAlbumAccess.mockResolvedValue(new Set([assetStub.image.id])); + assetMock.getById.mockResolvedValue(assetStub.image); + await sut.get(authStub.admin, assetStub.image.id); + expect(accessMock.asset.checkAlbumAccess).toHaveBeenCalledWith( + authStub.admin.user.id, + new Set([assetStub.image.id]), + ); + }); + + it('should throw an error for no access', async () => { + await expect(sut.get(authStub.admin, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException); + expect(assetMock.getById).not.toHaveBeenCalled(); + }); + + it('should throw an error for an invalid shared link', async () => { + await expect(sut.get(authStub.adminSharedLink, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException); + expect(accessMock.asset.checkOwnerAccess).not.toHaveBeenCalled(); + expect(assetMock.getById).not.toHaveBeenCalled(); + }); + }); + describe('update', () => { it('should require asset write access for the id', async () => { await expect(sut.update(authStub.admin, 'asset-1', { isArchived: false })).rejects.toBeInstanceOf( diff --git a/server/src/domain/asset/asset.service.ts b/server/src/domain/asset/asset.service.ts index dd0a0b39a6..732ae7ad19 100644 --- a/server/src/domain/asset/asset.service.ts +++ b/server/src/domain/asset/asset.service.ts @@ -15,7 +15,6 @@ import { IAccessRepository, IAssetRepository, ICommunicationRepository, - ICryptoRepository, IJobRepository, IPartnerRepository, IStorageRepository, @@ -87,7 +86,6 @@ export class AssetService { constructor( @Inject(IAccessRepository) accessRepository: IAccessRepository, @Inject(IAssetRepository) private assetRepository: IAssetRepository, - @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository, @Inject(IJobRepository) private jobRepository: IJobRepository, @Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository, @Inject(IStorageRepository) private storageRepository: IStorageRepository, @@ -400,6 +398,44 @@ export class AssetService { return this.assetRepository.getAllByDeviceId(auth.user.id, deviceId); } + async get(auth: AuthDto, id: string): Promise { + await this.access.requirePermission(auth, Permission.ASSET_READ, id); + + const asset = await this.assetRepository.getById(id, { + exifInfo: true, + tags: true, + sharedLinks: true, + smartInfo: true, + owner: true, + faces: { + person: true, + }, + stack: { + exifInfo: true, + }, + }); + + if (!asset) { + throw new BadRequestException('Asset not found'); + } + + if (auth.sharedLink && !auth.sharedLink.showExif) { + return mapAsset(asset, { stripMetadata: true, withStack: true }); + } + + const data = mapAsset(asset, { withStack: true }); + + if (auth.sharedLink) { + delete data.owner; + } + + if (data.ownerId !== auth.user.id) { + data.people = []; + } + + return data; + } + async update(auth: AuthDto, id: string, dto: UpdateAssetDto): Promise { await this.access.requirePermission(auth, Permission.ASSET_UPDATE, id); diff --git a/server/src/immich/api-v1/asset/asset-repository.ts b/server/src/immich/api-v1/asset/asset-repository.ts index 5ceb066013..39701ef2f5 100644 --- a/server/src/immich/api-v1/asset/asset-repository.ts +++ b/server/src/immich/api-v1/asset/asset-repository.ts @@ -20,12 +20,11 @@ export interface AssetOwnerCheck extends AssetCheck { ownerId: string; } -export interface IAssetRepository { +export interface IAssetRepositoryV1 { get(id: string): Promise; create(asset: AssetCreate): Promise; upsertExif(exif: Partial): Promise; getAllByUserId(userId: string, dto: AssetSearchDto): Promise; - getById(assetId: string): Promise; getLocationsByUserId(userId: string): Promise; getDetectedObjectsByUserId(userId: string): Promise; getSearchPropertiesByUserId(userId: string): Promise; @@ -34,10 +33,10 @@ export interface IAssetRepository { getByOriginalPath(originalPath: string): Promise; } -export const IAssetRepository = 'IAssetRepository'; +export const IAssetRepositoryV1 = 'IAssetRepositoryV1'; @Injectable() -export class AssetRepository implements IAssetRepository { +export class AssetRepositoryV1 implements IAssetRepositoryV1 { constructor( @InjectRepository(AssetEntity) private assetRepository: Repository, @InjectRepository(ExifEntity) private exifRepository: Repository, @@ -93,34 +92,6 @@ export class AssetRepository implements IAssetRepository { ); } - /** - * Get a single asset information by its ID - * - include exif info - * @param assetId - */ - getById(assetId: string): Promise { - return this.assetRepository.findOneOrFail({ - where: { - id: assetId, - }, - relations: { - exifInfo: true, - tags: true, - sharedLinks: true, - smartInfo: true, - owner: true, - faces: { - person: true, - }, - stack: { - exifInfo: true, - }, - }, - // We are specifically asking for this asset. Return it even if it is soft deleted - withDeleted: true, - }); - } - /** * Get all assets belong to the user on the database * @param ownerId diff --git a/server/src/immich/api-v1/asset/asset.controller.ts b/server/src/immich/api-v1/asset/asset.controller.ts index 0487d6f56d..ddcb8417c8 100644 --- a/server/src/immich/api-v1/asset/asset.controller.ts +++ b/server/src/immich/api-v1/asset/asset.controller.ts @@ -1,4 +1,4 @@ -import { AssetResponseDto, AuthDto } from '@app/domain'; +import { AssetResponseDto, AssetService, AuthDto } from '@app/domain'; import { Body, Controller, @@ -18,11 +18,11 @@ import { import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger'; import { NextFunction, Response } from 'express'; import { Auth, Authenticated, FileResponse, SharedLinkRoute } from '../../app.guard'; -import { sendFile } from '../../app.utils'; +import { UseValidation, sendFile } from '../../app.utils'; import { UUIDParamDto } from '../../controllers/dto/uuid-param.dto'; import { FileUploadInterceptor, ImmichFile, Route, mapToUploadFile } from '../../interceptors'; import FileNotEmptyValidator from '../validation/file-not-empty-validator'; -import { AssetService } from './asset.service'; +import { AssetService as AssetServiceV1 } from './asset.service'; import { AssetBulkUploadCheckDto } from './dto/asset-check.dto'; import { AssetSearchDto } from './dto/asset-search.dto'; import { CheckExistingAssetsDto } from './dto/check-existing-assets.dto'; @@ -45,7 +45,10 @@ interface UploadFiles { @Controller(Route.ASSET) @Authenticated() export class AssetController { - constructor(private assetService: AssetService) {} + constructor( + private serviceV1: AssetServiceV1, + private service: AssetService, + ) {} @SharedLinkRoute() @Post('upload') @@ -74,7 +77,7 @@ export class AssetController { sidecarFile = mapToUploadFile(_sidecarFile); } - const responseDto = await this.assetService.uploadFile(auth, dto, file, livePhotoFile, sidecarFile); + const responseDto = await this.serviceV1.uploadFile(auth, dto, file, livePhotoFile, sidecarFile); if (responseDto.duplicate) { res.status(HttpStatus.OK); } @@ -92,7 +95,7 @@ export class AssetController { @Param() { id }: UUIDParamDto, @Query(new ValidationPipe({ transform: true })) dto: ServeFileDto, ) { - await sendFile(res, next, () => this.assetService.serveFile(auth, id, dto)); + await sendFile(res, next, () => this.serviceV1.serveFile(auth, id, dto)); } @SharedLinkRoute() @@ -105,22 +108,22 @@ export class AssetController { @Param() { id }: UUIDParamDto, @Query(new ValidationPipe({ transform: true })) dto: GetAssetThumbnailDto, ) { - await sendFile(res, next, () => this.assetService.serveThumbnail(auth, id, dto)); + await sendFile(res, next, () => this.serviceV1.serveThumbnail(auth, id, dto)); } @Get('/curated-objects') getCuratedObjects(@Auth() auth: AuthDto): Promise { - return this.assetService.getCuratedObject(auth); + return this.serviceV1.getCuratedObject(auth); } @Get('/curated-locations') getCuratedLocations(@Auth() auth: AuthDto): Promise { - return this.assetService.getCuratedLocation(auth); + return this.serviceV1.getCuratedLocation(auth); } @Get('/search-terms') getAssetSearchTerms(@Auth() auth: AuthDto): Promise { - return this.assetService.getAssetSearchTerm(auth); + return this.serviceV1.getAssetSearchTerm(auth); } /** @@ -137,16 +140,18 @@ export class AssetController { @Auth() auth: AuthDto, @Query(new ValidationPipe({ transform: true })) dto: AssetSearchDto, ): Promise { - return this.assetService.getAllAssets(auth, dto); + return this.serviceV1.getAllAssets(auth, dto); } /** * Get a single asset's information + * @deprecated Use `/asset/:id` */ @SharedLinkRoute() + @UseValidation() @Get('/assetById/:id') getAssetById(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise { - return this.assetService.getAssetById(auth, id) as Promise; + return this.service.get(auth, id) as Promise; } /** @@ -158,7 +163,7 @@ export class AssetController { @Auth() auth: AuthDto, @Body(ValidationPipe) dto: CheckExistingAssetsDto, ): Promise { - return this.assetService.checkExistingAssets(auth, dto); + return this.serviceV1.checkExistingAssets(auth, dto); } /** @@ -170,6 +175,6 @@ export class AssetController { @Auth() auth: AuthDto, @Body(ValidationPipe) dto: AssetBulkUploadCheckDto, ): Promise { - return this.assetService.bulkUploadCheck(auth, dto); + return this.serviceV1.bulkUploadCheck(auth, dto); } } diff --git a/server/src/immich/api-v1/asset/asset.core.ts b/server/src/immich/api-v1/asset/asset.core.ts index e3fd6365e2..ec68c98a1e 100644 --- a/server/src/immich/api-v1/asset/asset.core.ts +++ b/server/src/immich/api-v1/asset/asset.core.ts @@ -2,12 +2,12 @@ import { AuthDto, IJobRepository, JobName, mimeTypes, UploadFile } from '@app/do import { AssetEntity } from '@app/infra/entities'; import { BadRequestException } from '@nestjs/common'; import { parse } from 'node:path'; -import { IAssetRepository } from './asset-repository'; +import { IAssetRepositoryV1 } from './asset-repository'; import { CreateAssetDto } from './dto/create-asset.dto'; export class AssetCore { constructor( - private repository: IAssetRepository, + private repository: IAssetRepositoryV1, private jobRepository: IJobRepository, ) {} diff --git a/server/src/immich/api-v1/asset/asset.service.spec.ts b/server/src/immich/api-v1/asset/asset.service.spec.ts index c91d3efab6..8d3046c3a3 100644 --- a/server/src/immich/api-v1/asset/asset.service.spec.ts +++ b/server/src/immich/api-v1/asset/asset.service.spec.ts @@ -1,19 +1,19 @@ -import { IJobRepository, ILibraryRepository, IUserRepository, JobName } from '@app/domain'; +import { IAssetRepository, IJobRepository, ILibraryRepository, IUserRepository, JobName } from '@app/domain'; import { ASSET_CHECKSUM_CONSTRAINT, AssetEntity, AssetType, ExifEntity } from '@app/infra/entities'; -import { BadRequestException } from '@nestjs/common'; import { IAccessRepositoryMock, assetStub, authStub, fileStub, newAccessRepositoryMock, + newAssetRepositoryMock, newJobRepositoryMock, newLibraryRepositoryMock, newUserRepositoryMock, } from '@test'; import { when } from 'jest-when'; import { QueryFailedError } from 'typeorm'; -import { IAssetRepository } from './asset-repository'; +import { IAssetRepositoryV1 } from './asset-repository'; import { AssetService } from './asset.service'; import { CreateAssetDto } from './dto/create-asset.dto'; import { AssetRejectReason, AssetUploadAction } from './response-dto/asset-check-response.dto'; @@ -59,19 +59,19 @@ const _getAsset_1 = () => { describe('AssetService', () => { let sut: AssetService; let accessMock: IAccessRepositoryMock; - let assetRepositoryMock: jest.Mocked; + let assetRepositoryMockV1: jest.Mocked; + let assetMock: jest.Mocked; let jobMock: jest.Mocked; let libraryMock: jest.Mocked; let userMock: jest.Mocked; beforeEach(() => { - assetRepositoryMock = { + assetRepositoryMockV1 = { get: jest.fn(), create: jest.fn(), upsertExif: jest.fn(), getAllByUserId: jest.fn(), - getById: jest.fn(), getDetectedObjectsByUserId: jest.fn(), getLocationsByUserId: jest.fn(), getSearchPropertiesByUserId: jest.fn(), @@ -81,16 +81,17 @@ describe('AssetService', () => { }; accessMock = newAccessRepositoryMock(); + assetMock = newAssetRepositoryMock(); jobMock = newJobRepositoryMock(); libraryMock = newLibraryRepositoryMock(); userMock = newUserRepositoryMock(); - sut = new AssetService(accessMock, assetRepositoryMock, jobMock, libraryMock, userMock); + sut = new AssetService(accessMock, assetRepositoryMockV1, assetMock, jobMock, libraryMock, userMock); - when(assetRepositoryMock.get) + when(assetRepositoryMockV1.get) .calledWith(assetStub.livePhotoStillAsset.id) .mockResolvedValue(assetStub.livePhotoStillAsset); - when(assetRepositoryMock.get) + when(assetRepositoryMockV1.get) .calledWith(assetStub.livePhotoMotionAsset.id) .mockResolvedValue(assetStub.livePhotoMotionAsset); }); @@ -108,12 +109,12 @@ describe('AssetService', () => { }; const dto = _getCreateAssetDto(); - assetRepositoryMock.create.mockResolvedValue(assetEntity); + assetRepositoryMockV1.create.mockResolvedValue(assetEntity); accessMock.library.checkOwnerAccess.mockResolvedValue(new Set([dto.libraryId!])); await expect(sut.uploadFile(authStub.user1, dto, file)).resolves.toEqual({ duplicate: false, id: 'id_1' }); - expect(assetRepositoryMock.create).toHaveBeenCalled(); + expect(assetRepositoryMockV1.create).toHaveBeenCalled(); expect(userMock.updateUsage).toHaveBeenCalledWith(authStub.user1.user.id, file.size); }); @@ -130,8 +131,8 @@ describe('AssetService', () => { const error = new QueryFailedError('', [], new Error('unique key violation')); (error as any).constraint = ASSET_CHECKSUM_CONSTRAINT; - assetRepositoryMock.create.mockRejectedValue(error); - assetRepositoryMock.getAssetsByChecksums.mockResolvedValue([_getAsset_1()]); + assetRepositoryMockV1.create.mockRejectedValue(error); + assetRepositoryMockV1.getAssetsByChecksums.mockResolvedValue([_getAsset_1()]); accessMock.library.checkOwnerAccess.mockResolvedValue(new Set([dto.libraryId!])); await expect(sut.uploadFile(authStub.user1, dto, file)).resolves.toEqual({ duplicate: true, id: 'id_1' }); @@ -148,8 +149,8 @@ describe('AssetService', () => { const error = new QueryFailedError('', [], new Error('unique key violation')); (error as any).constraint = ASSET_CHECKSUM_CONSTRAINT; - assetRepositoryMock.create.mockResolvedValueOnce(assetStub.livePhotoMotionAsset); - assetRepositoryMock.create.mockResolvedValueOnce(assetStub.livePhotoStillAsset); + assetRepositoryMockV1.create.mockResolvedValueOnce(assetStub.livePhotoMotionAsset); + assetRepositoryMockV1.create.mockResolvedValueOnce(assetStub.livePhotoStillAsset); accessMock.library.checkOwnerAccess.mockResolvedValue(new Set([dto.libraryId!])); await expect( @@ -177,7 +178,7 @@ describe('AssetService', () => { const file1 = Buffer.from('d2947b871a706081be194569951b7db246907957', 'hex'); const file2 = Buffer.from('53be335e99f18a66ff12e9a901c7a6171dd76573', 'hex'); - assetRepositoryMock.getAssetsByChecksums.mockResolvedValue([ + assetRepositoryMockV1.getAssetsByChecksums.mockResolvedValue([ { id: 'asset-1', checksum: file1 }, { id: 'asset-2', checksum: file2 }, ]); @@ -196,62 +197,7 @@ describe('AssetService', () => { ], }); - expect(assetRepositoryMock.getAssetsByChecksums).toHaveBeenCalledWith(authStub.admin.user.id, [file1, file2]); - }); - }); - - describe('getAssetById', () => { - it('should allow owner access', async () => { - accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); - assetRepositoryMock.getById.mockResolvedValue(assetStub.image); - await sut.getAssetById(authStub.admin, assetStub.image.id); - expect(accessMock.asset.checkOwnerAccess).toHaveBeenCalledWith( - authStub.admin.user.id, - new Set([assetStub.image.id]), - ); - }); - - it('should allow shared link access', async () => { - accessMock.asset.checkSharedLinkAccess.mockResolvedValue(new Set([assetStub.image.id])); - assetRepositoryMock.getById.mockResolvedValue(assetStub.image); - await sut.getAssetById(authStub.adminSharedLink, assetStub.image.id); - expect(accessMock.asset.checkSharedLinkAccess).toHaveBeenCalledWith( - authStub.adminSharedLink.sharedLink?.id, - new Set([assetStub.image.id]), - ); - }); - - it('should allow partner sharing access', async () => { - accessMock.asset.checkPartnerAccess.mockResolvedValue(new Set([assetStub.image.id])); - assetRepositoryMock.getById.mockResolvedValue(assetStub.image); - await sut.getAssetById(authStub.admin, assetStub.image.id); - expect(accessMock.asset.checkPartnerAccess).toHaveBeenCalledWith( - authStub.admin.user.id, - new Set([assetStub.image.id]), - ); - }); - - it('should allow shared album access', async () => { - accessMock.asset.checkAlbumAccess.mockResolvedValue(new Set([assetStub.image.id])); - assetRepositoryMock.getById.mockResolvedValue(assetStub.image); - await sut.getAssetById(authStub.admin, assetStub.image.id); - expect(accessMock.asset.checkAlbumAccess).toHaveBeenCalledWith( - authStub.admin.user.id, - new Set([assetStub.image.id]), - ); - }); - - it('should throw an error for no access', async () => { - await expect(sut.getAssetById(authStub.admin, assetStub.image.id)).rejects.toBeInstanceOf(BadRequestException); - expect(assetRepositoryMock.getById).not.toHaveBeenCalled(); - }); - - it('should throw an error for an invalid shared link', async () => { - await expect(sut.getAssetById(authStub.adminSharedLink, assetStub.image.id)).rejects.toBeInstanceOf( - BadRequestException, - ); - expect(accessMock.asset.checkOwnerAccess).not.toHaveBeenCalled(); - expect(assetRepositoryMock.getById).not.toHaveBeenCalled(); + expect(assetRepositoryMockV1.getAssetsByChecksums).toHaveBeenCalledWith(authStub.admin.user.id, [file1, file2]); }); }); }); diff --git a/server/src/immich/api-v1/asset/asset.service.ts b/server/src/immich/api-v1/asset/asset.service.ts index e611060ca5..efad71dc89 100644 --- a/server/src/immich/api-v1/asset/asset.service.ts +++ b/server/src/immich/api-v1/asset/asset.service.ts @@ -3,24 +3,24 @@ import { AssetResponseDto, AuthDto, CacheControl, - getLivePhotoMotionFilename, IAccessRepository, + IAssetRepository, IJobRepository, ILibraryRepository, - ImmichFileResponse, IUserRepository, + ImmichFileResponse, JobName, + Permission, + UploadFile, + getLivePhotoMotionFilename, mapAsset, mimeTypes, - Permission, - SanitizedAssetResponseDto, - UploadFile, } from '@app/domain'; import { ASSET_CHECKSUM_CONSTRAINT, AssetEntity, AssetType, LibraryType } from '@app/infra/entities'; import { ImmichLogger } from '@app/infra/logger'; import { Inject, Injectable, InternalServerErrorException, NotFoundException } from '@nestjs/common'; import { QueryFailedError } from 'typeorm'; -import { IAssetRepository } from './asset-repository'; +import { IAssetRepositoryV1 } from './asset-repository'; import { AssetCore } from './asset.core'; import { AssetBulkUploadCheckDto } from './dto/asset-check.dto'; import { AssetSearchDto } from './dto/asset-search.dto'; @@ -47,12 +47,13 @@ export class AssetService { constructor( @Inject(IAccessRepository) accessRepository: IAccessRepository, - @Inject(IAssetRepository) private _assetRepository: IAssetRepository, + @Inject(IAssetRepositoryV1) private assetRepositoryV1: IAssetRepositoryV1, + @Inject(IAssetRepository) private assetRepository: IAssetRepository, @Inject(IJobRepository) private jobRepository: IJobRepository, @Inject(ILibraryRepository) private libraryRepository: ILibraryRepository, @Inject(IUserRepository) private userRepository: IUserRepository, ) { - this.assetCore = new AssetCore(_assetRepository, jobRepository); + this.assetCore = new AssetCore(assetRepositoryV1, jobRepository); this.access = AccessCore.create(accessRepository); } @@ -102,7 +103,7 @@ export class AssetService { // handle duplicates with a success response if (error instanceof QueryFailedError && (error as any).constraint === ASSET_CHECKSUM_CONSTRAINT) { const checksums = [file.checksum, livePhotoFile?.checksum].filter((checksum): checksum is Buffer => !!checksum); - const [duplicate] = await this._assetRepository.getAssetsByChecksums(auth.user.id, checksums); + const [duplicate] = await this.assetRepositoryV1.getAssetsByChecksums(auth.user.id, checksums); return { id: duplicate.id, duplicate: true }; } @@ -114,35 +115,14 @@ export class AssetService { public async getAllAssets(auth: AuthDto, dto: AssetSearchDto): Promise { const userId = dto.userId || auth.user.id; await this.access.requirePermission(auth, Permission.TIMELINE_READ, userId); - const assets = await this._assetRepository.getAllByUserId(userId, dto); + const assets = await this.assetRepositoryV1.getAllByUserId(userId, dto); return assets.map((asset) => mapAsset(asset)); } - public async getAssetById(auth: AuthDto, assetId: string): Promise { - await this.access.requirePermission(auth, Permission.ASSET_READ, assetId); - - const asset = await this._assetRepository.getById(assetId); - if (!auth.sharedLink || auth.sharedLink?.showExif) { - const data = mapAsset(asset, { withStack: true }); - - if (data.ownerId !== auth.user.id) { - data.people = []; - } - - if (auth.sharedLink) { - delete data.owner; - } - - return data; - } else { - return mapAsset(asset, { stripMetadata: true, withStack: true }); - } - } - async serveThumbnail(auth: AuthDto, assetId: string, dto: GetAssetThumbnailDto): Promise { await this.access.requirePermission(auth, Permission.ASSET_VIEW, assetId); - const asset = await this._assetRepository.get(assetId); + const asset = await this.assetRepositoryV1.get(assetId); if (!asset) { throw new NotFoundException('Asset not found'); } @@ -160,7 +140,7 @@ export class AssetService { // this is not quite right as sometimes this returns the original still await this.access.requirePermission(auth, Permission.ASSET_VIEW, assetId); - const asset = await this._assetRepository.getById(assetId); + const asset = await this.assetRepository.getById(assetId); if (!asset) { throw new NotFoundException('Asset does not exist'); } @@ -182,7 +162,7 @@ export class AssetService { async getAssetSearchTerm(auth: AuthDto): Promise { const possibleSearchTerm = new Set(); - const rows = await this._assetRepository.getSearchPropertiesByUserId(auth.user.id); + const rows = await this.assetRepositoryV1.getSearchPropertiesByUserId(auth.user.id); rows.forEach((row: SearchPropertiesDto) => { // tags row.tags?.map((tag: string) => possibleSearchTerm.add(tag?.toLowerCase())); @@ -213,11 +193,11 @@ export class AssetService { } async getCuratedLocation(auth: AuthDto): Promise { - return this._assetRepository.getLocationsByUserId(auth.user.id); + return this.assetRepositoryV1.getLocationsByUserId(auth.user.id); } async getCuratedObject(auth: AuthDto): Promise { - return this._assetRepository.getDetectedObjectsByUserId(auth.user.id); + return this.assetRepositoryV1.getDetectedObjectsByUserId(auth.user.id); } async checkExistingAssets( @@ -225,7 +205,7 @@ export class AssetService { checkExistingAssetsDto: CheckExistingAssetsDto, ): Promise { return { - existingIds: await this._assetRepository.getExistingAssets(auth.user.id, checkExistingAssetsDto), + existingIds: await this.assetRepositoryV1.getExistingAssets(auth.user.id, checkExistingAssetsDto), }; } @@ -238,7 +218,7 @@ export class AssetService { } const checksums: Buffer[] = dto.assets.map((asset) => Buffer.from(asset.checksum, 'hex')); - const results = await this._assetRepository.getAssetsByChecksums(auth.user.id, checksums); + const results = await this.assetRepositoryV1.getAssetsByChecksums(auth.user.id, checksums); const checksumMap: Record = {}; for (const { id, checksum } of results) { diff --git a/server/src/immich/app.module.ts b/server/src/immich/app.module.ts index ff53401df0..364a3030bf 100644 --- a/server/src/immich/app.module.ts +++ b/server/src/immich/app.module.ts @@ -5,7 +5,7 @@ import { Module, OnModuleInit } from '@nestjs/common'; import { APP_GUARD, APP_INTERCEPTOR } from '@nestjs/core'; import { ScheduleModule } from '@nestjs/schedule'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { AssetRepository, IAssetRepository } from './api-v1/asset/asset-repository'; +import { AssetRepositoryV1, IAssetRepositoryV1 } from './api-v1/asset/asset-repository'; import { AssetController as AssetControllerV1 } from './api-v1/asset/asset.controller'; import { AssetService } from './api-v1/asset/asset.service'; import { AppGuard } from './app.guard'; @@ -45,8 +45,8 @@ import { ErrorInterceptor, FileUploadInterceptor } from './interceptors'; controllers: [ ActivityController, AssetsController, - AssetController, AssetControllerV1, + AssetController, AppController, AlbumController, APIKeyController, @@ -68,7 +68,7 @@ import { ErrorInterceptor, FileUploadInterceptor } from './interceptors'; providers: [ { provide: APP_INTERCEPTOR, useClass: ErrorInterceptor }, { provide: APP_GUARD, useClass: AppGuard }, - { provide: IAssetRepository, useClass: AssetRepository }, + { provide: IAssetRepositoryV1, useClass: AssetRepositoryV1 }, AppService, AssetService, FileUploadInterceptor, diff --git a/server/src/immich/controllers/asset.controller.ts b/server/src/immich/controllers/asset.controller.ts index fe4117be2a..10a5df3439 100644 --- a/server/src/immich/controllers/asset.controller.ts +++ b/server/src/immich/controllers/asset.controller.ts @@ -176,6 +176,12 @@ export class AssetController { return this.service.updateStackParent(auth, dto); } + @SharedLinkRoute() + @Get(':id') + getAssetInfo(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise { + return this.service.get(auth, id) as Promise; + } + @Put(':id') updateAsset(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto, @Body() dto: UpdateDto): Promise { return this.service.update(auth, id, dto); diff --git a/web/src/lib/components/asset-viewer/detail-panel.svelte b/web/src/lib/components/asset-viewer/detail-panel.svelte index 393ec01c0b..cce4cb810d 100644 --- a/web/src/lib/components/asset-viewer/detail-panel.svelte +++ b/web/src/lib/components/asset-viewer/detail-panel.svelte @@ -62,7 +62,7 @@ // Get latest description from server if (newAsset.id && !api.isSharedLink) { - const { data } = await api.assetApi.getAssetById({ id: asset.id }); + const { data } = await api.assetApi.getAssetInfo({ id: asset.id }); people = data?.people || []; description = data.exifInfo?.description || ''; @@ -126,7 +126,7 @@ }; const handleRefreshPeople = async () => { - await api.assetApi.getAssetById({ id: asset.id }).then((res) => { + await api.assetApi.getAssetInfo({ id: asset.id }).then((res) => { people = res.data?.people || []; textArea.value = res.data?.exifInfo?.description || ''; }); @@ -234,7 +234,7 @@ {/key} {:else if description} -

{description}

+

{description}

{/if} {#if !api.isSharedLink && people.length > 0} diff --git a/web/src/lib/components/sharedlinks-page/shared-link-card.svelte b/web/src/lib/components/sharedlinks-page/shared-link-card.svelte index d3be391b9d..92e4f5c49a 100644 --- a/web/src/lib/components/sharedlinks-page/shared-link-card.svelte +++ b/web/src/lib/components/sharedlinks-page/shared-link-card.svelte @@ -27,7 +27,7 @@ assetId = link.assets[0].id; } - const { data } = await api.assetApi.getAssetById({ id: assetId }); + const { data } = await api.assetApi.getAssetInfo({ id: assetId }); return data; }; diff --git a/web/src/lib/stores/asset-viewing.store.ts b/web/src/lib/stores/asset-viewing.store.ts index 3d1e2be5e4..c4a847f17b 100644 --- a/web/src/lib/stores/asset-viewing.store.ts +++ b/web/src/lib/stores/asset-viewing.store.ts @@ -6,7 +6,7 @@ function createAssetViewingStore() { const viewState = writable(false); const setAssetId = async (id: string) => { - const { data } = await api.assetApi.getAssetById({ id, key: api.getKey() }); + const { data } = await api.assetApi.getAssetInfo({ id, key: api.getKey() }); viewingAssetStoreState.set(data); viewState.set(true); }; diff --git a/web/src/routes/(user)/share/[key]/photos/[assetId]/+page.ts b/web/src/routes/(user)/share/[key]/photos/[assetId]/+page.ts index b76a84be24..7b15795180 100644 --- a/web/src/routes/(user)/share/[key]/photos/[assetId]/+page.ts +++ b/web/src/routes/(user)/share/[key]/photos/[assetId]/+page.ts @@ -3,7 +3,7 @@ import type { PageLoad } from './$types'; export const load = (async ({ params }) => { const { key, assetId } = params; - const { data: asset } = await api.assetApi.getAssetById({ id: assetId, key }); + const { data: asset } = await api.assetApi.getAssetInfo({ id: assetId, key }); return { asset,