perf(server): stop loading every album asset to build album responses (#32413)

* perf(server): stop loading every album asset to build album responses

Album create, update (rename, description, cover, sort order, activity
toggle) and add-users json_agg'd every asset of the album with its exif
row, only for mapAlbum to read the count and the first/last
localDateTime. Use getMetadataForIds for those fields, as GET /albums/:id
already does, and drop the now unused withAssets option from getById.

The responses now carry the same date format and lastModifiedAssetTimestamp
as GET /albums/:id.

PATCH /albums/:id on a 150k-asset album, PG 14 (response is ~900 B either
way): 3.4-3.6 s -> 0.13-0.16 s.

* refactor(server): share album metadata mapping

(cherry picked from commit 2a4e876ec990a4a8d5fdca00291cbeebd3d1bccf)

server/src/queries/album.repository.sql

@@ -61,27 +61,7 @@ select
         where
           "shared_link"."albumId" = "album"."id"
       ) as agg
-  ) as "sharedLinks",
-  (
-    select
-      json_agg("asset") as "assets"
-    from
-      (
-        select
-          "asset".*,
-          "asset_exif" as "exifInfo"
-        from
-          "asset"
-          left join "asset_exif" on "asset"."id" = "asset_exif"."assetId"
-          inner join "album_asset" on "album_asset"."assetId" = "asset"."id"
-        where
-          "album_asset"."albumId" = "album"."id"
-          and "asset"."deletedAt" is null
-          and "asset"."visibility" in ('archive', 'timeline')
-        order by
-          "asset"."fileCreatedAt" desc
-      ) as "asset"
-  ) as "assets"
+  ) as "sharedLinks"
 from
   "album"
 where
@@ -396,27 +376,7 @@ select
           "album_user"."role",
           "user"."name" asc
       ) as agg
-  ) as "albumUsers",
-  (
-    select
-      json_agg("asset") as "assets"
-    from
-      (
-        select
-          "asset".*,
-          "asset_exif" as "exifInfo"
-        from
-          "asset"
-          left join "asset_exif" on "asset"."id" = "asset_exif"."assetId"
-          inner join "album_asset" on "album_asset"."assetId" = "asset"."id"
-        where
-          "album_asset"."albumId" = "album"."id"
-          and "asset"."deletedAt" is null
-          and "asset"."visibility" in ('archive', 'timeline')
-        order by
-          "asset"."fileCreatedAt" desc
-      ) as "asset"
-  ) as "assets"
+  ) as "albumUsers"
 from
   "album"
 

server/src/repositories/album.repository.ts

@@ -1,14 +1,5 @@
 import { Injectable } from '@nestjs/common';
-import {
-  type ExpressionBuilder,
-  type Insertable,
-  type Kysely,
-  type NotNull,
-  type Selectable,
-  type ShallowDehydrateObject,
-  type Updateable,
-  sql,
-} from 'kysely';
+import { type ExpressionBuilder, type Insertable, type Kysely, type Updateable, sql } from 'kysely';
 import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres';
 import { InjectKysely } from 'nestjs-kysely';
 import { columns } from 'src/database.js';
@@ -17,7 +8,6 @@ import { AlbumUserCreateDto, MapAlbumDto } from 'src/dtos/album.dto.js';
 import { AlbumUserRole } from 'src/enum.js';
 import { DB } from 'src/schema/index.js';
 import { AlbumTable } from 'src/schema/tables/album.table.js';
-import { AssetExifTable } from 'src/schema/tables/asset-exif.table.js';
 import { asUuid, dummy, withDefaultVisibility } from 'src/utils/database.js';
 
 export interface AlbumAssetCount {
@@ -28,10 +18,6 @@ export interface AlbumAssetCount {
   lastModifiedAssetTimestamp: Date | null;
 }
 
-export interface AlbumInfoOptions {
-  withAssets: boolean;
-}
-
 const withAlbumUsers = (authUserId?: string) => (eb: ExpressionBuilder<DB, 'album'>) =>
   jsonArrayFrom(
     eb
@@ -52,27 +38,6 @@ const withSharedLink = (eb: ExpressionBuilder<DB, 'album'>) =>
     eb.selectFrom('shared_link').selectAll('shared_link').whereRef('shared_link.albumId', '=', 'album.id'),
   ).as('sharedLinks');
 
-const withAssets = (eb: ExpressionBuilder<DB, 'album'>) => {
-  return eb
-    .selectFrom((eb) =>
-      eb
-        .selectFrom('asset')
-        .selectAll('asset')
-        .leftJoin('asset_exif', 'asset.id', 'asset_exif.assetId')
-        .select((eb) =>
-          eb.table('asset_exif').$castTo<ShallowDehydrateObject<Selectable<AssetExifTable>>>().as('exifInfo'),
-        )
-        .innerJoin('album_asset', 'album_asset.assetId', 'asset.id')
-        .whereRef('album_asset.albumId', '=', 'album.id')
-        .where('asset.deletedAt', 'is', null)
-        .$call(withDefaultVisibility)
-        .orderBy('asset.fileCreatedAt', 'desc')
-        .as('asset'),
-    )
-    .select((eb) => eb.fn.jsonAgg('asset').as('assets'))
-    .as('assets');
-};
-
 const isAlbumOwned = (ownerId: string) => (eb: ExpressionBuilder<DB, 'album'>) =>
   eb.exists(
     eb
@@ -86,8 +51,8 @@ const isAlbumOwned = (ownerId: string) => (eb: ExpressionBuilder<DB, 'album'>) =
 export class AlbumRepository {
   constructor(@InjectKysely() private db: Kysely<DB>) {}
 
-  @GenerateSql({ params: [DummyValue.UUID, { withAssets: true }, DummyValue.UUID] })
-  getById(id: string, options: AlbumInfoOptions, authUserId?: string) {
+  @GenerateSql({ params: [DummyValue.UUID, DummyValue.UUID] })
+  getById(id: string, authUserId?: string) {
     return this.db
       .with('album_user', (qb) => qb.selectFrom('album_user').selectAll().where('album_user.albumId', '=', id))
       .selectFrom('album')
@@ -96,8 +61,6 @@ export class AlbumRepository {
       .where('album.deletedAt', 'is', null)
       .select(withAlbumUsers(authUserId))
       .select(withSharedLink)
-      .$if(options.withAssets, (eb) => eb.select(withAssets))
-      .$narrowType<{ assets: NotNull }>()
       .executeTakeFirst();
   }
 
@@ -351,8 +314,6 @@ export class AlbumRepository {
       .selectFrom('album')
       .selectAll('album')
       .select(withAlbumUsers(authUserId))
-      .select(withAssets)
-      .$narrowType<{ assets: NotNull }>()
       .executeTakeFirstOrThrow();
 
     return result;

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

@@ -18,6 +18,7 @@ describe(AlbumService.name, () => {
 
   beforeEach(() => {
     ({ sut, mocks } = newTestService(AlbumService));
+    mocks.album.getMetadataForIds.mockResolvedValue([]);
   });
 
   it('should work', () => {
@@ -591,7 +592,7 @@ describe(AlbumService.name, () => {
 
       expect(mocks.albumUser.delete).toHaveBeenCalledTimes(1);
       expect(mocks.albumUser.delete).toHaveBeenCalledWith({ albumId: album.id, userId });
-      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, { withAssets: false }, owner.id);
+      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, owner.id);
     });
 
     it('should prevent removing a shared user from a not-owned album (shared with auth user)', async () => {
@@ -693,7 +694,7 @@ describe(AlbumService.name, () => {
 
       await sut.get(AuthFactory.create(owner), album.id);
 
-      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, { withAssets: false }, owner.id);
+      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, owner.id);
       expect(mocks.access.album.checkOwnerAccess).toHaveBeenCalledWith(owner.id, new Set([album.id]));
     });
 
@@ -714,7 +715,7 @@ describe(AlbumService.name, () => {
       const auth = AuthFactory.from().sharedLink().build();
       await sut.get(auth, album.id);
 
-      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, { withAssets: false }, auth.user.id);
+      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, auth.user.id);
       expect(mocks.access.album.checkSharedLinkAccess).toHaveBeenCalledWith(auth.sharedLink!.id, new Set([album.id]));
     });
 
@@ -735,7 +736,7 @@ describe(AlbumService.name, () => {
 
       await sut.get(AuthFactory.create(user), album.id);
 
-      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, { withAssets: false }, user.id);
+      expect(mocks.album.getById).toHaveBeenCalledWith(album.id, user.id);
       expect(mocks.access.album.checkSharedAlbumAccess).toHaveBeenCalledWith(
         user.id,
         new Set([album.id]),

server/src/services/album.service.ts

@@ -7,6 +7,7 @@ import {
   AlbumsAddAssetsResponseDto,
   CreateAlbumDto,
   GetAlbumsDto,
+  MapAlbumDto,
   UpdateAlbumDto,
   UpdateAlbumUserDto,
   mapAlbum,
@@ -15,13 +16,21 @@ import { BulkIdErrorReason, BulkIdResponseDto, BulkIdsDto } from 'src/dtos/asset
 import { AuthDto } from 'src/dtos/auth.dto.js';
 import { MapMarkerResponseDto } from 'src/dtos/map.dto.js';
 import { AlbumUserRole, Permission } from 'src/enum.js';
-import { AlbumAssetCount, AlbumInfoOptions } from 'src/repositories/album.repository.js';
+import { AlbumAssetCount } from 'src/repositories/album.repository.js';
 import { BaseService } from 'src/services/base.service.js';
 import { addAssets, removeAssets } from 'src/utils/asset.util.js';
 import { asDateTimeString } from 'src/utils/date.js';
 import { findOrFail } from 'src/utils/misc.js';
 import { getPreferences } from 'src/utils/preferences.js';
 
+const mapAlbumMetadata = (metadata?: AlbumAssetCount) => ({
+  startDate: asDateTimeString(metadata?.startDate ?? undefined),
+  endDate: asDateTimeString(metadata?.endDate ?? undefined),
+  assetCount: metadata?.assetCount ?? 0,
+  // lastModifiedAssetTimestamp is only used in mobile app, please remove if not need
+  lastModifiedAssetTimestamp: asDateTimeString(metadata?.lastModifiedAssetTimestamp ?? undefined),
+});
+
 @Injectable()
 export class AlbumService extends BaseService {
   async getStatistics(auth: AuthDto): Promise<AlbumStatisticsResponseDto> {
@@ -60,30 +69,21 @@ export class AlbumService extends BaseService {
     return albums.map((album) => ({
       ...mapAlbum(album),
       sharedLinks: undefined,
-      startDate: asDateTimeString(albumMetadata[album.id]?.startDate ?? undefined),
-      endDate: asDateTimeString(albumMetadata[album.id]?.endDate ?? undefined),
-      assetCount: albumMetadata[album.id]?.assetCount ?? 0,
-      // lastModifiedAssetTimestamp is only used in mobile app, please remove if not need
-      lastModifiedAssetTimestamp: asDateTimeString(albumMetadata[album.id]?.lastModifiedAssetTimestamp ?? undefined),
+      ...mapAlbumMetadata(albumMetadata[album.id]),
     }));
   }
 
   async get(auth: AuthDto, id: string): Promise<AlbumResponseDto> {
     await this.requireAccess({ auth, permission: Permission.AlbumRead, ids: [id] });
     await this.albumRepository.updateThumbnails();
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: false });
-    const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]);
+    const album = await this.findOrFail(id, auth.user.id);
 
     const hasSharedUsers = album.albumUsers && album.albumUsers.length > 1;
     const hasSharedLink = album.sharedLinks && album.sharedLinks.length > 0;
     const isShared = hasSharedUsers || hasSharedLink;
 
     return {
-      ...mapAlbum(album),
-      startDate: asDateTimeString(albumMetadataForIds?.startDate ?? undefined),
-      endDate: asDateTimeString(albumMetadataForIds?.endDate ?? undefined),
-      assetCount: albumMetadataForIds?.assetCount ?? 0,
-      lastModifiedAssetTimestamp: asDateTimeString(albumMetadataForIds?.lastModifiedAssetTimestamp ?? undefined),
+      ...(await this.mapWithMetadata(album)),
       contributorCounts: isShared ? await this.albumRepository.getContributorCounts(album.id) : undefined,
     };
   }
@@ -134,13 +134,13 @@ export class AlbumService extends BaseService {
       await this.eventRepository.emit('AlbumInvite', { id: album.id, userId, senderName: auth.user.name });
     }
 
-    return mapAlbum(album);
+    return this.mapWithMetadata(album);
   }
 
   async update(auth: AuthDto, id: string, dto: UpdateAlbumDto): Promise<AlbumResponseDto> {
     await this.requireAccess({ auth, permission: Permission.AlbumUpdate, ids: [id] });
 
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: true });
+    const album = await this.findOrFail(id, auth.user.id);
 
     if (dto.albumThumbnailAssetId) {
       const results = await this.albumRepository.getAssetIds(id, [dto.albumThumbnailAssetId]);
@@ -161,7 +161,7 @@ export class AlbumService extends BaseService {
       auth.user.id,
     );
 
-    return mapAlbum({ ...updatedAlbum, assets: album.assets });
+    return this.mapWithMetadata(updatedAlbum);
   }
 
   async delete(auth: AuthDto, id: string): Promise<void> {
@@ -170,7 +170,7 @@ export class AlbumService extends BaseService {
   }
 
   async addAssets(auth: AuthDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> {
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: false });
+    const album = await this.findOrFail(id, auth.user.id);
     await this.requireAccess({ auth, permission: Permission.AlbumAssetCreate, ids: [id] });
 
     const results = await addAssets(
@@ -229,7 +229,7 @@ export class AlbumService extends BaseService {
       if (notPresentAssetIds.length === 0) {
         continue;
       }
-      const album = await this.findOrFail(albumId, auth.user.id, { withAssets: false });
+      const album = await this.findOrFail(albumId, auth.user.id);
       results.error = undefined;
       results.success = true;
 
@@ -261,7 +261,7 @@ export class AlbumService extends BaseService {
   async removeAssets(auth: AuthDto, id: string, dto: BulkIdsDto): Promise<BulkIdResponseDto[]> {
     await this.requireAccess({ auth, permission: Permission.AlbumAssetDelete, ids: [id] });
 
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: false });
+    const album = await this.findOrFail(id, auth.user.id);
     const results = await removeAssets(
       auth,
       { access: this.accessRepository, bulk: this.albumRepository },
@@ -287,7 +287,7 @@ export class AlbumService extends BaseService {
   async addUsers(auth: AuthDto, id: string, { albumUsers }: AddUsersDto): Promise<AlbumResponseDto> {
     await this.requireAccess({ auth, permission: Permission.AlbumShare, ids: [id] });
 
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: false });
+    const album = await this.findOrFail(id, auth.user.id);
 
     for (const { userId, role } of albumUsers) {
       if (role === AlbumUserRole.Owner) {
@@ -309,7 +309,7 @@ export class AlbumService extends BaseService {
       await this.eventRepository.emit('AlbumInvite', { id, userId, senderName: auth.user.name });
     }
 
-    return mapAlbum(await this.findOrFail(id, auth.user.id, { withAssets: true }));
+    return this.mapWithMetadata(await this.findOrFail(id, auth.user.id));
   }
 
   async removeUser(auth: AuthDto, id: string, userId: string | 'me'): Promise<void> {
@@ -317,7 +317,7 @@ export class AlbumService extends BaseService {
       userId = auth.user.id;
     }
 
-    const album = await this.findOrFail(id, auth.user.id, { withAssets: false });
+    const album = await this.findOrFail(id, auth.user.id);
 
     const exists = album.albumUsers.find(({ user: { id } }) => id === userId);
     if (!exists) {
@@ -342,7 +342,7 @@ export class AlbumService extends BaseService {
   async updateUser(auth: AuthDto, id: string, userId: string, dto: UpdateAlbumUserDto): Promise<void> {
     await this.requireAccess({ auth, permission: Permission.AlbumShare, ids: [id] });
 
-    const album = await this.findOrFail(id, userId, { withAssets: false });
+    const album = await this.findOrFail(id, userId);
     const owner = album.albumUsers[0];
 
     if (owner.user.id === userId) {
@@ -352,7 +352,12 @@ export class AlbumService extends BaseService {
     await this.albumUserRepository.update({ albumId: id, userId }, { role: dto.role });
   }
 
-  private findOrFail(id: string, authUserId: string, options: AlbumInfoOptions) {
-    return findOrFail(() => this.albumRepository.getById(id, options, authUserId), 'Album');
+  private findOrFail(id: string, authUserId: string) {
+    return findOrFail(() => this.albumRepository.getById(id, authUserId), 'Album');
+  }
+
+  private async mapWithMetadata(album: MapAlbumDto): Promise<AlbumResponseDto> {
+    const [metadata] = await this.albumRepository.getMetadataForIds([album.id]);
+    return { ...mapAlbum(album), ...mapAlbumMetadata(metadata) };
   }
 }

server/src/services/asset-media.service.ts

@@ -351,7 +351,7 @@ export class AssetMediaService extends BaseService {
       return;
     }
 
-    const album = await this.albumRepository.getById(sharedLink.albumId, { withAssets: false });
+    const album = await this.albumRepository.getById(sharedLink.albumId);
     if (!album) {
       return;
     }

server/src/services/notification.service.ts

@@ -324,7 +324,7 @@ export class NotificationService extends BaseService {
 
   @OnJob({ name: JobName.NotifyAlbumInvite, queue: QueueName.Notification })
   async handleAlbumInvite({ id, recipientId, senderName }: JobOf<JobName.NotifyAlbumInvite>) {
-    const album = await this.albumRepository.getById(id, { withAssets: false });
+    const album = await this.albumRepository.getById(id);
     if (!album) {
       return JobStatus.Skipped;
     }
@@ -374,7 +374,7 @@ export class NotificationService extends BaseService {
 
   @OnJob({ name: JobName.NotifyAlbumUpdate, queue: QueueName.Notification })
   async handleAlbumUpdate({ id, recipientId }: JobOf<JobName.NotifyAlbumUpdate>) {
-    const album = await this.albumRepository.getById(id, { withAssets: false });
+    const album = await this.albumRepository.getById(id);
 
     if (!album) {
       return JobStatus.Skipped;