From ccc0961ba3977491415e8e12a7d61d917ca4083c Mon Sep 17 00:00:00 2001 From: Brandon Wees Date: Fri, 23 Jan 2026 23:12:18 -0600 Subject: [PATCH] fix: return original thumbs when edited=false (#25485) --- server/src/repositories/asset.repository.ts | 6 +- .../src/services/asset-media.service.spec.ts | 58 ++++++- server/src/services/asset-media.service.ts | 6 +- .../services/asset-media.service.spec.ts | 161 ++++++++++++++++++ 4 files changed, 226 insertions(+), 5 deletions(-) diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index 468ded4773..6655184597 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -1030,8 +1030,8 @@ export class AssetRepository { .executeTakeFirstOrThrow(); } - @GenerateSql({ params: [DummyValue.UUID, AssetFileType.Preview] }) - async getForThumbnail(id: string, type: AssetFileType) { + @GenerateSql({ params: [DummyValue.UUID, AssetFileType.Preview, true] }) + async getForThumbnail(id: string, type: AssetFileType, isEdited: boolean) { return this.db .selectFrom('asset') .where('asset.id', '=', id) @@ -1039,7 +1039,7 @@ export class AssetRepository { join.onRef('asset.id', '=', 'asset_file.assetId').on('asset_file.type', '=', type), ) .select(['asset.originalPath', 'asset.originalFileName', 'asset_file.path as path']) - .orderBy('asset_file.isEdited', 'desc') + .orderBy('asset_file.isEdited', isEdited ? 'desc' : 'asc') .executeTakeFirstOrThrow(); } diff --git a/server/src/services/asset-media.service.spec.ts b/server/src/services/asset-media.service.spec.ts index 6c497e6098..049e7ec6ac 100644 --- a/server/src/services/asset-media.service.spec.ts +++ b/server/src/services/asset-media.service.spec.ts @@ -652,9 +652,65 @@ describe(AssetMediaService.name, () => { fileName: 'asset-id_thumbnail.ext', }), ); + expect(mocks.asset.getForThumbnail).toHaveBeenCalledWith(assetStub.image.id, AssetFileType.Thumbnail, false); }); - // TODO: Edited asset tests + it('should get original thumbnail by default', async () => { + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + mocks.asset.getForThumbnail.mockResolvedValue({ + ...assetStub.image, + path: '/uploads/user-id/thumbs/original-thumbnail.jpg', + }); + await expect( + sut.viewThumbnail(authStub.admin, assetStub.image.id, { size: AssetMediaSize.THUMBNAIL }), + ).resolves.toEqual( + new ImmichFileResponse({ + path: '/uploads/user-id/thumbs/original-thumbnail.jpg', + cacheControl: CacheControl.PrivateWithCache, + contentType: 'image/jpeg', + fileName: 'asset-id_thumbnail.jpg', + }), + ); + expect(mocks.asset.getForThumbnail).toHaveBeenCalledWith(assetStub.image.id, AssetFileType.Thumbnail, false); + }); + + it('should get edited thumbnail when edited=true', async () => { + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + mocks.asset.getForThumbnail.mockResolvedValue({ + ...assetStub.image, + path: '/uploads/user-id/thumbs/edited-thumbnail.jpg', + }); + await expect( + sut.viewThumbnail(authStub.admin, assetStub.image.id, { size: AssetMediaSize.THUMBNAIL, edited: true }), + ).resolves.toEqual( + new ImmichFileResponse({ + path: '/uploads/user-id/thumbs/edited-thumbnail.jpg', + cacheControl: CacheControl.PrivateWithCache, + contentType: 'image/jpeg', + fileName: 'asset-id_thumbnail.jpg', + }), + ); + expect(mocks.asset.getForThumbnail).toHaveBeenCalledWith(assetStub.image.id, AssetFileType.Thumbnail, true); + }); + + it('should get original thumbnail when edited=false', async () => { + mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + mocks.asset.getForThumbnail.mockResolvedValue({ + ...assetStub.image, + path: '/uploads/user-id/thumbs/original-thumbnail.jpg', + }); + await expect( + sut.viewThumbnail(authStub.admin, assetStub.image.id, { size: AssetMediaSize.THUMBNAIL, edited: false }), + ).resolves.toEqual( + new ImmichFileResponse({ + path: '/uploads/user-id/thumbs/original-thumbnail.jpg', + cacheControl: CacheControl.PrivateWithCache, + contentType: 'image/jpeg', + fileName: 'asset-id_thumbnail.jpg', + }), + ); + expect(mocks.asset.getForThumbnail).toHaveBeenCalledWith(assetStub.image.id, AssetFileType.Thumbnail, false); + }); }); describe('playbackVideo', () => { diff --git a/server/src/services/asset-media.service.ts b/server/src/services/asset-media.service.ts index e944a1e345..f008523029 100644 --- a/server/src/services/asset-media.service.ts +++ b/server/src/services/asset-media.service.ts @@ -223,7 +223,11 @@ export class AssetMediaService extends BaseService { } const size = (dto.size ?? AssetMediaSize.THUMBNAIL) as unknown as AssetFileType; - const { originalPath, originalFileName, path } = await this.assetRepository.getForThumbnail(id, size); + const { originalPath, originalFileName, path } = await this.assetRepository.getForThumbnail( + id, + size, + dto.edited ?? false, + ); if (size === AssetFileType.FullSize && mimeTypes.isWebSupportedImage(originalPath) && !dto.edited) { // use original file for web supported images diff --git a/server/test/medium/specs/services/asset-media.service.spec.ts b/server/test/medium/specs/services/asset-media.service.spec.ts index 5089850b6f..cdd47e3dc4 100644 --- a/server/test/medium/specs/services/asset-media.service.spec.ts +++ b/server/test/medium/specs/services/asset-media.service.spec.ts @@ -1,5 +1,7 @@ import { Kysely } from 'kysely'; import { AssetMediaStatus } from 'src/dtos/asset-media-response.dto'; +import { AssetMediaSize } from 'src/dtos/asset-media.dto'; +import { AssetFileType } from 'src/enum'; import { AccessRepository } from 'src/repositories/access.repository'; import { AssetRepository } from 'src/repositories/asset.repository'; import { EventRepository } from 'src/repositories/event.repository'; @@ -10,6 +12,7 @@ import { UserRepository } from 'src/repositories/user.repository'; import { DB } from 'src/schema'; import { AssetMediaService } from 'src/services/asset-media.service'; import { AssetService } from 'src/services/asset.service'; +import { ImmichFileResponse } from 'src/utils/file'; import { mediumFactory, newMediumService } from 'test/medium.factory'; import { factory } from 'test/small.factory'; import { getKyselyDB } from 'test/utils'; @@ -97,4 +100,162 @@ describe(AssetService.name, () => { }); }); }); + + describe('viewThumbnail', () => { + it('should return original thumbnail by default when both exist', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create both original and edited thumbnails + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/original/preview.jpg', + isEdited: false, + }); + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/edited/preview.jpg', + isEdited: true, + }); + + const auth = factory.auth({ user: { id: user.id } }); + const result = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.PREVIEW }); + + expect(result).toBeInstanceOf(ImmichFileResponse); + expect((result as ImmichFileResponse).path).toBe('/original/preview.jpg'); + }); + + it('should return edited thumbnail when edited=true', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create both original and edited thumbnails + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/original/preview.jpg', + isEdited: false, + }); + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/edited/preview.jpg', + isEdited: true, + }); + + const auth = factory.auth({ user: { id: user.id } }); + const result = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.PREVIEW, edited: true }); + + expect(result).toBeInstanceOf(ImmichFileResponse); + expect((result as ImmichFileResponse).path).toBe('/edited/preview.jpg'); + }); + + it('should return original thumbnail when edited=false', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create both original and edited thumbnails + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/original/preview.jpg', + isEdited: false, + }); + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/edited/preview.jpg', + isEdited: true, + }); + + const auth = factory.auth({ user: { id: user.id } }); + const result = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.PREVIEW, edited: false }); + + expect(result).toBeInstanceOf(ImmichFileResponse); + expect((result as ImmichFileResponse).path).toBe('/original/preview.jpg'); + }); + + it('should return original thumbnail when only original exists and edited=false', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create only original thumbnail + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/original/preview.jpg', + isEdited: false, + }); + + const auth = factory.auth({ user: { id: user.id } }); + const result = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.PREVIEW, edited: false }); + + expect(result).toBeInstanceOf(ImmichFileResponse); + expect((result as ImmichFileResponse).path).toBe('/original/preview.jpg'); + }); + + it('should return original thumbnail when only original exists and edited=true', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create only original thumbnail + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Preview, + path: '/original/preview.jpg', + isEdited: false, + }); + + const auth = factory.auth({ user: { id: user.id } }); + const result = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.PREVIEW, edited: true }); + + expect(result).toBeInstanceOf(ImmichFileResponse); + expect((result as ImmichFileResponse).path).toBe('/original/preview.jpg'); + }); + + it('should work with thumbnail size', async () => { + const { sut, ctx } = setup(); + + const { user } = await ctx.newUser(); + const { asset } = await ctx.newAsset({ ownerId: user.id }); + + // Create both original and edited thumbnails + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Thumbnail, + path: '/original/thumbnail.jpg', + isEdited: false, + }); + await ctx.newAssetFile({ + assetId: asset.id, + type: AssetFileType.Thumbnail, + path: '/edited/thumbnail.jpg', + isEdited: true, + }); + + const auth = factory.auth({ user: { id: user.id } }); + + // Test default (should get original) + const resultDefault = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.THUMBNAIL }); + expect(resultDefault).toBeInstanceOf(ImmichFileResponse); + expect((resultDefault as ImmichFileResponse).path).toBe('/original/thumbnail.jpg'); + + // Test edited=true (should get edited) + const resultEdited = await sut.viewThumbnail(auth, asset.id, { size: AssetMediaSize.THUMBNAIL, edited: true }); + expect(resultEdited).toBeInstanceOf(ImmichFileResponse); + expect((resultEdited as ImmichFileResponse).path).toBe('/edited/thumbnail.jpg'); + }); + }); });