From f3b0e8a5e68c2f323ffec3145952bd8974d3f3bb Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Tue, 26 Aug 2025 11:27:11 -0400 Subject: [PATCH 1/3] fix: sidecar check job --- server/src/enum.ts | 3 +- server/src/queries/asset.job.repository.sql | 12 ++ .../src/repositories/asset-job.repository.ts | 18 ++- server/src/services/job.service.spec.ts | 4 +- server/src/services/job.service.ts | 3 +- server/src/services/library.service.spec.ts | 4 +- server/src/services/library.service.ts | 2 +- server/src/services/metadata.service.spec.ts | 148 ++++++------------ server/src/services/metadata.service.ts | 118 ++++++-------- server/src/types.ts | 5 +- 10 files changed, 133 insertions(+), 184 deletions(-) diff --git a/server/src/enum.ts b/server/src/enum.ts index 02ef222883..39110a49d9 100644 --- a/server/src/enum.ts +++ b/server/src/enum.ts @@ -566,8 +566,7 @@ export enum JobName { SendMail = 'SendMail', SidecarQueueAll = 'SidecarQueueAll', - SidecarDiscovery = 'SidecarDiscovery', - SidecarSync = 'SidecarSync', + SidecarCheck = 'SidecarCheck', SidecarWrite = 'SidecarWrite', SmartSearchQueueAll = 'SmartSearchQueueAll', diff --git a/server/src/queries/asset.job.repository.sql b/server/src/queries/asset.job.repository.sql index df8163be3e..09ecc958c2 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/server/src/queries/asset.job.repository.sql @@ -43,6 +43,18 @@ where limit $2 +-- AssetJobRepository.getForSidecarCheckJob +select + "id", + "sidecarPath", + "originalPath" +from + "asset" +where + "asset"."id" = $1::uuid +limit + $2 + -- AssetJobRepository.streamForThumbnailJob select "asset"."id", diff --git a/server/src/repositories/asset-job.repository.ts b/server/src/repositories/asset-job.repository.ts index 0500bb867f..4ddbc374b7 100644 --- a/server/src/repositories/asset-job.repository.ts +++ b/server/src/repositories/asset-job.repository.ts @@ -39,10 +39,8 @@ export class AssetJobRepository { return this.db .selectFrom('asset') .where('asset.id', '=', asUuid(id)) - .select((eb) => [ - 'id', - 'sidecarPath', - 'originalPath', + .select(['id', 'sidecarPath', 'originalPath']) + .select((eb) => jsonArrayFrom( eb .selectFrom('tag') @@ -50,7 +48,17 @@ export class AssetJobRepository { .innerJoin('tag_asset', 'tag.id', 'tag_asset.tagsId') .whereRef('asset.id', '=', 'tag_asset.assetsId'), ).as('tags'), - ]) + ) + .limit(1) + .executeTakeFirst(); + } + + @GenerateSql({ params: [DummyValue.UUID] }) + getForSidecarCheckJob(id: string) { + return this.db + .selectFrom('asset') + .where('asset.id', '=', asUuid(id)) + .select(['id', 'sidecarPath', 'originalPath']) .limit(1) .executeTakeFirst(); } diff --git a/server/src/services/job.service.spec.ts b/server/src/services/job.service.spec.ts index 63d5fb2d06..a758f39244 100644 --- a/server/src/services/job.service.spec.ts +++ b/server/src/services/job.service.spec.ts @@ -238,11 +238,11 @@ describe(JobService.name, () => { const tests: Array<{ item: JobItem; jobs: JobName[]; stub?: any }> = [ { - item: { name: JobName.SidecarSync, data: { id: 'asset-1' } }, + item: { name: JobName.SidecarCheck, data: { id: 'asset-1' } }, jobs: [JobName.AssetExtractMetadata], }, { - item: { name: JobName.SidecarDiscovery, data: { id: 'asset-1' } }, + item: { name: JobName.SidecarCheck, data: { id: 'asset-1' } }, jobs: [JobName.AssetExtractMetadata], }, { diff --git a/server/src/services/job.service.ts b/server/src/services/job.service.ts index 0116c869c6..a5c48ab43a 100644 --- a/server/src/services/job.service.ts +++ b/server/src/services/job.service.ts @@ -309,8 +309,7 @@ export class JobService extends BaseService { */ private async onDone(item: JobItem) { switch (item.name) { - case JobName.SidecarSync: - case JobName.SidecarDiscovery: { + case JobName.SidecarCheck: { await this.jobRepository.queue({ name: JobName.AssetExtractMetadata, data: item.data }); break; } diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index edd0d46bce..64f0915698 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -527,7 +527,7 @@ describe(LibraryService.name, () => { expect(mocks.job.queueAll).toHaveBeenCalledWith([ { - name: JobName.SidecarDiscovery, + name: JobName.SidecarCheck, data: { id: assetStub.external.id, source: 'upload', @@ -573,7 +573,7 @@ describe(LibraryService.name, () => { expect(mocks.job.queueAll).toHaveBeenCalledWith([ { - name: JobName.SidecarDiscovery, + name: JobName.SidecarCheck, data: { id: assetStub.image.id, source: 'upload', diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index c5f6971a83..f4a1992d91 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -414,7 +414,7 @@ export class LibraryService extends BaseService { // We queue a sidecar discovery which, in turn, queues metadata extraction await this.jobRepository.queueAll( assetIds.map((assetId) => ({ - name: JobName.SidecarDiscovery, + name: JobName.SidecarCheck, data: { id: assetId, source: 'upload' }, })), ); diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 1e304137e4..413b20a954 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -1,7 +1,6 @@ import { BinaryField, ExifDateTime } from 'exiftool-vendored'; import { randomBytes } from 'node:crypto'; import { Stats } from 'node:fs'; -import { constants } from 'node:fs/promises'; import { defaults } from 'src/config'; import { MapAsset } from 'src/dtos/asset-response.dto'; import { AssetType, AssetVisibility, ExifOrientation, ImmichWorker, JobName, JobStatus, SourceType } from 'src/enum'; @@ -15,6 +14,21 @@ import { tagStub } from 'test/fixtures/tag.stub'; import { factory } from 'test/small.factory'; import { makeStream, newTestService, ServiceMocks } from 'test/utils'; +const forSidecarJob = ( + asset: { + id?: string; + originalPath?: string; + sidecarPath?: string | null; + } = {}, +) => { + return { + id: factory.uuid(), + originalPath: '/path/to/IMG_123.jpg', + sidecarPath: null, + ...asset, + }; +}; + const makeFaceTags = (face: Partial<{ Name: string }> = {}, orientation?: ImmichTags['Orientation']) => ({ Orientation: orientation, RegionInfo: { @@ -1457,7 +1471,7 @@ describe(MetadataService.name, () => { expect(mocks.job.queueAll).toHaveBeenCalledWith([ { - name: JobName.SidecarSync, + name: JobName.SidecarCheck, data: { id: assetStub.sidecar.id }, }, ]); @@ -1471,133 +1485,65 @@ describe(MetadataService.name, () => { expect(mocks.assetJob.streamForSidecar).toHaveBeenCalledWith(false); expect(mocks.job.queueAll).toHaveBeenCalledWith([ { - name: JobName.SidecarDiscovery, + name: JobName.SidecarCheck, data: { id: assetStub.image.id }, }, ]); }); }); - describe('handleSidecarSync', () => { + describe('handleSidecarCheck', () => { it('should do nothing if asset could not be found', async () => { - mocks.asset.getByIds.mockResolvedValue([]); - await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.Failed); + mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(void 0); + + await expect(sut.handleSidecarCheck({ id: assetStub.image.id })).resolves.toBeUndefined(); + expect(mocks.asset.update).not.toHaveBeenCalled(); }); - it('should do nothing if asset has no sidecar path', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); - await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.Failed); - expect(mocks.asset.update).not.toHaveBeenCalled(); + it('should detect a new sidecar at .jpg.xmp', async () => { + const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg' }); + + mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); + mocks.storage.checkFileExists.mockResolvedValueOnce(true); + + await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Success); + + expect(mocks.asset.update).toHaveBeenCalledWith({ id: asset.id, sidecarPath: `/path/to/IMG_123.jpg.xmp` }); }); - it('should set sidecar path if exists (sidecar named photo.ext.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); - mocks.storage.checkFileExists.mockResolvedValue(true); + it('should detect a new sidecar at .xmp', async () => { + const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg' }); - await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); - expect(mocks.storage.checkFileExists).toHaveBeenCalledWith( - `${assetStub.sidecar.originalPath}.xmp`, - constants.R_OK, - ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: assetStub.sidecar.sidecarPath, - }); - }); - - it('should set sidecar path if exists (sidecar named photo.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecarWithoutExt as any]); + mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); mocks.storage.checkFileExists.mockResolvedValueOnce(false); mocks.storage.checkFileExists.mockResolvedValueOnce(true); - await expect(sut.handleSidecarSync({ id: assetStub.sidecarWithoutExt.id })).resolves.toBe(JobStatus.Success); - expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith( - 2, - assetStub.sidecarWithoutExt.sidecarPath, - constants.R_OK, - ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecarWithoutExt.id, - sidecarPath: assetStub.sidecarWithoutExt.sidecarPath, - }); - }); + await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Success); - it('should set sidecar path if exists (two sidecars named photo.ext.xmp and photo.xmp, should pick photo.ext.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); - mocks.storage.checkFileExists.mockResolvedValueOnce(true); - mocks.storage.checkFileExists.mockResolvedValueOnce(true); - - await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); - expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith(1, assetStub.sidecar.sidecarPath, constants.R_OK); - expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith( - 2, - assetStub.sidecarWithoutExt.sidecarPath, - constants.R_OK, - ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: assetStub.sidecar.sidecarPath, - }); + expect(mocks.asset.update).toHaveBeenCalledWith({ id: asset.id, sidecarPath: '/path/to/IMG_123.xmp' }); }); it('should unset sidecar path if file does not exist anymore', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); + const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg', sidecarPath: '/path/to/IMG_123.jpg.xmp' }); + mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); mocks.storage.checkFileExists.mockResolvedValue(false); - await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); - expect(mocks.storage.checkFileExists).toHaveBeenCalledWith( - `${assetStub.sidecar.originalPath}.xmp`, - constants.R_OK, - ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: null, - }); - }); - }); + await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Success); - describe('handleSidecarDiscovery', () => { - it('should skip hidden assets', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset as any]); - await sut.handleSidecarDiscovery({ id: assetStub.livePhotoMotionAsset.id }); - expect(mocks.storage.checkFileExists).not.toHaveBeenCalled(); + expect(mocks.asset.update).toHaveBeenCalledWith({ id: asset.id, sidecarPath: null }); }); - it('should skip assets with a sidecar path', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); - await sut.handleSidecarDiscovery({ id: assetStub.sidecar.id }); - expect(mocks.storage.checkFileExists).not.toHaveBeenCalled(); - }); + it('should do nothing if the sidecar file still exists', async () => { + const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg', sidecarPath: '/path/to/IMG_123.jpg' }); + + mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); + mocks.storage.checkFileExists.mockResolvedValueOnce(true); + + await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Skipped); - it('should do nothing when a sidecar is not found ', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); - mocks.storage.checkFileExists.mockResolvedValue(false); - await sut.handleSidecarDiscovery({ id: assetStub.image.id }); expect(mocks.asset.update).not.toHaveBeenCalled(); }); - - it('should update a image asset when a sidecar is found', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); - mocks.storage.checkFileExists.mockResolvedValue(true); - await sut.handleSidecarDiscovery({ id: assetStub.image.id }); - expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.jpg.xmp', constants.R_OK); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.image.id, - sidecarPath: '/original/path.jpg.xmp', - }); - }); - - it('should update a video asset when a sidecar is found', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.video]); - mocks.storage.checkFileExists.mockResolvedValue(true); - await sut.handleSidecarDiscovery({ id: assetStub.video.id }); - expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.R_OK); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.image.id, - sidecarPath: '/original/path.ext.xmp', - }); - }); }); describe('handleSidecarWrite', () => { diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index c675b7200d..fd4a2ffa48 100644 --- a/server/src/services/metadata.service.ts +++ b/server/src/services/metadata.service.ts @@ -5,7 +5,7 @@ import _ from 'lodash'; import { Duration } from 'luxon'; import { Stats } from 'node:fs'; import { constants } from 'node:fs/promises'; -import path from 'node:path'; +import { join, parse } from 'node:path'; import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants'; import { StorageCore } from 'src/cores/storage.core'; import { Asset, AssetFace } from 'src/database'; @@ -330,7 +330,7 @@ export class MetadataService extends BaseService { const assets = this.assetJobRepository.streamForSidecar(force); for await (const asset of assets) { - jobs.push({ name: force ? JobName.SidecarSync : JobName.SidecarDiscovery, data: { id: asset.id } }); + jobs.push({ name: JobName.SidecarCheck, data: { id: asset.id } }); if (jobs.length >= JOBS_ASSET_PAGINATION_SIZE) { await queueAll(); } @@ -341,14 +341,37 @@ export class MetadataService extends BaseService { return JobStatus.Success; } - @OnJob({ name: JobName.SidecarSync, queue: QueueName.Sidecar }) - handleSidecarSync({ id }: JobOf): Promise { - return this.processSidecar(id, true); - } + @OnJob({ name: JobName.SidecarCheck, queue: QueueName.Sidecar }) + async handleSidecarCheck({ id }: JobOf): Promise { + const asset = await this.assetJobRepository.getForSidecarCheckJob(id); + if (!asset) { + return; + } - @OnJob({ name: JobName.SidecarDiscovery, queue: QueueName.Sidecar }) - handleSidecarDiscovery({ id }: JobOf): Promise { - return this.processSidecar(id, false); + let sidecarPath = null; + for (const candidate of this.getSidecarCandidates(asset)) { + const exists = await this.storageRepository.checkFileExists(candidate, constants.R_OK); + if (!exists) { + continue; + } + + sidecarPath = candidate; + break; + } + + const isChanged = sidecarPath !== asset.sidecarPath; + + this.logger.debug( + `Sidecar check found old=${asset.sidecarPath}, new=${sidecarPath} will ${isChanged ? 'update' : 'do nothing for'} asset ${asset.id}: ${asset.originalPath}`, + ); + + if (!isChanged) { + return JobStatus.Skipped; + } + + await this.assetRepository.update({ id: asset.id, sidecarPath }); + + return JobStatus.Success; } @OnEvent({ name: 'AssetTag' }) @@ -398,6 +421,25 @@ export class MetadataService extends BaseService { return JobStatus.Success; } + private getSidecarCandidates({ sidecarPath, originalPath }: { sidecarPath: string | null; originalPath: string }) { + const candidates: string[] = []; + + if (sidecarPath) { + candidates.push(sidecarPath); + } + + const assetPath = parse(originalPath); + + candidates.push( + // IMG_123.jpg.xmp + `${originalPath}.xmp`, + // IMG_123.xmp + `${join(assetPath.dir, assetPath.name)}.xmp`, + ); + + return candidates; + } + private getImageDimensions(exifTags: ImmichTags): { width?: number; height?: number } { /* * The "true" values for width and height are a bit hidden, depending on the camera model and file format. @@ -577,7 +619,7 @@ export class MetadataService extends BaseService { checksum, ownerId: asset.ownerId, originalPath: StorageCore.getAndroidMotionPath(asset, motionAssetId), - originalFileName: `${path.parse(asset.originalFileName).name}.mp4`, + originalFileName: `${parse(asset.originalFileName).name}.mp4`, visibility: AssetVisibility.Hidden, deviceAssetId: 'NONE', deviceId: 'NONE', @@ -889,60 +931,4 @@ export class MetadataService extends BaseService { return tags; } - - private async processSidecar(id: string, isSync: boolean): Promise { - const [asset] = await this.assetRepository.getByIds([id]); - - if (!asset) { - return JobStatus.Failed; - } - - if (isSync && !asset.sidecarPath) { - return JobStatus.Failed; - } - - if (!isSync && (asset.visibility === AssetVisibility.Hidden || asset.sidecarPath) && !asset.isExternal) { - return JobStatus.Failed; - } - - // XMP sidecars can come in two filename formats. For a photo named photo.ext, the filenames are photo.ext.xmp and photo.xmp - const assetPath = path.parse(asset.originalPath); - const assetPathWithoutExt = path.join(assetPath.dir, assetPath.name); - const sidecarPathWithoutExt = `${assetPathWithoutExt}.xmp`; - const sidecarPathWithExt = `${asset.originalPath}.xmp`; - - const [sidecarPathWithExtExists, sidecarPathWithoutExtExists] = await Promise.all([ - this.storageRepository.checkFileExists(sidecarPathWithExt, constants.R_OK), - this.storageRepository.checkFileExists(sidecarPathWithoutExt, constants.R_OK), - ]); - - let sidecarPath = null; - if (sidecarPathWithExtExists) { - sidecarPath = sidecarPathWithExt; - } else if (sidecarPathWithoutExtExists) { - sidecarPath = sidecarPathWithoutExt; - } - - if (asset.isExternal) { - if (sidecarPath !== asset.sidecarPath) { - await this.assetRepository.update({ id: asset.id, sidecarPath }); - } - return JobStatus.Success; - } - - if (sidecarPath) { - this.logger.debug(`Detected sidecar at '${sidecarPath}' for asset ${asset.id}: ${asset.originalPath}`); - await this.assetRepository.update({ id: asset.id, sidecarPath }); - return JobStatus.Success; - } - - if (!isSync) { - return JobStatus.Failed; - } - - this.logger.debug(`No sidecar found for asset ${asset.id}: ${asset.originalPath}`); - await this.assetRepository.update({ id: asset.id, sidecarPath: null }); - - return JobStatus.Success; - } } diff --git a/server/src/types.ts b/server/src/types.ts index 9cd1aa996b..0b346e1ea6 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -306,8 +306,7 @@ export type JobItem = // Sidecar Scanning | { name: JobName.SidecarQueueAll; data: IBaseJob } - | { name: JobName.SidecarDiscovery; data: IEntityJob } - | { name: JobName.SidecarSync; data: IEntityJob } + | { name: JobName.SidecarCheck; data: IEntityJob } | { name: JobName.SidecarWrite; data: ISidecarWriteJob } // Facial Recognition @@ -394,8 +393,8 @@ export interface VectorUpdateResult { } export interface ImmichFile extends Express.Multer.File { - /** sha1 hash of file */ uuid: string; + /** sha1 hash of file */ checksum: Buffer; } From 430af9a1450822f27c61b4df8de965b754826f39 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Thu, 21 Aug 2025 00:47:32 +0200 Subject: [PATCH 2/3] feat: move sidecars to asset_files --- e2e/src/api/specs/library.e2e-spec.ts | 2 +- server/src/cores/storage.core.ts | 2 +- server/src/database.ts | 2 - server/src/dtos/asset-response.dto.ts | 1 - server/src/enum.ts | 1 + server/src/queries/asset.job.repository.sql | 87 +++++++-- .../src/repositories/asset-job.repository.ts | 26 ++- server/src/repositories/asset.repository.ts | 8 + .../src/repositories/database.repository.ts | 1 - .../1755984055382-SidecarInAssetFile.ts | 24 +++ server/src/schema/tables/asset.table.ts | 3 - .../src/services/asset-media.service.spec.ts | 25 ++- server/src/services/asset-media.service.ts | 23 ++- server/src/services/asset.service.spec.ts | 4 +- server/src/services/asset.service.ts | 4 +- server/src/services/metadata.service.spec.ts | 174 +++++++++++------- server/src/services/metadata.service.ts | 81 +++++--- server/src/types.ts | 2 +- server/src/utils/asset.util.ts | 1 + server/test/fixtures/asset.stub.ts | 40 ++-- .../repositories/asset.repository.mock.ts | 1 + server/test/small.factory.ts | 18 +- 22 files changed, 370 insertions(+), 160 deletions(-) create mode 100644 server/src/schema/migrations/1755984055382-SidecarInAssetFile.ts diff --git a/e2e/src/api/specs/library.e2e-spec.ts b/e2e/src/api/specs/library.e2e-spec.ts index 59ff74cc43..4d67a84647 100644 --- a/e2e/src/api/specs/library.e2e-spec.ts +++ b/e2e/src/api/specs/library.e2e-spec.ts @@ -1006,7 +1006,7 @@ describe('/libraries', () => { rmSync(`${testAssetDir}/temp/xmp`, { recursive: true, force: true }); }); - it('should switch from using file metadata to file.xmp metadata when asset refreshes', async () => { + it('should switch from using file metadata to file.ext.xmp metadata when asset refreshes', async () => { const library = await utils.createLibrary(admin.accessToken, { ownerId: admin.userId, importPaths: [`${testAssetDirInternal}/temp/xmp`], diff --git a/server/src/cores/storage.core.ts b/server/src/cores/storage.core.ts index 06dde4644c..33a727e75a 100644 --- a/server/src/cores/storage.core.ts +++ b/server/src/cores/storage.core.ts @@ -303,7 +303,7 @@ export class StorageCore { return this.assetRepository.update({ id, encodedVideoPath: newPath }); } case AssetPathType.Sidecar: { - return this.assetRepository.update({ id, sidecarPath: newPath }); + return this.assetRepository.upsertFile({ assetId: id, type: AssetFileType.Sidecar, path: newPath }); } case PersonPathType.Face: { return this.personRepository.update({ id, thumbnailPath: newPath }); diff --git a/server/src/database.ts b/server/src/database.ts index f472c643ee..38865a1987 100644 --- a/server/src/database.ts +++ b/server/src/database.ts @@ -117,7 +117,6 @@ export type Asset = { originalFileName: string; originalPath: string; ownerId: string; - sidecarPath: string | null; type: AssetType; }; @@ -302,7 +301,6 @@ export const columns = { 'asset.originalFileName', 'asset.originalPath', 'asset.ownerId', - 'asset.sidecarPath', 'asset.type', ], assetFiles: ['asset_file.id', 'asset_file.path', 'asset_file.type'], diff --git a/server/src/dtos/asset-response.dto.ts b/server/src/dtos/asset-response.dto.ts index 98ed8669f0..33322989bf 100644 --- a/server/src/dtos/asset-response.dto.ts +++ b/server/src/dtos/asset-response.dto.ts @@ -117,7 +117,6 @@ export type MapAsset = { originalPath: string; owner?: User | null; ownerId: string; - sidecarPath: string | null; stack?: Stack | null; stackId: string | null; tags?: Tag[]; diff --git a/server/src/enum.ts b/server/src/enum.ts index 02ef222883..47ed7aca6b 100644 --- a/server/src/enum.ts +++ b/server/src/enum.ts @@ -43,6 +43,7 @@ export enum AssetFileType { FullSize = 'fullsize', Preview = 'preview', Thumbnail = 'thumbnail', + Sidecar = 'sidecar', } export enum AlbumUserRole { diff --git a/server/src/queries/asset.job.repository.sql b/server/src/queries/asset.job.repository.sql index df8163be3e..a4bd94db7c 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/server/src/queries/asset.job.repository.sql @@ -20,7 +20,6 @@ limit -- AssetJobRepository.getForSidecarWriteJob select "id", - "sidecarPath", "originalPath", ( select @@ -35,13 +34,29 @@ select where "asset"."id" = "tag_asset"."assetsId" ) as agg - ) as "tags" + ) as "tags", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "asset_file"."id", + "asset_file"."path", + "asset_file"."type" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 + ) as agg + ) as "files" from "asset" where - "asset"."id" = $1::uuid + "asset"."id" = $2::uuid limit - $2 + $3 -- AssetJobRepository.streamForThumbnailJob select @@ -146,7 +161,6 @@ select "asset"."originalFileName", "asset"."originalPath", "asset"."ownerId", - "asset"."sidecarPath", "asset"."type", ( select @@ -161,11 +175,27 @@ select "asset_face"."assetId" = "asset"."id" and "asset_face"."deletedAt" is null ) as agg - ) as "faces" + ) as "faces", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "asset_file"."id", + "asset_file"."path", + "asset_file"."type" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 + ) as agg + ) as "files" from "asset" where - "asset"."id" = $1 + "asset"."id" = $2 -- AssetJobRepository.getAlbumThumbnailFiles select @@ -293,7 +323,6 @@ select "asset"."libraryId", "asset"."ownerId", "asset"."livePhotoVideoId", - "asset"."sidecarPath", "asset"."encodedVideoPath", "asset"."originalPath", to_json("asset_exif") as "exifInfo", @@ -404,18 +433,28 @@ select "asset"."checksum", "asset"."originalPath", "asset"."isExternal", - "asset"."sidecarPath", "asset"."originalFileName", "asset"."livePhotoVideoId", "asset"."fileCreatedAt", "asset_exif"."timeZone", - "asset_exif"."fileSizeInByte" + "asset_exif"."fileSizeInByte", + ( + select + "asset_file"."path" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 + limit + $2 + ) as "sidecarPath" from "asset" inner join "asset_exif" on "asset"."id" = "asset_exif"."assetId" where "asset"."deletedAt" is null - and "asset"."id" = $1 + and "asset"."id" = $3 -- AssetJobRepository.streamForStorageTemplateJob select @@ -425,12 +464,22 @@ select "asset"."checksum", "asset"."originalPath", "asset"."isExternal", - "asset"."sidecarPath", "asset"."originalFileName", "asset"."livePhotoVideoId", "asset"."fileCreatedAt", "asset_exif"."timeZone", - "asset_exif"."fileSizeInByte" + "asset_exif"."fileSizeInByte", + ( + select + "asset_file"."path" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 + limit + $2 + ) as "sidecarPath" from "asset" inner join "asset_exif" on "asset"."id" = "asset_exif"."assetId" @@ -452,11 +501,15 @@ select from "asset" where - ( - "asset"."sidecarPath" = $1 - or "asset"."sidecarPath" is null + not exists ( + select + "asset_file"."id" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 ) - and "asset"."visibility" != $2 -- AssetJobRepository.streamForDetectFacesJob select diff --git a/server/src/repositories/asset-job.repository.ts b/server/src/repositories/asset-job.repository.ts index 0500bb867f..112070f124 100644 --- a/server/src/repositories/asset-job.repository.ts +++ b/server/src/repositories/asset-job.repository.ts @@ -41,7 +41,6 @@ export class AssetJobRepository { .where('asset.id', '=', asUuid(id)) .select((eb) => [ 'id', - 'sidecarPath', 'originalPath', jsonArrayFrom( eb @@ -51,6 +50,7 @@ export class AssetJobRepository { .whereRef('asset.id', '=', 'tag_asset.assetsId'), ).as('tags'), ]) + .select((eb) => withFiles(eb, AssetFileType.Sidecar)) .limit(1) .executeTakeFirst(); } @@ -113,6 +113,7 @@ export class AssetJobRepository { .selectFrom('asset') .select(columns.asset) .select(withFaces) + .select((eb) => withFiles(eb, AssetFileType.Sidecar)) .where('asset.id', '=', id) .executeTakeFirst(); } @@ -210,7 +211,6 @@ export class AssetJobRepository { 'asset.libraryId', 'asset.ownerId', 'asset.livePhotoVideoId', - 'asset.sidecarPath', 'asset.encodedVideoPath', 'asset.originalPath', ]) @@ -288,12 +288,19 @@ export class AssetJobRepository { 'asset.checksum', 'asset.originalPath', 'asset.isExternal', - 'asset.sidecarPath', 'asset.originalFileName', 'asset.livePhotoVideoId', 'asset.fileCreatedAt', 'asset_exif.timeZone', 'asset_exif.fileSizeInByte', + (eb) => + eb + .selectFrom('asset_file') + .select('asset_file.path') + .whereRef('asset_file.assetId', '=', 'asset.id') + .where('asset_file.type', '=', AssetFileType.Sidecar) + .limit(1) + .as('sidecarPath'), ]) .where('asset.deletedAt', 'is', null); } @@ -325,9 +332,18 @@ export class AssetJobRepository { .selectFrom('asset') .select(['asset.id']) .$if(!force, (qb) => - qb.where((eb) => eb.or([eb('asset.sidecarPath', '=', ''), eb('asset.sidecarPath', 'is', null)])), + qb.where((eb) => + eb.not( + eb.exists( + eb + .selectFrom('asset_file') + .select('asset_file.id') + .whereRef('asset_file.assetId', '=', 'asset.id') + .where('asset_file.type', '=', AssetFileType.Sidecar), + ), + ), + ), ) - .where('asset.visibility', '!=', AssetVisibility.Hidden) .stream(); } diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index 61ccbf6541..621004c99b 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -794,6 +794,14 @@ export class AssetRepository { .execute(); } + async deleteFile(file: Pick, 'assetId' | 'type'>): Promise { + await this.db + .deleteFrom('asset_file') + .where('assetId', '=', asUuid(file.assetId)) + .where('type', '=', file.type) + .execute(); + } + async deleteFiles(files: Pick, 'id'>[]): Promise { if (files.length === 0) { return; diff --git a/server/src/repositories/database.repository.ts b/server/src/repositories/database.repository.ts index e5d88339c8..f6e6bffd18 100644 --- a/server/src/repositories/database.repository.ts +++ b/server/src/repositories/database.repository.ts @@ -414,7 +414,6 @@ export class DatabaseRepository { .set((eb) => ({ originalPath: eb.fn('REGEXP_REPLACE', ['originalPath', source, target]), encodedVideoPath: eb.fn('REGEXP_REPLACE', ['encodedVideoPath', source, target]), - sidecarPath: eb.fn('REGEXP_REPLACE', ['sidecarPath', source, target]), })) .execute(); diff --git a/server/src/schema/migrations/1755984055382-SidecarInAssetFile.ts b/server/src/schema/migrations/1755984055382-SidecarInAssetFile.ts new file mode 100644 index 0000000000..f700733feb --- /dev/null +++ b/server/src/schema/migrations/1755984055382-SidecarInAssetFile.ts @@ -0,0 +1,24 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + await sql`INSERT INTO asset_file ("assetId", path, type) + SELECT + id, "sidecarPath", 'sidecar' + FROM asset + WHERE "sidecarPath" IS NOT NULL;`.execute(db); + + await sql`ALTER TABLE "asset" DROP COLUMN "sidecarPath";`.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`ALTER TABLE "asset" ADD "sidecarPath" character varying;`.execute(db); + + await sql` + UPDATE asset + SET "sidecarPath" = asset_file.path + FROM asset_file + WHERE asset.id = asset_file."assetId"; + `.execute(db); + + await sql`DELETE FROM asset_file WHERE type = 'sidecar';`.execute(db); +} diff --git a/server/src/schema/tables/asset.table.ts b/server/src/schema/tables/asset.table.ts index e92e01a1bd..b28fc99e4a 100644 --- a/server/src/schema/tables/asset.table.ts +++ b/server/src/schema/tables/asset.table.ts @@ -105,9 +105,6 @@ export class AssetTable { @Column({ index: true }) originalFileName!: string; - @Column({ nullable: true }) - sidecarPath!: string | null; - @Column({ type: 'bytea', nullable: true }) thumbhash!: Buffer | null; diff --git a/server/src/services/asset-media.service.spec.ts b/server/src/services/asset-media.service.spec.ts index 91bddeb6e9..eb34f142c6 100644 --- a/server/src/services/asset-media.service.spec.ts +++ b/server/src/services/asset-media.service.spec.ts @@ -171,7 +171,6 @@ const assetEntity = Object.freeze({ longitude: 10.703_075, }, livePhotoVideoId: null, - sidecarPath: null, } as MapAsset); const existingAsset = Object.freeze({ @@ -709,18 +708,22 @@ describe(AssetMediaService.name, () => { expect(mocks.asset.update).toHaveBeenCalledWith( expect.objectContaining({ id: existingAsset.id, - sidecarPath: null, originalFileName: 'photo1.jpeg', originalPath: 'fake_path/photo1.jpeg', }), ); expect(mocks.asset.create).toHaveBeenCalledWith( expect.objectContaining({ - sidecarPath: null, originalFileName: 'existing-filename.jpeg', originalPath: 'fake_path/asset_1.jpeg', }), ); + expect(mocks.asset.deleteFile).toHaveBeenCalledWith( + expect.objectContaining({ + assetId: existingAsset.id, + type: AssetFileType.Sidecar, + }), + ); expect(mocks.asset.updateAll).toHaveBeenCalledWith([copiedAsset.id], { deletedAt: expect.any(Date), @@ -757,6 +760,13 @@ describe(AssetMediaService.name, () => { deletedAt: expect.any(Date), status: AssetStatus.Trashed, }); + expect(mocks.asset.upsertFile).toHaveBeenCalledWith( + expect.objectContaining({ + assetId: existingAsset.id, + path: sidecarFile.originalPath, + type: AssetFileType.Sidecar, + }), + ); expect(mocks.user.updateUsage).toHaveBeenCalledWith(authStub.user1.user.id, updatedFile.size); expect(mocks.storage.utimes).toHaveBeenCalledWith( updatedFile.originalPath, @@ -786,6 +796,12 @@ describe(AssetMediaService.name, () => { deletedAt: expect.any(Date), status: AssetStatus.Trashed, }); + expect(mocks.asset.deleteFile).toHaveBeenCalledWith( + expect.objectContaining({ + assetId: existingAsset.id, + type: AssetFileType.Sidecar, + }), + ); expect(mocks.user.updateUsage).toHaveBeenCalledWith(authStub.user1.user.id, updatedFile.size); expect(mocks.storage.utimes).toHaveBeenCalledWith( updatedFile.originalPath, @@ -815,6 +831,9 @@ describe(AssetMediaService.name, () => { expect(mocks.asset.create).not.toHaveBeenCalled(); expect(mocks.asset.updateAll).not.toHaveBeenCalled(); + expect(mocks.asset.upsertFile).not.toHaveBeenCalled(); + expect(mocks.asset.deleteFile).not.toHaveBeenCalled(); + expect(mocks.job.queue).toHaveBeenCalledWith({ name: JobName.FileDelete, data: { files: [updatedFile.originalPath, undefined] }, diff --git a/server/src/services/asset-media.service.ts b/server/src/services/asset-media.service.ts index 517a1f665f..9ae200a2ec 100644 --- a/server/src/services/asset-media.service.ts +++ b/server/src/services/asset-media.service.ts @@ -21,7 +21,16 @@ import { UploadFieldName, } from 'src/dtos/asset-media.dto'; import { AuthDto } from 'src/dtos/auth.dto'; -import { AssetStatus, AssetType, AssetVisibility, CacheControl, JobName, Permission, StorageFolder } from 'src/enum'; +import { + AssetFileType, + AssetStatus, + AssetType, + AssetVisibility, + CacheControl, + JobName, + Permission, + StorageFolder, +} from 'src/enum'; import { AuthRequest } from 'src/middleware/auth.guard'; import { BaseService } from 'src/services/base.service'; import { UploadFile } from 'src/types'; @@ -360,9 +369,12 @@ export class AssetMediaService extends BaseService { duration: dto.duration || null, livePhotoVideoId: null, - sidecarPath: sidecarPath || null, }); + await (sidecarPath + ? this.assetRepository.upsertFile({ assetId, path: sidecarPath, type: AssetFileType.Sidecar }) + : this.assetRepository.deleteFile({ assetId, type: AssetFileType.Sidecar })); + await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt)); await this.assetRepository.upsertExif({ assetId, fileSizeInByte: file.size }); await this.jobRepository.queue({ @@ -390,7 +402,6 @@ export class AssetMediaService extends BaseService { localDateTime: asset.localDateTime, fileModifiedAt: asset.fileModifiedAt, livePhotoVideoId: asset.livePhotoVideoId, - sidecarPath: asset.sidecarPath, }); const { size } = await this.storageRepository.stat(created.originalPath); @@ -420,10 +431,14 @@ export class AssetMediaService extends BaseService { visibility: dto.visibility ?? AssetVisibility.Timeline, livePhotoVideoId: dto.livePhotoVideoId, originalFileName: dto.filename || file.originalName, - sidecarPath: sidecarFile?.originalPath, }); if (sidecarFile) { + await this.assetRepository.upsertFile({ + assetId: asset.id, + path: sidecarFile.originalPath, + type: AssetFileType.Sidecar, + }); await this.storageRepository.utimes(sidecarFile.originalPath, new Date(), new Date(dto.fileModifiedAt)); } await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt)); diff --git a/server/src/services/asset.service.spec.ts b/server/src/services/asset.service.spec.ts index 7b29b8ab96..d05f4f0945 100755 --- a/server/src/services/asset.service.spec.ts +++ b/server/src/services/asset.service.spec.ts @@ -585,8 +585,8 @@ describe(AssetService.name, () => { '/uploads/user-id/webp/path.ext', '/uploads/user-id/thumbs/path.jpg', '/uploads/user-id/fullsize/path.webp', - assetWithFace.encodedVideoPath, - assetWithFace.sidecarPath, + assetWithFace.encodedVideoPath, // this value is null + undefined, // no sidecar path assetWithFace.originalPath, ], }, diff --git a/server/src/services/asset.service.ts b/server/src/services/asset.service.ts index 9a2c580707..7c43eba61b 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -247,11 +247,11 @@ export class AssetService extends BaseService { } } - const { fullsizeFile, previewFile, thumbnailFile } = getAssetFiles(asset.files ?? []); + const { fullsizeFile, previewFile, thumbnailFile, sidecarFile } = getAssetFiles(asset.files ?? []); const files = [thumbnailFile?.path, previewFile?.path, fullsizeFile?.path, asset.encodedVideoPath]; if (deleteOnDisk) { - files.push(asset.sidecarPath, asset.originalPath); + files.push(sidecarFile?.path, asset.originalPath); } await this.jobRepository.queue({ name: JobName.FileDelete, data: { files } }); diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 1e304137e4..ba4fce9f47 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -4,7 +4,16 @@ import { Stats } from 'node:fs'; import { constants } from 'node:fs/promises'; import { defaults } from 'src/config'; import { MapAsset } from 'src/dtos/asset-response.dto'; -import { AssetType, AssetVisibility, ExifOrientation, ImmichWorker, JobName, JobStatus, SourceType } from 'src/enum'; +import { + AssetFileType, + AssetType, + AssetVisibility, + ExifOrientation, + ImmichWorker, + JobName, + JobStatus, + SourceType, +} from 'src/enum'; import { ImmichTags } from 'src/repositories/metadata.repository'; import { firstDateTime, MetadataService } from 'src/services/metadata.service'; import { assetStub } from 'test/fixtures/asset.stub'; @@ -35,6 +44,13 @@ const makeFaceTags = (face: Partial<{ Name: string }> = {}, orientation?: Immich }, }); +function removeNonSidecarFiles(asset: any) { + return { + ...asset, + files: asset.files.filter((file: any) => file.type === AssetFileType.Sidecar), + }; +} + describe(MetadataService.name, () => { let sut: MetadataService; let mocks: ServiceMocks; @@ -151,7 +167,7 @@ describe(MetadataService.name, () => { it('should handle a date in a sidecar file', async () => { const originalDate = new Date('2023-11-21T16:13:17.517Z'); const sidecarDate = new Date('2022-01-01T00:00:00.000Z'); - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.sidecar); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.sidecar)); mockReadTags({ CreationDate: originalDate.toISOString() }, { CreationDate: sidecarDate.toISOString() }); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -170,7 +186,7 @@ describe(MetadataService.name, () => { it('should take the file modification date when missing exif and earlier than creation date', async () => { const fileCreatedAt = new Date('2022-01-01T00:00:00.000Z'); const fileModifiedAt = new Date('2021-01-01T00:00:00.000Z'); - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mocks.storage.stat.mockResolvedValue({ size: 123_456, mtime: fileModifiedAt, @@ -196,7 +212,7 @@ describe(MetadataService.name, () => { it('should take the file creation date when missing exif and earlier than modification date', async () => { const fileCreatedAt = new Date('2021-01-01T00:00:00.000Z'); const fileModifiedAt = new Date('2022-01-01T00:00:00.000Z'); - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mocks.storage.stat.mockResolvedValue({ size: 123_456, mtime: fileModifiedAt, @@ -219,7 +235,7 @@ describe(MetadataService.name, () => { it('should account for the server being in a non-UTC timezone', async () => { process.env.TZ = 'America/Los_Angeles'; - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.sidecar); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.sidecar)); mockReadTags({ DateTimeOriginal: '2022:01:01 00:00:00' }); await sut.handleMetadataExtraction({ id: assetStub.image.id }); @@ -237,7 +253,7 @@ describe(MetadataService.name, () => { }); it('should handle lists of numbers', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mocks.storage.stat.mockResolvedValue({ size: 123_456, mtime: assetStub.image.fileModifiedAt, @@ -290,7 +306,7 @@ describe(MetadataService.name, () => { }); it('should apply reverse geocoding', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.withLocation); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.withLocation)); mocks.systemMetadata.get.mockResolvedValue({ reverseGeocoding: { enabled: true } }); mocks.map.reverseGeocode.mockResolvedValue({ city: 'City', state: 'State', country: 'Country' }); mocks.storage.stat.mockResolvedValue({ @@ -319,7 +335,7 @@ describe(MetadataService.name, () => { }); it('should discard latitude and longitude on null island', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.withLocation); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.withLocation)); mockReadTags({ GPSLatitude: 0, GPSLongitude: 0, @@ -331,7 +347,7 @@ describe(MetadataService.name, () => { }); it('should extract tags from TagsList', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ TagsList: ['Parent'] }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -341,7 +357,7 @@ describe(MetadataService.name, () => { }); it('should extract hierarchy from TagsList', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ TagsList: ['Parent/Child'] }); mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); @@ -361,7 +377,7 @@ describe(MetadataService.name, () => { }); it('should extract tags from Keywords as a string', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ Keywords: 'Parent' }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -371,7 +387,7 @@ describe(MetadataService.name, () => { }); it('should extract tags from Keywords as a list', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ Keywords: ['Parent'] }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -381,7 +397,7 @@ describe(MetadataService.name, () => { }); it('should extract tags from Keywords as a list with a number', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ Keywords: ['Parent', 2024] }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -392,7 +408,7 @@ describe(MetadataService.name, () => { }); it('should extract hierarchal tags from Keywords', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ Keywords: 'Parent/Child' }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -411,7 +427,7 @@ describe(MetadataService.name, () => { }); it('should ignore Keywords when TagsList is present', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ Keywords: 'Child', TagsList: ['Parent/Child'] }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -430,7 +446,7 @@ describe(MetadataService.name, () => { }); it('should extract hierarchy from HierarchicalSubject', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ HierarchicalSubject: ['Parent|Child', 'TagA'] }); mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.parentUpsert); mocks.tag.upsertValue.mockResolvedValueOnce(tagStub.childUpsert); @@ -451,7 +467,7 @@ describe(MetadataService.name, () => { }); it('should extract tags from HierarchicalSubject as a list with a number', async () => { - mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); mockReadTags({ HierarchicalSubject: ['Parent', 2024] }); mocks.tag.upsertValue.mockResolvedValue(tagStub.parentUpsert); @@ -1479,20 +1495,20 @@ describe(MetadataService.name, () => { }); describe('handleSidecarSync', () => { - it('should do nothing if asset could not be found', async () => { - mocks.asset.getByIds.mockResolvedValue([]); + it("should do nothing if asset isn't found", async () => { + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(void 0); await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.Failed); expect(mocks.asset.update).not.toHaveBeenCalled(); }); - it('should do nothing if asset has no sidecar path', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); + it('should do nothing if asset has no sidecar file', async () => { + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); await expect(sut.handleSidecarSync({ id: assetStub.image.id })).resolves.toBe(JobStatus.Failed); expect(mocks.asset.update).not.toHaveBeenCalled(); }); it('should set sidecar path if exists (sidecar named photo.ext.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.sidecar)); mocks.storage.checkFileExists.mockResolvedValue(true); await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); @@ -1500,49 +1516,54 @@ describe(MetadataService.name, () => { `${assetStub.sidecar.originalPath}.xmp`, constants.R_OK, ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: assetStub.sidecar.sidecarPath, + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: assetStub.sidecar.id, + path: assetStub.sidecar.files[1].path, + type: AssetFileType.Sidecar, }); }); it('should set sidecar path if exists (sidecar named photo.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecarWithoutExt as any]); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue( + removeNonSidecarFiles(assetStub.sidecarWithoutExt as any), + ); mocks.storage.checkFileExists.mockResolvedValueOnce(false); mocks.storage.checkFileExists.mockResolvedValueOnce(true); await expect(sut.handleSidecarSync({ id: assetStub.sidecarWithoutExt.id })).resolves.toBe(JobStatus.Success); expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith( 2, - assetStub.sidecarWithoutExt.sidecarPath, + assetStub.sidecarWithoutExt.files[1].path, constants.R_OK, ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecarWithoutExt.id, - sidecarPath: assetStub.sidecarWithoutExt.sidecarPath, + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: assetStub.sidecarWithoutExt.id, + path: assetStub.sidecarWithoutExt.files[1].path, + type: AssetFileType.Sidecar, }); }); it('should set sidecar path if exists (two sidecars named photo.ext.xmp and photo.xmp, should pick photo.ext.xmp)', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.sidecar)); mocks.storage.checkFileExists.mockResolvedValueOnce(true); mocks.storage.checkFileExists.mockResolvedValueOnce(true); await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); - expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith(1, assetStub.sidecar.sidecarPath, constants.R_OK); + expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith(1, assetStub.sidecar.files[1].path, constants.R_OK); expect(mocks.storage.checkFileExists).toHaveBeenNthCalledWith( 2, - assetStub.sidecarWithoutExt.sidecarPath, + assetStub.sidecarWithoutExt.files[1].path, constants.R_OK, ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: assetStub.sidecar.sidecarPath, + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: assetStub.sidecar.id, + path: assetStub.sidecar.files[1].path, + type: AssetFileType.Sidecar, }); }); it('should unset sidecar path if file does not exist anymore', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.sidecar)); mocks.storage.checkFileExists.mockResolvedValue(false); await expect(sut.handleSidecarSync({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Success); @@ -1550,58 +1571,79 @@ describe(MetadataService.name, () => { `${assetStub.sidecar.originalPath}.xmp`, constants.R_OK, ); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.sidecar.id, - sidecarPath: null, + expect(mocks.asset.deleteFile).toHaveBeenCalledWith({ + assetId: assetStub.sidecar.id, + type: AssetFileType.Sidecar, }); }); }); describe('handleSidecarDiscovery', () => { it('should skip hidden assets', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.livePhotoMotionAsset as any]); - await sut.handleSidecarDiscovery({ id: assetStub.livePhotoMotionAsset.id }); - expect(mocks.storage.checkFileExists).not.toHaveBeenCalled(); - }); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.livePhotoMotionAsset as any); + await expect(sut.handleSidecarDiscovery({ id: assetStub.livePhotoMotionAsset.id })).resolves.toBe( + JobStatus.Skipped, + ); - it('should skip assets with a sidecar path', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.sidecar]); - await sut.handleSidecarDiscovery({ id: assetStub.sidecar.id }); expect(mocks.storage.checkFileExists).not.toHaveBeenCalled(); - }); - - it('should do nothing when a sidecar is not found ', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); - mocks.storage.checkFileExists.mockResolvedValue(false); - await sut.handleSidecarDiscovery({ id: assetStub.image.id }); expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.upsertFile).not.toHaveBeenCalled(); }); - it('should update a image asset when a sidecar is found', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.image]); - mocks.storage.checkFileExists.mockResolvedValue(true); - await sut.handleSidecarDiscovery({ id: assetStub.image.id }); + it('should skip assets that already have a known sidecar', async () => { + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.sidecar); + await expect(sut.handleSidecarDiscovery({ id: assetStub.sidecar.id })).resolves.toBe(JobStatus.Skipped); + + expect(mocks.storage.checkFileExists).not.toHaveBeenCalled(); + expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.upsertFile).not.toHaveBeenCalled(); + }); + + it('should do nothing when no sidecar is found on disk', async () => { + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); + mocks.storage.checkFileExists.mockResolvedValue(false); + await expect(sut.handleSidecarDiscovery({ id: assetStub.image.id })).resolves.toBe(JobStatus.Success); + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.jpg.xmp', constants.R_OK); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.image.id, - sidecarPath: '/original/path.jpg.xmp', + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.xmp', constants.R_OK); + + expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.upsertFile).not.toHaveBeenCalled(); + }); + + it('should update a image asset when a sidecar is found on disk', async () => { + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(removeNonSidecarFiles(assetStub.image)); + mocks.storage.checkFileExists.mockResolvedValue(true); + await expect(sut.handleSidecarDiscovery({ id: assetStub.image.id })).resolves.toBe(JobStatus.Success); + + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.jpg.xmp', constants.R_OK); + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.xmp', constants.R_OK); + + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: assetStub.image.id, + path: '/original/path.jpg.xmp', + type: AssetFileType.Sidecar, }); }); it('should update a video asset when a sidecar is found', async () => { - mocks.asset.getByIds.mockResolvedValue([assetStub.video]); + mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.video); mocks.storage.checkFileExists.mockResolvedValue(true); - await sut.handleSidecarDiscovery({ id: assetStub.video.id }); + await expect(sut.handleSidecarDiscovery({ id: assetStub.video.id })).resolves.toBe(JobStatus.Success); + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.R_OK); - expect(mocks.asset.update).toHaveBeenCalledWith({ - id: assetStub.image.id, - sidecarPath: '/original/path.ext.xmp', + expect(mocks.storage.checkFileExists).toHaveBeenCalledWith('/original/path.xmp', constants.R_OK); + + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: assetStub.image.id, + path: '/original/path.ext.xmp', + type: AssetFileType.Sidecar, }); }); }); describe('handleSidecarWrite', () => { - it('should skip assets that do not exist anymore', async () => { + it('should skip assets that no longer exist', async () => { mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(void 0); await expect(sut.handleSidecarWrite({ id: 'asset-123' })).resolves.toBe(JobStatus.Failed); expect(mocks.metadata.writeTags).not.toHaveBeenCalled(); diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index c675b7200d..3aa53bc0f2 100644 --- a/server/src/services/metadata.service.ts +++ b/server/src/services/metadata.service.ts @@ -8,9 +8,10 @@ import { constants } from 'node:fs/promises'; import path from 'node:path'; import { JOBS_ASSET_PAGINATION_SIZE } from 'src/constants'; import { StorageCore } from 'src/cores/storage.core'; -import { Asset, AssetFace } from 'src/database'; +import { Asset, AssetFace, AssetFile } from 'src/database'; import { OnEvent, OnJob } from 'src/decorators'; import { + AssetFileType, AssetType, AssetVisibility, DatabaseLock, @@ -371,7 +372,7 @@ export class MetadataService extends BaseService { const tagsList = (asset.tags || []).map((tag) => tag.value); - const sidecarPath = asset.sidecarPath || `${asset.originalPath}.xmp`; + const sidecarPath = asset.files[0]?.path || `${asset.originalPath}.xmp`; const exif = _.omitBy( { Description: description, @@ -391,8 +392,8 @@ export class MetadataService extends BaseService { await this.metadataRepository.writeTags(sidecarPath, exif); - if (!asset.sidecarPath) { - await this.assetRepository.update({ id, sidecarPath }); + if (asset.files.length === 0) { + await this.assetRepository.upsertFile({ assetId: id, type: AssetFileType.Sidecar, path: sidecarPath }); } return JobStatus.Success; @@ -411,13 +412,17 @@ export class MetadataService extends BaseService { return { width, height }; } - private getExifTags(asset: { - originalPath: string; - sidecarPath: string | null; - type: AssetType; - }): Promise { - if (!asset.sidecarPath && asset.type === AssetType.Image) { - return this.metadataRepository.readTags(asset.originalPath); + private getExifTags(asset: { originalPath: string; files: AssetFile[]; type: AssetType }): Promise { + if (asset.type === AssetType.Image) { + let hasSidecar = false; + + if (asset.files && asset.files.length > 0) { + hasSidecar = asset.files.some((file) => file.type === AssetFileType.Sidecar); + } + + if (!hasSidecar) { + return this.metadataRepository.readTags(asset.originalPath); + } } return this.mergeExifTags(asset); @@ -425,12 +430,16 @@ export class MetadataService extends BaseService { private async mergeExifTags(asset: { originalPath: string; - sidecarPath: string | null; + files: AssetFile[]; type: AssetType; }): Promise { + if (asset.files && asset.files.length > 1) { + throw new Error(`Asset ${asset.originalPath} has multiple sidecar files`); + } + const [mediaTags, sidecarTags, videoTags] = await Promise.all([ this.metadataRepository.readTags(asset.originalPath), - asset.sidecarPath ? this.metadataRepository.readTags(asset.sidecarPath) : null, + asset.files && asset.files.length > 0 ? this.metadataRepository.readTags(asset.files[0].path) : null, asset.type === AssetType.Video ? this.getVideoTags(asset.originalPath) : null, ]); @@ -891,18 +900,38 @@ export class MetadataService extends BaseService { } private async processSidecar(id: string, isSync: boolean): Promise { - const [asset] = await this.assetRepository.getByIds([id]); + const asset = await this.assetJobRepository.getForMetadataExtraction(id); if (!asset) { return JobStatus.Failed; } - if (isSync && !asset.sidecarPath) { - return JobStatus.Failed; + if (isSync) { + // We are performing a sidecar sync + if (asset.files.length === 0) { + return JobStatus.Failed; + } + } else if (!asset.isExternal) { + // We are performing a sidecar discovery and not on an external library + if (asset.visibility === AssetVisibility.Hidden) { + // Skip hidden assets + return JobStatus.Skipped; + } else if (asset.files.length > 1) { + // Skip assets that already have a sidecar + return JobStatus.Skipped; + } } - if (!isSync && (asset.visibility === AssetVisibility.Hidden || asset.sidecarPath) && !asset.isExternal) { - return JobStatus.Failed; + let currentSidecar: AssetFile | null; + + if (asset.files.length === 0) { + currentSidecar = null; + } else if (asset.files.length === 1) { + currentSidecar = asset.files[0]; + } else { + throw new Error( + `Multiple sidecar files found for asset ${asset.id}: ${asset.files.map((file) => file.path).join(', ')}`, + ); } // XMP sidecars can come in two filename formats. For a photo named photo.ext, the filenames are photo.ext.xmp and photo.xmp @@ -918,30 +947,30 @@ export class MetadataService extends BaseService { let sidecarPath = null; if (sidecarPathWithExtExists) { + // Sidecars with the extension have precedence over those without sidecarPath = sidecarPathWithExt; } else if (sidecarPathWithoutExtExists) { sidecarPath = sidecarPathWithoutExt; } if (asset.isExternal) { - if (sidecarPath !== asset.sidecarPath) { - await this.assetRepository.update({ id: asset.id, sidecarPath }); + if (sidecarPath !== currentSidecar?.path) { + await (sidecarPath + ? this.assetRepository.upsertFile({ assetId: asset.id, path: sidecarPath, type: AssetFileType.Sidecar }) + : this.assetRepository.deleteFile({ assetId: asset.id, type: AssetFileType.Sidecar })); } + return JobStatus.Success; } if (sidecarPath) { this.logger.debug(`Detected sidecar at '${sidecarPath}' for asset ${asset.id}: ${asset.originalPath}`); - await this.assetRepository.update({ id: asset.id, sidecarPath }); + await this.assetRepository.upsertFile({ assetId: asset.id, path: sidecarPath, type: AssetFileType.Sidecar }); return JobStatus.Success; } - if (!isSync) { - return JobStatus.Failed; - } - this.logger.debug(`No sidecar found for asset ${asset.id}: ${asset.originalPath}`); - await this.assetRepository.update({ id: asset.id, sidecarPath: null }); + await this.assetRepository.deleteFile({ assetId: asset.id, type: AssetFileType.Sidecar }); return JobStatus.Success; } diff --git a/server/src/types.ts b/server/src/types.ts index 9cd1aa996b..bbe5a16096 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -435,7 +435,7 @@ export type StorageAsset = { fileCreatedAt: Date; originalPath: string; originalFileName: string; - sidecarPath: string | null; + sidecarPath?: string | null; fileSizeInByte: number | null; }; diff --git a/server/src/utils/asset.util.ts b/server/src/utils/asset.util.ts index 1b9e12c1cd..2d1bb96c53 100644 --- a/server/src/utils/asset.util.ts +++ b/server/src/utils/asset.util.ts @@ -21,6 +21,7 @@ export const getAssetFiles = (files: AssetFile[]) => ({ fullsizeFile: getAssetFile(files, AssetFileType.FullSize), previewFile: getAssetFile(files, AssetFileType.Preview), thumbnailFile: getAssetFile(files, AssetFileType.Thumbnail), + sidecarFile: getAssetFile(files, AssetFileType.Sidecar), }); export const addAssets = async ( diff --git a/server/test/fixtures/asset.stub.ts b/server/test/fixtures/asset.stub.ts index 066996ead5..d4bbdeeb18 100644 --- a/server/test/fixtures/asset.stub.ts +++ b/server/test/fixtures/asset.stub.ts @@ -24,6 +24,18 @@ const fullsizeFile: AssetFile = { path: '/uploads/user-id/fullsize/path.webp', }; +const sidecarFileWithExt: AssetFile = { + id: 'sidecar-with-ext', + type: AssetFileType.Sidecar, + path: '/original/path.ext.xmp', +}; + +const sidecarFileWithoutExt: AssetFile = { + id: 'sidecar-without-ext', + type: AssetFileType.Sidecar, + path: '/original/path.xmp', +}; + const files: AssetFile[] = [fullsizeFile, previewFile, thumbnailFile]; export const stackStub = (stackId: string, assets: (MapAsset & { exifInfo: Exif })[]) => { @@ -51,7 +63,6 @@ export const assetStub = { fileCreatedAt: new Date('2022-06-19T23:41:36.910Z'), originalPath: '/original/path.jpg', originalFileName: 'IMG_123.jpg', - sidecarPath: null, fileSizeInByte: 12_345, ...asset, }), @@ -81,7 +92,6 @@ export const assetStub = { sharedLinks: [], faces: [], exifInfo: {} as Exif, - sidecarPath: null, deletedAt: null, isExternal: false, duplicateId: null, @@ -117,7 +127,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'IMG_456.jpg', faces: [], - sidecarPath: null, isExternal: false, exifInfo: { fileSizeInByte: 123_000, @@ -157,7 +166,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.ext', faces: [], - sidecarPath: null, deletedAt: null, duplicateId: null, isOffline: false, @@ -194,7 +202,6 @@ export const assetStub = { originalFileName: 'asset-id.jpg', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, exifImageHeight: 1000, @@ -243,7 +250,6 @@ export const assetStub = { originalFileName: 'asset-id.jpg', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, exifImageHeight: 3840, @@ -285,7 +291,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.jpg', faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, exifImageHeight: 3840, @@ -328,7 +333,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.jpg', faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, exifImageHeight: 3840, @@ -367,7 +371,6 @@ export const assetStub = { originalFileName: 'asset-id.jpg', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, exifImageHeight: 3840, @@ -409,7 +412,6 @@ export const assetStub = { originalFileName: 'asset-id.jpg', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, } as Exif, @@ -448,7 +450,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.ext', faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, } as Exif, @@ -490,7 +491,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.ext', faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, } as Exif, @@ -526,7 +526,6 @@ export const assetStub = { livePhotoVideoId: null, sharedLinks: [], faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 100_000, exifImageHeight: 2160, @@ -573,7 +572,7 @@ export const assetStub = { files, faces: [] as AssetFace[], visibility: AssetVisibility.Timeline, - } as MapAsset & { faces: AssetFace[] }), + } as MapAsset & { faces: AssetFace[]; files: AssetFile[] }), livePhotoWithOriginalFileName: Object.freeze({ id: 'live-photo-still-asset', @@ -592,7 +591,7 @@ export const assetStub = { libraryId: null, faces: [] as AssetFace[], visibility: AssetVisibility.Timeline, - } as MapAsset & { faces: AssetFace[] }), + } as MapAsset & { faces: AssetFace[]; files: AssetFile[] }), withLocation: Object.freeze({ id: 'asset-with-favorite-id', @@ -605,7 +604,6 @@ export const assetStub = { deviceId: 'device-id', checksum: Buffer.from('file hash', 'utf8'), originalPath: '/original/path.ext', - sidecarPath: null, type: AssetType.Image, files: [previewFile], thumbhash: null, @@ -652,7 +650,7 @@ export const assetStub = { thumbhash: null, checksum: Buffer.from('file hash', 'utf8'), type: AssetType.Image, - files: [previewFile], + files: [previewFile, sidecarFileWithExt], encodedVideoPath: null, createdAt: new Date('2023-02-23T05:06:29.716Z'), updatedAt: new Date('2023-02-23T05:06:29.716Z'), @@ -665,7 +663,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.ext', faces: [], - sidecarPath: '/original/path.ext.xmp', deletedAt: null, duplicateId: null, isOffline: false, @@ -688,7 +685,7 @@ export const assetStub = { thumbhash: null, checksum: Buffer.from('file hash', 'utf8'), type: AssetType.Image, - files: [previewFile], + files: [previewFile, sidecarFileWithoutExt], encodedVideoPath: null, createdAt: new Date('2023-02-23T05:06:29.716Z'), updatedAt: new Date('2023-02-23T05:06:29.716Z'), @@ -701,7 +698,6 @@ export const assetStub = { sharedLinks: [], originalFileName: 'asset-id.ext', faces: [], - sidecarPath: '/original/path.xmp', deletedAt: null, duplicateId: null, isOffline: false, @@ -734,7 +730,6 @@ export const assetStub = { livePhotoVideoId: null, sharedLinks: [], faces: [], - sidecarPath: null, exifInfo: { fileSizeInByte: 100_000, } as Exif, @@ -776,7 +771,6 @@ export const assetStub = { originalFileName: 'photo.jpg', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, } as Exif, @@ -812,7 +806,6 @@ export const assetStub = { originalFileName: 'asset-id.dng', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, profileDescription: 'Adobe RGB', @@ -853,7 +846,6 @@ export const assetStub = { originalFileName: 'asset-id.hif', faces: [], deletedAt: null, - sidecarPath: null, exifInfo: { fileSizeInByte: 5000, profileDescription: 'Adobe RGB', diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index 79e3d506f3..26ef9dcf9c 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -36,6 +36,7 @@ export const newAssetRepositoryMock = (): Mocked randomUUID() as string; @@ -305,6 +314,13 @@ const assetSidecarWriteFactory = (asset: Partial = {}) => ({ sidecarPath: '/path/to/original-path.jpg.xmp', originalPath: '/path/to/original-path.jpg.xmp', tags: [], + files: [ + { + id: newUuid(), + path: '/path/to/original-path.jpg.xmp', + type: AssetFileType.Sidecar, + }, + ], ...asset, }); From 6b91b31dbc9a0c72572a5e92e0255e0ab9b23269 Mon Sep 17 00:00:00 2001 From: Jonathan Jogenfors Date: Wed, 27 Aug 2025 15:14:27 +0200 Subject: [PATCH 3/3] feat: combine with handleSidecarCheck --- server/src/queries/asset.job.repository.sql | 42 +++++++++++++------- server/src/services/metadata.service.spec.ts | 10 +---- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/server/src/queries/asset.job.repository.sql b/server/src/queries/asset.job.repository.sql index 5a987dd78f..d4418edc56 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/server/src/queries/asset.job.repository.sql @@ -21,6 +21,22 @@ limit select "id", "originalPath", + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "asset_file"."id", + "asset_file"."path", + "asset_file"."type" + from + "asset_file" + where + "asset_file"."assetId" = "asset"."id" + and "asset_file"."type" = $1 + ) as agg + ) as "files", ( select coalesce(json_agg(agg), '[]') @@ -34,7 +50,18 @@ select where "asset"."id" = "tag_asset"."assetsId" ) as agg - ) as "tags", + ) as "tags" +from + "asset" +where + "asset"."id" = $2::uuid +limit + $3 + +-- AssetJobRepository.getForSidecarCheckJob +select + "id", + "originalPath", ( select coalesce(json_agg(agg), '[]') @@ -48,23 +75,10 @@ select "asset_file" where "asset_file"."assetId" = "asset"."id" - and "asset_file"."type" = $1 ) as agg ) as "files" from "asset" -where - "asset"."id" = $2::uuid -limit - $3 - --- AssetJobRepository.getForSidecarCheckJob -select - "id", - "sidecarPath", - "originalPath" -from - "asset" where "asset"."id" = $1::uuid limit diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 31a014f31e..f483541e72 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -27,13 +27,12 @@ const forSidecarJob = ( asset: { id?: string; originalPath?: string; - sidecarPath?: string | null; + files: { id: string; type: AssetFileType; path: string }[]; } = {}, ) => { return { id: factory.uuid(), originalPath: '/path/to/IMG_123.jpg', - sidecarPath: null, ...asset, }; }; @@ -58,13 +57,6 @@ const makeFaceTags = (face: Partial<{ Name: string }> = {}, orientation?: Immich }, }); -function removeNonSidecarFiles(asset: any) { - return { - ...asset, - files: asset.files.filter((file: any) => file.type === AssetFileType.Sidecar), - }; -} - describe(MetadataService.name, () => { let sut: MetadataService; let mocks: ServiceMocks;