feat: improve performance for GET /api/album & /api/album/:id (#17124)

* fix(server) optimize number of sql calls for GET /api/albums

remove unnecessary join for getMetadataForIds
remove separate call to getLastUpdatedAssetForAlbumId

* fix(server) remove unnecessary getLastUpdatedAssetForAlbumId call for GET /api/album/:id

also remove getLastUpdatedAssetForAlbumId query as it is no longer referenced

* fix(server): correct lastModifiedAssetTimestamp return type + formatting and typing

* chore(server): address type issue with tests found via npm:check

tests & lint still pass before this commit.

server/src/queries/album.repository.sql

@@ -201,19 +201,23 @@ order by
 
 -- AlbumRepository.getMetadataForIds
 select
-  "albums"."id" as "albumId",
+  "album_assets"."albumsId" as "albumId",
-  min("assets"."localDateTime") as "startDate",
+  min(
-  max("assets"."localDateTime") as "endDate",
+    ("assets"."localDateTime" AT TIME ZONE 'UTC'::text)::date
+  ) as "startDate",
+  max(
+    ("assets"."localDateTime" AT TIME ZONE 'UTC'::text)::date
+  ) as "endDate",
+  max("assets"."updatedAt") as "lastModifiedAssetTimestamp",
   count("assets"."id")::int as "assetCount"
 from
-  "albums"
+  "assets"
-  inner join "albums_assets_assets" as "album_assets" on "album_assets"."albumsId" = "albums"."id"
+  inner join "albums_assets_assets" as "album_assets" on "album_assets"."assetsId" = "assets"."id"
-  inner join "assets" on "assets"."id" = "album_assets"."assetsId"
 where
-  "albums"."id" in ($1)
+  "album_assets"."albumsId" in ($1)
   and "assets"."deletedAt" is null
 group by
-  "albums"."id"
+  "album_assets"."albumsId"
 
 -- AlbumRepository.getOwned
 select

server/src/repositories/album.repository.ts

@@ -12,6 +12,7 @@ export interface AlbumAssetCount {
   assetCount: number;
   startDate: Date | null;
   endDate: Date | null;
+  lastModifiedAssetTimestamp: Date | null;
 }
 
 export interface AlbumInfoOptions {
@@ -132,18 +133,21 @@ export class AlbumRepository {
       return [];
     }
 
-    return this.db
+    return (
-      .selectFrom('albums')
+      this.db
-      .innerJoin('albums_assets_assets as album_assets', 'album_assets.albumsId', 'albums.id')
+        .selectFrom('assets')
-      .innerJoin('assets', 'assets.id', 'album_assets.assetsId')
+        .innerJoin('albums_assets_assets as album_assets', 'album_assets.assetsId', 'assets.id')
-      .select('albums.id as albumId')
+        .select('album_assets.albumsId as albumId')
-      .select((eb) => eb.fn.min('assets.localDateTime').as('startDate'))
+        .select((eb) => eb.fn.min(sql<Date>`("assets"."localDateTime" AT TIME ZONE 'UTC'::text)::date`).as('startDate'))
-      .select((eb) => eb.fn.max('assets.localDateTime').as('endDate'))
+        .select((eb) => eb.fn.max(sql<Date>`("assets"."localDateTime" AT TIME ZONE 'UTC'::text)::date`).as('endDate'))
-      .select((eb) => sql<number>`${eb.fn.count('assets.id')}::int`.as('assetCount'))
+        // lastModifiedAssetTimestamp is only used in mobile app, please remove if not need
-      .where('albums.id', 'in', ids)
+        .select((eb) => eb.fn.max('assets.updatedAt').as('lastModifiedAssetTimestamp'))
-      .where('assets.deletedAt', 'is', null)
+        .select((eb) => sql<number>`${eb.fn.count('assets.id')}::int`.as('assetCount'))
-      .groupBy('albums.id')
+        .where('album_assets.albumsId', 'in', ids)
-      .execute();
+        .where('assets.deletedAt', 'is', null)
+        .groupBy('album_assets.albumsId')
+        .execute()
+    );
   }
 
   @GenerateSql({ params: [DummyValue.UUID] })

server/src/repositories/asset.repository.ts

@@ -728,17 +728,6 @@ export class AssetRepository {
     return paginationHelper(items as any as AssetEntity[], pagination.take);
   }
 
-  getLastUpdatedAssetForAlbumId(albumId: string): Promise<AssetEntity | undefined> {
-    return this.db
-      .selectFrom('assets')
-      .selectAll('assets')
-      .innerJoin('albums_assets_assets', 'assets.id', 'albums_assets_assets.assetsId')
-      .where('albums_assets_assets.albumsId', '=', asUuid(albumId))
-      .orderBy('updatedAt', 'desc')
-      .limit(1)
-      .executeTakeFirst() as Promise<AssetEntity | undefined>;
-  }
-
   getStatistics(ownerId: string, { isArchived, isFavorite, isTrashed }: AssetStatsOptions): Promise<AssetStats> {
     return this.db
       .selectFrom('assets')

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

@@ -41,8 +41,20 @@ describe(AlbumService.name, () => {
     it('gets list of albums for auth user', async () => {
       mocks.album.getOwned.mockResolvedValue([albumStub.empty, albumStub.sharedWithUser]);
       mocks.album.getMetadataForIds.mockResolvedValue([
-        { albumId: albumStub.empty.id, assetCount: 0, startDate: null, endDate: null },
+        {
-        { albumId: albumStub.sharedWithUser.id, assetCount: 0, startDate: null, endDate: null },
+          albumId: albumStub.empty.id,
+          assetCount: 0,
+          startDate: null,
+          endDate: null,
+          lastModifiedAssetTimestamp: null,
+        },
+        {
+          albumId: albumStub.sharedWithUser.id,
+          assetCount: 0,
+          startDate: null,
+          endDate: null,
+          lastModifiedAssetTimestamp: null,
+        },
       ]);
 
       const result = await sut.getAll(authStub.admin, {});
@@ -59,6 +71,7 @@ describe(AlbumService.name, () => {
           assetCount: 1,
           startDate: new Date('1970-01-01'),
           endDate: new Date('1970-01-01'),
+          lastModifiedAssetTimestamp: new Date('1970-01-01'),
         },
       ]);
 
@@ -71,7 +84,13 @@ describe(AlbumService.name, () => {
     it('gets list of albums that are shared', async () => {
       mocks.album.getShared.mockResolvedValue([albumStub.sharedWithUser]);
       mocks.album.getMetadataForIds.mockResolvedValue([
-        { albumId: albumStub.sharedWithUser.id, assetCount: 0, startDate: null, endDate: null },
+        {
+          albumId: albumStub.sharedWithUser.id,
+          assetCount: 0,
+          startDate: null,
+          endDate: null,
+          lastModifiedAssetTimestamp: null,
+        },
       ]);
 
       const result = await sut.getAll(authStub.admin, { shared: true });
@@ -83,7 +102,13 @@ describe(AlbumService.name, () => {
     it('gets list of albums that are NOT shared', async () => {
       mocks.album.getNotShared.mockResolvedValue([albumStub.empty]);
       mocks.album.getMetadataForIds.mockResolvedValue([
-        { albumId: albumStub.empty.id, assetCount: 0, startDate: null, endDate: null },
+        {
+          albumId: albumStub.empty.id,
+          assetCount: 0,
+          startDate: null,
+          endDate: null,
+          lastModifiedAssetTimestamp: null,
+        },
       ]);
 
       const result = await sut.getAll(authStub.admin, { shared: false });
@@ -101,6 +126,7 @@ describe(AlbumService.name, () => {
         assetCount: 1,
         startDate: new Date('1970-01-01'),
         endDate: new Date('1970-01-01'),
+        lastModifiedAssetTimestamp: new Date('1970-01-01'),
       },
     ]);
 
@@ -447,6 +473,7 @@ describe(AlbumService.name, () => {
           assetCount: 1,
           startDate: new Date('1970-01-01'),
           endDate: new Date('1970-01-01'),
+          lastModifiedAssetTimestamp: new Date('1970-01-01'),
         },
       ]);
 
@@ -468,6 +495,7 @@ describe(AlbumService.name, () => {
           assetCount: 1,
           startDate: new Date('1970-01-01'),
           endDate: new Date('1970-01-01'),
+          lastModifiedAssetTimestamp: new Date('1970-01-01'),
         },
       ]);
 
@@ -489,6 +517,7 @@ describe(AlbumService.name, () => {
           assetCount: 1,
           startDate: new Date('1970-01-01'),
           endDate: new Date('1970-01-01'),
+          lastModifiedAssetTimestamp: new Date('1970-01-01'),
         },
       ]);
 

server/src/services/album.service.ts

@@ -58,19 +58,15 @@ export class AlbumService extends BaseService {
       albumMetadata[metadata.albumId] = metadata;
     }
 
-    return Promise.all(
+    return albums.map((album) => ({
-      albums.map(async (album) => {
+      ...mapAlbumWithoutAssets(album),
-        const lastModifiedAsset = await this.assetRepository.getLastUpdatedAssetForAlbumId(album.id);
+      sharedLinks: undefined,
-        return {
+      startDate: albumMetadata[album.id]?.startDate ?? undefined,
-          ...mapAlbumWithoutAssets(album),
+      endDate: albumMetadata[album.id]?.endDate ?? undefined,
-          sharedLinks: undefined,
+      assetCount: albumMetadata[album.id]?.assetCount ?? 0,
-          startDate: albumMetadata[album.id]?.startDate ?? undefined,
+      // lastModifiedAssetTimestamp is only used in mobile app, please remove if not need
-          endDate: albumMetadata[album.id]?.endDate ?? undefined,
+      lastModifiedAssetTimestamp: albumMetadata[album.id]?.lastModifiedAssetTimestamp ?? undefined,
-          assetCount: albumMetadata[album.id]?.assetCount ?? 0,
+    }));
-          lastModifiedAssetTimestamp: lastModifiedAsset?.updatedAt,
-        };
-      }),
-    );
   }
 
   async get(auth: AuthDto, id: string, dto: AlbumInfoDto): Promise<AlbumResponseDto> {
@@ -79,14 +75,13 @@ export class AlbumService extends BaseService {
     const withAssets = dto.withoutAssets === undefined ? true : !dto.withoutAssets;
     const album = await this.findOrFail(id, { withAssets });
     const [albumMetadataForIds] = await this.albumRepository.getMetadataForIds([album.id]);
-    const lastModifiedAsset = await this.assetRepository.getLastUpdatedAssetForAlbumId(album.id);
 
     return {
       ...mapAlbum(album, withAssets, auth),
       startDate: albumMetadataForIds?.startDate ?? undefined,
       endDate: albumMetadataForIds?.endDate ?? undefined,
       assetCount: albumMetadataForIds?.assetCount ?? 0,
-      lastModifiedAssetTimestamp: lastModifiedAsset?.updatedAt,
+      lastModifiedAssetTimestamp: albumMetadataForIds?.lastModifiedAssetTimestamp ?? undefined,
     };
   }
 

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

@@ -21,7 +21,6 @@ export const newAssetRepositoryMock = (): Mocked<RepositoryInterface<AssetReposi
     getByChecksums: vitest.fn(),
     getUploadAssetIdByChecksum: vitest.fn(),
     getRandom: vitest.fn(),
-    getLastUpdatedAssetForAlbumId: vitest.fn(),
     getAll: vitest.fn().mockResolvedValue({ items: [], hasNextPage: false }),
     getAllByDeviceId: vitest.fn(),
     getLivePhotoCount: vitest.fn(),