From ec48fccb30d6e60306489738333ba4335f19a8f8 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Fri, 29 Mar 2024 03:51:07 +0100 Subject: [PATCH] fix(server): add missing file extensions to library files (#8342) * fix file extensions * fix tests * fix formatting * fixed bug * fix merts comments --- server/src/services/library.service.spec.ts | 37 +++++++--- server/src/services/library.service.ts | 11 ++- server/test/fixtures/asset.stub.ts | 78 +++++++++++++++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 1af419aff6..df59d0c150 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -501,17 +501,17 @@ describe(LibraryService.name, () => { const mockLibraryJob: ILibraryFileJob = { id: libraryStub.externalLibrary1.id, ownerId: mockUser.id, - assetPath: '/data/user1/photo.jpg', + assetPath: assetStub.hasFileExtension.originalPath, force: false, }; storageMock.stat.mockResolvedValue({ size: 100, - mtime: assetStub.image.fileModifiedAt, + mtime: assetStub.hasFileExtension.fileModifiedAt, ctime: new Date('2023-01-01'), } as Stats); - assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.image); + assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.hasFileExtension); await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SKIPPED); @@ -548,6 +548,26 @@ describe(LibraryService.name, () => { }); }); + it('should import an asset that is missing a file extension', async () => { + // This tests for the case where the file extension is missing from the asset path. + // This happened in previous versions of Immich + const mockLibraryJob: ILibraryFileJob = { + id: libraryStub.externalLibrary1.id, + ownerId: mockUser.id, + assetPath: assetStub.missingFileExtension.originalPath, + force: false, + }; + + assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.missingFileExtension); + + await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS); + + expect(assetMock.updateAll).toHaveBeenCalledWith( + [assetStub.missingFileExtension.id], + expect.objectContaining({ originalFileName: 'photo.jpg' }), + ); + }); + it('should set a missing asset to offline', async () => { storageMock.stat.mockRejectedValue(new Error('Path not found')); @@ -618,19 +638,20 @@ describe(LibraryService.name, () => { it('should refresh an existing asset if forced', async () => { const mockLibraryJob: ILibraryFileJob = { id: assetStub.image.id, - ownerId: assetStub.image.ownerId, - assetPath: '/data/user1/photo.jpg', + ownerId: assetStub.hasFileExtension.ownerId, + assetPath: assetStub.hasFileExtension.originalPath, force: true, }; - assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.image); - assetMock.create.mockResolvedValue(assetStub.image); + assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(assetStub.hasFileExtension); + assetMock.create.mockResolvedValue(assetStub.hasFileExtension); await expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(JobStatus.SUCCESS); - expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.image.id], { + expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.hasFileExtension.id], { fileCreatedAt: new Date('2023-01-01'), fileModifiedAt: new Date('2023-01-01'), + originalFileName: assetStub.hasFileExtension.originalFileName, }); }); diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index b3c4915352..d0e41da172 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -443,6 +443,8 @@ export class LibraryService extends EventEmitter { doRefresh = true; } + const originalFileName = parse(assetPath).base; + if (!existingAssetEntity) { // This asset is new to us, read it from disk this.logger.debug(`Importing new asset: ${assetPath}`); @@ -453,6 +455,12 @@ export class LibraryService extends EventEmitter { `File modification time has changed, re-importing asset: ${assetPath}. Old mtime: ${existingAssetEntity.fileModifiedAt}. New mtime: ${stats.mtime}`, ); doRefresh = true; + } else if (existingAssetEntity.originalFileName !== originalFileName) { + // TODO: We can likely remove this check in the second half of 2024 when all assets have likely been re-imported by all users + this.logger.debug( + `Asset is missing file extension, re-importing: ${assetPath}. Current incorrect filename: ${existingAssetEntity.originalFileName}.`, + ); + doRefresh = true; } else if (!job.force && stats && !existingAssetEntity.isOffline) { // Asset exists on disk and in db and mtime has not changed. Also, we are not forcing refresn. Therefore, do nothing this.logger.debug(`Asset already exists in database and on disk, will not import: ${assetPath}`); @@ -510,7 +518,7 @@ export class LibraryService extends EventEmitter { fileModifiedAt: stats.mtime, localDateTime: stats.mtime, type: assetType, - originalFileName: parse(assetPath).base, + originalFileName, sidecarPath, isReadOnly: true, isExternal: true, @@ -521,6 +529,7 @@ export class LibraryService extends EventEmitter { await this.assetRepository.updateAll([existingAssetEntity.id], { fileCreatedAt: stats.mtime, fileModifiedAt: stats.mtime, + originalFileName, }); } else { // Not importing and not refreshing, do nothing diff --git a/server/test/fixtures/asset.stub.ts b/server/test/fixtures/asset.stub.ts index e9b4650c28..fc94a363a1 100644 --- a/server/test/fixtures/asset.stub.ts +++ b/server/test/fixtures/asset.stub.ts @@ -639,4 +639,82 @@ export const assetStub = { } as ExifEntity, deletedAt: null, }), + missingFileExtension: 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', + resizePath: '/uploads/user-id/thumbs/path.jpg', + checksum: Buffer.from('file hash', 'utf8'), + type: AssetType.IMAGE, + webpPath: '/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: false, + libraryId: 'library-id', + library: libraryStub.externalLibrary1, + tags: [], + sharedLinks: [], + originalFileName: 'photo', + faces: [], + deletedAt: null, + sidecarPath: null, + exifInfo: { + fileSizeInByte: 5000, + } as ExifEntity, + }), + hasFileExtension: 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', + resizePath: '/uploads/user-id/thumbs/path.jpg', + checksum: Buffer.from('file hash', 'utf8'), + type: AssetType.IMAGE, + webpPath: '/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: false, + libraryId: 'library-id', + library: libraryStub.externalLibrary1, + tags: [], + sharedLinks: [], + originalFileName: 'photo.jpg', + faces: [], + deletedAt: null, + sidecarPath: null, + exifInfo: { + fileSizeInByte: 5000, + } as ExifEntity, + }), };