fix(server): check if sidecarPath exists (#6293)

* check if sidecarPath exists

* Revert "check if sidecarPath exists"

This reverts commit 954a1097b870585afee34974d466e51c5172fed9.

* sidecar sync remove dead sidecarPaths and discover new ones

* tests and minor cleanup

* chore: linting

---------

Co-authored-by: Daniel Dietzler <mail@ddietzler.dev>
Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

server/src/domain/metadata/metadata.service.spec.ts

@@ -37,7 +37,6 @@ import {
   IStorageRepository,
   ISystemConfigRepository,
   ImmichTags,
-  WithProperty,
   WithoutProperty,
 } from '../repositories';
 import { MetadataService, Orientation } from './metadata.service';
@@ -598,11 +597,11 @@ describe(MetadataService.name, () => {
 
   describe('handleQueueSidecar', () => {
     it('should queue assets with sidecar files', async () => {
-      assetMock.getWith.mockResolvedValue({ items: [assetStub.sidecar], hasNextPage: false });
+      assetMock.getAll.mockResolvedValue({ items: [assetStub.sidecar], hasNextPage: false });
 
       await sut.handleQueueSidecar({ force: true });
 
-      expect(assetMock.getWith).toHaveBeenCalledWith({ take: 1000, skip: 0 }, WithProperty.SIDECAR);
+      expect(assetMock.getAll).toHaveBeenCalledWith({ take: 1000, skip: 0 });
       expect(assetMock.getWithout).not.toHaveBeenCalled();
       expect(jobMock.queueAll).toHaveBeenCalledWith([
         {
@@ -618,7 +617,7 @@ describe(MetadataService.name, () => {
       await sut.handleQueueSidecar({ force: false });
 
       expect(assetMock.getWithout).toHaveBeenCalledWith({ take: 1000, skip: 0 }, WithoutProperty.SIDECAR);
-      expect(assetMock.getWith).not.toHaveBeenCalled();
+      expect(assetMock.getAll).not.toHaveBeenCalled();
       expect(jobMock.queueAll).toHaveBeenCalledWith([
         {
           name: JobName.SIDECAR_DISCOVERY,
@@ -629,8 +628,46 @@ describe(MetadataService.name, () => {
   });
 
   describe('handleSidecarSync', () => {
-    it('should not error', async () => {
+    it('should do nothing if asset could not be found', async () => {
-      await sut.handleSidecarSync();
+      assetMock.getByIds.mockResolvedValue([]);
+      await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(false);
+      expect(assetMock.save).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(false);
+      expect(assetMock.save).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(false);
+      expect(assetMock.save).not.toHaveBeenCalled();
+    });
+
+    it('should set sidecar path if exists', async () => {
+      assetMock.getByIds.mockResolvedValue([assetStub.sidecar]);
+      storageMock.checkFileExists.mockResolvedValue(true);
+
+      await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(true);
+      expect(storageMock.checkFileExists).toHaveBeenCalledWith(`${assetStub.sidecar.originalPath}.xmp`, constants.R_OK);
+      expect(assetMock.save).toHaveBeenCalledWith({
+        id: assetStub.sidecar.id,
+        sidecarPath: assetStub.sidecar.sidecarPath,
+      });
+    });
+
+    it('should unset sidecar path if file does not exist anymore', async () => {
+      assetMock.getByIds.mockResolvedValue([assetStub.sidecar]);
+      storageMock.checkFileExists.mockResolvedValue(false);
+
+      await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(true);
+      expect(storageMock.checkFileExists).toHaveBeenCalledWith(`${assetStub.sidecar.originalPath}.xmp`, constants.R_OK);
+      expect(assetMock.save).toHaveBeenCalledWith({
+        id: assetStub.sidecar.id,
+        sidecarPath: null,
+      });
     });
   });
 

server/src/domain/metadata/metadata.service.ts

@@ -25,7 +25,6 @@ import {
   IStorageRepository,
   ISystemConfigRepository,
   ImmichTags,
-  WithProperty,
   WithoutProperty,
 } from '../repositories';
 import { StorageCore } from '../storage';
@@ -267,7 +266,7 @@ export class MetadataService {
     const { force } = job;
     const assetPagination = usePagination(JOBS_ASSET_PAGINATION_SIZE, (pagination) => {
       return force
-        ? this.assetRepository.getWith(pagination, WithProperty.SIDECAR)
+        ? this.assetRepository.getAll(pagination)
         : this.assetRepository.getWithout(pagination, WithoutProperty.SIDECAR);
     });
 
@@ -283,26 +282,12 @@ export class MetadataService {
     return true;
   }
 
-  async handleSidecarSync() {
+  handleSidecarSync({ id }: IEntityJob) {
-    // TODO: optimize to only queue assets with recent xmp changes
+    return this.processSidecar(id, true);
-    return true;
   }
 
-  async handleSidecarDiscovery({ id }: IEntityJob) {
+  handleSidecarDiscovery({ id }: IEntityJob) {
-    const [asset] = await this.assetRepository.getByIds([id]);
+    return this.processSidecar(id, false);
-    if (!asset || !asset.isVisible || asset.sidecarPath) {
-      return false;
-    }
-
-    const sidecarPath = `${asset.originalPath}.xmp`;
-    const exists = await this.storageRepository.checkFileExists(sidecarPath, constants.R_OK);
-    if (!exists) {
-      return false;
-    }
-
-    await this.assetRepository.save({ id: asset.id, sidecarPath });
-
-    return true;
   }
 
   async handleSidecarWrite(job: ISidecarWriteJob) {
@@ -565,4 +550,36 @@ export class MetadataService {
 
     return Duration.fromObject({ seconds: _seconds }).toFormat('hh:mm:ss.SSS');
   }
+
+  private async processSidecar(id: string, isSync: boolean) {
+    const [asset] = await this.assetRepository.getByIds([id]);
+
+    if (!asset) {
+      return false;
+    }
+
+    if (isSync && !asset.sidecarPath) {
+      return false;
+    }
+
+    if (!isSync && (!asset.isVisible || asset.sidecarPath)) {
+      return false;
+    }
+
+    const sidecarPath = `${asset.originalPath}.xmp`;
+    const exists = await this.storageRepository.checkFileExists(sidecarPath, constants.R_OK);
+    if (exists) {
+      await this.assetRepository.save({ id: asset.id, sidecarPath });
+      return true;
+    }
+
+    if (!isSync) {
+      return false;
+    }
+
+    this.logger.debug(`Sidecar File '${sidecarPath}' was not found, removing sidecarPath for asset ${asset.id}`);
+    await this.assetRepository.save({ id: asset.id, sidecarPath: null });
+
+    return true;
+  }
 }

server/src/microservices/app.service.ts

@@ -72,7 +72,7 @@ export class AppService {
       [JobName.PERSON_CLEANUP]: () => this.personService.handlePersonCleanup(),
       [JobName.QUEUE_SIDECAR]: (data) => this.metadataService.handleQueueSidecar(data),
       [JobName.SIDECAR_DISCOVERY]: (data) => this.metadataService.handleSidecarDiscovery(data),
-      [JobName.SIDECAR_SYNC]: () => this.metadataService.handleSidecarSync(),
+      [JobName.SIDECAR_SYNC]: (data) => this.metadataService.handleSidecarSync(data),
       [JobName.SIDECAR_WRITE]: (data) => this.metadataService.handleSidecarWrite(data),
       [JobName.LIBRARY_SCAN_ASSET]: (data) => this.libraryService.handleAssetRefresh(data),
       [JobName.LIBRARY_SCAN]: (data) => this.libraryService.handleQueueAssetRefresh(data),