From caac3bfc956982e8fbbfd9c690ee139b8c7b5692 Mon Sep 17 00:00:00 2001
From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com>
Date: Sun, 12 Feb 2023 20:26:24 +0100
Subject: [PATCH] fix(server): only update album when required (#1739)

* fix(server): only update album when required

* remove thumbnail from empty album
---
 .../src/api-v1/album/album.service.spec.ts    | 69 ++++++++++++++++++-
 .../immich/src/api-v1/album/album.service.ts  | 22 +++---
 2 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/server/apps/immich/src/api-v1/album/album.service.spec.ts b/server/apps/immich/src/api-v1/album/album.service.spec.ts
index c009e16508..150ddb2506 100644
--- a/server/apps/immich/src/api-v1/album/album.service.spec.ts
+++ b/server/apps/immich/src/api-v1/album/album.service.spec.ts
@@ -1,7 +1,7 @@
 import { AlbumService } from './album.service';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { BadRequestException, NotFoundException, ForbiddenException } from '@nestjs/common';
-import { AlbumEntity, UserEntity } from '@app/infra';
+import { AlbumEntity, AssetEntity, UserEntity } from '@app/infra';
 import { AlbumResponseDto, ICryptoRepository, mapUser } from '@app/domain';
 import { AddAssetsResponseDto } from './response-dto/add-assets-response.dto';
 import { IAlbumRepository } from './album-repository';
@@ -513,4 +513,71 @@ describe('Album service', () => {
     expect(result).toHaveLength(1);
     expect(result[0].assetCount).toEqual(1);
   });
+
+  it('updates the album thumbnail by listing all albums', async () => {
+    const albumEntity = _getOwnedAlbum();
+    const assetEntity = new AssetEntity();
+    const newThumbnailAssetId = 'e5e65c02-b889-4f3c-afe1-a39a96d578ed';
+
+    albumEntity.albumThumbnailAssetId = 'nonexistent';
+    assetEntity.id = newThumbnailAssetId;
+    albumEntity.assets = [
+      {
+        id: '760841c1-f7c4-42b1-96af-c7d007a26126',
+        assetId: assetEntity.id,
+        albumId: albumEntity.id,
+        albumInfo: albumEntity,
+        assetInfo: assetEntity,
+      },
+    ];
+    albumRepositoryMock.getList.mockImplementation(async () => [albumEntity]);
+    albumRepositoryMock.updateAlbum.mockImplementation(async () => ({
+      ...albumEntity,
+      albumThumbnailAssetId: newThumbnailAssetId,
+    }));
+
+    const result = await sut.getAllAlbums(authUser, {});
+
+    expect(result).toHaveLength(1);
+    expect(result[0].albumThumbnailAssetId).toEqual(newThumbnailAssetId);
+    expect(albumRepositoryMock.getList).toHaveBeenCalledTimes(1);
+    expect(albumRepositoryMock.updateAlbum).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);
+
+    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.getList).toHaveBeenCalledWith(albumEntity.ownerId, {});
+  });
 });
diff --git a/server/apps/immich/src/api-v1/album/album.service.ts b/server/apps/immich/src/api-v1/album/album.service.ts
index 30c0eb53a4..1f8def394d 100644
--- a/server/apps/immich/src/api-v1/album/album.service.ts
+++ b/server/apps/immich/src/api-v1/album/album.service.ts
@@ -187,15 +187,19 @@ export class AlbumService {
 
   async _checkValidThumbnail(album: AlbumEntity) {
     const assets = album.assets || [];
-    const valid = assets.some((asset) => asset.assetId === album.albumThumbnailAssetId);
-    if (!valid) {
-      let dto: UpdateAlbumDto = {};
-      if (assets.length > 0) {
-        const albumThumbnailAssetId = assets[0].assetId;
-        dto = { albumThumbnailAssetId };
-      }
-      await this._albumRepository.updateAlbum(album, dto);
-      album.albumThumbnailAssetId = dto.albumThumbnailAssetId || null;
+
+    // Check if the album's thumbnail is invalid by referencing
+    // an asset outside the album.
+    const invalid = assets.length > 0 && !assets.some((asset) => asset.assetId === 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]?.assetId;
+
+      album.albumThumbnailAssetId = albumThumbnailAssetId || null;
+      await this._albumRepository.updateAlbum(album, { albumThumbnailAssetId });
     }
   }