fix(server): album perf query (#5232)

* Revert "fix: album performances (#5224)" This reverts commit c438e17954. * Revert "fix: album sorting options (#5127)" This reverts commit 725f30c494.
2025-12-20 01:11:46 +03:00 · 2023-11-21 10:07:49 -06:00
parent a13052e24c
commit f094ff2aa1
13 changed files with 108 additions and 184 deletions
--- a/server/src/domain/album/album-response.dto.ts
+++ b/server/src/domain/album/album-response.dto.ts
@@ -37,6 +37,15 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons
  const hasSharedLink = entity.sharedLinks?.length > 0;
  const hasSharedUser = sharedUsers.length > 0;

+  let startDate = assets.at(0)?.fileCreatedAt || undefined;
+  let endDate = assets.at(-1)?.fileCreatedAt || undefined;
+  // Swap dates if start date is greater than end date.
+  if (startDate && endDate && startDate > endDate) {
+    const temp = startDate;
+    startDate = endDate;
+    endDate = temp;
+  }
+
  return {
    albumName: entity.albumName,
    description: entity.description,
@@ -49,10 +58,10 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons
    sharedUsers,
    shared: hasSharedUser || hasSharedLink,
    hasSharedLink,
-    startDate: entity.startDate ? entity.startDate : undefined,
-    endDate: entity.endDate ? entity.endDate : undefined,
+    startDate,
+    endDate,
    assets: (withAssets ? assets : []).map((asset) => mapAsset(asset)),
-    assetCount: entity.assetCount,
+    assetCount: entity.assets?.length || 0,
    isActivityEnabled: entity.isActivityEnabled,
  };
 };
--- a/server/src/domain/album/album.service.spec.ts
+++ b/server/src/domain/album/album.service.spec.ts
@@ -58,6 +58,10 @@ describe(AlbumService.name, () => {
  describe('getAll', () => {
    it('gets list of albums for auth user', async () => {
      albumMock.getOwned.mockResolvedValue([albumStub.empty, albumStub.sharedWithUser]);
+      albumMock.getAssetCountForIds.mockResolvedValue([
+        { albumId: albumStub.empty.id, assetCount: 0 },
+        { albumId: albumStub.sharedWithUser.id, assetCount: 0 },
+      ]);
      albumMock.getInvalidThumbnail.mockResolvedValue([]);

      const result = await sut.getAll(authStub.admin, {});
@@ -68,6 +72,7 @@ describe(AlbumService.name, () => {

    it('gets list of albums that have a specific asset', async () => {
      albumMock.getByAssetId.mockResolvedValue([albumStub.oneAsset]);
+      albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
      albumMock.getInvalidThumbnail.mockResolvedValue([]);

      const result = await sut.getAll(authStub.admin, { assetId: albumStub.oneAsset.id });
@@ -78,6 +83,7 @@ describe(AlbumService.name, () => {

    it('gets list of albums that are shared', async () => {
      albumMock.getShared.mockResolvedValue([albumStub.sharedWithUser]);
+      albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.sharedWithUser.id, assetCount: 0 }]);
      albumMock.getInvalidThumbnail.mockResolvedValue([]);

      const result = await sut.getAll(authStub.admin, { shared: true });
@@ -88,6 +94,7 @@ describe(AlbumService.name, () => {

    it('gets list of albums that are NOT shared', async () => {
      albumMock.getNotShared.mockResolvedValue([albumStub.empty]);
+      albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.empty.id, assetCount: 0 }]);
      albumMock.getInvalidThumbnail.mockResolvedValue([]);

      const result = await sut.getAll(authStub.admin, { shared: false });
@@ -99,6 +106,7 @@ describe(AlbumService.name, () => {

  it('counts assets correctly', async () => {
    albumMock.getOwned.mockResolvedValue([albumStub.oneAsset]);
+    albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
    albumMock.getInvalidThumbnail.mockResolvedValue([]);

    const result = await sut.getAll(authStub.admin, {});
@@ -110,6 +118,9 @@ describe(AlbumService.name, () => {

  it('updates the album thumbnail by listing all albums', async () => {
    albumMock.getOwned.mockResolvedValue([albumStub.oneAssetInvalidThumbnail]);
+    albumMock.getAssetCountForIds.mockResolvedValue([
+      { albumId: albumStub.oneAssetInvalidThumbnail.id, assetCount: 1 },
+    ]);
    albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.oneAssetInvalidThumbnail.id]);
    albumMock.update.mockResolvedValue(albumStub.oneAssetValidThumbnail);
    assetMock.getFirstAssetForAlbumId.mockResolvedValue(albumStub.oneAssetInvalidThumbnail.assets[0]);
@@ -123,6 +134,9 @@ describe(AlbumService.name, () => {

  it('removes the thumbnail for an empty album', async () => {
    albumMock.getOwned.mockResolvedValue([albumStub.emptyWithInvalidThumbnail]);
+    albumMock.getAssetCountForIds.mockResolvedValue([
+      { albumId: albumStub.emptyWithInvalidThumbnail.id, assetCount: 1 },
+    ]);
    albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.emptyWithInvalidThumbnail.id]);
    albumMock.update.mockResolvedValue(albumStub.emptyWithValidThumbnail);
    assetMock.getFirstAssetForAlbumId.mockResolvedValue(null);
--- a/server/src/domain/album/album.service.ts
+++ b/server/src/domain/album/album.service.ts
@@ -66,12 +66,21 @@ export class AlbumService {
      albums = await this.albumRepository.getOwned(ownerId);
    }

+    // Get asset count for each album. Then map the result to an object:
+    // { [albumId]: assetCount }
+    const albumsAssetCount = await this.albumRepository.getAssetCountForIds(albums.map((album) => album.id));
+    const albumsAssetCountObj = albumsAssetCount.reduce((obj: Record<string, number>, { albumId, assetCount }) => {
+      obj[albumId] = assetCount;
+      return obj;
+    }, {});
+
    return Promise.all(
      albums.map(async (album) => {
        const lastModifiedAsset = await this.assetRepository.getLastUpdatedAssetForAlbumId(album.id);
        return {
          ...mapAlbumWithoutAssets(album),
          sharedLinks: undefined,
+          assetCount: albumsAssetCountObj[album.id],
          lastModifiedAssetTimestamp: lastModifiedAsset?.fileModifiedAt,
        };
      }),
@@ -81,8 +90,7 @@ export class AlbumService {
  async get(authUser: AuthUserDto, id: string, dto: AlbumInfoDto): Promise<AlbumResponseDto> {
    await this.access.requirePermission(authUser, Permission.ALBUM_READ, id);
    await this.albumRepository.updateThumbnails();
-    const withAssets = dto.withoutAssets === undefined ? true : !dto.withoutAssets;
-    return mapAlbum(await this.findOrFail(id, { withAssets }), !dto.withoutAssets);
+    return mapAlbum(await this.findOrFail(id, { withAssets: true }), !dto.withoutAssets);
  }

  async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
--- a/server/src/domain/repositories/album.repository.ts
+++ b/server/src/domain/repositories/album.repository.ts
@@ -30,6 +30,7 @@ export interface IAlbumRepository {
  hasAsset(asset: AlbumAsset): Promise<boolean>;
  removeAsset(assetId: string): Promise<void>;
  removeAssets(assets: AlbumAssets): Promise<void>;
+  getAssetCountForIds(ids: string[]): Promise<AlbumAssetCount[]>;
  getInvalidThumbnail(): Promise<string[]>;
  getOwned(ownerId: string): Promise<AlbumEntity[]>;
  getShared(ownerId: string): Promise<AlbumEntity[]>;
--- a/server/src/domain/shared-link/shared-link-response.dto.ts
+++ b/server/src/domain/shared-link/shared-link-response.dto.ts
@@ -40,7 +40,7 @@ export function mapSharedLink(sharedLink: SharedLinkEntity): SharedLinkResponseD
    createdAt: sharedLink.createdAt,
    expiresAt: sharedLink.expiresAt,
    assets: assets.map((asset) => mapAsset(asset)),
-    album: sharedLink.album?.id ? mapAlbumWithoutAssets(sharedLink.album) : undefined,
+    album: sharedLink.album ? mapAlbumWithoutAssets(sharedLink.album) : undefined,
    allowUpload: sharedLink.allowUpload,
    allowDownload: sharedLink.allowDownload,
    showMetadata: sharedLink.showExif,
--- a/server/src/infra/entities/album.entity.ts
+++ b/server/src/infra/entities/album.entity.ts
@@ -9,7 +9,6 @@ import {
  OneToMany,
  PrimaryGeneratedColumn,
  UpdateDateColumn,
-  VirtualColumn,
 } from 'typeorm';
 import { AssetEntity } from './asset.entity';
 import { SharedLinkEntity } from './shared-link.entity';
@@ -60,34 +59,4 @@ export class AlbumEntity {

  @Column({ default: true })
  isActivityEnabled!: boolean;
-
-  @VirtualColumn({
-    query: (alias) => `
-    SELECT MIN(assets."fileCreatedAt") 
-    FROM "assets" assets
-    JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
-    WHERE aa."albumsId" = ${alias}.id
-    `,
-  })
-  startDate!: Date | null;
-
-  @VirtualColumn({
-    query: (alias) => `
-    SELECT MAX(assets."fileCreatedAt") 
-    FROM "assets" assets
-    JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
-    WHERE aa."albumsId" = ${alias}.id
-    `,
-  })
-  endDate!: Date | null;
-
-  @VirtualColumn({
-    query: (alias) => `
-    SELECT COUNT(assets."id") 
-    FROM "assets" assets
-    JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
-    WHERE aa."albumsId" = ${alias}.id
-    `,
-  })
-  assetCount!: number;
 }
--- a/server/src/infra/repositories/album.repository.ts
+++ b/server/src/infra/repositories/album.repository.ts
@@ -1,4 +1,4 @@
-import { AlbumAsset, AlbumAssets, AlbumInfoOptions, IAlbumRepository } from '@app/domain';
+import { AlbumAsset, AlbumAssetCount, AlbumAssets, AlbumInfoOptions, IAlbumRepository } from '@app/domain';
 import { Injectable } from '@nestjs/common';
 import { InjectDataSource, InjectRepository } from '@nestjs/typeorm';
 import { DataSource, FindOptionsOrder, FindOptionsRelations, In, IsNull, Not, Repository } from 'typeorm';
@@ -56,10 +56,31 @@ export class AlbumRepository implements IAlbumRepository {
      ],
      relations: { owner: true, sharedUsers: true },
      order: { createdAt: 'DESC' },
-      relationLoadStrategy: 'query',
    });
  }

+  async getAssetCountForIds(ids: string[]): Promise<AlbumAssetCount[]> {
+    // Guard against running invalid query when ids list is empty.
+    if (!ids.length) {
+      return [];
+    }
+
+    // Only possible with query builder because of GROUP BY.
+    const countByAlbums = await this.repository
+      .createQueryBuilder('album')
+      .select('album.id')
+      .addSelect('COUNT(albums_assets.assetsId)', 'asset_count')
+      .leftJoin('albums_assets_assets', 'albums_assets', 'albums_assets.albumsId = album.id')
+      .where('album.id IN (:...ids)', { ids })
+      .groupBy('album.id')
+      .getRawMany();
+
+    return countByAlbums.map<AlbumAssetCount>((albumCount) => ({
+      albumId: albumCount['album_id'],
+      assetCount: Number(albumCount['asset_count']),
+    }));
+  }
+
  /**
   * Returns the album IDs that have an invalid thumbnail, when:
   *  - Thumbnail references an asset outside the album
@@ -92,7 +113,6 @@ export class AlbumRepository implements IAlbumRepository {
      relations: { sharedUsers: true, sharedLinks: true, owner: true },
      where: { ownerId },
      order: { createdAt: 'DESC' },
-      relationLoadStrategy: 'query',
    });
  }

@@ -108,7 +128,6 @@ export class AlbumRepository implements IAlbumRepository {
        { ownerId, sharedUsers: { id: Not(IsNull()) } },
      ],
      order: { createdAt: 'DESC' },
-      relationLoadStrategy: 'query',
    });
  }

@@ -120,7 +139,6 @@ export class AlbumRepository implements IAlbumRepository {
      relations: { sharedUsers: true, sharedLinks: true, owner: true },
      where: { ownerId, sharedUsers: { id: IsNull() }, sharedLinks: { id: IsNull() } },
      order: { createdAt: 'DESC' },
-      relationLoadStrategy: 'query',
    });
  }