fix(web,server): album share performance (#3698)

This commit is contained in:
Jason Rasmussen 2023-08-15 14:34:02 -04:00 committed by GitHub
parent af1f00dff9
commit 0abbd85134
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 47 additions and 40 deletions

View File

@ -7,8 +7,12 @@ export interface AlbumAssetCount {
assetCount: number; assetCount: number;
} }
export interface AlbumInfoOptions {
withAssets: boolean;
}
export interface IAlbumRepository { export interface IAlbumRepository {
getById(id: string): Promise<AlbumEntity | null>; getById(id: string, options: AlbumInfoOptions): Promise<AlbumEntity | null>;
getByIds(ids: string[]): Promise<AlbumEntity[]>; getByIds(ids: string[]): Promise<AlbumEntity[]>;
getByAssetId(ownerId: string, assetId: string): Promise<AlbumEntity[]>; getByAssetId(ownerId: string, assetId: string): Promise<AlbumEntity[]>;
hasAsset(id: string, assetId: string): Promise<boolean>; hasAsset(id: string, assetId: string): Promise<boolean>;

View File

@ -364,6 +364,7 @@ describe(AlbumService.name, () => {
updatedAt: expect.any(Date), updatedAt: expect.any(Date),
sharedUsers: [], sharedUsers: [],
}); });
expect(albumMock.getById).toHaveBeenCalledWith(albumStub.sharedWithUser.id, { withAssets: false });
}); });
it('should prevent removing a shared user from a not-owned album (shared with auth user)', async () => { it('should prevent removing a shared user from a not-owned album (shared with auth user)', async () => {
@ -432,7 +433,7 @@ describe(AlbumService.name, () => {
await sut.get(authStub.admin, albumStub.oneAsset.id, {}); await sut.get(authStub.admin, albumStub.oneAsset.id, {});
expect(albumMock.getById).toHaveBeenCalledWith(albumStub.oneAsset.id); expect(albumMock.getById).toHaveBeenCalledWith(albumStub.oneAsset.id, { withAssets: true });
expect(accessMock.album.hasOwnerAccess).toHaveBeenCalledWith(authStub.admin.id, albumStub.oneAsset.id); expect(accessMock.album.hasOwnerAccess).toHaveBeenCalledWith(authStub.admin.id, albumStub.oneAsset.id);
}); });
@ -442,7 +443,7 @@ describe(AlbumService.name, () => {
await sut.get(authStub.adminSharedLink, 'album-123', {}); await sut.get(authStub.adminSharedLink, 'album-123', {});
expect(albumMock.getById).toHaveBeenCalledWith('album-123'); expect(albumMock.getById).toHaveBeenCalledWith('album-123', { withAssets: true });
expect(accessMock.album.hasSharedLinkAccess).toHaveBeenCalledWith( expect(accessMock.album.hasSharedLinkAccess).toHaveBeenCalledWith(
authStub.adminSharedLink.sharedLinkId, authStub.adminSharedLink.sharedLinkId,
'album-123', 'album-123',
@ -455,7 +456,7 @@ describe(AlbumService.name, () => {
await sut.get(authStub.user1, 'album-123', {}); await sut.get(authStub.user1, 'album-123', {});
expect(albumMock.getById).toHaveBeenCalledWith('album-123'); expect(albumMock.getById).toHaveBeenCalledWith('album-123', { withAssets: true });
expect(accessMock.album.hasSharedAlbumAccess).toHaveBeenCalledWith(authStub.user1.id, 'album-123'); expect(accessMock.album.hasSharedAlbumAccess).toHaveBeenCalledWith(authStub.user1.id, 'album-123');
}); });

View File

@ -12,7 +12,7 @@ import {
mapAlbumWithAssets, mapAlbumWithAssets,
mapAlbumWithoutAssets, mapAlbumWithoutAssets,
} from './album-response.dto'; } from './album-response.dto';
import { IAlbumRepository } from './album.repository'; import { AlbumInfoOptions, IAlbumRepository } from './album.repository';
import { AddUsersDto, AlbumInfoDto, CreateAlbumDto, GetAlbumsDto, UpdateAlbumDto } from './dto'; import { AddUsersDto, AlbumInfoDto, CreateAlbumDto, GetAlbumsDto, UpdateAlbumDto } from './dto';
@Injectable() @Injectable()
@ -84,7 +84,7 @@ export class AlbumService {
async get(authUser: AuthUserDto, id: string, dto: AlbumInfoDto) { async get(authUser: AuthUserDto, id: string, dto: AlbumInfoDto) {
await this.access.requirePermission(authUser, Permission.ALBUM_READ, id); await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
await this.albumRepository.updateThumbnails(); await this.albumRepository.updateThumbnails();
return mapAlbum(await this.findOrFail(id), !dto.withoutAssets); return mapAlbum(await this.findOrFail(id, { withAssets: true }), !dto.withoutAssets);
} }
async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> { async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
@ -111,7 +111,7 @@ export class AlbumService {
async update(authUser: AuthUserDto, id: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> { async update(authUser: AuthUserDto, id: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> {
await this.access.requirePermission(authUser, Permission.ALBUM_UPDATE, id); await this.access.requirePermission(authUser, Permission.ALBUM_UPDATE, id);
const album = await this.findOrFail(id); const album = await this.findOrFail(id, { withAssets: true });
if (dto.albumThumbnailAssetId) { if (dto.albumThumbnailAssetId) {
const valid = await this.albumRepository.hasAsset(id, dto.albumThumbnailAssetId); const valid = await this.albumRepository.hasAsset(id, dto.albumThumbnailAssetId);
@ -129,13 +129,13 @@ export class AlbumService {
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [updatedAlbum.id] } }); await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ALBUM, data: { ids: [updatedAlbum.id] } });
return mapAlbumWithAssets(updatedAlbum); return mapAlbumWithoutAssets(updatedAlbum);
} }
async delete(authUser: AuthUserDto, id: string): Promise<void> { async delete(authUser: AuthUserDto, id: string): Promise<void> {
await this.access.requirePermission(authUser, Permission.ALBUM_DELETE, id); await this.access.requirePermission(authUser, Permission.ALBUM_DELETE, id);
const album = await this.albumRepository.getById(id); const album = await this.findOrFail(id, { withAssets: false });
if (!album) { if (!album) {
throw new BadRequestException('Album not found'); throw new BadRequestException('Album not found');
} }
@ -145,7 +145,7 @@ export class AlbumService {
} }
async addAssets(authUser: AuthUserDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> { async addAssets(authUser: AuthUserDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> {
const album = await this.findOrFail(id); const album = await this.findOrFail(id, { withAssets: true });
await this.access.requirePermission(authUser, Permission.ALBUM_READ, id); await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
@ -181,7 +181,7 @@ export class AlbumService {
} }
async removeAssets(authUser: AuthUserDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> { async removeAssets(authUser: AuthUserDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> {
const album = await this.findOrFail(id); const album = await this.findOrFail(id, { withAssets: true });
await this.access.requirePermission(authUser, Permission.ALBUM_READ, id); await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
@ -225,7 +225,7 @@ export class AlbumService {
async addUsers(authUser: AuthUserDto, id: string, dto: AddUsersDto): Promise<AlbumResponseDto> { async addUsers(authUser: AuthUserDto, id: string, dto: AddUsersDto): Promise<AlbumResponseDto> {
await this.access.requirePermission(authUser, Permission.ALBUM_SHARE, id); await this.access.requirePermission(authUser, Permission.ALBUM_SHARE, id);
const album = await this.findOrFail(id); const album = await this.findOrFail(id, { withAssets: false });
for (const userId of dto.sharedUserIds) { for (const userId of dto.sharedUserIds) {
const exists = album.sharedUsers.find((user) => user.id === userId); const exists = album.sharedUsers.find((user) => user.id === userId);
@ -247,7 +247,7 @@ export class AlbumService {
updatedAt: new Date(), updatedAt: new Date(),
sharedUsers: album.sharedUsers, sharedUsers: album.sharedUsers,
}) })
.then(mapAlbumWithAssets); .then(mapAlbumWithoutAssets);
} }
async removeUser(authUser: AuthUserDto, id: string, userId: string | 'me'): Promise<void> { async removeUser(authUser: AuthUserDto, id: string, userId: string | 'me'): Promise<void> {
@ -255,7 +255,7 @@ export class AlbumService {
userId = authUser.id; userId = authUser.id;
} }
const album = await this.findOrFail(id); const album = await this.findOrFail(id, { withAssets: false });
if (album.ownerId === userId) { if (album.ownerId === userId) {
throw new BadRequestException('Cannot remove album owner'); throw new BadRequestException('Cannot remove album owner');
@ -278,8 +278,8 @@ export class AlbumService {
}); });
} }
private async findOrFail(id: string) { private async findOrFail(id: string, options: AlbumInfoOptions) {
const album = await this.albumRepository.getById(id); const album = await this.albumRepository.getById(id, options);
if (!album) { if (!album) {
throw new BadRequestException('Album not found'); throw new BadRequestException('Album not found');
} }

View File

@ -1,7 +1,7 @@
import { AlbumAssetCount, IAlbumRepository } from '@app/domain'; import { AlbumAssetCount, AlbumInfoOptions, IAlbumRepository } from '@app/domain';
import { Injectable } from '@nestjs/common'; import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm'; import { InjectRepository } from '@nestjs/typeorm';
import { In, IsNull, Not, Repository } from 'typeorm'; import { FindOptionsOrder, FindOptionsRelations, In, IsNull, Not, Repository } from 'typeorm';
import { dataSource } from '../database.config'; import { dataSource } from '../database.config';
import { AlbumEntity, AssetEntity } from '../entities'; import { AlbumEntity, AssetEntity } from '../entities';
@ -12,25 +12,27 @@ export class AlbumRepository implements IAlbumRepository {
@InjectRepository(AlbumEntity) private repository: Repository<AlbumEntity>, @InjectRepository(AlbumEntity) private repository: Repository<AlbumEntity>,
) {} ) {}
getById(id: string): Promise<AlbumEntity | null> { getById(id: string, options: AlbumInfoOptions): Promise<AlbumEntity | null> {
return this.repository.findOne({ const relations: FindOptionsRelations<AlbumEntity> = {
where: {
id,
},
relations: {
owner: true, owner: true,
sharedUsers: true, sharedUsers: true,
assets: { assets: false,
exifInfo: true,
},
sharedLinks: true, sharedLinks: true,
}, };
order: {
assets: { const order: FindOptionsOrder<AlbumEntity> = {};
if (options.withAssets) {
relations.assets = {
exifInfo: true,
};
order.assets = {
fileCreatedAt: 'DESC', fileCreatedAt: 'DESC',
}, };
}, }
});
return this.repository.findOne({ where: { id }, relations, order });
} }
getByIds(ids: string[]): Promise<AlbumEntity[]> { getByIds(ids: string[]): Promise<AlbumEntity[]> {

View File

@ -100,7 +100,7 @@
}); });
const refreshAlbum = async () => { const refreshAlbum = async () => {
const { data } = await api.albumApi.getAlbumInfo({ id: album.id, withoutAssets: false }); const { data } = await api.albumApi.getAlbumInfo({ id: album.id, withoutAssets: true });
album = data; album = data;
}; };
@ -261,9 +261,9 @@
} }
}; };
const handleUpdateDescription = (description: string) => { const handleUpdateDescription = async (description: string) => {
try { try {
api.albumApi.updateAlbumInfo({ await api.albumApi.updateAlbumInfo({
id: album.id, id: album.id,
updateAlbumDto: { updateAlbumDto: {
description, description,