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 25573cb08e..96623092f1 100644 --- a/server/src/cores/storage.core.ts +++ b/server/src/cores/storage.core.ts @@ -305,7 +305,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 f60f2a8824..93831d9d44 100644 --- a/server/src/dtos/asset-response.dto.ts +++ b/server/src/dtos/asset-response.dto.ts @@ -124,7 +124,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 b8e6e5209f..045f041319 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 471de1ac34..9c561d06fc 100644 --- a/server/src/queries/asset.job.repository.sql +++ b/server/src/queries/asset.job.repository.sql @@ -20,8 +20,23 @@ limit -- AssetJobRepository.getForSidecarWriteJob select "id", - "sidecarPath", "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), '[]') @@ -39,21 +54,36 @@ select from "asset" where - "asset"."id" = $1::uuid + "asset"."id" = $2::uuid limit - $2 + $3 -- AssetJobRepository.getForSidecarCheckJob select "id", - "sidecarPath", - "originalPath" + "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" from "asset" where - "asset"."id" = $1::uuid + "asset"."id" = $2::uuid limit - $2 + $3 -- AssetJobRepository.streamForThumbnailJob select @@ -158,7 +188,6 @@ select "asset"."originalFileName", "asset"."originalPath", "asset"."ownerId", - "asset"."sidecarPath", "asset"."type", ( select @@ -173,11 +202,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 @@ -305,7 +350,6 @@ select "asset"."libraryId", "asset"."ownerId", "asset"."livePhotoVideoId", - "asset"."sidecarPath", "asset"."encodedVideoPath", "asset"."originalPath", to_json("asset_exif") as "exifInfo", @@ -416,18 +460,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 @@ -437,12 +491,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" @@ -464,11 +528,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 ca1291b852..f4b8899341 100644 --- a/server/src/repositories/asset-job.repository.ts +++ b/server/src/repositories/asset-job.repository.ts @@ -39,7 +39,8 @@ export class AssetJobRepository { return this.db .selectFrom('asset') .where('asset.id', '=', asUuid(id)) - .select(['id', 'sidecarPath', 'originalPath']) + .select(['id', 'originalPath']) + .select((eb) => withFiles(eb, AssetFileType.Sidecar)) .select((eb) => jsonArrayFrom( eb @@ -58,7 +59,8 @@ export class AssetJobRepository { return this.db .selectFrom('asset') .where('asset.id', '=', asUuid(id)) - .select(['id', 'sidecarPath', 'originalPath']) + .select(['id', 'originalPath']) + .select((eb) => withFiles(eb, AssetFileType.Sidecar)) .limit(1) .executeTakeFirst(); } @@ -121,6 +123,7 @@ export class AssetJobRepository { .selectFrom('asset') .select(columns.asset) .select(withFaces) + .select((eb) => withFiles(eb, AssetFileType.Sidecar)) .where('asset.id', '=', id) .executeTakeFirst(); } @@ -218,7 +221,6 @@ export class AssetJobRepository { 'asset.libraryId', 'asset.ownerId', 'asset.livePhotoVideoId', - 'asset.sidecarPath', 'asset.encodedVideoPath', 'asset.originalPath', ]) @@ -296,12 +298,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); } @@ -333,9 +342,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 5c3bd8996c..8634b629c5 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -840,6 +840,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 a338c30d78..267261bc82 100644 --- a/server/src/services/asset-media.service.spec.ts +++ b/server/src/services/asset-media.service.spec.ts @@ -173,7 +173,6 @@ const assetEntity = Object.freeze({ longitude: 10.703_075, }, livePhotoVideoId: null, - sidecarPath: null, } as MapAsset); const existingAsset = Object.freeze({ @@ -711,18 +710,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), @@ -759,6 +762,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, @@ -788,6 +798,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, @@ -817,6 +833,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 0747bd7b7b..d1bd2d1327 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, UploadRequest } from 'src/types'; @@ -354,9 +363,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({ @@ -384,7 +396,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); @@ -414,7 +425,6 @@ export class AssetMediaService extends BaseService { visibility: dto.visibility ?? AssetVisibility.Timeline, livePhotoVideoId: dto.livePhotoVideoId, originalFileName: dto.filename || file.originalName, - sidecarPath: sidecarFile?.originalPath, }); if (dto.metadata) { @@ -422,6 +432,11 @@ export class AssetMediaService extends BaseService { } 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 93861149c3..4a7af35d67 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 6cb0219745..a22ea364e4 100644 --- a/server/src/services/asset.service.ts +++ b/server/src/services/asset.service.ts @@ -258,11 +258,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 0adb390f6a..94704a4707 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -3,7 +3,16 @@ import { randomBytes } from 'node:crypto'; import { Stats } from 'node:fs'; 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'; @@ -14,17 +23,24 @@ import { tagStub } from 'test/fixtures/tag.stub'; import { factory } from 'test/small.factory'; import { makeStream, newTestService, ServiceMocks } from 'test/utils'; +function removeNonSidecarFiles(asset: any) { + return { + ...asset, + files: asset.files.filter((file: any) => file.type === AssetFileType.Sidecar), + }; +} + 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, + files: [], ...asset, }; }; @@ -165,7 +181,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 }); @@ -184,7 +200,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, @@ -210,7 +226,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, @@ -233,7 +249,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 }); @@ -251,7 +267,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, @@ -304,7 +320,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({ @@ -333,7 +349,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, @@ -345,7 +361,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); @@ -355,7 +371,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); @@ -375,7 +391,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); @@ -385,7 +401,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); @@ -395,7 +411,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); @@ -406,7 +422,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); @@ -425,7 +441,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); @@ -444,7 +460,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); @@ -465,7 +481,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); @@ -1502,18 +1518,25 @@ describe(MetadataService.name, () => { }); it('should detect a new sidecar at .jpg.xmp', async () => { - const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg' }); + const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg', files: [] }); 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` }); + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: asset.id, + type: AssetFileType.Sidecar, + path: '/path/to/IMG_123.jpg.xmp', + }); }); it('should detect a new sidecar at .xmp', async () => { - const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg' }); + const asset = forSidecarJob({ + originalPath: '/path/to/IMG_123.jpg', + files: [], + }); mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); mocks.storage.checkFileExists.mockResolvedValueOnce(false); @@ -1521,33 +1544,44 @@ describe(MetadataService.name, () => { await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Success); - expect(mocks.asset.update).toHaveBeenCalledWith({ id: asset.id, sidecarPath: '/path/to/IMG_123.xmp' }); + expect(mocks.asset.upsertFile).toHaveBeenCalledWith({ + assetId: asset.id, + type: AssetFileType.Sidecar, + path: '/path/to/IMG_123.xmp', + }); }); - it('should unset sidecar path if file does not exist anymore', async () => { - const asset = forSidecarJob({ originalPath: '/path/to/IMG_123.jpg', sidecarPath: '/path/to/IMG_123.jpg.xmp' }); + it('should unset sidecar path if file no longer exist', async () => { + const asset = forSidecarJob({ + originalPath: '/path/to/IMG_123.jpg', + files: [{ id: 'sidecar', path: '/path/to/IMG_123.jpg.xmp', type: AssetFileType.Sidecar }], + }); mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); mocks.storage.checkFileExists.mockResolvedValue(false); await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Success); - expect(mocks.asset.update).toHaveBeenCalledWith({ id: asset.id, sidecarPath: null }); + expect(mocks.asset.deleteFile).toHaveBeenCalledWith({ assetId: asset.id, type: AssetFileType.Sidecar }); }); 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' }); + const asset = forSidecarJob({ + originalPath: '/path/to/IMG_123.jpg', + files: [{ id: 'sidecar', path: '/path/to/IMG_123.jpg.xmp', type: AssetFileType.Sidecar }], + }); mocks.assetJob.getForSidecarCheckJob.mockResolvedValue(asset); mocks.storage.checkFileExists.mockResolvedValueOnce(true); await expect(sut.handleSidecarCheck({ id: asset.id })).resolves.toBe(JobStatus.Skipped); - expect(mocks.asset.update).not.toHaveBeenCalled(); + expect(mocks.asset.upsertFile).not.toHaveBeenCalled(); + expect(mocks.asset.deleteFile).not.toHaveBeenCalled(); }); }); 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 7d3de76550..790bd81c92 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 { 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'; +import { Asset, AssetFace, AssetFile } from 'src/database'; import { OnEvent, OnJob } from 'src/decorators'; import { + AssetFileType, AssetType, AssetVisibility, DatabaseLock, @@ -360,17 +361,21 @@ export class MetadataService extends BaseService { break; } - const isChanged = sidecarPath !== asset.sidecarPath; + const existingSidecar = asset.files ? asset.files.find((file) => file.type === AssetFileType.Sidecar) : null; + + const isChanged = sidecarPath !== existingSidecar?.path; this.logger.debug( - `Sidecar check found old=${asset.sidecarPath}, new=${sidecarPath} will ${isChanged ? 'update' : 'do nothing for'} asset ${asset.id}: ${asset.originalPath}`, + `Sidecar check found old=${existingSidecar?.path}, 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 }); + await (sidecarPath === null + ? this.assetRepository.deleteFile({ assetId: asset.id, type: AssetFileType.Sidecar }) + : this.assetRepository.upsertFile({ assetId: asset.id, type: AssetFileType.Sidecar, path: sidecarPath })); return JobStatus.Success; } @@ -395,7 +400,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, @@ -415,18 +420,20 @@ 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; } - private getSidecarCandidates({ sidecarPath, originalPath }: { sidecarPath: string | null; originalPath: string }) { + private getSidecarCandidates({ files, originalPath }: { files: AssetFile[] | null; originalPath: string }) { const candidates: string[] = []; - if (sidecarPath) { - candidates.push(sidecarPath); + const existingSidecar = files?.find((file) => file.type === AssetFileType.Sidecar); + + if (existingSidecar) { + candidates.push(existingSidecar.path); } const assetPath = parse(originalPath); @@ -454,13 +461,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); @@ -468,12 +479,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, ]); diff --git a/server/src/types.ts b/server/src/types.ts index da3889ef7c..3cfa4ed93f 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -453,7 +453,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 629b3bf819..f3f807c829 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 0fd2189268..ee3bb09b2f 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 e735b37564..e853deb270 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 = {}) => ({ 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, });