fix(server): user delete with stacked assets (#10642)

e2e/src/api/specs/user-admin.e2e-spec.ts

@@ -5,6 +5,7 @@ import {
  getUserAdmin,
  getUserPreferencesAdmin,
  login,
+ updateAssets,
 } from '@immich/sdk';
 import { Socket } from 'socket.io-client';
 import { createUserDto, uuidDto } from 'src/fixtures';
@@ -20,18 +21,16 @@ describe('/admin/users', () => {
  let nonAdmin: LoginResponseDto;
  let deletedUser: LoginResponseDto;
  let userToDelete: LoginResponseDto;
- let userToHardDelete: LoginResponseDto;
 
  beforeAll(async () => {
  await utils.resetDatabase();
  admin = await utils.adminSetup({ onboarding: false });
 
- [websocket, nonAdmin, deletedUser, userToDelete, userToHardDelete] = await Promise.all([
+ [websocket, nonAdmin, deletedUser, userToDelete] = await Promise.all([
  utils.connectWebsocket(admin.accessToken),
  utils.userSetup(admin.accessToken, createUserDto.user1),
  utils.userSetup(admin.accessToken, createUserDto.user2),
  utils.userSetup(admin.accessToken, createUserDto.user3),
- utils.userSetup(admin.accessToken, createUserDto.user4),
  ]);
 
  await deleteUserAdmin(
@@ -64,13 +63,12 @@ describe('/admin/users', () => {
  .get(`/admin/users`)
  .set('Authorization', `Bearer ${admin.accessToken}`);
  expect(status).toBe(200);
- expect(body).toHaveLength(4);
+ expect(body).toHaveLength(3);
  expect(body).toEqual(
  expect.arrayContaining([
  expect.objectContaining({ email: admin.userEmail }),
  expect.objectContaining({ email: nonAdmin.userEmail }),
  expect.objectContaining({ email: userToDelete.userEmail }),
- expect.objectContaining({ email: userToHardDelete.userEmail }),
  ]),
  );
  });
@@ -81,13 +79,12 @@ describe('/admin/users', () => {
  .set('Authorization', `Bearer ${admin.accessToken}`);
 
  expect(status).toBe(200);
- expect(body).toHaveLength(5);
+ expect(body).toHaveLength(4);
  expect(body).toEqual(
  expect.arrayContaining([
  expect.objectContaining({ email: admin.userEmail }),
  expect.objectContaining({ email: nonAdmin.userEmail }),
  expect.objectContaining({ email: userToDelete.userEmail }),
- expect.objectContaining({ email: userToHardDelete.userEmail }),
  expect.objectContaining({ email: deletedUser.userEmail }),
  ]),
  );
@@ -299,19 +296,49 @@ describe('/admin/users', () => {
  });
 
  it('should hard delete a user', async () => {
+ const user = await utils.userSetup(admin.accessToken, createUserDto.create('hard-delete-1'));
+
+ const { status, body } = await request(app)
+ .delete(`/admin/users/${user.userId}`)
+ .send({ force: true })
+ .set('Authorization', `Bearer ${admin.accessToken}`);
+
+ expect(status).toBe(200);
+ expect(body).toMatchObject({
+ id: user.userId,
+ updatedAt: expect.any(String),
+ deletedAt: expect.any(String),
+ });
+
+ await utils.waitForWebsocketEvent({ event: 'userDelete', id: user.userId, timeout: 5000 });
+ });
+
+ it('should hard delete a user with stacked assets', async () => {
+ const user = await utils.userSetup(admin.accessToken, createUserDto.create('hard-delete-1'));
+
+ const [asset1, asset2] = await Promise.all([
+ utils.createAsset(user.accessToken),
+ utils.createAsset(user.accessToken),
+ ]);
+
+ await updateAssets(
+ { assetBulkUpdateDto: { stackParentId: asset1.id, ids: [asset2.id] } },
+ { headers: asBearerAuth(user.accessToken) },
+ );
+
  const { status, body } = await request(app)
- .delete(`/admin/users/${userToHardDelete.userId}`)
+ .delete(`/admin/users/${user.userId}`)
  .send({ force: true })
  .set('Authorization', `Bearer ${admin.accessToken}`);
 
  expect(status).toBe(200);
  expect(body).toMatchObject({
- id: userToHardDelete.userId,
+ id: user.userId,
  updatedAt: expect.any(String),
  deletedAt: expect.any(String),
  });
 
- await utils.waitForWebsocketEvent({ event: 'userDelete', id: userToHardDelete.userId, timeout: 5000 });
+ await utils.waitForWebsocketEvent({ event: 'userDelete', id: user.userId, timeout: 5000 });
  });
  });
 

server/src/interfaces/asset-stack.interface.ts

@@ -7,4 +7,5 @@ export interface IAssetStackRepository {
  update(asset: Pick<AssetStackEntity, 'id'> & Partial<AssetStackEntity>): Promise<AssetStackEntity>;
  delete(id: string): Promise<void>;
  getById(id: string): Promise<AssetStackEntity | null>;
+ deleteAll(userId: string): Promise<void>;
 }

server/src/repositories/asset-stack.repository.ts

@@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';
 import { AssetStackEntity } from 'src/entities/asset-stack.entity';
 import { IAssetStackRepository } from 'src/interfaces/asset-stack.interface';
 import { Instrumentation } from 'src/utils/instrumentation';
-import { Repository } from 'typeorm';
+import { In, Repository } from 'typeorm';
 
 @Instrumentation()
 @Injectable()
@@ -34,6 +34,13 @@ export class AssetStackRepository implements IAssetStackRepository {
  });
  }
 
+ async deleteAll(userId: string): Promise<void> {
+ // TODO add owner to stack entity
+ const stacks = await this.repository.find({ where: { primaryAsset: { ownerId: userId } } });
+ const stackIds = new Set(stacks.map((stack) => stack.id));
+ await this.repository.delete({ id: In([...stackIds]) });
+ }
+
  private async save(entity: Partial<AssetStackEntity>) {
  const { id } = await this.repository.save(entity);
  return this.repository.findOneOrFail({

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

@@ -1,6 +1,7 @@
 import { BadRequestException, InternalServerErrorException, NotFoundException } from '@nestjs/common';
 import { UserEntity } from 'src/entities/user.entity';
 import { IAlbumRepository } from 'src/interfaces/album.interface';
+import { IAssetStackRepository } from 'src/interfaces/asset-stack.interface';
 import { ICryptoRepository } from 'src/interfaces/crypto.interface';
 import { IJobRepository, JobName } from 'src/interfaces/job.interface';
 import { ILoggerRepository } from 'src/interfaces/logger.interface';
@@ -13,6 +14,7 @@ import { authStub } from 'test/fixtures/auth.stub';
 import { systemConfigStub } from 'test/fixtures/system-config.stub';
 import { userStub } from 'test/fixtures/user.stub';
 import { newAlbumRepositoryMock } from 'test/repositories/album.repository.mock';
+import { newAssetStackRepositoryMock } from 'test/repositories/asset-stack.repository.mock';
 import { newCryptoRepositoryMock } from 'test/repositories/crypto.repository.mock';
 import { newJobRepositoryMock } from 'test/repositories/job.repository.mock';
 import { newLoggerRepositoryMock } from 'test/repositories/logger.repository.mock';
@@ -34,6 +36,7 @@ describe(UserService.name, () => {
 
  let albumMock: Mocked<IAlbumRepository>;
  let jobMock: Mocked<IJobRepository>;
+ let stackMock: Mocked<IAssetStackRepository>;
  let storageMock: Mocked<IStorageRepository>;
  let systemMock: Mocked<ISystemMetadataRepository>;
  let loggerMock: Mocked<ILoggerRepository>;
@@ -43,11 +46,21 @@ describe(UserService.name, () => {
  systemMock = newSystemMetadataRepositoryMock();
  cryptoRepositoryMock = newCryptoRepositoryMock();
  jobMock = newJobRepositoryMock();
+ stackMock = newAssetStackRepositoryMock();
  storageMock = newStorageRepositoryMock();
  userMock = newUserRepositoryMock();
  loggerMock = newLoggerRepositoryMock();
 
- sut = new UserService(albumMock, cryptoRepositoryMock, jobMock, storageMock, systemMock, userMock, loggerMock);
+ sut = new UserService(
+ albumMock,
+ cryptoRepositoryMock,
+ jobMock,
+ stackMock,
+ storageMock,
+ systemMock,
+ userMock,
+ loggerMock,
+ );
 
  userMock.get.mockImplementation((userId) =>
  Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? null),

server/src/services/user.service.ts

@@ -10,6 +10,7 @@ import { UserAdminResponseDto, UserResponseDto, UserUpdateMeDto, mapUser, mapUse
 import { UserMetadataKey } from 'src/entities/user-metadata.entity';
 import { UserEntity } from 'src/entities/user.entity';
 import { IAlbumRepository } from 'src/interfaces/album.interface';
+import { IAssetStackRepository } from 'src/interfaces/asset-stack.interface';
 import { ICryptoRepository } from 'src/interfaces/crypto.interface';
 import { IEntityJob, IJobRepository, JobName, JobStatus } from 'src/interfaces/job.interface';
 import { ILoggerRepository } from 'src/interfaces/logger.interface';
@@ -27,6 +28,7 @@ export class UserService {
  @Inject(IAlbumRepository) private albumRepository: IAlbumRepository,
  @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository,
  @Inject(IJobRepository) private jobRepository: IJobRepository,
+ @Inject(IAssetStackRepository) private stackRepository: IAssetStackRepository,
  @Inject(IStorageRepository) private storageRepository: IStorageRepository,
  @Inject(ISystemMetadataRepository) systemMetadataRepository: ISystemMetadataRepository,
  @Inject(IUserRepository) private userRepository: IUserRepository,
@@ -168,6 +170,7 @@ export class UserService {
  }
 
  this.logger.warn(`Removing user from database: ${user.id}`);
+ await this.stackRepository.deleteAll(user.id);
  await this.albumRepository.deleteAll(user.id);
  await this.userRepository.delete(user, true);
 

server/test/repositories/asset-stack.repository.mock.ts

@@ -7,5 +7,6 @@ export const newAssetStackRepositoryMock = (): Mocked<IAssetStackRepository> =>
  update: vitest.fn(),
  delete: vitest.fn(),
  getById: vitest.fn(),
+ deleteAll: vitest.fn(),
  };
 };