refactor(server): stacks (#11453)

* refactor: stacks

* mobile: get it built

* chore: feedback

* fix: sync and duplicates

* mobile: remove old stack reference

* chore: add primary asset id

* revert change to asset entity

* mobile: refactor mobile api

* mobile: sync stack info after creating stack

* mobile: update timeline after deleting stack

* server: update asset updatedAt when stack is deleted

* mobile: simplify action

* mobile: rename to match dto property

* fix: web test

---------

Co-authored-by: Alex <alex.tran1502@gmail.com>

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

@@ -4,7 +4,7 @@ import { AssetJobName, AssetStatsResponseDto } from 'src/dtos/asset.dto';
 import { AssetEntity } from 'src/entities/asset.entity';
 import { AssetType } from 'src/enum';
 import { AssetStats, IAssetRepository } from 'src/interfaces/asset.interface';
-import { ClientEvent, IEventRepository } from 'src/interfaces/event.interface';
+import { IEventRepository } from 'src/interfaces/event.interface';
 import { IJobRepository, JobName } from 'src/interfaces/job.interface';
 import { ILoggerRepository } from 'src/interfaces/logger.interface';
 import { IPartnerRepository } from 'src/interfaces/partner.interface';
@@ -12,7 +12,7 @@ import { IStackRepository } from 'src/interfaces/stack.interface';
 import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface';
 import { IUserRepository } from 'src/interfaces/user.interface';
 import { AssetService } from 'src/services/asset.service';
-import { assetStub, stackStub } from 'test/fixtures/asset.stub';
+import { assetStub } from 'test/fixtures/asset.stub';
 import { authStub } from 'test/fixtures/auth.stub';
 import { faceStub } from 'test/fixtures/face.stub';
 import { partnerStub } from 'test/fixtures/partner.stub';
@@ -253,134 +253,6 @@ describe(AssetService.name, () => {
       await sut.updateAll(authStub.admin, { ids: ['asset-1', 'asset-2'], isArchived: true });
       expect(assetMock.updateAll).toHaveBeenCalledWith(['asset-1', 'asset-2'], { isArchived: true });
     });
-
-    /// Stack related
-
-    it('should require asset update access for parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['asset-1']));
-      await expect(
-        sut.updateAll(authStub.user1, {
-          ids: ['asset-1'],
-          stackParentId: 'parent',
-        }),
-      ).rejects.toBeInstanceOf(BadRequestException);
-    });
-
-    it('should update parent asset updatedAt when children are added', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['parent']));
-      mockGetById([{ ...assetStub.image, id: 'parent' }]);
-      await sut.updateAll(authStub.user1, {
-        ids: [],
-        stackParentId: 'parent',
-      });
-      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',
-          stackId: 'stack-1',
-          stack: stackStub('stack-1', [{ id: 'parent' } as AssetEntity, { id: 'child-1' } as AssetEntity]),
-        } as AssetEntity,
-      ]);
-      stackMock.getById.mockResolvedValue(stackStub('stack-1', [{ id: 'parent' } as AssetEntity]));
-
-      await sut.updateAll(authStub.user1, {
-        ids: ['child-1'],
-        removeParent: true,
-      });
-      expect(assetMock.updateAll).toHaveBeenCalledWith(expect.arrayContaining(['child-1']), { stack: null });
-      expect(assetMock.updateAll).toHaveBeenCalledWith(expect.arrayContaining(['parent']), {
-        updatedAt: expect.any(Date),
-      });
-      expect(stackMock.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 = stackStub('stack-1', [
-        { id: 'parent' } as AssetEntity,
-        { id: 'child-1' } as AssetEntity,
-        { id: 'child-2' } as AssetEntity,
-      ]);
-      assetMock.getById.mockResolvedValue({
-        id: 'child-1',
-        stack,
-      } as AssetEntity);
-
-      await sut.updateAll(authStub.user1, {
-        stackParentId: 'parent',
-        ids: ['child-1', 'child-2'],
-      });
-
-      expect(stackMock.update).toHaveBeenCalledWith({
-        ...stackStub('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('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'], { 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']));
-      assetMock.getById.mockResolvedValue({ ...assetStub.image, id: 'parent' });
-      assetMock.getByIds.mockResolvedValue([
-        {
-          id: 'child-1',
-          stackId: 'stack-1',
-          stack: stackStub('stack-1', [{ id: 'child-1' } as AssetEntity, { id: 'child-2' } as AssetEntity]),
-        } as AssetEntity,
-      ]);
-      stackMock.getById.mockResolvedValue(stackStub('stack-1', [{ id: 'parent' } as AssetEntity]));
-
-      await sut.updateAll(authStub.user1, {
-        ids: ['child-1'],
-        stackParentId: 'parent',
-      });
-
-      expect(stackMock.delete).toHaveBeenCalledWith('stack-1');
-      expect(stackMock.create).toHaveBeenCalledWith({
-        assets: [{ id: 'child-1' }, { id: 'parent' }, { id: 'child-1' }, { id: 'child-2' }],
-        ownerId: 'user-id',
-        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']));
-      assetMock.getById.mockResolvedValue(assetStub.image);
-
-      await sut.updateAll(authStub.user1, {
-        ids: ['asset-1'],
-        stackParentId: 'parent',
-      });
-
-      expect(eventMock.clientSend).toHaveBeenCalledWith(ClientEvent.ASSET_STACK_UPDATE, authStub.user1.user.id, [
-        'asset-1',
-        'parent',
-      ]);
-    });
   });
 
   describe('deleteAll', () => {
@@ -530,53 +402,17 @@ describe(AssetService.name, () => {
     });
   });
 
-  describe('updateStackParent', () => {
-    it('should require asset update access for new parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['old']));
-      await expect(
-        sut.updateStackParent(authStub.user1, {
-          oldParentId: 'old',
-          newParentId: 'new',
-        }),
-      ).rejects.toBeInstanceOf(BadRequestException);
+  describe('getUserAssetsByDeviceId', () => {
+    it('get assets by device id', async () => {
+      const assets = [assetStub.image, assetStub.image1];
+
+      assetMock.getAllByDeviceId.mockResolvedValue(assets.map((asset) => asset.deviceAssetId));
+
+      const deviceId = 'device-id';
+      const result = await sut.getUserAssetsByDeviceId(authStub.user1, deviceId);
+
+      expect(result.length).toEqual(2);
+      expect(result).toEqual(assets.map((asset) => asset.deviceAssetId));
     });
-
-    it('should require asset read access for old parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set(['new']));
-      await expect(
-        sut.updateStackParent(authStub.user1, {
-          oldParentId: 'old',
-          newParentId: 'new',
-        }),
-      ).rejects.toBeInstanceOf(BadRequestException);
-    });
-
-    it('make old parent the child of new parent', async () => {
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set([assetStub.image.id]));
-      accessMock.asset.checkOwnerAccess.mockResolvedValueOnce(new Set(['new']));
-      assetMock.getById.mockResolvedValue({ ...assetStub.image, stackId: 'stack-1' });
-
-      await sut.updateStackParent(authStub.user1, {
-        oldParentId: assetStub.image.id,
-        newParentId: 'new',
-      });
-
-      expect(stackMock.update).toBeCalledWith({ id: 'stack-1', primaryAssetId: 'new' });
-      expect(assetMock.updateAll).toBeCalledWith([assetStub.image.id, 'new', assetStub.image.id], {
-        updatedAt: expect.any(Date),
-      });
-    });
-  });
-
-  it('get assets by device id', async () => {
-    const assets = [assetStub.image, assetStub.image1];
-
-    assetMock.getAllByDeviceId.mockResolvedValue(assets.map((asset) => asset.deviceAssetId));
-
-    const deviceId = 'device-id';
-    const result = await sut.getUserAssetsByDeviceId(authStub.user1, deviceId);
-
-    expect(result.length).toEqual(2);
-    expect(result).toEqual(assets.map((asset) => asset.deviceAssetId));
   });
 });

server/src/services/asset.service.ts

@@ -20,7 +20,6 @@ import {
 } from 'src/dtos/asset.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
 import { MemoryLaneDto } from 'src/dtos/search.dto';
-import { UpdateStackParentDto } from 'src/dtos/stack.dto';
 import { AssetEntity } from 'src/entities/asset.entity';
 import { Permission } from 'src/enum';
 import { IAccessRepository } from 'src/interfaces/access.interface';
@@ -179,68 +178,14 @@ export class AssetService {
   }
 
   async updateAll(auth: AuthDto, dto: AssetBulkUpdateDto): Promise<void> {
-    const { ids, removeParent, dateTimeOriginal, latitude, longitude, ...options } = dto;
+    const { ids, 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>).stack = null;
-      const assets = await this.assetRepository.getByIds(ids, { stack: true });
-      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
-      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) {
-        await this.stackRepository.update({
-          id: stack.id,
-          primaryAssetId: primaryAsset.id,
-          assets: ids.map((id) => ({ id }) as AssetEntity),
-        });
-      } else {
-        stack = await this.stackRepository.create({
-          primaryAssetId: primaryAsset.id,
-          ownerId: primaryAsset.ownerId,
-          assets: ids.map((id) => ({ id }) as AssetEntity),
-        });
-      }
-
-      // Merge stacks
-      options.stackParentId = undefined;
-      (options as Partial<AssetEntity>).updatedAt = new Date();
-    }
-
     for (const id of ids) {
       await this.updateMetadata({ id, dateTimeOriginal, latitude, longitude });
     }
 
     await this.assetRepository.updateAll(ids, options);
-    const stackIdsToDelete = await Promise.all(stackIdsToCheckForDelete.map((id) => this.stackRepository.getById(id)));
-    const stacksToDelete = stackIdsToDelete
-      .flatMap((stack) => (stack ? [stack] : []))
-      .filter((stack) => stack.assets.length < 2);
-    await Promise.all(stacksToDelete.map((as) => this.stackRepository.delete(as.id)));
-    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, ids);
   }
 
   async handleAssetDeletionCheck(): Promise<JobStatus> {
@@ -343,41 +288,6 @@ export class AssetService {
     }
   }
 
-  async updateStackParent(auth: AuthDto, dto: UpdateStackParentDto): Promise<void> {
-    const { oldParentId, newParentId } = dto;
-    await this.access.requirePermission(auth, Permission.ASSET_READ, oldParentId);
-    await this.access.requirePermission(auth, Permission.ASSET_UPDATE, newParentId);
-
-    const childIds: string[] = [];
-    const oldParent = await this.assetRepository.getById(oldParentId, {
-      faces: {
-        person: true,
-      },
-      library: true,
-      stack: {
-        assets: true,
-      },
-    });
-    if (!oldParent?.stackId) {
-      throw new Error('Asset not found or not in a stack');
-    }
-    if (oldParent != null) {
-      // Get all children of old parent
-      childIds.push(oldParent.id, ...(oldParent.stack?.assets.map((a) => a.id) ?? []));
-    }
-    await this.stackRepository.update({
-      id: oldParent.stackId,
-      primaryAssetId: newParentId,
-    });
-
-    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, [
-      ...childIds,
-      newParentId,
-      oldParentId,
-    ]);
-    await this.assetRepository.updateAll([oldParentId, newParentId, ...childIds], { updatedAt: new Date() });
-  }
-
   async run(auth: AuthDto, dto: AssetJobsDto) {
     await this.access.requirePermission(auth, Permission.ASSET_UPDATE, dto.assetIds);
 

server/src/services/duplicate.service.ts

@@ -39,7 +39,7 @@ export class DuplicateService {
   async getDuplicates(auth: AuthDto): Promise<DuplicateResponseDto[]> {
     const res = await this.assetRepository.getDuplicates({ userIds: [auth.user.id] });
 
-    return mapDuplicateResponse(res.map((a) => mapAsset(a, { auth })));
+    return mapDuplicateResponse(res.map((a) => mapAsset(a, { auth, withStack: true })));
   }
 
   async handleQueueSearchDuplicates({ force }: IBaseJob): Promise<JobStatus> {

server/src/services/index.ts

@@ -25,6 +25,7 @@ import { ServerService } from 'src/services/server.service';
 import { SessionService } from 'src/services/session.service';
 import { SharedLinkService } from 'src/services/shared-link.service';
 import { SmartInfoService } from 'src/services/smart-info.service';
+import { StackService } from 'src/services/stack.service';
 import { StorageTemplateService } from 'src/services/storage-template.service';
 import { StorageService } from 'src/services/storage.service';
 import { SyncService } from 'src/services/sync.service';
@@ -65,6 +66,7 @@ export const services = [
   SessionService,
   SharedLinkService,
   SmartInfoService,
+  StackService,
   StorageService,
   StorageTemplateService,
   SyncService,

server/src/services/stack.service.ts

@@ -0,0 +1,84 @@
+import { BadRequestException, Inject, Injectable } from '@nestjs/common';
+import { AccessCore } from 'src/cores/access.core';
+import { BulkIdsDto } from 'src/dtos/asset-ids.response.dto';
+import { AuthDto } from 'src/dtos/auth.dto';
+import { StackCreateDto, StackResponseDto, StackSearchDto, StackUpdateDto, mapStack } from 'src/dtos/stack.dto';
+import { Permission } from 'src/enum';
+import { IAccessRepository } from 'src/interfaces/access.interface';
+import { ClientEvent, IEventRepository } from 'src/interfaces/event.interface';
+import { IStackRepository } from 'src/interfaces/stack.interface';
+
+@Injectable()
+export class StackService {
+  private access: AccessCore;
+
+  constructor(
+    @Inject(IAccessRepository) accessRepository: IAccessRepository,
+    @Inject(IEventRepository) private eventRepository: IEventRepository,
+    @Inject(IStackRepository) private stackRepository: IStackRepository,
+  ) {
+    this.access = AccessCore.create(accessRepository);
+  }
+
+  async search(auth: AuthDto, dto: StackSearchDto): Promise<StackResponseDto[]> {
+    const stacks = await this.stackRepository.search({
+      ownerId: auth.user.id,
+      primaryAssetId: dto.primaryAssetId,
+    });
+
+    return stacks.map((stack) => mapStack(stack, { auth }));
+  }
+
+  async create(auth: AuthDto, dto: StackCreateDto): Promise<StackResponseDto> {
+    await this.access.requirePermission(auth, Permission.ASSET_UPDATE, dto.assetIds);
+
+    const stack = await this.stackRepository.create({ ownerId: auth.user.id, assetIds: dto.assetIds });
+
+    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, []);
+
+    return mapStack(stack, { auth });
+  }
+
+  async get(auth: AuthDto, id: string): Promise<StackResponseDto> {
+    await this.access.requirePermission(auth, Permission.STACK_READ, id);
+    const stack = await this.findOrFail(id);
+    return mapStack(stack, { auth });
+  }
+
+  async update(auth: AuthDto, id: string, dto: StackUpdateDto): Promise<StackResponseDto> {
+    await this.access.requirePermission(auth, Permission.STACK_UPDATE, id);
+    const stack = await this.findOrFail(id);
+    if (dto.primaryAssetId && !stack.assets.some(({ id }) => id === dto.primaryAssetId)) {
+      throw new BadRequestException('Primary asset must be in the stack');
+    }
+
+    const updatedStack = await this.stackRepository.update({ id, primaryAssetId: dto.primaryAssetId });
+
+    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, []);
+
+    return mapStack(updatedStack, { auth });
+  }
+
+  async delete(auth: AuthDto, id: string): Promise<void> {
+    await this.access.requirePermission(auth, Permission.STACK_DELETE, id);
+    await this.stackRepository.delete(id);
+
+    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, []);
+  }
+
+  async deleteAll(auth: AuthDto, dto: BulkIdsDto): Promise<void> {
+    await this.access.requirePermission(auth, Permission.STACK_DELETE, dto.ids);
+    await this.stackRepository.deleteAll(dto.ids);
+
+    this.eventRepository.clientSend(ClientEvent.ASSET_STACK_UPDATE, auth.user.id, []);
+  }
+
+  private async findOrFail(id: string) {
+    const stack = await this.stackRepository.getById(id);
+    if (!stack) {
+      throw new Error('Asset stack not found');
+    }
+
+    return stack;
+  }
+}