From 2daed747cd62a0ec86dac8a71e070d31b1a95686 Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Tue, 19 Mar 2024 22:42:10 -0400 Subject: [PATCH] chore(server): change `save` -> `update` in asset repository (#8055) * `save` -> `update` * change return type * include relations * fix tests * remove when mocks * fix * stricter typing * simpler type --- server/src/domain/asset/asset.service.spec.ts | 8 +-- server/src/domain/asset/asset.service.ts | 14 ++++- server/src/domain/audit/audit.service.ts | 10 ++-- .../domain/library/library.service.spec.ts | 8 +-- server/src/domain/library/library.service.ts | 6 +- server/src/domain/media/media.service.spec.ts | 20 +++---- server/src/domain/media/media.service.ts | 10 ++-- .../domain/metadata/metadata.service.spec.ts | 60 +++++++++---------- .../src/domain/metadata/metadata.service.ts | 14 ++--- .../domain/repositories/asset.repository.ts | 23 ++++++- .../storage-template.service.spec.ts | 60 +++++-------------- server/src/domain/storage/storage.core.ts | 10 ++-- .../infra/repositories/asset.repository.ts | 21 ++----- .../repositories/asset.repository.mock.ts | 2 +- 14 files changed, 128 insertions(+), 138 deletions(-) diff --git a/server/src/domain/asset/asset.service.spec.ts b/server/src/domain/asset/asset.service.spec.ts index 361946f6151b7..11d2ed00b9b3e 100644 --- a/server/src/domain/asset/asset.service.spec.ts +++ b/server/src/domain/asset/asset.service.spec.ts @@ -548,19 +548,19 @@ describe(AssetService.name, () => { await expect(sut.update(authStub.admin, 'asset-1', { isArchived: false })).rejects.toBeInstanceOf( BadRequestException, ); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should update the asset', async () => { accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1'])); - assetMock.save.mockResolvedValue(assetStub.image); + assetMock.getById.mockResolvedValue(assetStub.image); await sut.update(authStub.admin, 'asset-1', { isFavorite: true }); - expect(assetMock.save).toHaveBeenCalledWith({ id: 'asset-1', isFavorite: true }); + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-1', isFavorite: true }); }); it('should update the exif description', async () => { accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1'])); - assetMock.save.mockResolvedValue(assetStub.image); + assetMock.getById.mockResolvedValue(assetStub.image); await sut.update(authStub.admin, 'asset-1', { description: 'Test description' }); expect(assetMock.upsertExif).toHaveBeenCalledWith({ assetId: 'asset-1', description: 'Test description' }); }); diff --git a/server/src/domain/asset/asset.service.ts b/server/src/domain/asset/asset.service.ts index e54eb84391e35..fbe4e91bd1e9c 100644 --- a/server/src/domain/asset/asset.service.ts +++ b/server/src/domain/asset/asset.service.ts @@ -324,7 +324,19 @@ export class AssetService { const { description, dateTimeOriginal, latitude, longitude, ...rest } = dto; await this.updateMetadata({ id, description, dateTimeOriginal, latitude, longitude }); - const asset = await this.assetRepository.save({ id, ...rest }); + await this.assetRepository.update({ id, ...rest }); + const asset = await this.assetRepository.getById(id, { + exifInfo: true, + owner: true, + smartInfo: true, + tags: true, + faces: { + person: true, + }, + }); + if (!asset) { + throw new BadRequestException('Asset not found'); + } return mapAsset(asset, { auth }); } diff --git a/server/src/domain/audit/audit.service.ts b/server/src/domain/audit/audit.service.ts index 91ebd78ee9863..c96f36d740d6e 100644 --- a/server/src/domain/audit/audit.service.ts +++ b/server/src/domain/audit/audit.service.ts @@ -93,27 +93,27 @@ export class AuditService { switch (pathType) { case AssetPathType.ENCODED_VIDEO: { - await this.assetRepository.save({ id, encodedVideoPath: pathValue }); + await this.assetRepository.update({ id, encodedVideoPath: pathValue }); break; } case AssetPathType.JPEG_THUMBNAIL: { - await this.assetRepository.save({ id, resizePath: pathValue }); + await this.assetRepository.update({ id, resizePath: pathValue }); break; } case AssetPathType.WEBP_THUMBNAIL: { - await this.assetRepository.save({ id, webpPath: pathValue }); + await this.assetRepository.update({ id, webpPath: pathValue }); break; } case AssetPathType.ORIGINAL: { - await this.assetRepository.save({ id, originalPath: pathValue }); + await this.assetRepository.update({ id, originalPath: pathValue }); break; } case AssetPathType.SIDECAR: { - await this.assetRepository.save({ id, sidecarPath: pathValue }); + await this.assetRepository.update({ id, sidecarPath: pathValue }); break; } diff --git a/server/src/domain/library/library.service.spec.ts b/server/src/domain/library/library.service.spec.ts index 3b5258b975af6..0c0daa165dd5c 100644 --- a/server/src/domain/library/library.service.spec.ts +++ b/server/src/domain/library/library.service.spec.ts @@ -584,7 +584,7 @@ describe(LibraryService.name, () => { await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS); - expect(assetMock.save).toHaveBeenCalledWith({ id: assetStub.image.id, isOffline: true }); + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, isOffline: true }); expect(jobMock.queue).not.toHaveBeenCalled(); expect(jobMock.queueAll).not.toHaveBeenCalled(); }); @@ -602,7 +602,7 @@ describe(LibraryService.name, () => { await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS); - expect(assetMock.save).toHaveBeenCalledWith({ id: assetStub.offline.id, isOffline: false }); + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.offline.id, isOffline: false }); expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.METADATA_EXTRACTION, @@ -631,7 +631,7 @@ describe(LibraryService.name, () => { assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.image); assetMock.create.mockResolvedValue(assetStub.image); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS); }); @@ -1257,7 +1257,7 @@ describe(LibraryService.name, () => { await sut.watchAll(); - expect(assetMock.save).toHaveBeenCalledWith({ id: assetStub.external.id, isOffline: true }); + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.external.id, isOffline: true }); }); it('should handle an error event', async () => { diff --git a/server/src/domain/library/library.service.ts b/server/src/domain/library/library.service.ts index 000acac29df05..3ef59c9190292 100644 --- a/server/src/domain/library/library.service.ts +++ b/server/src/domain/library/library.service.ts @@ -173,7 +173,7 @@ export class LibraryService extends EventEmitter { this.logger.debug(`Detected deleted file at ${path} in library ${library.id}`); const asset = await this.assetRepository.getByLibraryIdAndOriginalPath(library.id, path); if (asset && matcher(path)) { - await this.assetRepository.save({ id: asset.id, isOffline: true }); + await this.assetRepository.update({ id: asset.id, isOffline: true }); } this.emit(StorageEventType.UNLINK, path); }; @@ -429,7 +429,7 @@ export class LibraryService extends EventEmitter { // Mark asset as offline this.logger.debug(`Marking asset as offline: ${assetPath}`); - await this.assetRepository.save({ id: existingAssetEntity.id, isOffline: true }); + await this.assetRepository.update({ id: existingAssetEntity.id, isOffline: true }); return JobStatus.SUCCESS; } else { // File can't be accessed and does not already exist in db @@ -462,7 +462,7 @@ export class LibraryService extends EventEmitter { if (stats && existingAssetEntity?.isOffline) { // File was previously offline but is now online this.logger.debug(`Marking previously-offline asset as online: ${assetPath}`); - await this.assetRepository.save({ id: existingAssetEntity.id, isOffline: false }); + await this.assetRepository.update({ id: existingAssetEntity.id, isOffline: false }); doRefresh = true; } diff --git a/server/src/domain/media/media.service.spec.ts b/server/src/domain/media/media.service.spec.ts index beea126bf671b..36d2cfdbac845 100644 --- a/server/src/domain/media/media.service.spec.ts +++ b/server/src/domain/media/media.service.spec.ts @@ -205,7 +205,7 @@ describe(MediaService.name, () => { assetMock.getByIds.mockResolvedValue([]); await sut.handleGenerateJpegThumbnail({ id: assetStub.image.id }); expect(mediaMock.resize).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalledWith(); + expect(assetMock.update).not.toHaveBeenCalledWith(); }); it('should skip video thumbnail generation if no video stream', async () => { @@ -213,7 +213,7 @@ describe(MediaService.name, () => { assetMock.getByIds.mockResolvedValue([assetStub.video]); await sut.handleGenerateJpegThumbnail({ id: assetStub.image.id }); expect(mediaMock.resize).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalledWith(); + expect(assetMock.update).not.toHaveBeenCalledWith(); }); it('should generate a thumbnail for an image', async () => { @@ -227,7 +227,7 @@ describe(MediaService.name, () => { quality: 80, colorspace: Colorspace.SRGB, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', resizePath: 'upload/thumbs/user-id/as/se/asset-id.jpeg', }); @@ -246,7 +246,7 @@ describe(MediaService.name, () => { quality: 80, colorspace: Colorspace.P3, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', resizePath: 'upload/thumbs/user-id/as/se/asset-id.jpeg', }); @@ -271,7 +271,7 @@ describe(MediaService.name, () => { twoPass: false, }, ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', resizePath: 'upload/thumbs/user-id/as/se/asset-id.jpeg', }); @@ -296,7 +296,7 @@ describe(MediaService.name, () => { twoPass: false, }, ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', resizePath: 'upload/thumbs/user-id/as/se/asset-id.jpeg', }); @@ -337,7 +337,7 @@ describe(MediaService.name, () => { assetMock.getByIds.mockResolvedValue([]); await sut.handleGenerateWebpThumbnail({ id: assetStub.image.id }); expect(mediaMock.resize).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalledWith(); + expect(assetMock.update).not.toHaveBeenCalledWith(); }); it('should generate a thumbnail', async () => { @@ -350,7 +350,7 @@ describe(MediaService.name, () => { quality: 80, colorspace: Colorspace.SRGB, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', webpPath: 'upload/thumbs/user-id/as/se/asset-id.webp', }); @@ -370,7 +370,7 @@ describe(MediaService.name, () => { quality: 80, colorspace: Colorspace.P3, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', webpPath: 'upload/thumbs/user-id/as/se/asset-id.webp', }); @@ -397,7 +397,7 @@ describe(MediaService.name, () => { await sut.handleGenerateThumbhashThumbnail({ id: assetStub.image.id }); expect(mediaMock.generateThumbhash).toHaveBeenCalledWith('/uploads/user-id/thumbs/path.jpg'); - expect(assetMock.save).toHaveBeenCalledWith({ id: 'asset-id', thumbhash: thumbhashBuffer }); + expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', thumbhash: thumbhashBuffer }); }); }); diff --git a/server/src/domain/media/media.service.ts b/server/src/domain/media/media.service.ts index 9d522d10406d4..31eafcbcfadaa 100644 --- a/server/src/domain/media/media.service.ts +++ b/server/src/domain/media/media.service.ts @@ -172,7 +172,7 @@ export class MediaService { } const resizePath = await this.generateThumbnail(asset, 'jpeg'); - await this.assetRepository.save({ id: asset.id, resizePath }); + await this.assetRepository.update({ id: asset.id, resizePath }); return JobStatus.SUCCESS; } @@ -222,7 +222,7 @@ export class MediaService { } const webpPath = await this.generateThumbnail(asset, 'webp'); - await this.assetRepository.save({ id: asset.id, webpPath }); + await this.assetRepository.update({ id: asset.id, webpPath }); return JobStatus.SUCCESS; } @@ -233,7 +233,7 @@ export class MediaService { } const thumbhash = await this.mediaRepository.generateThumbhash(asset.resizePath); - await this.assetRepository.save({ id: asset.id, thumbhash }); + await this.assetRepository.update({ id: asset.id, thumbhash }); return JobStatus.SUCCESS; } @@ -286,7 +286,7 @@ export class MediaService { if (asset.encodedVideoPath) { this.logger.log(`Transcoded video exists for asset ${asset.id}, but is no longer required. Deleting...`); await this.jobRepository.queue({ name: JobName.DELETE_FILES, data: { files: [asset.encodedVideoPath] } }); - await this.assetRepository.save({ id: asset.id, encodedVideoPath: null }); + await this.assetRepository.update({ id: asset.id, encodedVideoPath: null }); } return JobStatus.SKIPPED; @@ -321,7 +321,7 @@ export class MediaService { this.logger.log(`Successfully encoded ${asset.id}`); - await this.assetRepository.save({ id: asset.id, encodedVideoPath: output }); + await this.assetRepository.update({ id: asset.id, encodedVideoPath: output }); return JobStatus.SUCCESS; } diff --git a/server/src/domain/metadata/metadata.service.spec.ts b/server/src/domain/metadata/metadata.service.spec.ts index c28c61f2218ce..69d31cbd57590 100644 --- a/server/src/domain/metadata/metadata.service.spec.ts +++ b/server/src/domain/metadata/metadata.service.spec.ts @@ -117,7 +117,7 @@ describe(MetadataService.name, () => { await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true }); expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); expect(albumMock.removeAsset).not.toHaveBeenCalled(); }); @@ -127,7 +127,7 @@ describe(MetadataService.name, () => { await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true }); expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); expect(albumMock.removeAsset).not.toHaveBeenCalled(); }); @@ -137,7 +137,7 @@ describe(MetadataService.name, () => { await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.SKIPPED); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true }); expect(assetMock.findLivePhotoMatch).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); expect(albumMock.removeAsset).not.toHaveBeenCalled(); }); @@ -159,7 +159,7 @@ describe(MetadataService.name, () => { otherAssetId: assetStub.livePhotoMotionAsset.id, type: AssetType.IMAGE, }); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); expect(albumMock.removeAsset).not.toHaveBeenCalled(); }); @@ -182,11 +182,11 @@ describe(MetadataService.name, () => { otherAssetId: assetStub.livePhotoStillAsset.id, type: AssetType.VIDEO, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.livePhotoStillAsset.id, livePhotoVideoId: assetStub.livePhotoMotionAsset.id, }); - expect(assetMock.save).toHaveBeenCalledWith({ id: assetStub.livePhotoMotionAsset.id, isVisible: false }); + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.livePhotoMotionAsset.id, isVisible: false }); expect(albumMock.removeAsset).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.id); }); @@ -248,7 +248,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should handle a date in a sidecar file', async () => { @@ -267,7 +267,7 @@ describe(MetadataService.name, () => { await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.sidecar.id]); expect(assetMock.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ dateTimeOriginal: sidecarDate })); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, duration: null, fileCreatedAt: sidecarDate, @@ -282,7 +282,7 @@ describe(MetadataService.name, () => { await sut.handleMetadataExtraction({ id: assetStub.image.id }); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ iso: 160 })); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, duration: null, fileCreatedAt: assetStub.image.createdAt, @@ -304,7 +304,7 @@ describe(MetadataService.name, () => { expect(assetMock.upsertExif).toHaveBeenCalledWith( expect.objectContaining({ city: 'City', state: 'State', country: 'Country' }), ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.withLocation.id, duration: null, fileCreatedAt: assetStub.withLocation.createdAt, @@ -333,7 +333,7 @@ describe(MetadataService.name, () => { expect(storageMock.writeFile).not.toHaveBeenCalled(); expect(jobMock.queue).not.toHaveBeenCalled(); expect(jobMock.queueAll).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalledWith( + expect(assetMock.update).not.toHaveBeenCalledWith( expect.objectContaining({ assetType: AssetType.VIDEO, isVisible: false }), ); }); @@ -376,7 +376,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]); expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video); - expect(assetMock.save).toHaveBeenNthCalledWith(1, { + expect(assetMock.update).toHaveBeenNthCalledWith(1, { id: assetStub.livePhotoStillAsset.id, livePhotoVideoId: fileStub.livePhotoMotion.uuid, }); @@ -404,7 +404,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]); expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video); - expect(assetMock.save).toHaveBeenNthCalledWith(1, { + expect(assetMock.update).toHaveBeenNthCalledWith(1, { id: assetStub.livePhotoStillAsset.id, livePhotoVideoId: fileStub.livePhotoMotion.uuid, }); @@ -430,7 +430,7 @@ describe(MetadataService.name, () => { expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object)); expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video); - expect(assetMock.save).toHaveBeenNthCalledWith(1, { + expect(assetMock.update).toHaveBeenNthCalledWith(1, { id: assetStub.livePhotoStillAsset.id, livePhotoVideoId: fileStub.livePhotoMotion.uuid, }); @@ -470,7 +470,7 @@ describe(MetadataService.name, () => { expect(assetMock.create).toHaveBeenCalledTimes(0); expect(storageMock.writeFile).toHaveBeenCalledTimes(0); // The still asset gets saved by handleMetadataExtraction, but not the video - expect(assetMock.save).toHaveBeenCalledTimes(1); + expect(assetMock.update).toHaveBeenCalledTimes(1); expect(jobMock.queue).toHaveBeenCalledTimes(0); }); @@ -529,7 +529,7 @@ describe(MetadataService.name, () => { projectionType: 'EQUIRECTANGULAR', timeZone: tags.tz, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, duration: null, fileCreatedAt: new Date('1970-01-01'), @@ -545,7 +545,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).toHaveBeenCalled(); - expect(assetMock.save).toHaveBeenCalledWith( + expect(assetMock.update).toHaveBeenCalledWith( expect.objectContaining({ id: assetStub.image.id, duration: '00:00:06.210', @@ -561,7 +561,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).toHaveBeenCalled(); - expect(assetMock.save).toHaveBeenCalledWith( + expect(assetMock.update).toHaveBeenCalledWith( expect.objectContaining({ id: assetStub.image.id, duration: '00:00:08.410', @@ -577,7 +577,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).toHaveBeenCalled(); - expect(assetMock.save).toHaveBeenCalledWith( + expect(assetMock.update).toHaveBeenCalledWith( expect.objectContaining({ id: assetStub.image.id, duration: '00:00:06.200', @@ -593,7 +593,7 @@ describe(MetadataService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.image.id]); expect(assetMock.upsertExif).toHaveBeenCalled(); - expect(assetMock.save).toHaveBeenCalledWith( + expect(assetMock.update).toHaveBeenCalledWith( expect.objectContaining({ id: assetStub.image.id, duration: '00:00:06.207', @@ -638,13 +638,13 @@ describe(MetadataService.name, () => { it('should do nothing if asset could not be found', async () => { assetMock.getByIds.mockResolvedValue([]); await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should do nothing if asset has no sidecar path', async () => { assetMock.getByIds.mockResolvedValue([assetStub.image]); await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should set sidecar path if exists (sidecar named photo.ext.xmp)', async () => { @@ -653,7 +653,7 @@ describe(MetadataService.name, () => { await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.SUCCESS); expect(storageMock.checkFileExists).toHaveBeenCalledWith(`${assetStub.sidecar.originalPath}.xmp`, constants.R_OK); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.sidecar.id, sidecarPath: assetStub.sidecar.sidecarPath, }); @@ -670,7 +670,7 @@ describe(MetadataService.name, () => { assetStub.sidecarWithoutExt.sidecarPath, constants.R_OK, ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.sidecarWithoutExt.id, sidecarPath: assetStub.sidecarWithoutExt.sidecarPath, }); @@ -688,7 +688,7 @@ describe(MetadataService.name, () => { assetStub.sidecarWithoutExt.sidecarPath, constants.R_OK, ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.sidecar.id, sidecarPath: assetStub.sidecar.sidecarPath, }); @@ -700,7 +700,7 @@ describe(MetadataService.name, () => { await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.SUCCESS); expect(storageMock.checkFileExists).toHaveBeenCalledWith(`${assetStub.sidecar.originalPath}.xmp`, constants.R_OK); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.sidecar.id, sidecarPath: null, }); @@ -724,16 +724,15 @@ describe(MetadataService.name, () => { assetMock.getByIds.mockResolvedValue([assetStub.image]); storageMock.checkFileExists.mockResolvedValue(false); await sut.handleSidecarDiscovery({ id: assetStub.image.id }); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should update a image asset when a sidecar is found', async () => { assetMock.getByIds.mockResolvedValue([assetStub.image]); - assetMock.save.mockResolvedValue(assetStub.image); storageMock.checkFileExists.mockResolvedValue(true); await sut.handleSidecarDiscovery({ id: assetStub.image.id }); expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.jpg.xmp', constants.R_OK); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, sidecarPath: '/original/path.jpg.xmp', }); @@ -741,11 +740,10 @@ describe(MetadataService.name, () => { it('should update a video asset when a sidecar is found', async () => { assetMock.getByIds.mockResolvedValue([assetStub.video]); - assetMock.save.mockResolvedValue(assetStub.video); storageMock.checkFileExists.mockResolvedValue(true); await sut.handleSidecarDiscovery({ id: assetStub.video.id }); expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.R_OK); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, sidecarPath: '/original/path.ext.xmp', }); diff --git a/server/src/domain/metadata/metadata.service.ts b/server/src/domain/metadata/metadata.service.ts index 5f0b28fc47f95..75838330d22ca 100644 --- a/server/src/domain/metadata/metadata.service.ts +++ b/server/src/domain/metadata/metadata.service.ts @@ -177,8 +177,8 @@ export class MetadataService { const [photoAsset, motionAsset] = asset.type === AssetType.IMAGE ? [asset, match] : [match, asset]; - await this.assetRepository.save({ id: photoAsset.id, livePhotoVideoId: motionAsset.id }); - await this.assetRepository.save({ id: motionAsset.id, isVisible: false }); + await this.assetRepository.update({ id: photoAsset.id, livePhotoVideoId: motionAsset.id }); + await this.assetRepository.update({ id: motionAsset.id, isVisible: false }); await this.albumRepository.removeAsset(motionAsset.id); // Notify clients to hide the linked live photo asset @@ -249,7 +249,7 @@ export class MetadataService { if (dateTimeOriginal && timeZoneOffset) { localDateTime = new Date(dateTimeOriginal.getTime() + timeZoneOffset * 60_000); } - await this.assetRepository.save({ + await this.assetRepository.update({ id: asset.id, duration: tags.Duration ? this.getDuration(tags.Duration) : null, localDateTime, @@ -317,7 +317,7 @@ export class MetadataService { await this.repository.writeTags(sidecarPath, exif); if (!asset.sidecarPath) { - await this.assetRepository.save({ id, sidecarPath }); + await this.assetRepository.update({ id, sidecarPath }); } return JobStatus.SUCCESS; @@ -435,7 +435,7 @@ export class MetadataService { this.storageCore.ensureFolders(motionPath); await this.storageRepository.writeFile(motionAsset.originalPath, video); await this.jobRepository.queue({ name: JobName.METADATA_EXTRACTION, data: { id: motionAsset.id } }); - await this.assetRepository.save({ id: asset.id, livePhotoVideoId: motionAsset.id }); + await this.assetRepository.update({ id: asset.id, livePhotoVideoId: motionAsset.id }); // If the asset already had an associated livePhotoVideo, delete it, because // its checksum doesn't match the checksum of the motionAsset we just extracted @@ -587,7 +587,7 @@ export class MetadataService { } if (sidecarPath) { - await this.assetRepository.save({ id: asset.id, sidecarPath }); + await this.assetRepository.update({ id: asset.id, sidecarPath }); return JobStatus.SUCCESS; } @@ -598,7 +598,7 @@ export class MetadataService { this.logger.debug( `Sidecar file was not found. Checked paths '${sidecarPathWithExt}' and '${sidecarPathWithoutExt}'. Removing sidecarPath for asset ${asset.id}`, ); - await this.assetRepository.save({ id: asset.id, sidecarPath: null }); + await this.assetRepository.update({ id: asset.id, sidecarPath: null }); return JobStatus.SUCCESS; } diff --git a/server/src/domain/repositories/asset.repository.ts b/server/src/domain/repositories/asset.repository.ts index c4ddb31076501..c504bbb7f2629 100644 --- a/server/src/domain/repositories/asset.repository.ts +++ b/server/src/domain/repositories/asset.repository.ts @@ -90,6 +90,25 @@ export type AssetCreate = Pick< > & Partial; +export type AssetWithoutRelations = Omit< + AssetEntity, + | 'livePhotoVideo' + | 'stack' + | 'albums' + | 'faces' + | 'owner' + | 'library' + | 'exifInfo' + | 'sharedLinks' + | 'smartInfo' + | 'smartSearch' + | 'tags' +>; + +export type AssetUpdateOptions = Pick & Partial; + +export type AssetUpdateAllOptions = Omit, 'id'>; + export interface MonthDay { day: number; month: number; @@ -138,8 +157,8 @@ export interface IAssetRepository { deleteAll(ownerId: string): Promise; getAll(pagination: PaginationOptions, options?: AssetSearchOptions): Paginated; getAllByDeviceId(userId: string, deviceId: string): Promise; - updateAll(ids: string[], options: Partial): Promise; - save(asset: Pick & Partial): Promise; + updateAll(ids: string[], options: Partial): Promise; + update(asset: AssetUpdateOptions): Promise; remove(asset: AssetEntity): Promise; softDeleteAll(ids: string[]): Promise; restoreAll(ids: string[]): Promise; diff --git a/server/src/domain/storage-template/storage-template.service.spec.ts b/server/src/domain/storage-template/storage-template.service.spec.ts index 21fa6ef7dffed..a81e27c8f93cf 100644 --- a/server/src/domain/storage-template/storage-template.service.spec.ts +++ b/server/src/domain/storage-template/storage-template.service.spec.ts @@ -111,7 +111,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.checkFileExists).not.toHaveBeenCalled(); expect(storageMock.rename).not.toHaveBeenCalled(); expect(storageMock.copyFile).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); expect(moveMock.create).not.toHaveBeenCalled(); expect(moveMock.update).not.toHaveBeenCalled(); expect(storageMock.stat).not.toHaveBeenCalled(); @@ -122,14 +122,6 @@ describe(StorageTemplateService.name, () => { const newMotionPicturePath = `upload/library/${userStub.user1.id}/2022/2022-06-19/${assetStub.livePhotoStillAsset.id}.mp4`; const newStillPicturePath = `upload/library/${userStub.user1.id}/2022/2022-06-19/${assetStub.livePhotoStillAsset.id}.jpeg`; - when(assetMock.save) - .calledWith({ id: assetStub.livePhotoStillAsset.id, originalPath: newStillPicturePath }) - .mockResolvedValue(assetStub.livePhotoStillAsset); - - when(assetMock.save) - .calledWith({ id: assetStub.livePhotoMotionAsset.id, originalPath: newMotionPicturePath }) - .mockResolvedValue(assetStub.livePhotoMotionAsset); - when(assetMock.getByIds) .calledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true }) .mockResolvedValue([assetStub.livePhotoStillAsset]); @@ -175,11 +167,11 @@ describe(StorageTemplateService.name, () => { expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true }); expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true }); expect(storageMock.checkFileExists).toHaveBeenCalledTimes(2); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.livePhotoStillAsset.id, originalPath: newStillPicturePath, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.livePhotoMotionAsset.id, originalPath: newMotionPicturePath, }); @@ -200,10 +192,6 @@ describe(StorageTemplateService.name, () => { newPath: previousFailedNewPath, }); - when(assetMock.save) - .calledWith({ id: assetStub.image.id, originalPath: newPath }) - .mockResolvedValue(assetStub.image); - when(assetMock.getByIds) .calledWith([assetStub.image.id], { exifInfo: true }) .mockResolvedValue([assetStub.image]); @@ -232,7 +220,7 @@ describe(StorageTemplateService.name, () => { oldPath: assetStub.image.originalPath, newPath, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: newPath, }); @@ -257,10 +245,6 @@ describe(StorageTemplateService.name, () => { newPath: previousFailedNewPath, }); - when(assetMock.save) - .calledWith({ id: assetStub.image.id, originalPath: newPath }) - .mockResolvedValue(assetStub.image); - when(assetMock.getByIds) .calledWith([assetStub.image.id], { exifInfo: true }) .mockResolvedValue([assetStub.image]); @@ -291,7 +275,7 @@ describe(StorageTemplateService.name, () => { oldPath: previousFailedNewPath, newPath, }); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: newPath, }); @@ -307,10 +291,6 @@ describe(StorageTemplateService.name, () => { .mockResolvedValue({ size: 5000 } as Stats); when(cryptoMock.hashFile).calledWith(newPath).mockResolvedValue(Buffer.from('different-hash', 'utf8')); - when(assetMock.save) - .calledWith({ id: assetStub.image.id, originalPath: newPath }) - .mockResolvedValue(assetStub.image); - when(assetMock.getByIds) .calledWith([assetStub.image.id], { exifInfo: true }) .mockResolvedValue([assetStub.image]); @@ -345,7 +325,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.copyFile).toHaveBeenCalledWith(assetStub.image.originalPath, newPath); expect(storageMock.unlink).toHaveBeenCalledWith(newPath); expect(storageMock.unlink).toHaveBeenCalledTimes(1); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it.each` @@ -374,10 +354,6 @@ describe(StorageTemplateService.name, () => { newPath: previousFailedNewPath, }); - when(assetMock.save) - .calledWith({ id: assetStub.image.id, originalPath: newPath }) - .mockResolvedValue(assetStub.image); - when(assetMock.getByIds) .calledWith([assetStub.image.id], { exifInfo: true }) .mockResolvedValue([assetStub.image]); @@ -404,7 +380,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.rename).not.toHaveBeenCalled(); expect(storageMock.copyFile).not.toHaveBeenCalled(); expect(moveMock.update).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }, ); }); @@ -427,7 +403,6 @@ describe(StorageTemplateService.name, () => { items: [assetStub.image], hasNextPage: false, }); - assetMock.save.mockResolvedValue(assetStub.image); userMock.getList.mockResolvedValue([userStub.user1]); moveMock.create.mockResolvedValue({ id: '123', @@ -449,7 +424,7 @@ describe(StorageTemplateService.name, () => { expect(assetMock.getAll).toHaveBeenCalled(); expect(storageMock.checkFileExists).toHaveBeenCalledTimes(2); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id+1.jpg', }); @@ -474,7 +449,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.rename).not.toHaveBeenCalled(); expect(storageMock.copyFile).not.toHaveBeenCalled(); expect(storageMock.checkFileExists).not.toHaveBeenCalledTimes(2); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should skip when an asset is probably a duplicate', async () => { @@ -495,7 +470,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.rename).not.toHaveBeenCalled(); expect(storageMock.copyFile).not.toHaveBeenCalled(); expect(storageMock.checkFileExists).not.toHaveBeenCalledTimes(2); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should move an asset', async () => { @@ -503,7 +478,6 @@ describe(StorageTemplateService.name, () => { items: [assetStub.image], hasNextPage: false, }); - assetMock.save.mockResolvedValue(assetStub.image); userMock.getList.mockResolvedValue([userStub.user1]); moveMock.create.mockResolvedValue({ id: '123', @@ -520,7 +494,7 @@ describe(StorageTemplateService.name, () => { '/original/path.jpg', 'upload/library/user-id/2023/2023-02-23/asset-id.jpg', ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: 'upload/library/user-id/2023/2023-02-23/asset-id.jpg', }); @@ -531,7 +505,6 @@ describe(StorageTemplateService.name, () => { items: [assetStub.image], hasNextPage: false, }); - assetMock.save.mockResolvedValue(assetStub.image); userMock.getList.mockResolvedValue([userStub.storageLabel]); moveMock.create.mockResolvedValue({ id: '123', @@ -548,7 +521,7 @@ describe(StorageTemplateService.name, () => { '/original/path.jpg', 'upload/library/label-1/2023/2023-02-23/asset-id.jpg', ); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: 'upload/library/label-1/2023/2023-02-23/asset-id.jpg', }); @@ -592,7 +565,7 @@ describe(StorageTemplateService.name, () => { expect(storageMock.utimes).toHaveBeenCalledWith(newPath, expect.any(Date), expect.any(Date)); expect(storageMock.unlink).toHaveBeenCalledWith(assetStub.image.originalPath); expect(storageMock.unlink).toHaveBeenCalledTimes(1); - expect(assetMock.save).toHaveBeenCalledWith({ + expect(assetMock.update).toHaveBeenCalledWith({ id: assetStub.image.id, originalPath: newPath, }); @@ -630,7 +603,7 @@ describe(StorageTemplateService.name, () => { 'upload/library/user-id/2023/2023-02-23/asset-id.jpg', ); expect(storageMock.stat).toHaveBeenCalledWith('upload/library/user-id/2023/2023-02-23/asset-id.jpg'); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should not update the database if the move fails', async () => { @@ -656,7 +629,7 @@ describe(StorageTemplateService.name, () => { '/original/path.jpg', 'upload/library/user-id/2023/2023-02-23/asset-id.jpg', ); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); it('should not move read-only asset', async () => { @@ -670,7 +643,6 @@ describe(StorageTemplateService.name, () => { ], hasNextPage: false, }); - assetMock.save.mockResolvedValue(assetStub.image); userMock.getList.mockResolvedValue([userStub.user1]); await sut.handleMigration(); @@ -678,7 +650,7 @@ describe(StorageTemplateService.name, () => { expect(assetMock.getAll).toHaveBeenCalled(); expect(storageMock.rename).not.toHaveBeenCalled(); expect(storageMock.copyFile).not.toHaveBeenCalled(); - expect(assetMock.save).not.toHaveBeenCalled(); + expect(assetMock.update).not.toHaveBeenCalled(); }); }); }); diff --git a/server/src/domain/storage/storage.core.ts b/server/src/domain/storage/storage.core.ts index 36e600b24d484..5cf65ad7cc46c 100644 --- a/server/src/domain/storage/storage.core.ts +++ b/server/src/domain/storage/storage.core.ts @@ -286,19 +286,19 @@ export class StorageCore { private savePath(pathType: PathType, id: string, newPath: string) { switch (pathType) { case AssetPathType.ORIGINAL: { - return this.assetRepository.save({ id, originalPath: newPath }); + return this.assetRepository.update({ id, originalPath: newPath }); } case AssetPathType.JPEG_THUMBNAIL: { - return this.assetRepository.save({ id, resizePath: newPath }); + return this.assetRepository.update({ id, resizePath: newPath }); } case AssetPathType.WEBP_THUMBNAIL: { - return this.assetRepository.save({ id, webpPath: newPath }); + return this.assetRepository.update({ id, webpPath: newPath }); } case AssetPathType.ENCODED_VIDEO: { - return this.assetRepository.save({ id, encodedVideoPath: newPath }); + return this.assetRepository.update({ id, encodedVideoPath: newPath }); } case AssetPathType.SIDECAR: { - return this.assetRepository.save({ id, sidecarPath: newPath }); + return this.assetRepository.update({ id, sidecarPath: newPath }); } case PersonPathType.FACE: { return this.personRepository.update({ id, thumbnailPath: newPath }); diff --git a/server/src/infra/repositories/asset.repository.ts b/server/src/infra/repositories/asset.repository.ts index 09cf2c7794a01..3af6eb1509997 100644 --- a/server/src/infra/repositories/asset.repository.ts +++ b/server/src/infra/repositories/asset.repository.ts @@ -6,6 +6,8 @@ import { AssetSearchOptions, AssetStats, AssetStatsOptions, + AssetUpdateAllOptions, + AssetUpdateOptions, IAssetRepository, LivePhotoSearchOptions, MapMarker, @@ -275,7 +277,7 @@ export class AssetRepository implements IAssetRepository { @GenerateSql({ params: [[DummyValue.UUID], { deviceId: DummyValue.STRING }] }) @Chunked() - async updateAll(ids: string[], options: Partial): Promise { + async updateAll(ids: string[], options: AssetUpdateAllOptions): Promise { await this.repository.update({ id: In(ids) }, options); } @@ -289,21 +291,8 @@ export class AssetRepository implements IAssetRepository { await this.repository.restore({ id: In(ids) }); } - async save(asset: Partial): Promise { - const { id } = await this.repository.save(asset); - return this.repository.findOneOrFail({ - where: { id }, - relations: { - exifInfo: true, - owner: true, - smartInfo: true, - tags: true, - faces: { - person: true, - }, - }, - withDeleted: true, - }); + async update(asset: AssetUpdateOptions): Promise { + await this.repository.update(asset.id, asset); } async remove(asset: AssetEntity): Promise { diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index b291b7183c63b..b9451f34f577c 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -24,7 +24,7 @@ export const newAssetRepositoryMock = (): jest.Mocked => { getLibraryAssetPaths: jest.fn(), getByLibraryIdAndOriginalPath: jest.fn(), deleteAll: jest.fn(), - save: jest.fn(), + update: jest.fn(), remove: jest.fn(), findLivePhotoMatch: jest.fn(), getMapMarkers: jest.fn(),