From 334fc250d396e590a1f3279fb8442586dc7e3b57 Mon Sep 17 00:00:00 2001 From: Nikhil Alapati <35281285+NikhilAlapati@users.noreply.github.com> Date: Fri, 27 Feb 2026 03:46:55 -0800 Subject: [PATCH] fix(server): Live Photo migration bug when album is in template (#25329) Co-authored-by: Nikhil Alapati --- server/src/queries/asset.job.repository.sql | 4 + .../src/repositories/asset-job.repository.ts | 10 +- .../services/storage-template.service.spec.ts | 109 +++++++++++++++++- .../src/services/storage-template.service.ts | 38 ++++-- server/test/mappers.ts | 1 + 5 files changed, 144 insertions(+), 18 deletions(-) diff --git a/server/src/queries/asset.job.repository.sql b/server/src/queries/asset.job.repository.sql index 54b3c92dd4..a9c407782b 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/server/src/queries/asset.job.repository.sql @@ -562,6 +562,7 @@ select "asset"."checksum", "asset"."originalPath", "asset"."isExternal", + "asset"."visibility", "asset"."originalFileName", "asset"."livePhotoVideoId", "asset"."fileCreatedAt", @@ -593,6 +594,7 @@ from where "asset"."deletedAt" is null and "asset"."id" = $2 + and "asset"."visibility" != $3 -- AssetJobRepository.streamForStorageTemplateJob select @@ -602,6 +604,7 @@ select "asset"."checksum", "asset"."originalPath", "asset"."isExternal", + "asset"."visibility", "asset"."originalFileName", "asset"."livePhotoVideoId", "asset"."fileCreatedAt", @@ -632,6 +635,7 @@ from inner join "asset_exif" on "asset"."id" = "asset_exif"."assetId" where "asset"."deletedAt" is null + and "asset"."visibility" != $2 -- AssetJobRepository.streamForDeletedJob select diff --git a/server/src/repositories/asset-job.repository.ts b/server/src/repositories/asset-job.repository.ts index f4b93a775b..df9b50791f 100644 --- a/server/src/repositories/asset-job.repository.ts +++ b/server/src/repositories/asset-job.repository.ts @@ -353,6 +353,7 @@ export class AssetJobRepository { 'asset.checksum', 'asset.originalPath', 'asset.isExternal', + 'asset.visibility', 'asset.originalFileName', 'asset.livePhotoVideoId', 'asset.fileCreatedAt', @@ -367,13 +368,16 @@ export class AssetJobRepository { } @GenerateSql({ params: [DummyValue.UUID] }) - getForStorageTemplateJob(id: string) { - return this.storageTemplateAssetQuery().where('asset.id', '=', id).executeTakeFirst(); + getForStorageTemplateJob(id: string, options?: { includeHidden?: boolean }) { + return this.storageTemplateAssetQuery() + .where('asset.id', '=', id) + .$if(!options?.includeHidden, (qb) => qb.where('asset.visibility', '!=', AssetVisibility.Hidden)) + .executeTakeFirst(); } @GenerateSql({ params: [], stream: true }) streamForStorageTemplateJob() { - return this.storageTemplateAssetQuery().stream(); + return this.storageTemplateAssetQuery().where('asset.visibility', '!=', AssetVisibility.Hidden).stream(); } @GenerateSql({ params: [DummyValue.DATE], stream: true }) diff --git a/server/src/services/storage-template.service.spec.ts b/server/src/services/storage-template.service.spec.ts index 09e0c10b80..57343bb622 100644 --- a/server/src/services/storage-template.service.spec.ts +++ b/server/src/services/storage-template.service.spec.ts @@ -9,6 +9,9 @@ import { userStub } from 'test/fixtures/user.stub'; import { getForStorageTemplate } from 'test/mappers'; import { makeStream, newTestService, ServiceMocks } from 'test/utils'; +const motionAsset = AssetFactory.from({ type: AssetType.Video }).exif().build(); +const stillAsset = AssetFactory.from({ livePhotoVideoId: motionAsset.id }).exif().build(); + describe(StorageTemplateService.name, () => { let sut: StorageTemplateService; let mocks: ServiceMocks; @@ -153,6 +156,58 @@ describe(StorageTemplateService.name, () => { expect(mocks.asset.update).toHaveBeenCalledWith({ id: motionAsset.id, originalPath: newMotionPicturePath }); }); + it('should migrate live photo motion video alongside the still image using album in path', async () => { + const motionAsset = AssetFactory.from({ + type: AssetType.Video, + fileCreatedAt: new Date('2022-06-19T23:41:36.910Z'), + }) + .exif() + .build(); + const stillAsset = AssetFactory.from({ + livePhotoVideoId: motionAsset.id, + fileCreatedAt: new Date('2022-06-19T23:41:36.910Z'), + }) + .exif() + .build(); + + const album = AlbumFactory.from().asset().build(); + const config = structuredClone(defaults); + config.storageTemplate.template = '{{y}}/{{#if album}}{{album}}{{else}}other/{{MM}}{{/if}}/{{filename}}'; + sut.onConfigInit({ newConfig: config }); + + mocks.user.get.mockResolvedValue(userStub.user1); + + const newMotionPicturePath = `/data/library/${motionAsset.ownerId}/2022/${album.albumName}/${stillAsset.originalFileName.slice(0, -4)}.mp4`; + const newStillPicturePath = `/data/library/${stillAsset.ownerId}/2022/${album.albumName}/${stillAsset.originalFileName}`; + + mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(stillAsset)); + mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(motionAsset)); + mocks.album.getByAssetId.mockResolvedValue([album]); + + mocks.move.create.mockResolvedValueOnce({ + id: '123', + entityId: stillAsset.id, + pathType: AssetPathType.Original, + oldPath: stillAsset.originalPath, + newPath: newStillPicturePath, + }); + + mocks.move.create.mockResolvedValueOnce({ + id: '124', + entityId: motionAsset.id, + pathType: AssetPathType.Original, + oldPath: motionAsset.originalPath, + newPath: newMotionPicturePath, + }); + + await expect(sut.handleMigrationSingle({ id: stillAsset.id })).resolves.toBe(JobStatus.Success); + + expect(mocks.storage.checkFileExists).toHaveBeenCalledTimes(2); + expect(mocks.album.getByAssetId).toHaveBeenCalledWith(stillAsset.ownerId, stillAsset.id); + expect(mocks.asset.update).toHaveBeenCalledWith({ id: stillAsset.id, originalPath: newStillPicturePath }); + expect(mocks.asset.update).toHaveBeenCalledWith({ id: motionAsset.id, originalPath: newMotionPicturePath }); + }); + it('should use handlebar if condition for album', async () => { const user = UserFactory.create(); const asset = AssetFactory.from().owner(user).exif().build(); @@ -709,12 +764,18 @@ describe(StorageTemplateService.name, () => { }) .exif() .build(); - const newMotionPicturePath = `/data/library/${motionAsset.ownerId}/2022/2022-06-19/${stillAsset.originalFileName.slice(0, -4)}.mp4`; - const newStillPicturePath = `/data/library/${stillAsset.ownerId}/2022/2022-06-19/${stillAsset.originalFileName}`; + const album = AlbumFactory.from().asset().build(); + const config = structuredClone(defaults); + config.storageTemplate.template = '{{y}}/{{#if album}}{{album}}{{else}}other/{{MM}}{{/if}}/{{filename}}'; + sut.onConfigInit({ newConfig: config }); + + const newMotionPicturePath = `/data/library/${motionAsset.ownerId}/2022/${album.albumName}/${stillAsset.originalFileName.slice(0, -4)}.mp4`; + const newStillPicturePath = `/data/library/${stillAsset.ownerId}/2022/${album.albumName}/${stillAsset.originalFileName}`; mocks.assetJob.streamForStorageTemplateJob.mockReturnValue(makeStream([getForStorageTemplate(stillAsset)])); mocks.user.getList.mockResolvedValue([userStub.user1]); mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(motionAsset)); + mocks.album.getByAssetId.mockResolvedValue([album]); mocks.move.create.mockResolvedValueOnce({ id: '123', @@ -735,11 +796,53 @@ describe(StorageTemplateService.name, () => { await sut.handleMigration(); expect(mocks.assetJob.streamForStorageTemplateJob).toHaveBeenCalled(); - expect(mocks.assetJob.getForStorageTemplateJob).toHaveBeenCalledWith(motionAsset.id); expect(mocks.storage.checkFileExists).toHaveBeenCalledTimes(2); expect(mocks.asset.update).toHaveBeenCalledWith({ id: stillAsset.id, originalPath: newStillPicturePath }); expect(mocks.asset.update).toHaveBeenCalledWith({ id: motionAsset.id, originalPath: newMotionPicturePath }); }); + + it('should use still photo album info when migrating live photo motion video', async () => { + const user = userStub.user1; + const album = AlbumFactory.from().asset().build(); + const config = structuredClone(defaults); + config.storageTemplate.template = '{{y}}/{{#if album}}{{album}}{{else}}other{{/if}}/{{filename}}'; + + sut.onConfigInit({ newConfig: config }); + + mocks.assetJob.streamForStorageTemplateJob.mockReturnValue(makeStream([getForStorageTemplate(stillAsset)])); + mocks.user.getList.mockResolvedValue([user]); + mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(motionAsset)); + mocks.album.getByAssetId.mockResolvedValue([album]); + + mocks.move.create.mockResolvedValueOnce({ + id: '123', + entityId: stillAsset.id, + pathType: AssetPathType.Original, + oldPath: stillAsset.originalPath, + newPath: `/data/library/${user.id}/2022/${album.albumName}/${stillAsset.originalFileName}`, + }); + + mocks.move.create.mockResolvedValueOnce({ + id: '124', + entityId: motionAsset.id, + pathType: AssetPathType.Original, + oldPath: motionAsset.originalPath, + newPath: `/data/library/${user.id}/2022/${album.albumName}/${motionAsset.originalFileName}`, + }); + + await sut.handleMigration(); + + expect(mocks.album.getByAssetId).toHaveBeenCalledWith(stillAsset.ownerId, stillAsset.id); + expect(mocks.album.getByAssetId).toHaveBeenCalledTimes(2); + expect(mocks.asset.update).toHaveBeenCalledWith({ + id: stillAsset.id, + originalPath: expect.stringContaining(`/${album.albumName}/`), + }); + expect(mocks.asset.update).toHaveBeenCalledWith({ + id: motionAsset.id, + originalPath: expect.stringContaining(`/${album.albumName}/`), + }); + }); }); describe('file rename correctness', () => { diff --git a/server/src/services/storage-template.service.ts b/server/src/services/storage-template.service.ts index a8f4e6a185..3d1bc8f835 100644 --- a/server/src/services/storage-template.service.ts +++ b/server/src/services/storage-template.service.ts @@ -158,12 +158,14 @@ export class StorageTemplateService extends BaseService { // move motion part of live photo if (asset.livePhotoVideoId) { - const livePhotoVideo = await this.assetJobRepository.getForStorageTemplateJob(asset.livePhotoVideoId); + const livePhotoVideo = await this.assetJobRepository.getForStorageTemplateJob(asset.livePhotoVideoId, { + includeHidden: true, + }); if (!livePhotoVideo) { return JobStatus.Failed; } const motionFilename = getLivePhotoMotionFilename(filename, livePhotoVideo.originalPath); - await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename }); + await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename }, asset); } return JobStatus.Success; } @@ -191,10 +193,12 @@ export class StorageTemplateService extends BaseService { // move motion part of live photo if (asset.livePhotoVideoId) { - const livePhotoVideo = await this.assetJobRepository.getForStorageTemplateJob(asset.livePhotoVideoId); + const livePhotoVideo = await this.assetJobRepository.getForStorageTemplateJob(asset.livePhotoVideoId, { + includeHidden: true, + }); if (livePhotoVideo) { const motionFilename = getLivePhotoMotionFilename(filename, livePhotoVideo.originalPath); - await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename }); + await this.moveAsset(livePhotoVideo, { storageLabel, filename: motionFilename }, asset); } } } @@ -214,7 +218,7 @@ export class StorageTemplateService extends BaseService { await this.moveRepository.cleanMoveHistorySingle(assetId); } - async moveAsset(asset: StorageAsset, metadata: MoveAssetMetadata) { + async moveAsset(asset: StorageAsset, metadata: MoveAssetMetadata, stillPhoto?: StorageAsset) { if (asset.isExternal || StorageCore.isAndroidMotionPath(asset.originalPath)) { // External assets are not affected by storage template // TODO: shouldn't this only apply to external assets? @@ -224,7 +228,7 @@ export class StorageTemplateService extends BaseService { return this.databaseRepository.withLock(DatabaseLock.StorageTemplateMigration, async () => { const { id, originalPath, checksum, fileSizeInByte } = asset; const oldPath = originalPath; - const newPath = await this.getTemplatePath(asset, metadata); + const newPath = await this.getTemplatePath(asset, metadata, stillPhoto); if (!fileSizeInByte) { this.logger.error(`Asset ${id} missing exif info, skipping storage template migration`); @@ -255,7 +259,11 @@ export class StorageTemplateService extends BaseService { }); } - private async getTemplatePath(asset: StorageAsset, metadata: MoveAssetMetadata): Promise { + private async getTemplatePath( + asset: StorageAsset, + metadata: MoveAssetMetadata, + stillPhoto?: StorageAsset, + ): Promise { const { storageLabel, filename } = metadata; try { @@ -296,8 +304,12 @@ export class StorageTemplateService extends BaseService { let albumName = null; let albumStartDate = null; let albumEndDate = null; + const assetForMetadata = stillPhoto || asset; + if (this.template.needsAlbum) { - const albums = await this.albumRepository.getByAssetId(asset.ownerId, asset.id); + // For motion videos, use the still photo's album information since motion videos + // don't have album metadata attached directly + const albums = await this.albumRepository.getByAssetId(assetForMetadata.ownerId, assetForMetadata.id); const album = albums?.[0]; if (album) { albumName = album.albumName || null; @@ -310,16 +322,18 @@ export class StorageTemplateService extends BaseService { } } + // For motion videos that are part of live photos, use the still photo's date + // to ensure both parts end up in the same folder const storagePath = this.render(this.template.compiled, { - asset, + asset: assetForMetadata, filename: sanitized, extension, albumName, albumStartDate, albumEndDate, - make: asset.make, - model: asset.model, - lensModel: asset.lensModel, + make: assetForMetadata.make, + model: assetForMetadata.model, + lensModel: assetForMetadata.lensModel, }); const fullPath = path.normalize(path.join(rootPath, storagePath)); let destination = `${fullPath}.${extension}`; diff --git a/server/test/mappers.ts b/server/test/mappers.ts index eb57c10e2e..7ccd61a48c 100644 --- a/server/test/mappers.ts +++ b/server/test/mappers.ts @@ -12,6 +12,7 @@ export const getForStorageTemplate = (asset: ReturnType) isExternal: asset.isExternal, checksum: asset.checksum, timeZone: asset.exifInfo.timeZone, + visibility: asset.visibility, fileCreatedAt: asset.fileCreatedAt, originalPath: asset.originalPath, originalFileName: asset.originalFileName,