refactor(server): link live photos as part of metadata extraction instead of queueing job (#16390)

* link live photos helper instead of job

* update test

* queue storage template migration

* queue in onDone

* remove link live photos job

server/src/enum.ts

@@ -437,7 +437,6 @@ export enum JobName {
   // metadata
   QUEUE_METADATA_EXTRACTION = 'queue-metadata-extraction',
   METADATA_EXTRACTION = 'metadata-extraction',
-  LINK_LIVE_PHOTOS = 'link-live-photos',
 
   // user
   USER_DELETION = 'user-deletion',

server/src/services/job.service.spec.ts

@@ -233,10 +233,6 @@ describe(JobService.name, () => {
       },
       {
         item: { name: JobName.METADATA_EXTRACTION, data: { id: 'asset-1' } },
-        jobs: [JobName.LINK_LIVE_PHOTOS],
-      },
-      {
-        item: { name: JobName.LINK_LIVE_PHOTOS, data: { id: 'asset-1' } },
         jobs: [JobName.STORAGE_TEMPLATE_MIGRATION_SINGLE],
       },
       {

server/src/services/job.service.ts

@@ -255,11 +255,6 @@ export class JobService extends BaseService {
             this.eventRepository.clientSend('on_asset_update', asset.ownerId, mapAsset(asset));
           }
         }
-        await this.jobRepository.queue({ name: JobName.LINK_LIVE_PHOTOS, data: item.data });
-        break;
-      }
-
-      case JobName.LINK_LIVE_PHOTOS: {
         await this.jobRepository.queue({ name: JobName.STORAGE_TEMPLATE_MIGRATION_SINGLE, data: item.data });
         break;
       }

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

@@ -76,125 +76,6 @@ describe(MetadataService.name, () => {
     });
   });
 
-  describe('handleLivePhotoLinking', () => {
-    it('should handle an asset that could not be found', async () => {
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED);
-      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
-      expect(mocks.asset.findLivePhotoMatch).not.toHaveBeenCalled();
-      expect(mocks.asset.update).not.toHaveBeenCalled();
-      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
-    });
-
-    it('should handle an asset without exif info', async () => {
-      mocks.asset.getByIds.mockResolvedValue([{ ...assetStub.image, exifInfo: undefined }]);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.FAILED);
-      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
-      expect(mocks.asset.findLivePhotoMatch).not.toHaveBeenCalled();
-      expect(mocks.asset.update).not.toHaveBeenCalled();
-      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
-    });
-
-    it('should handle livePhotoCID not set', async () => {
-      mocks.asset.getByIds.mockResolvedValue([{ ...assetStub.image }]);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.image.id })).resolves.toBe(JobStatus.SKIPPED);
-      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.image.id], { exifInfo: true });
-      expect(mocks.asset.findLivePhotoMatch).not.toHaveBeenCalled();
-      expect(mocks.asset.update).not.toHaveBeenCalled();
-      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
-    });
-
-    it('should handle not finding a match', async () => {
-      mocks.asset.getByIds.mockResolvedValue([
-        {
-          ...assetStub.livePhotoMotionAsset,
-          exifInfo: { livePhotoCID: assetStub.livePhotoStillAsset.id } as ExifEntity,
-        },
-      ]);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoMotionAsset.id })).resolves.toBe(
-        JobStatus.SKIPPED,
-      );
-      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id], { exifInfo: true });
-      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
-        livePhotoCID: assetStub.livePhotoStillAsset.id,
-        ownerId: assetStub.livePhotoMotionAsset.ownerId,
-        otherAssetId: assetStub.livePhotoMotionAsset.id,
-        type: AssetType.IMAGE,
-      });
-      expect(mocks.asset.update).not.toHaveBeenCalled();
-      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
-    });
-
-    it('should link photo and video', async () => {
-      mocks.asset.getByIds.mockResolvedValue([
-        {
-          ...assetStub.livePhotoStillAsset,
-          exifInfo: { livePhotoCID: assetStub.livePhotoMotionAsset.id } as ExifEntity,
-        },
-      ]);
-      mocks.asset.findLivePhotoMatch.mockResolvedValue(assetStub.livePhotoMotionAsset);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
-        JobStatus.SUCCESS,
-      );
-      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id], { exifInfo: true });
-      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
-        livePhotoCID: assetStub.livePhotoMotionAsset.id,
-        ownerId: assetStub.livePhotoStillAsset.ownerId,
-        otherAssetId: assetStub.livePhotoStillAsset.id,
-        type: AssetType.VIDEO,
-      });
-      expect(mocks.asset.update).toHaveBeenCalledWith({
-        id: assetStub.livePhotoStillAsset.id,
-        livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
-      });
-      expect(mocks.asset.update).toHaveBeenCalledWith({ id: assetStub.livePhotoMotionAsset.id, isVisible: false });
-      expect(mocks.album.removeAsset).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.id);
-    });
-
-    it('should notify clients on live photo link', async () => {
-      mocks.asset.getByIds.mockResolvedValue([
-        {
-          ...assetStub.livePhotoStillAsset,
-          exifInfo: { livePhotoCID: assetStub.livePhotoMotionAsset.id } as ExifEntity,
-        },
-      ]);
-      mocks.asset.findLivePhotoMatch.mockResolvedValue(assetStub.livePhotoMotionAsset);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
-        JobStatus.SUCCESS,
-      );
-      expect(mocks.event.emit).toHaveBeenCalledWith('asset.hide', {
-        userId: assetStub.livePhotoMotionAsset.ownerId,
-        assetId: assetStub.livePhotoMotionAsset.id,
-      });
-    });
-
-    it('should search by libraryId', async () => {
-      mocks.asset.getByIds.mockResolvedValue([
-        {
-          ...assetStub.livePhotoStillAsset,
-          libraryId: 'library-id',
-          exifInfo: { livePhotoCID: 'CID' } as ExifEntity,
-        },
-      ]);
-
-      await expect(sut.handleLivePhotoLinking({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
-        JobStatus.SKIPPED,
-      );
-
-      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
-        ownerId: 'user-id',
-        otherAssetId: 'live-photo-still-asset',
-        livePhotoCID: 'CID',
-        libraryId: 'library-id',
-        type: 'VIDEO',
-      });
-    });
-  });
-
   describe('handleQueueMetadataExtraction', () => {
     it('should queue metadata extraction for all assets without exif values', async () => {
       mocks.asset.getWithout.mockResolvedValue({ items: [assetStub.image], hasNextPage: false });
@@ -746,11 +627,11 @@ describe(MetadataService.name, () => {
       mocks.storage.checkFileExists.mockResolvedValue(true);
 
       await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
-      expect(mocks.asset.create).toHaveBeenCalledTimes(0);
+      expect(mocks.asset.create).not.toHaveBeenCalled();
-      expect(mocks.storage.createOrOverwriteFile).toHaveBeenCalledTimes(0);
+      expect(mocks.storage.createOrOverwriteFile).not.toHaveBeenCalled();
       // The still asset gets saved by handleMetadataExtraction, but not the video
       expect(mocks.asset.update).toHaveBeenCalledTimes(1);
-      expect(mocks.job.queue).toHaveBeenCalledTimes(0);
+      expect(mocks.job.queue).not.toHaveBeenCalled();
     });
 
     it('should link and hide motion video asset to still asset if the hash of the extracted video matches an existing asset', async () => {
@@ -1178,6 +1059,107 @@ describe(MetadataService.name, () => {
         }),
       );
     });
+
+    it('should handle livePhotoCID not set', async () => {
+      mocks.asset.getByIds.mockResolvedValue([assetStub.image]);
+
+      await expect(sut.handleMetadataExtraction({ id: assetStub.image.id })).resolves.toBe(JobStatus.SUCCESS);
+
+      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.image.id], { faces: { person: false } });
+      expect(mocks.asset.findLivePhotoMatch).not.toHaveBeenCalled();
+      expect(mocks.asset.update).not.toHaveBeenCalledWith(expect.objectContaining({ isVisible: false }));
+      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
+    });
+
+    it('should handle not finding a match', async () => {
+      mocks.media.probe.mockResolvedValue(probeStub.videoStreamVertical2160p);
+      mocks.asset.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset]);
+      mockReadTags({ ContentIdentifier: 'CID' });
+
+      await expect(sut.handleMetadataExtraction({ id: assetStub.livePhotoMotionAsset.id })).resolves.toBe(
+        JobStatus.SUCCESS,
+      );
+
+      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id], {
+        faces: { person: false },
+      });
+      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
+        livePhotoCID: 'CID',
+        ownerId: assetStub.livePhotoMotionAsset.ownerId,
+        otherAssetId: assetStub.livePhotoMotionAsset.id,
+        type: AssetType.IMAGE,
+      });
+      expect(mocks.asset.update).not.toHaveBeenCalledWith(expect.objectContaining({ isVisible: false }));
+      expect(mocks.album.removeAsset).not.toHaveBeenCalled();
+    });
+
+    it('should link photo and video', async () => {
+      mocks.asset.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
+      mocks.asset.findLivePhotoMatch.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      mockReadTags({ ContentIdentifier: 'CID' });
+
+      await expect(sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
+        JobStatus.SUCCESS,
+      );
+
+      expect(mocks.asset.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id], {
+        faces: { person: false },
+      });
+      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
+        livePhotoCID: 'CID',
+        ownerId: assetStub.livePhotoStillAsset.ownerId,
+        otherAssetId: assetStub.livePhotoStillAsset.id,
+        type: AssetType.VIDEO,
+      });
+      expect(mocks.asset.update).toHaveBeenCalledWith({
+        id: assetStub.livePhotoStillAsset.id,
+        livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
+      });
+      expect(mocks.asset.update).toHaveBeenCalledWith({ id: assetStub.livePhotoMotionAsset.id, isVisible: false });
+      expect(mocks.album.removeAsset).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.id);
+    });
+
+    it('should notify clients on live photo link', async () => {
+      mocks.asset.getByIds.mockResolvedValue([
+        {
+          ...assetStub.livePhotoStillAsset,
+          exifInfo: { livePhotoCID: assetStub.livePhotoMotionAsset.id } as ExifEntity,
+        },
+      ]);
+      mocks.asset.findLivePhotoMatch.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      mockReadTags({ ContentIdentifier: 'CID' });
+
+      await expect(sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
+        JobStatus.SUCCESS,
+      );
+
+      expect(mocks.event.emit).toHaveBeenCalledWith('asset.hide', {
+        userId: assetStub.livePhotoMotionAsset.ownerId,
+        assetId: assetStub.livePhotoMotionAsset.id,
+      });
+    });
+
+    it('should search by libraryId', async () => {
+      mocks.asset.getByIds.mockResolvedValue([
+        {
+          ...assetStub.livePhotoStillAsset,
+          libraryId: 'library-id',
+        },
+      ]);
+      mockReadTags({ ContentIdentifier: 'CID' });
+
+      await expect(sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id })).resolves.toBe(
+        JobStatus.SUCCESS,
+      );
+
+      expect(mocks.asset.findLivePhotoMatch).toHaveBeenCalledWith({
+        ownerId: 'user-id',
+        otherAssetId: 'live-photo-still-asset',
+        livePhotoCID: 'CID',
+        libraryId: 'library-id',
+        type: 'VIDEO',
+      });
+    });
   });
 
   describe('handleQueueSidecar', () => {

server/src/services/metadata.service.ts

@@ -113,21 +113,14 @@ export class MetadataService extends BaseService {
     }
   }
 
-  @OnJob({ name: JobName.LINK_LIVE_PHOTOS, queue: QueueName.METADATA_EXTRACTION })
+  private async linkLivePhotos(asset: AssetEntity, exifInfo: Insertable<Exif>): Promise<void> {
-  async handleLivePhotoLinking(job: JobOf<JobName.LINK_LIVE_PHOTOS>): Promise<JobStatus> {
+    if (!exifInfo.livePhotoCID) {
-    const { id } = job;
+      return;
-    const [asset] = await this.assetRepository.getByIds([id], { exifInfo: true });
-    if (!asset?.exifInfo) {
-      return JobStatus.FAILED;
-    }
-
-    if (!asset.exifInfo.livePhotoCID) {
-      return JobStatus.SKIPPED;
     }
 
     const otherType = asset.type === AssetType.VIDEO ? AssetType.IMAGE : AssetType.VIDEO;
     const match = await this.assetRepository.findLivePhotoMatch({
-      livePhotoCID: asset.exifInfo.livePhotoCID,
+      livePhotoCID: exifInfo.livePhotoCID,
       ownerId: asset.ownerId,
       libraryId: asset.libraryId,
       otherAssetId: asset.id,
@@ -135,18 +128,17 @@ export class MetadataService extends BaseService {
     });
 
     if (!match) {
-      return JobStatus.SKIPPED;
+      return;
     }
 
     const [photoAsset, motionAsset] = asset.type === AssetType.IMAGE ? [asset, match] : [match, asset];
-
+    await Promise.all([
-    await this.assetRepository.update({ id: photoAsset.id, livePhotoVideoId: motionAsset.id });
+      this.assetRepository.update({ id: photoAsset.id, livePhotoVideoId: motionAsset.id }),
-    await this.assetRepository.update({ id: motionAsset.id, isVisible: false });
+      this.assetRepository.update({ id: motionAsset.id, isVisible: false }),
-    await this.albumRepository.removeAsset(motionAsset.id);
+      this.albumRepository.removeAsset(motionAsset.id),
+    ]);
 
     await this.eventRepository.emit('asset.hide', { assetId: motionAsset.id, userId: motionAsset.ownerId });
-
-    return JobStatus.SUCCESS;
   }
 
   @OnJob({ name: JobName.QUEUE_METADATA_EXTRACTION, queue: QueueName.METADATA_EXTRACTION })
@@ -168,9 +160,9 @@ export class MetadataService extends BaseService {
   }
 
   @OnJob({ name: JobName.METADATA_EXTRACTION, queue: QueueName.METADATA_EXTRACTION })
-  async handleMetadataExtraction({ id }: JobOf<JobName.METADATA_EXTRACTION>): Promise<JobStatus> {
+  async handleMetadataExtraction(data: JobOf<JobName.METADATA_EXTRACTION>): Promise<JobStatus> {
     const { metadata, reverseGeocoding } = await this.getConfig({ withCache: true });
-    const [asset] = await this.assetRepository.getByIds([id], { faces: { person: false } });
+    const [asset] = await this.assetRepository.getByIds([data.id], { faces: { person: false } });
     if (!asset) {
       return JobStatus.FAILED;
     }
@@ -251,15 +243,16 @@ export class MetadataService extends BaseService {
       fileModifiedAt: stats.mtime,
     });
 
-    await this.assetRepository.upsertJobStatus({
+    if (exifData.livePhotoCID) {
-      assetId: asset.id,
+      await this.linkLivePhotos(asset, exifData);
-      metadataExtractedAt: new Date(),
+    }
-    });
 
     if (isFaceImportEnabled(metadata)) {
       await this.applyTaggedFaces(asset, exifTags);
     }
 
+    await this.assetRepository.upsertJobStatus({ assetId: asset.id, metadataExtractedAt: new Date() });
+
     return JobStatus.SUCCESS;
   }
 

server/src/types.ts

@@ -303,7 +303,6 @@ export type JobItem =
   // Metadata Extraction
   | { name: JobName.QUEUE_METADATA_EXTRACTION; data: IBaseJob }
   | { name: JobName.METADATA_EXTRACTION; data: IEntityJob }
-  | { name: JobName.LINK_LIVE_PHOTOS; data: IEntityJob }
   // Sidecar Scanning
   | { name: JobName.QUEUE_SIDECAR; data: IBaseJob }
   | { name: JobName.SIDECAR_DISCOVERY; data: IEntityJob }