From e1f25b44d23ca6204b4987f1ae0f0bfa03cf66b3 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Fri, 5 Jul 2024 16:16:53 -0400 Subject: [PATCH] refactor(server): stack owner (#10900) --- e2e/src/utils.ts | 4 ---- server/src/dtos/user.dto.ts | 2 +- server/src/entities/stack.entity.ts | 9 +++++++- server/src/interfaces/stack.interface.ts | 3 +-- .../migrations/1720207981949-AddStackOwner.ts | 22 +++++++++++++++++++ server/src/queries/asset.repository.sql | 6 +++++ server/src/queries/search.repository.sql | 2 ++ server/src/repositories/stack.repository.ts | 9 +------- server/src/services/asset.service.spec.ts | 1 + server/src/services/asset.service.ts | 1 + server/src/services/user.service.spec.ts | 15 +------------ server/src/services/user.service.ts | 3 --- server/test/fixtures/asset.stub.ts | 2 ++ .../repositories/stack.repository.mock.ts | 1 - 14 files changed, 46 insertions(+), 34 deletions(-) create mode 100644 server/src/migrations/1720207981949-AddStackOwner.ts diff --git a/e2e/src/utils.ts b/e2e/src/utils.ts index 3459e51376..e018e87b59 100644 --- a/e2e/src/utils.ts +++ b/e2e/src/utils.ts @@ -152,10 +152,6 @@ export const utils = { const sql: string[] = []; - if (tables.includes('asset_stack')) { - sql.push('UPDATE "assets" SET "stackId" = NULL;'); - } - for (const table of tables) { if (table === 'system_metadata') { // prevent reverse geocoder from being re-initialized diff --git a/server/src/dtos/user.dto.ts b/server/src/dtos/user.dto.ts index 11767ed4a6..ce9ff5c99f 100644 --- a/server/src/dtos/user.dto.ts +++ b/server/src/dtos/user.dto.ts @@ -4,7 +4,7 @@ import { IsBoolean, IsEmail, IsNotEmpty, IsNumber, IsPositive, IsString } from ' import { UserAvatarColor, UserMetadataEntity, UserMetadataKey } from 'src/entities/user-metadata.entity'; import { UserEntity, UserStatus } from 'src/entities/user.entity'; import { getPreferences } from 'src/utils/preferences'; -import { Optional, toEmail, toSanitized, ValidateBoolean } from 'src/validation'; +import { Optional, ValidateBoolean, toEmail, toSanitized } from 'src/validation'; export class UserUpdateMeDto { @Optional() diff --git a/server/src/entities/stack.entity.ts b/server/src/entities/stack.entity.ts index dac75b6362..883f5cf246 100644 --- a/server/src/entities/stack.entity.ts +++ b/server/src/entities/stack.entity.ts @@ -1,11 +1,18 @@ import { AssetEntity } from 'src/entities/asset.entity'; -import { Column, Entity, JoinColumn, OneToMany, OneToOne, PrimaryGeneratedColumn } from 'typeorm'; +import { UserEntity } from 'src/entities/user.entity'; +import { Column, Entity, JoinColumn, ManyToOne, OneToMany, OneToOne, PrimaryGeneratedColumn } from 'typeorm'; @Entity('asset_stack') export class StackEntity { @PrimaryGeneratedColumn('uuid') id!: string; + @ManyToOne(() => UserEntity, { onDelete: 'CASCADE', onUpdate: 'CASCADE' }) + owner!: UserEntity; + + @Column() + ownerId!: string; + @OneToMany(() => AssetEntity, (asset) => asset.stack) assets!: AssetEntity[]; diff --git a/server/src/interfaces/stack.interface.ts b/server/src/interfaces/stack.interface.ts index c06d7cd5ec..0e6baf0a34 100644 --- a/server/src/interfaces/stack.interface.ts +++ b/server/src/interfaces/stack.interface.ts @@ -3,9 +3,8 @@ import { StackEntity } from 'src/entities/stack.entity'; export const IStackRepository = 'IStackRepository'; export interface IStackRepository { - create(stack: Partial): Promise; + create(stack: Partial & { ownerId: string }): Promise; update(stack: Pick & Partial): Promise; delete(id: string): Promise; getById(id: string): Promise; - deleteAll(userId: string): Promise; } diff --git a/server/src/migrations/1720207981949-AddStackOwner.ts b/server/src/migrations/1720207981949-AddStackOwner.ts new file mode 100644 index 0000000000..61394cc985 --- /dev/null +++ b/server/src/migrations/1720207981949-AddStackOwner.ts @@ -0,0 +1,22 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class AddStackOwner1720207981949 implements MigrationInterface { + name = 'AddStackOwner1720207981949' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "asset_stack" ADD "ownerId" uuid`); + await queryRunner.query(` + UPDATE "asset_stack" stack + SET "ownerId" = asset."ownerId" + FROM "assets" asset + WHERE stack."primaryAssetId" = asset."id" + `) + await queryRunner.query('ALTER TABLE "asset_stack" ALTER COLUMN "ownerId" SET NOT NULL') + await queryRunner.query(`ALTER TABLE "asset_stack" ADD CONSTRAINT "FK_c05079e542fd74de3b5ecb5c1c8" FOREIGN KEY ("ownerId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "asset_stack" DROP CONSTRAINT "FK_c05079e542fd74de3b5ecb5c1c8"`); + await queryRunner.query(`ALTER TABLE "asset_stack" DROP COLUMN "ownerId"`); + } +} diff --git a/server/src/queries/asset.repository.sql b/server/src/queries/asset.repository.sql index 38c2209c2d..ba0707cfe7 100644 --- a/server/src/queries/asset.repository.sql +++ b/server/src/queries/asset.repository.sql @@ -205,6 +205,7 @@ SELECT "8258e303a73a72cf6abb13d73fb592dde0d68280"."faceAssetId" AS "8258e303a73a72cf6abb13d73fb592dde0d68280_faceAssetId", "8258e303a73a72cf6abb13d73fb592dde0d68280"."isHidden" AS "8258e303a73a72cf6abb13d73fb592dde0d68280_isHidden", "AssetEntity__AssetEntity_stack"."id" AS "AssetEntity__AssetEntity_stack_id", + "AssetEntity__AssetEntity_stack"."ownerId" AS "AssetEntity__AssetEntity_stack_ownerId", "AssetEntity__AssetEntity_stack"."primaryAssetId" AS "AssetEntity__AssetEntity_stack_primaryAssetId", "bd93d5747511a4dad4923546c51365bf1a803774"."id" AS "bd93d5747511a4dad4923546c51365bf1a803774_id", "bd93d5747511a4dad4923546c51365bf1a803774"."deviceAssetId" AS "bd93d5747511a4dad4923546c51365bf1a803774_deviceAssetId", @@ -629,6 +630,7 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId", "stackedAssets"."id" AS "stackedAssets_id", "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", @@ -769,6 +771,7 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId", "stackedAssets"."id" AS "stackedAssets_id", "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", @@ -885,6 +888,7 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId", "stackedAssets"."id" AS "stackedAssets_id", "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", @@ -1051,6 +1055,7 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId" FROM "assets" "asset" @@ -1126,6 +1131,7 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId" FROM "assets" "asset" diff --git a/server/src/queries/search.repository.sql b/server/src/queries/search.repository.sql index e058fc685c..58a288a0cd 100644 --- a/server/src/queries/search.repository.sql +++ b/server/src/queries/search.repository.sql @@ -37,6 +37,7 @@ FROM "asset"."stackId" AS "asset_stackId", "asset"."duplicateId" AS "asset_duplicateId", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId", "stackedAssets"."id" AS "stackedAssets_id", "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", @@ -133,6 +134,7 @@ SELECT "asset"."stackId" AS "asset_stackId", "asset"."duplicateId" AS "asset_duplicateId", "stack"."id" AS "stack_id", + "stack"."ownerId" AS "stack_ownerId", "stack"."primaryAssetId" AS "stack_primaryAssetId", "stackedAssets"."id" AS "stackedAssets_id", "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", diff --git a/server/src/repositories/stack.repository.ts b/server/src/repositories/stack.repository.ts index 4a99ba01e4..46cc14e713 100644 --- a/server/src/repositories/stack.repository.ts +++ b/server/src/repositories/stack.repository.ts @@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { StackEntity } from 'src/entities/stack.entity'; import { IStackRepository } from 'src/interfaces/stack.interface'; import { Instrumentation } from 'src/utils/instrumentation'; -import { In, Repository } from 'typeorm'; +import { Repository } from 'typeorm'; @Instrumentation() @Injectable() @@ -34,13 +34,6 @@ export class StackRepository implements IStackRepository { }); } - async deleteAll(userId: string): Promise { - // 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) { const { id } = await this.repository.save(entity); return this.repository.findOneOrFail({ diff --git a/server/src/services/asset.service.spec.ts b/server/src/services/asset.service.spec.ts index 8efa78edf3..5e5e555383 100755 --- a/server/src/services/asset.service.spec.ts +++ b/server/src/services/asset.service.spec.ts @@ -350,6 +350,7 @@ describe(AssetService.name, () => { 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'], { diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index 8a62dd89a7..5c6cce27d8 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -219,6 +219,7 @@ export class AssetService { } else { stack = await this.stackRepository.create({ primaryAssetId: primaryAsset.id, + ownerId: primaryAsset.ownerId, assets: ids.map((id) => ({ id }) as AssetEntity), }); } diff --git a/server/src/services/user.service.spec.ts b/server/src/services/user.service.spec.ts index 91cdfb30d0..d07e69df6e 100644 --- a/server/src/services/user.service.spec.ts +++ b/server/src/services/user.service.spec.ts @@ -5,7 +5,6 @@ import { IAlbumRepository } from 'src/interfaces/album.interface'; import { ICryptoRepository } from 'src/interfaces/crypto.interface'; import { IJobRepository, JobName } from 'src/interfaces/job.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; -import { IStackRepository } from 'src/interfaces/stack.interface'; import { IStorageRepository } from 'src/interfaces/storage.interface'; import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface'; import { IUserRepository } from 'src/interfaces/user.interface'; @@ -18,7 +17,6 @@ import { newAlbumRepositoryMock } from 'test/repositories/album.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'; -import { newStackRepositoryMock } from 'test/repositories/stack.repository.mock'; import { newStorageRepositoryMock } from 'test/repositories/storage.repository.mock'; import { newSystemMetadataRepositoryMock } from 'test/repositories/system-metadata.repository.mock'; import { newUserRepositoryMock } from 'test/repositories/user.repository.mock'; @@ -37,7 +35,6 @@ describe(UserService.name, () => { let albumMock: Mocked; let jobMock: Mocked; - let stackMock: Mocked; let storageMock: Mocked; let systemMock: Mocked; let loggerMock: Mocked; @@ -47,21 +44,11 @@ describe(UserService.name, () => { systemMock = newSystemMetadataRepositoryMock(); cryptoRepositoryMock = newCryptoRepositoryMock(); jobMock = newJobRepositoryMock(); - stackMock = newStackRepositoryMock(); storageMock = newStorageRepositoryMock(); userMock = newUserRepositoryMock(); loggerMock = newLoggerRepositoryMock(); - sut = new UserService( - albumMock, - cryptoRepositoryMock, - jobMock, - stackMock, - storageMock, - systemMock, - userMock, - loggerMock, - ); + sut = new UserService(albumMock, cryptoRepositoryMock, jobMock, storageMock, systemMock, userMock, loggerMock); userMock.get.mockImplementation((userId) => Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? null), diff --git a/server/src/services/user.service.ts b/server/src/services/user.service.ts index b92e2822df..641f75e9e1 100644 --- a/server/src/services/user.service.ts +++ b/server/src/services/user.service.ts @@ -15,7 +15,6 @@ import { IAlbumRepository } from 'src/interfaces/album.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'; -import { IStackRepository } from 'src/interfaces/stack.interface'; import { IStorageRepository } from 'src/interfaces/storage.interface'; import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface'; import { IUserRepository, UserFindOptions } from 'src/interfaces/user.interface'; @@ -30,7 +29,6 @@ export class UserService { @Inject(IAlbumRepository) private albumRepository: IAlbumRepository, @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository, @Inject(IJobRepository) private jobRepository: IJobRepository, - @Inject(IStackRepository) private stackRepository: IStackRepository, @Inject(IStorageRepository) private storageRepository: IStorageRepository, @Inject(ISystemMetadataRepository) systemMetadataRepository: ISystemMetadataRepository, @Inject(IUserRepository) private userRepository: IUserRepository, @@ -213,7 +211,6 @@ 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); diff --git a/server/test/fixtures/asset.stub.ts b/server/test/fixtures/asset.stub.ts index 290dd1623f..aa141a9964 100644 --- a/server/test/fixtures/asset.stub.ts +++ b/server/test/fixtures/asset.stub.ts @@ -10,6 +10,8 @@ export const stackStub = (stackId: string, assets: AssetEntity[]): StackEntity = return { id: stackId, assets: assets, + owner: assets[0].owner, + ownerId: assets[0].ownerId, primaryAsset: assets[0], primaryAssetId: assets[0].id, }; diff --git a/server/test/repositories/stack.repository.mock.ts b/server/test/repositories/stack.repository.mock.ts index 374afd1fbd..5567d2e1ac 100644 --- a/server/test/repositories/stack.repository.mock.ts +++ b/server/test/repositories/stack.repository.mock.ts @@ -7,6 +7,5 @@ export const newStackRepositoryMock = (): Mocked => { update: vitest.fn(), delete: vitest.fn(), getById: vitest.fn(), - deleteAll: vitest.fn(), }; };