From 85df3f1e995ae3be6001979804cc2e694e4c20ed Mon Sep 17 00:00:00 2001 From: Kevin Huang <44907675+Ynng@users.noreply.github.com> Date: Sun, 14 Apr 2024 16:55:44 -0700 Subject: [PATCH] fix(server): external library motion photo video asset handling (#8721) * added "isExternal" to the getLibraryAssetPaths query * handleQueueAssetRefresh skip "non external" video asset, closes #8562 * correctly implements live photo deletion for external library * use "external asset" for external library tests * minor: external library asset checksum is "path hash" not file hash * renamed to getExternalLibraryAssetPaths and added isExternal where clause * generated sql * reverted leftover change --- server/src/interfaces/asset.interface.ts | 2 +- server/src/queries/asset.repository.sql | 3 +- server/src/repositories/asset.repository.ts | 4 +- server/src/services/asset.service.ts | 5 ++- server/src/services/library.service.spec.ts | 18 ++++---- server/src/services/library.service.ts | 2 +- server/test/fixtures/asset.stub.ts | 42 ++++++++++++++++++- .../repositories/asset.repository.mock.ts | 2 +- 8 files changed, 61 insertions(+), 17 deletions(-) diff --git a/server/src/interfaces/asset.interface.ts b/server/src/interfaces/asset.interface.ts index eab392a586..c5905f3b55 100644 --- a/server/src/interfaces/asset.interface.ts +++ b/server/src/interfaces/asset.interface.ts @@ -155,7 +155,7 @@ export interface IAssetRepository { getRandom(userId: string, count: number): Promise; getFirstAssetForAlbumId(albumId: string): Promise; getLastUpdatedAssetForAlbumId(albumId: string): Promise; - getLibraryAssetPaths(pagination: PaginationOptions, libraryId: string): Paginated; + getExternalLibraryAssetPaths(pagination: PaginationOptions, libraryId: string): Paginated; getByLibraryIdAndOriginalPath(libraryId: string, originalPath: string): Promise; deleteAll(ownerId: string): Promise; getAll(pagination: PaginationOptions, options?: AssetSearchOptions): Paginated; diff --git a/server/src/queries/asset.repository.sql b/server/src/queries/asset.repository.sql index d8edfdb12c..e223539dc0 100644 --- a/server/src/queries/asset.repository.sql +++ b/server/src/queries/asset.repository.sql @@ -253,7 +253,7 @@ DELETE FROM "assets" WHERE "ownerId" = $1 --- AssetRepository.getLibraryAssetPaths +-- AssetRepository.getExternalLibraryAssetPaths SELECT DISTINCT "distinctAlias"."AssetEntity_id" AS "ids_AssetEntity_id" FROM @@ -272,6 +272,7 @@ FROM ( ( ((("AssetEntity__AssetEntity_library"."id" = $1))) + AND ("AssetEntity"."isExternal" = $2) ) ) AND ("AssetEntity"."deletedAt" IS NULL) diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index b7ed3e4f00..724c1b0c0f 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -160,10 +160,10 @@ export class AssetRepository implements IAssetRepository { } @GenerateSql({ params: [{ take: 1, skip: 0 }, DummyValue.UUID] }) - getLibraryAssetPaths(pagination: PaginationOptions, libraryId: string): Paginated { + getExternalLibraryAssetPaths(pagination: PaginationOptions, libraryId: string): Paginated { return paginate(this.repository, pagination, { select: { id: true, originalPath: true, isOffline: true }, - where: { library: { id: libraryId } }, + where: { library: { id: libraryId }, isExternal: true }, }); } diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index 606a5f62e2..f2a67775f8 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -396,7 +396,10 @@ export class AssetService { // TODO refactor this to use cascades if (asset.livePhotoVideoId) { - await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } }); + await this.jobRepository.queue({ + name: JobName.ASSET_DELETION, + data: { id: asset.livePhotoVideoId, fromExternal }, + }); } const files = [asset.thumbnailPath, asset.previewPath, asset.encodedVideoPath]; diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 95e3655cb3..5f3dc45de1 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -160,7 +160,7 @@ describe(LibraryService.name, () => { storageMock.walk.mockImplementation(async function* generator() { yield '/data/user1/photo.jpg'; }); - assetMock.getLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); + assetMock.getExternalLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); await sut.handleQueueAssetRefresh(mockLibraryJob); @@ -189,7 +189,7 @@ describe(LibraryService.name, () => { storageMock.walk.mockImplementation(async function* generator() { yield '/data/user1/photo.jpg'; }); - assetMock.getLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); + assetMock.getExternalLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); await sut.handleQueueAssetRefresh(mockLibraryJob); @@ -238,7 +238,7 @@ describe(LibraryService.name, () => { }; libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1); - assetMock.getLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); + assetMock.getExternalLibraryAssetPaths.mockResolvedValue({ items: [], hasNextPage: false }); await sut.handleQueueAssetRefresh(mockLibraryJob); @@ -256,8 +256,8 @@ describe(LibraryService.name, () => { }; libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1); - assetMock.getLibraryAssetPaths.mockResolvedValue({ - items: [assetStub.image], + assetMock.getExternalLibraryAssetPaths.mockResolvedValue({ + items: [assetStub.external], hasNextPage: false, }); @@ -278,16 +278,16 @@ describe(LibraryService.name, () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1); // eslint-disable-next-line @typescript-eslint/require-await storageMock.walk.mockImplementation(async function* generator() { - yield assetStub.offline.originalPath; + yield assetStub.externalOffline.originalPath; }); - assetMock.getLibraryAssetPaths.mockResolvedValue({ - items: [assetStub.offline], + assetMock.getExternalLibraryAssetPaths.mockResolvedValue({ + items: [assetStub.externalOffline], hasNextPage: false, }); await sut.handleQueueAssetRefresh(mockLibraryJob); - expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.offline.id], { isOffline: false }); + expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.externalOffline.id], { isOffline: false }); expect(assetMock.updateAll).not.toHaveBeenCalledWith(expect.anything(), { isOffline: true }); expect(jobMock.queueAll).not.toHaveBeenCalled(); }); diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index 63066866fb..4dd47f9ad5 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -616,7 +616,7 @@ export class LibraryService extends EventEmitter { const assetIdsToMarkOffline = []; const assetIdsToMarkOnline = []; const pagination = usePagination(LIBRARY_SCAN_BATCH_SIZE, (pagination) => - this.assetRepository.getLibraryAssetPaths(pagination, library.id), + this.assetRepository.getExternalLibraryAssetPaths(pagination, library.id), ); this.logger.verbose(`Crawled asset paths paginated`); diff --git a/server/test/fixtures/asset.stub.ts b/server/test/fixtures/asset.stub.ts index 0b2ff82a3d..7aa49866d0 100644 --- a/server/test/fixtures/asset.stub.ts +++ b/server/test/fixtures/asset.stub.ts @@ -225,7 +225,7 @@ export const assetStub = { deviceId: 'device-id', originalPath: '/data/user1/photo.jpg', previewPath: '/uploads/user-id/thumbs/path.jpg', - checksum: Buffer.from('file hash', 'utf8'), + checksum: Buffer.from('path hash', 'utf8'), type: AssetType.IMAGE, thumbnailPath: '/uploads/user-id/webp/path.ext', thumbhash: Buffer.from('blablabla', 'base64'), @@ -295,6 +295,46 @@ export const assetStub = { deletedAt: null, }), + externalOffline: Object.freeze({ + id: 'asset-id', + deviceAssetId: 'device-asset-id', + fileModifiedAt: new Date('2023-02-23T05:06:29.716Z'), + fileCreatedAt: new Date('2023-02-23T05:06:29.716Z'), + owner: userStub.user1, + ownerId: 'user-id', + deviceId: 'device-id', + originalPath: '/data/user1/photo.jpg', + previewPath: '/uploads/user-id/thumbs/path.jpg', + checksum: Buffer.from('path hash', 'utf8'), + type: AssetType.IMAGE, + thumbnailPath: '/uploads/user-id/webp/path.ext', + thumbhash: Buffer.from('blablabla', 'base64'), + encodedVideoPath: null, + createdAt: new Date('2023-02-23T05:06:29.716Z'), + updatedAt: new Date('2023-02-23T05:06:29.716Z'), + localDateTime: new Date('2023-02-23T05:06:29.716Z'), + isFavorite: true, + isArchived: false, + isReadOnly: false, + isExternal: true, + duration: null, + isVisible: true, + livePhotoVideo: null, + livePhotoVideoId: null, + isOffline: true, + libraryId: 'library-id', + library: libraryStub.externalLibrary1, + tags: [], + sharedLinks: [], + originalFileName: 'asset-id.jpg', + faces: [], + sidecarPath: null, + exifInfo: { + fileSizeInByte: 5000, + } as ExifEntity, + deletedAt: null, + }), + image1: Object.freeze({ id: 'asset-id-1', deviceAssetId: 'device-asset-id', diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index 67770cd93b..4133880d6f 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -20,7 +20,7 @@ export const newAssetRepositoryMock = (): jest.Mocked => { getAll: jest.fn().mockResolvedValue({ items: [], hasNextPage: false }), getAllByDeviceId: jest.fn(), updateAll: jest.fn(), - getLibraryAssetPaths: jest.fn(), + getExternalLibraryAssetPaths: jest.fn(), getByLibraryIdAndOriginalPath: jest.fn(), deleteAll: jest.fn(), update: jest.fn(),