feat(server): improve thumbnail relation and updating (#1897)

* feat(server): improve thumbnail relation and updating

* improve query + update tests and migration

* make sure uuids are valid in migration

* fix unit test
This commit is contained in:
Michel Heusschen 2023-03-04 15:16:48 +01:00 committed by GitHub
parent 2ac54ce4bd
commit bdf35b6688
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 138 additions and 60 deletions

View file

@ -1,4 +1,4 @@
import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra';
import { AlbumEntity, AssetEntity, dataSource, UserEntity } from '@app/infra';
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository, Not, IsNull, FindManyOptions } from 'typeorm';
@ -22,6 +22,7 @@ export interface IAlbumRepository {
removeAssets(album: AlbumEntity, removeAssets: RemoveAssetsDto): Promise<number>;
addAssets(album: AlbumEntity, addAssetsDto: AddAssetsDto): Promise<AddAssetsResponseDto>;
updateAlbum(album: AlbumEntity, updateAlbumDto: UpdateAlbumDto): Promise<AlbumEntity>;
updateThumbnails(): Promise<number | undefined>;
getListByAssetId(userId: string, assetId: string): Promise<AlbumEntity[]>;
getCountByUserId(userId: string): Promise<AlbumCountResponseDto>;
getSharedWithUserAlbumCount(userId: string, assetId: string): Promise<number>;
@ -34,6 +35,9 @@ export class AlbumRepository implements IAlbumRepository {
constructor(
@InjectRepository(AlbumEntity)
private albumRepository: Repository<AlbumEntity>,
@InjectRepository(AssetEntity)
private assetRepository: Repository<AssetEntity>,
) {}
async getPublicSharingList(ownerId: string): Promise<AlbumEntity[]> {
@ -208,6 +212,48 @@ export class AlbumRepository implements IAlbumRepository {
return this.albumRepository.save(album);
}
/**
* Makes sure all thumbnails for albums are updated by:
* - Removing thumbnails from albums without assets
* - Removing references of thumbnails to assets outside the album
* - Setting a thumbnail when none is set and the album contains assets
*
* @returns Amount of updated album thumbnails or undefined when unknown
*/
async updateThumbnails(): Promise<number | undefined> {
// Subquery for getting a new thumbnail.
const newThumbnail = this.assetRepository
.createQueryBuilder('assets')
.select('albums_assets2.assetsId')
.addFrom('albums_assets_assets', 'albums_assets2')
.where('albums_assets2.assetsId = assets.id')
.andWhere('albums_assets2.albumsId = "albums"."id"') // Reference to albums.id outside this query
.orderBy('assets.fileCreatedAt', 'DESC')
.limit(1);
// Using dataSource, because there is no direct access to albums_assets_assets.
const albumHasAssets = dataSource
.createQueryBuilder()
.select('1')
.from('albums_assets_assets', 'albums_assets')
.where('"albums"."id" = "albums_assets"."albumsId"');
const albumContainsThumbnail = albumHasAssets
.clone()
.andWhere('"albums"."albumThumbnailAssetId" = "albums_assets"."assetsId"');
const updateAlbums = this.albumRepository
.createQueryBuilder('albums')
.update(AlbumEntity)
.set({ albumThumbnailAssetId: () => `(${newThumbnail.getQuery()})` })
.where(`"albums"."albumThumbnailAssetId" IS NULL AND EXISTS (${albumHasAssets.getQuery()})`)
.orWhere(`"albums"."albumThumbnailAssetId" IS NOT NULL AND NOT EXISTS (${albumContainsThumbnail.getQuery()})`);
const result = await updateAlbums.execute();
return result.affected;
}
async getSharedWithUserAlbumCount(userId: string, assetId: string): Promise<number> {
return this.albumRepository.count({
where: [

View file

@ -2,7 +2,7 @@ import { Module } from '@nestjs/common';
import { AlbumService } from './album.service';
import { AlbumController } from './album.controller';
import { TypeOrmModule } from '@nestjs/typeorm';
import { AlbumEntity } from '@app/infra';
import { AlbumEntity, AssetEntity } from '@app/infra';
import { AlbumRepository, IAlbumRepository } from './album-repository';
import { DownloadModule } from '../../modules/download/download.module';
@ -12,7 +12,7 @@ const ALBUM_REPOSITORY_PROVIDER = {
};
@Module({
imports: [TypeOrmModule.forFeature([AlbumEntity]), DownloadModule],
imports: [TypeOrmModule.forFeature([AlbumEntity, AssetEntity]), DownloadModule],
controllers: [AlbumController],
providers: [AlbumService, ALBUM_REPOSITORY_PROVIDER],
exports: [ALBUM_REPOSITORY_PROVIDER],

View file

@ -129,6 +129,7 @@ describe('Album service', () => {
removeAssets: jest.fn(),
removeUser: jest.fn(),
updateAlbum: jest.fn(),
updateThumbnails: jest.fn(),
getListByAssetId: jest.fn(),
getCountByUserId: jest.fn(),
getSharedWithUserAlbumCount: jest.fn(),
@ -502,59 +503,47 @@ describe('Album service', () => {
it('updates the album thumbnail by listing all albums', async () => {
const albumEntity = _getOwnedAlbum();
const assetEntity = new AssetEntity();
const newThumbnailAssetId = 'e5e65c02-b889-4f3c-afe1-a39a96d578ed';
const newThumbnailAsset = new AssetEntity();
newThumbnailAsset.id = 'e5e65c02-b889-4f3c-afe1-a39a96d578ed';
albumEntity.albumThumbnailAssetId = 'nonexistent';
assetEntity.id = newThumbnailAssetId;
assetEntity.id = newThumbnailAsset.id;
albumEntity.assets = [assetEntity];
albumRepositoryMock.getList.mockImplementation(async () => [albumEntity]);
albumRepositoryMock.updateAlbum.mockImplementation(async () => ({
...albumEntity,
albumThumbnailAssetId: newThumbnailAssetId,
}));
albumRepositoryMock.updateThumbnails.mockImplementation(async () => {
albumEntity.albumThumbnailAsset = newThumbnailAsset;
albumEntity.albumThumbnailAssetId = newThumbnailAsset.id;
return 1;
});
const result = await sut.getAllAlbums(authUser, {});
expect(result).toHaveLength(1);
expect(result[0].albumThumbnailAssetId).toEqual(newThumbnailAssetId);
expect(result[0].albumThumbnailAssetId).toEqual(newThumbnailAsset.id);
expect(albumRepositoryMock.getList).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.updateThumbnails).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.getList).toHaveBeenCalledWith(albumEntity.ownerId, {});
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledWith(albumEntity, {
albumThumbnailAssetId: newThumbnailAssetId,
});
});
it('removes the thumbnail for an empty album', async () => {
const albumEntity = _getOwnedAlbum();
const newAlbumEntity = { ...albumEntity, albumThumbnailAssetId: null };
albumEntity.albumThumbnailAssetId = 'e5e65c02-b889-4f3c-afe1-a39a96d578ed';
albumRepositoryMock.getList.mockImplementation(async () => [albumEntity]);
albumRepositoryMock.updateAlbum.mockImplementation(async () => newAlbumEntity);
albumRepositoryMock.updateThumbnails.mockImplementation(async () => {
albumEntity.albumThumbnailAsset = null;
albumEntity.albumThumbnailAssetId = null;
return 1;
});
const result = await sut.getAllAlbums(authUser, {});
expect(result).toHaveLength(1);
expect(result[0].albumThumbnailAssetId).toBeNull();
expect(albumRepositoryMock.getList).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.getList).toHaveBeenCalledWith(albumEntity.ownerId, {});
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledWith(newAlbumEntity, {
albumThumbnailAssetId: undefined,
});
});
it('listing empty albums does not unnecessarily update the album', async () => {
const albumEntity = _getOwnedAlbum();
albumRepositoryMock.getList.mockImplementation(async () => [albumEntity]);
albumRepositoryMock.updateAlbum.mockImplementation(async () => albumEntity);
const result = await sut.getAllAlbums(authUser, {});
expect(result).toHaveLength(1);
expect(albumRepositoryMock.getList).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.updateAlbum).toHaveBeenCalledTimes(0);
expect(albumRepositoryMock.updateThumbnails).toHaveBeenCalledTimes(1);
expect(albumRepositoryMock.getList).toHaveBeenCalledWith(albumEntity.ownerId, {});
});
});

View file

@ -41,6 +41,8 @@ export class AlbumService {
albumId: string;
validateIsOwner?: boolean;
}): Promise<AlbumEntity> {
await this.albumRepository.updateThumbnails();
const album = await this.albumRepository.get(albumId);
if (!album) {
throw new NotFoundException('Album Not Found');
@ -67,6 +69,8 @@ export class AlbumService {
* @returns All Shared Album And Its Members
*/
async getAllAlbums(authUser: AuthUserDto, getAlbumsDto: GetAlbumsDto): Promise<AlbumResponseDto[]> {
await this.albumRepository.updateThumbnails();
let albums: AlbumEntity[];
if (typeof getAlbumsDto.assetId === 'string') {
albums = await this.albumRepository.getListByAssetId(authUser.id, getAlbumsDto.assetId);
@ -81,10 +85,6 @@ export class AlbumService {
albums = _.uniqBy(albums, (album) => album.id);
for (const album of albums) {
await this._checkValidThumbnail(album);
}
return albums.map((album) => mapAlbumExcludeAssetInfo(album));
}
@ -131,10 +131,6 @@ export class AlbumService {
const deletedCount = await this.albumRepository.removeAssets(album, removeAssetsDto);
const newAlbum = await this._getAlbum({ authUser, albumId });
if (newAlbum) {
await this._checkValidThumbnail(newAlbum);
}
if (deletedCount !== removeAssetsDto.assetIds.length) {
throw new BadRequestException('Some assets were not found in the album');
}
@ -191,24 +187,6 @@ export class AlbumService {
return this.downloadService.downloadArchive(album.albumName, assets);
}
async _checkValidThumbnail(album: AlbumEntity) {
const assets = album.assets || [];
// Check if the album's thumbnail is invalid by referencing
// an asset outside the album.
const invalid = assets.length > 0 && !assets.some((asset) => asset.id === album.albumThumbnailAssetId);
// Check if an empty album still has a thumbnail.
const isEmptyWithThumbnail = assets.length === 0 && album.albumThumbnailAssetId !== null;
if (invalid || isEmptyWithThumbnail) {
const albumThumbnailAssetId = assets[0]?.id;
album.albumThumbnailAssetId = albumThumbnailAssetId || null;
await this.albumRepository.updateAlbum(album, { albumThumbnailAssetId });
}
}
async createAlbumSharedLink(authUser: AuthUserDto, dto: CreateAlbumShareLinkDto): Promise<SharedLinkResponseDto> {
const album = await this._getAlbum({ authUser, albumId: dto.albumId });

View file

@ -163,6 +163,7 @@ export const albumStub = {
ownerId: authStub.admin.id,
owner: userEntityStub.admin,
assets: [],
albumThumbnailAsset: null,
albumThumbnailAssetId: null,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
@ -422,6 +423,7 @@ export const sharedLinkStub = {
albumName: 'Test Album',
createdAt: today.toISOString(),
updatedAt: today.toISOString(),
albumThumbnailAsset: null,
albumThumbnailAssetId: null,
sharedUsers: [],
sharedLinks: [],

View file

@ -33,7 +33,10 @@ export class AlbumEntity {
@UpdateDateColumn({ type: 'timestamptz' })
updatedAt!: string;
@Column({ comment: 'Asset ID to be used as thumbnail', type: 'varchar', nullable: true })
@ManyToOne(() => AssetEntity, { nullable: true, onDelete: 'SET NULL', onUpdate: 'CASCADE' })
albumThumbnailAsset!: AssetEntity | null;
@Column({ comment: 'Asset ID to be used as thumbnail', nullable: true })
albumThumbnailAssetId!: string | null;
@ManyToMany(() => UserEntity)

View file

@ -0,0 +1,60 @@
import { MigrationInterface, QueryRunner } from 'typeorm';
export class AlbumThumbnailRelation1677613712565 implements MigrationInterface {
name = 'AlbumThumbnailRelation1677613712565';
public async up(queryRunner: QueryRunner): Promise<void> {
// Make sure all albums have a valid albumThumbnailAssetId UUID or NULL.
await queryRunner.query(`
UPDATE "albums"
SET
"albumThumbnailAssetId" = (
SELECT
"albums_assets2"."assetsId"
FROM
"assets" "assets",
"albums_assets_assets" "albums_assets2"
WHERE
"albums_assets2"."assetsId" = "assets"."id"
AND "albums_assets2"."albumsId" = "albums"."id"
ORDER BY
"assets"."fileCreatedAt" DESC
LIMIT 1
),
"updatedAt" = CURRENT_TIMESTAMP
WHERE
"albums"."albumThumbnailAssetId" IS NULL
AND EXISTS (
SELECT 1
FROM "albums_assets_assets" "albums_assets"
WHERE "albums"."id" = "albums_assets"."albumsId"
)
OR "albums"."albumThumbnailAssetId" IS NOT NULL
AND NOT EXISTS (
SELECT 1
FROM "albums_assets_assets" "albums_assets"
WHERE
"albums"."id" = "albums_assets"."albumsId"
AND "albums"."albumThumbnailAssetId" = "albums_assets"."assetsId"::varchar
)
`);
await queryRunner.query(`
ALTER TABLE "albums"
ALTER COLUMN "albumThumbnailAssetId"
TYPE uuid USING "albumThumbnailAssetId"::uuid
`);
await queryRunner.query(`
ALTER TABLE "albums" ADD CONSTRAINT "FK_05895aa505a670300d4816debce" FOREIGN KEY ("albumThumbnailAssetId") REFERENCES "assets"("id") ON DELETE SET NULL ON UPDATE CASCADE
`);
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`ALTER TABLE "albums" DROP CONSTRAINT "FK_05895aa505a670300d4816debce"`);
await queryRunner.query(`
ALTER TABLE "albums" ALTER COLUMN "albumThumbnailAssetId" TYPE varchar USING "albumThumbnailAssetId"::varchar
`);
}
}