refactor: move asset stacks to their own entity (#6353)

* feat: auto-stack burst photos * feat: move stacks to asset stack entity * chore: pin node version with volta in server * chore: update e2e cases * chore: cleanup * feat: migrate existing stacks --------- Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
2024-01-27 18:52:14 +00:00
parent c4b8c853bc
commit 25cad79657
29 changed files with 538 additions and 243 deletions
@@ -2,11 +2,13 @@ import { AssetEntity, AssetType } from '@app/infra/entities';
 import { BadRequestException, UnauthorizedException } from '@nestjs/common';
 import {
  IAccessRepositoryMock,
+  assetStackStub,
  assetStub,
  authStub,
  faceStub,
  newAccessRepositoryMock,
  newAssetRepositoryMock,
+  newAssetStackRepositoryMock,
  newCommunicationRepositoryMock,
  newJobRepositoryMock,
  newPartnerRepositoryMock,
@@ -20,6 +22,7 @@ import {
  AssetStats,
  ClientEvent,
  IAssetRepository,
+  IAssetStackRepository,
  ICommunicationRepository,
  IJobRepository,
  IPartnerRepository,
@@ -160,6 +163,7 @@ describe(AssetService.name, () => {
  let communicationMock: jest.Mocked<ICommunicationRepository>;
  let configMock: jest.Mocked<ISystemConfigRepository>;
  let partnerMock: jest.Mocked<IPartnerRepository>;
+  let assetStackMock: jest.Mocked<IAssetStackRepository>;

  it('should work', () => {
    expect(sut).toBeDefined();
@@ -174,6 +178,7 @@ describe(AssetService.name, () => {
    userMock = newUserRepositoryMock();
    configMock = newSystemConfigRepositoryMock();
    partnerMock = newPartnerRepositoryMock();
+    assetStackMock = newAssetStackRepositoryMock();

    sut = new AssetService(
      accessMock,
@@ -184,6 +189,7 @@ describe(AssetService.name, () => {
      userMock,
      communicationMock,
      partnerMock,
+      assetStackMock,
    );

    when(assetMock.getById)
@@ -578,65 +584,121 @@ describe(AssetService.name, () => {
      ).rejects.toBeInstanceOf(BadRequestException);
    });

-    it('should update parent asset when children are added', async () => {
+    it('should update parent asset updatedAt when children are added', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['parent']));
+      when(assetMock.getById)
+        .calledWith('parent', { stack: { assets: true } })
+        .mockResolvedValue(assetStub.image);
      await sut.updateAll(authStub.user1, {
        ids: [],
        stackParentId: 'parent',
      }),
-        expect(assetMock.updateAll).toHaveBeenCalledWith(['parent'], { stackParentId: null });
+        expect(assetMock.updateAll).toHaveBeenCalledWith(['parent'], { updatedAt: expect.any(Date) });
    });

    it('should update parent asset when children are removed', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['child-1']));
-      assetMock.getByIds.mockResolvedValue([{ id: 'child-1', stackParentId: 'parent' } as AssetEntity]);
+      assetMock.getByIds.mockResolvedValue([
+        {
+          id: 'child-1',
+          stackId: 'stack-1',
+          stack: assetStackStub('stack-1', [{ id: 'parent' } as AssetEntity, { id: 'child-1' } as AssetEntity]),
+        } as AssetEntity,
+      ]);
+      when(assetStackMock.getById)
+        .calledWith('stack-1')
+        .mockResolvedValue(assetStackStub('stack-1', [{ id: 'parent' } as AssetEntity]));

      await sut.updateAll(authStub.user1, {
        ids: ['child-1'],
        removeParent: true,
-      }),
-        expect(assetMock.updateAll).toHaveBeenCalledWith(expect.arrayContaining(['parent']), { stackParentId: null });
+      });
+      expect(assetMock.updateAll).toHaveBeenCalledWith(expect.arrayContaining(['child-1']), { stack: null });
+      expect(assetMock.updateAll).toHaveBeenCalledWith(expect.arrayContaining(['parent']), {
+        updatedAt: expect.any(Date),
+      });
+      expect(assetStackMock.delete).toHaveBeenCalledWith('stack-1');
    });

    it('update parentId for new children', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['child-1', 'child-2']));
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['parent']));
+      const stack = assetStackStub('stack-1', [
+        { id: 'parent' } as AssetEntity,
+        { id: 'child-1' } as AssetEntity,
+        { id: 'child-2' } as AssetEntity,
+      ]);
+      when(assetMock.getById)
+        .calledWith('parent', { stack: { assets: true } })
+        .mockResolvedValue({
+          id: 'child-1',
+          stack,
+        } as AssetEntity);
+
      await sut.updateAll(authStub.user1, {
        stackParentId: 'parent',
        ids: ['child-1', 'child-2'],
      });

-      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'child-2'], { stackParentId: 'parent' });
+      expect(assetStackMock.update).toHaveBeenCalledWith({
+        ...assetStackStub('stack-1', [
+          { id: 'child-1' } as AssetEntity,
+          { id: 'child-2' } as AssetEntity,
+          { id: 'parent' } as AssetEntity,
+        ]),
+        primaryAsset: undefined,
+      });
+      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'child-2', 'parent'], { updatedAt: expect.any(Date) });
    });

-    it('nullify parentId for remove children', async () => {
+    it('remove stack for removed children', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['child-1', 'child-2']));
      await sut.updateAll(authStub.user1, {
        removeParent: true,
        ids: ['child-1', 'child-2'],
      });

-      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'child-2'], { stackParentId: null });
+      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'child-2'], { stack: null });
    });

    it('merge stacks if new child has children', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['child-1']));
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['parent']));
+      when(assetMock.getById)
+        .calledWith('parent', { stack: { assets: true } })
+        .mockResolvedValue({ ...assetStub.image, id: 'parent' });
      assetMock.getByIds.mockResolvedValue([
-        { id: 'child-1', stack: [{ id: 'child-2' } as AssetEntity] } as AssetEntity,
+        {
+          id: 'child-1',
+          stackId: 'stack-1',
+          stack: assetStackStub('stack-1', [{ id: 'child-1' } as AssetEntity, { id: 'child-2' } as AssetEntity]),
+        } as AssetEntity,
      ]);
+      when(assetStackMock.getById)
+        .calledWith('stack-1')
+        .mockResolvedValue(assetStackStub('stack-1', [{ id: 'parent' } as AssetEntity]));

      await sut.updateAll(authStub.user1, {
        ids: ['child-1'],
        stackParentId: 'parent',
      });

-      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'child-2'], { stackParentId: 'parent' });
+      expect(assetStackMock.delete).toHaveBeenCalledWith('stack-1');
+      expect(assetStackMock.create).toHaveBeenCalledWith({
+        assets: [{ id: 'child-1' }, { id: 'parent' }, { id: 'child-1' }, { id: 'child-2' }],
+        primaryAssetId: 'parent',
+      });
+      expect(assetMock.updateAll).toBeCalledWith(['child-1', 'parent', 'child-1', 'child-2'], {
+        updatedAt: expect.any(Date),
+      });
    });

    it('should send ws asset update event', async () => {
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['asset-1']));
      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['parent']));
+      when(assetMock.getById)
+        .calledWith('parent', { stack: { assets: true } })
+        .mockResolvedValue(assetStub.image);

      await sut.updateAll(authStub.user1, {
        ids: ['asset-1'],
@@ -645,6 +707,7 @@ describe(AssetService.name, () => {

      expect(communicationMock.send).toHaveBeenCalledWith(ClientEvent.ASSET_UPDATE, authStub.user1.user.id, [
        'asset-1',
+        'parent',
      ]);
    });
  });
@@ -702,7 +765,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetWithFace);
@@ -729,25 +792,23 @@ describe(AssetService.name, () => {
      expect(assetMock.remove).toHaveBeenCalledWith(assetWithFace);
    });

-    it('should update stack parent if asset has stack children', async () => {
+    it('should update stack primary asset if deleted asset was primary asset in a stack', async () => {
      when(assetMock.getById)
        .calledWith(assetStub.primaryImage.id, {
          faces: {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
-        .mockResolvedValue(assetStub.primaryImage);
+        .mockResolvedValue(assetStub.primaryImage as AssetEntity);

      await sut.handleAssetDeletion({ id: assetStub.primaryImage.id });

-      expect(assetMock.updateAll).toHaveBeenCalledWith(['stack-child-asset-2'], {
-        stackParentId: 'stack-child-asset-1',
-      });
-      expect(assetMock.updateAll).toHaveBeenCalledWith(['stack-child-asset-1'], {
-        stackParentId: null,
+      expect(assetStackMock.update).toHaveBeenCalledWith({
+        id: 'stack-1',
+        primaryAssetId: 'stack-child-asset-1',
      });
    });

@@ -758,7 +819,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetStub.readOnly);
@@ -787,7 +848,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetStub.external);
@@ -819,7 +880,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetStub.livePhotoStillAsset);
@@ -829,7 +890,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetStub.livePhotoMotionAsset);
@@ -864,7 +925,7 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: { assets: true },
          exifInfo: true,
        })
        .mockResolvedValue(assetStub.image);
@@ -927,54 +988,21 @@ describe(AssetService.name, () => {
            person: true,
          },
          library: true,
-          stack: true,
+          stack: {
+            assets: true,
+          },
        })
-        .mockResolvedValue(assetStub.image as AssetEntity);
+        .mockResolvedValue({ ...assetStub.image, stackId: 'stack-1' });

      await sut.updateStackParent(authStub.user1, {
        oldParentId: assetStub.image.id,
        newParentId: 'new',
      });

-      expect(assetMock.updateAll).toBeCalledWith([assetStub.image.id], { stackParentId: 'new' });
-    });
-
-    it('remove stackParentId of new parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set([assetStub.primaryImage.id]));
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['new']));
-
-      await sut.updateStackParent(authStub.user1, {
-        oldParentId: assetStub.primaryImage.id,
-        newParentId: 'new',
+      expect(assetStackMock.update).toBeCalledWith({ id: 'stack-1', primaryAssetId: 'new' });
+      expect(assetMock.updateAll).toBeCalledWith([assetStub.image.id, 'new', assetStub.image.id], {
+        updatedAt: expect.any(Date),
      });
-
-      expect(assetMock.updateAll).toBeCalledWith(['new'], { stackParentId: null });
-    });
-
-    it('update stackParentId of old parents children to new parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set([assetStub.primaryImage.id]));
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['new']));
-      when(assetMock.getById)
-        .calledWith(assetStub.primaryImage.id, {
-          faces: {
-            person: true,
-          },
-          library: true,
-          stack: true,
-        })
-        .mockResolvedValue(assetStub.primaryImage as AssetEntity);
-
-      await sut.updateStackParent(authStub.user1, {
-        oldParentId: assetStub.primaryImage.id,
-        newParentId: 'new',
-      });
-
-      expect(assetMock.updateAll).toBeCalledWith(
-        [assetStub.primaryImage.id, 'stack-child-asset-1', 'stack-child-asset-2'],
-        {
-          stackParentId: 'new',
-        },
-      );
    });
  });

@@ -14,6 +14,7 @@ import {
  ClientEvent,
  IAccessRepository,
  IAssetRepository,
+  IAssetStackRepository,
  ICommunicationRepository,
  IJobRepository,
  IPartnerRepository,
@@ -85,6 +86,7 @@ export class AssetService {
    @Inject(IUserRepository) private userRepository: IUserRepository,
    @Inject(ICommunicationRepository) private communicationRepository: ICommunicationRepository,
    @Inject(IPartnerRepository) private partnerRepository: IPartnerRepository,
+    @Inject(IAssetStackRepository) private assetStackRepository: IAssetStackRepository,
  ) {
    this.access = AccessCore.create(accessRepository);
    this.configCore = SystemConfigCore.create(configRepository);
@@ -299,7 +301,9 @@ export class AssetService {
        person: true,
      },
      stack: {
-        exifInfo: true,
+        assets: {
+          exifInfo: true,
+        },
      },
    });

@@ -338,25 +342,51 @@ export class AssetService {
    const { ids, removeParent, dateTimeOriginal, latitude, longitude, ...options } = dto;
    await this.access.requirePermission(auth, Permission.ASSET_UPDATE, ids);

+    // TODO: refactor this logic into separate API calls POST /stack, PUT /stack, etc.
+    const stackIdsToCheckForDelete: string[] = [];
    if (removeParent) {
-      (options as Partial<AssetEntity>).stackParentId = null;
+      (options as Partial<AssetEntity>).stack = null;
      const assets = await this.assetRepository.getByIds(ids);
+      stackIdsToCheckForDelete.push(...new Set(assets.filter((a) => !!a.stackId).map((a) => a.stackId!)));
      // This updates the updatedAt column of the parents to indicate that one of its children is removed
      // All the unique parent's -> parent is set to null
-      ids.push(...new Set(assets.filter((a) => !!a.stackParentId).map((a) => a.stackParentId!)));
+      await this.assetRepository.updateAll(
+        assets.filter((a) => !!a.stack?.primaryAssetId).map((a) => a.stack!.primaryAssetId!),
+        { updatedAt: new Date() },
+      );
    } else if (options.stackParentId) {
+      //Creating new stack if parent doesn't have one already. If it does, then we add to the existing stack
      await this.access.requirePermission(auth, Permission.ASSET_UPDATE, options.stackParentId);
+      const primaryAsset = await this.assetRepository.getById(options.stackParentId, { stack: { assets: true } });
+      if (!primaryAsset) {
+        throw new BadRequestException('Asset not found for given stackParentId');
+      }
+      let stack = primaryAsset.stack;
+
+      ids.push(options.stackParentId);
+      const assets = await this.assetRepository.getByIds(ids, { stack: { assets: true } });
+      stackIdsToCheckForDelete.push(
+        ...new Set(assets.filter((a) => !!a.stackId && stack?.id !== a.stackId).map((a) => a.stackId!)),
+      );
+      const assetsWithChildren = assets.filter((a) => a.stack && a.stack.assets.length > 0);
+      ids.push(...assetsWithChildren.flatMap((child) => child.stack!.assets.map((gChild) => gChild.id)));
+
+      if (!stack) {
+        stack = await this.assetStackRepository.create({
+          primaryAssetId: primaryAsset.id,
+          assets: ids.map((id) => ({ id }) as AssetEntity),
+        });
+      } else {
+        await this.assetStackRepository.update({
+          id: stack.id,
+          primaryAssetId: primaryAsset.id,
+          assets: ids.map((id) => ({ id }) as AssetEntity),
+        });
+      }
+
      // Merge stacks
-      const assets = await this.assetRepository.getByIds(ids);
-      const assetsWithChildren = assets.filter((a) => a.stack && a.stack.length > 0);
-      ids.push(...assetsWithChildren.flatMap((child) => child.stack!.map((gChild) => gChild.id)));
-
-      // This updates the updatedAt column of the parent to indicate that a new child has been added
-      await this.assetRepository.updateAll([options.stackParentId], { stackParentId: null });
-    }
-
-    for (const id of ids) {
-      await this.updateMetadata({ id, dateTimeOriginal, latitude, longitude });
+      options.stackParentId = undefined;
+      (options as Partial<AssetEntity>).updatedAt = new Date();
    }

    for (const id of ids) {
@@ -364,6 +394,12 @@ export class AssetService {
    }

    await this.assetRepository.updateAll(ids, options);
+    const stacksToDelete = (
+      await Promise.all(stackIdsToCheckForDelete.map((id) => this.assetStackRepository.getById(id)))
+    )
+      .flatMap((stack) => (stack ? [stack] : []))
+      .filter((stack) => stack.assets.length < 2);
+    await Promise.all(stacksToDelete.map((as) => this.assetStackRepository.delete(as.id)));
    this.communicationRepository.send(ClientEvent.ASSET_UPDATE, auth.user.id, ids);
  }

@@ -394,7 +430,7 @@ export class AssetService {
        person: true,
      },
      library: true,
-      stack: true,
+      stack: { assets: true },
      exifInfo: true,
    });

@@ -408,11 +444,17 @@ export class AssetService {
    }

    // Replace the parent of the stack children with a new asset
-    if (asset.stack && asset.stack.length != 0) {
-      const stackIds = asset.stack.map((a) => a.id);
-      const newParentId = stackIds[0];
-      await this.assetRepository.updateAll(stackIds.slice(1), { stackParentId: newParentId });
-      await this.assetRepository.updateAll([newParentId], { stackParentId: null });
+    if (asset.stack?.primaryAssetId === id) {
+      const stackAssetIds = asset.stack.assets.map((a) => a.id);
+      if (stackAssetIds.length > 2) {
+        const newPrimaryAssetId = stackAssetIds.find((a) => a !== id)!;
+        await this.assetStackRepository.update({
+          id: asset.stack.id,
+          primaryAssetId: newPrimaryAssetId,
+        });
+      } else {
+        await this.assetStackRepository.delete(asset.stack.id);
+      }
    }

    await this.assetRepository.remove(asset);
@@ -460,18 +502,25 @@ export class AssetService {
        person: true,
      },
      library: true,
-      stack: true,
+      stack: {
+        assets: true,
+      },
    });
+    if (!oldParent?.stackId) {
+      throw new Error('Asset not found or not in a stack');
+    }
    if (oldParent != null) {
      childIds.push(oldParent.id);
      // Get all children of old parent
-      childIds.push(...(oldParent.stack?.map((a) => a.id) ?? []));
+      childIds.push(...(oldParent.stack?.assets.map((a) => a.id) ?? []));
    }
+    await this.assetStackRepository.update({
+      id: oldParent.stackId,
+      primaryAssetId: newParentId,
+    });

-    this.communicationRepository.send(ClientEvent.ASSET_UPDATE, auth.user.id, [...childIds, newParentId]);
-    await this.assetRepository.updateAll(childIds, { stackParentId: newParentId });
-    // Remove ParentId of new parent if this was previously a child of some other asset
-    return this.assetRepository.updateAll([newParentId], { stackParentId: null });
+    this.communicationRepository.send(ClientEvent.ASSET_UPDATE, auth.user.id, [...childIds, newParentId, oldParentId]);
+    await this.assetRepository.updateAll([oldParentId, newParentId, ...childIds], { updatedAt: new Date() });
  }

  async run(auth: AuthDto, dto: AssetJobsDto) {
@@ -116,9 +116,13 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As
    tags: entity.tags?.map(mapTag),
    people: peopleWithFaces(entity.faces),
    checksum: entity.checksum.toString('base64'),
-    stackParentId: entity.stackParentId,
-    stack: withStack ? entity.stack?.map((a) => mapAsset(a, { stripMetadata })) ?? undefined : undefined,
-    stackCount: entity.stack?.length ?? null,
+    stackParentId: withStack ? entity.stack?.primaryAssetId : undefined,
+    stack: withStack
+      ? entity.stack?.assets
+          .filter((a) => a.id !== entity.stack?.primaryAssetId)
+          .map((a) => mapAsset(a, { stripMetadata }))
+      : undefined,
+    stackCount: entity.stack?.assets?.length ?? null,
    isExternal: entity.isExternal,
    isOffline: entity.isOffline,
    isReadOnly: entity.isReadOnly,
@@ -499,6 +499,7 @@ describe(MetadataService.name, () => {
      expect(assetMock.upsertExif).toHaveBeenCalledWith({
        assetId: assetStub.image.id,
        bitsPerSample: expect.any(Number),
+        autoStackId: null,
        colorspace: tags.ColorSpace,
        dateTimeOriginal: new Date('1970-01-01'),
        description: tags.ImageDescription,
@@ -499,6 +499,7 @@ export class MetadataService {
      latitude: validate(tags.GPSLatitude),
      lensModel: tags.LensModel ?? null,
      livePhotoCID: (tags.ContentIdentifier || tags.MediaGroupUUID) ?? null,
+      autoStackId: this.getAutoStackId(tags),
      longitude: validate(tags.GPSLongitude),
      make: tags.Make ?? null,
      model: tags.Model ?? null,
@@ -518,6 +519,13 @@ export class MetadataService {
    return { exifData, tags };
  }

+  private getAutoStackId(tags: ImmichTags | null): string | null {
+    if (!tags) {
+      return null;
+    }
+    return tags.BurstID ?? tags.BurstUUID ?? tags.CameraBurstID ?? tags.MediaUniqueID ?? null;
+  }
+
  private getDateTimeOriginal(tags: ImmichTags | Tags | null) {
    if (!tags) {
      return null;
@@ -0,0 +1,10 @@
+import { AssetStackEntity } from '@app/infra/entities/asset-stack.entity';
+
+export const IAssetStackRepository = 'IAssetStackRepository';
+
+export interface IAssetStackRepository {
+  create(assetStack: Partial<AssetStackEntity>): Promise<AssetStackEntity>;
+  update(asset: Pick<AssetStackEntity, 'id'> & Partial<AssetStackEntity>): Promise<AssetStackEntity>;
+  delete(id: string): Promise<void>;
+  getById(id: string): Promise<AssetStackEntity | null>;
+}
@@ -2,6 +2,7 @@ export * from './access.repository';
 export * from './activity.repository';
 export * from './album.repository';
 export * from './api-key.repository';
+export * from './asset-stack.repository';
 export * from './asset.repository';
 export * from './audit.repository';
 export * from './communication.repository';