fix(server): add missing file extensions to library files (#8342)

* fix file extensions * fix tests * fix formatting * fixed bug * fix merts comments
immich-app · Mar 29, 2024 · ec48fcc · ec48fcc
1 parent 3f61019
commit ec48fcc
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 9 deletions.
diff --git 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
@@ -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
@@ -639,4 +639,82 @@ export const assetStub = {
     } as ExifEntity,
     deletedAt: null,
   }),
+  missingFileExtension: Object.freeze<AssetEntity>({
+    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<AssetEntity>({
+    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,
+  }),
 };