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; }