fix: asset update race condition

This commit is contained in:
Daniel Dietzler
2025-12-04 15:33:44 +01:00
parent 75a7c9c06c
commit 29827f6f08
17 changed files with 316 additions and 133 deletions

View File

@@ -240,7 +240,7 @@ export type Session = {
isPendingSyncReset: boolean; isPendingSyncReset: boolean;
}; };
export type Exif = Omit<Selectable<AssetExifTable>, 'updatedAt' | 'updateId'>; export type Exif = Omit<Selectable<AssetExifTable>, 'updatedAt' | 'updateId' | 'lockedProperties'>;
export type Person = { export type Person = {
createdAt: Date; createdAt: Date;

View File

@@ -50,9 +50,11 @@ select
where where
"asset"."id" = "tag_asset"."assetId" "asset"."id" = "tag_asset"."assetId"
) as agg ) as agg
) as "tags" ) as "tags",
to_json("asset_exif") as "exifInfo"
from from
"asset" "asset"
inner join "asset_exif" on "asset"."id" = "asset_exif"."assetId"
where where
"asset"."id" = $2::uuid "asset"."id" = $2::uuid
limit limit
@@ -224,6 +226,14 @@ from
where where
"asset"."id" = $2 "asset"."id" = $2
-- AssetJobRepository.getLockedPropertiesForMetadataExtraction
select
"asset_exif"."lockedProperties"
from
"asset_exif"
where
"asset_exif"."assetId" = $1
-- AssetJobRepository.getAlbumThumbnailFiles -- AssetJobRepository.getAlbumThumbnailFiles
select select
"asset_file"."id", "asset_file"."id",

View File

@@ -3,17 +3,25 @@
-- AssetRepository.updateAllExif -- AssetRepository.updateAllExif
update "asset_exif" update "asset_exif"
set set
"model" = $1 "model" = $1,
"lockedProperties" = array(
select distinct
unnest(array_cat("lockedProperties", $2))
)
where where
"assetId" in ($2) "assetId" in ($3)
-- AssetRepository.updateDateTimeOriginal -- AssetRepository.updateDateTimeOriginal
update "asset_exif" update "asset_exif"
set set
"dateTimeOriginal" = "dateTimeOriginal" + $1::interval, "dateTimeOriginal" = "dateTimeOriginal" + $1::interval,
"timeZone" = $2 "timeZone" = $2,
"lockedProperties" = array(
select distinct
unnest(array_cat("lockedProperties", $3))
)
where where
"assetId" in ($3) "assetId" in ($4)
returning returning
"assetId", "assetId",
"dateTimeOriginal", "dateTimeOriginal",

View File

@@ -50,6 +50,7 @@ export class AssetJobRepository {
.whereRef('asset.id', '=', 'tag_asset.assetId'), .whereRef('asset.id', '=', 'tag_asset.assetId'),
).as('tags'), ).as('tags'),
) )
.$call(withExifInner)
.limit(1) .limit(1)
.executeTakeFirst(); .executeTakeFirst();
} }
@@ -128,6 +129,16 @@ export class AssetJobRepository {
.executeTakeFirst(); .executeTakeFirst();
} }
@GenerateSql({ params: [DummyValue.UUID] })
async getLockedPropertiesForMetadataExtraction(assetId: string) {
return this.db
.selectFrom('asset_exif')
.select('asset_exif.lockedProperties')
.where('asset_exif.assetId', '=', assetId)
.executeTakeFirst()
.then((row) => row?.lockedProperties ?? []);
}
@GenerateSql({ params: [DummyValue.UUID, AssetFileType.Thumbnail] }) @GenerateSql({ params: [DummyValue.UUID, AssetFileType.Thumbnail] })
getAlbumThumbnailFiles(id: string, fileType?: AssetFileType) { getAlbumThumbnailFiles(id: string, fileType?: AssetFileType) {
return this.db return this.db

View File

@@ -1,13 +1,13 @@
import { Injectable } from '@nestjs/common'; import { Injectable } from '@nestjs/common';
import { Insertable, Kysely, NotNull, Selectable, sql, Updateable, UpdateResult } from 'kysely'; import { Insertable, Kysely, NotNull, Selectable, sql, Updateable, UpdateResult } from 'kysely';
import { isEmpty, isUndefined, omitBy } from 'lodash'; import { intersection, isEmpty, isUndefined, omit, omitBy, union } from 'lodash';
import { InjectKysely } from 'nestjs-kysely'; import { InjectKysely } from 'nestjs-kysely';
import { Stack } from 'src/database'; import { Stack } from 'src/database';
import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators'; import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators';
import { AuthDto } from 'src/dtos/auth.dto'; import { AuthDto } from 'src/dtos/auth.dto';
import { AssetFileType, AssetMetadataKey, AssetOrder, AssetStatus, AssetType, AssetVisibility } from 'src/enum'; import { AssetFileType, AssetMetadataKey, AssetOrder, AssetStatus, AssetType, AssetVisibility } from 'src/enum';
import { DB } from 'src/schema'; import { DB } from 'src/schema';
import { AssetExifTable } from 'src/schema/tables/asset-exif.table'; import { AssetExifTable, lockableProperties, LockableProperty } from 'src/schema/tables/asset-exif.table';
import { AssetFileTable } from 'src/schema/tables/asset-file.table'; import { AssetFileTable } from 'src/schema/tables/asset-file.table';
import { AssetJobStatusTable } from 'src/schema/tables/asset-job-status.table'; import { AssetJobStatusTable } from 'src/schema/tables/asset-job-status.table';
import { AssetTable } from 'src/schema/tables/asset.table'; import { AssetTable } from 'src/schema/tables/asset.table';
@@ -117,49 +117,83 @@ interface GetByIdsRelations {
export class AssetRepository { export class AssetRepository {
constructor(@InjectKysely() private db: Kysely<DB>) {} constructor(@InjectKysely() private db: Kysely<DB>) {}
async upsertExif(exif: Insertable<AssetExifTable>): Promise<void> { async upsertExif(
const value = { ...exif, assetId: asUuid(exif.assetId) }; exif: Insertable<AssetExifTable>,
await this.db { lockedPropertiesBehavior }: { lockedPropertiesBehavior: 'none' | 'update' | 'skip' },
.insertInto('asset_exif') ): Promise<void> {
.values(value) await this.db.transaction().execute(async (tx) => {
.onConflict((oc) => const lockedProperties = await tx
oc.column('assetId').doUpdateSet((eb) => .selectFrom('asset_exif')
removeUndefinedKeys( .select('asset_exif.lockedProperties')
{ .where('asset_exif.assetId', '=', exif.assetId)
description: eb.ref('excluded.description'), .executeTakeFirst()
exifImageWidth: eb.ref('excluded.exifImageWidth'), .then((result) => result?.lockedProperties ?? []);
exifImageHeight: eb.ref('excluded.exifImageHeight'),
fileSizeInByte: eb.ref('excluded.fileSizeInByte'), let value = { ...exif, assetId: asUuid(exif.assetId) };
orientation: eb.ref('excluded.orientation'),
dateTimeOriginal: eb.ref('excluded.dateTimeOriginal'), switch (lockedPropertiesBehavior) {
modifyDate: eb.ref('excluded.modifyDate'), case 'skip': {
timeZone: eb.ref('excluded.timeZone'), value = omit(value, [...lockedProperties, 'lockedProperties']);
latitude: eb.ref('excluded.latitude'), break;
longitude: eb.ref('excluded.longitude'), }
projectionType: eb.ref('excluded.projectionType'),
city: eb.ref('excluded.city'), case 'update': {
livePhotoCID: eb.ref('excluded.livePhotoCID'), const updatedLockableProperties = intersection(lockableProperties, Object.keys(exif)) as LockableProperty[];
autoStackId: eb.ref('excluded.autoStackId'), value = {
state: eb.ref('excluded.state'), ...value,
country: eb.ref('excluded.country'), lockedProperties: union(updatedLockableProperties, lockedProperties),
make: eb.ref('excluded.make'), };
model: eb.ref('excluded.model'), break;
lensModel: eb.ref('excluded.lensModel'), }
fNumber: eb.ref('excluded.fNumber'), }
focalLength: eb.ref('excluded.focalLength'),
iso: eb.ref('excluded.iso'), if (Object.keys(value).length <= 1) {
exposureTime: eb.ref('excluded.exposureTime'), return;
profileDescription: eb.ref('excluded.profileDescription'), }
colorspace: eb.ref('excluded.colorspace'),
bitsPerSample: eb.ref('excluded.bitsPerSample'), return tx
rating: eb.ref('excluded.rating'), .insertInto('asset_exif')
fps: eb.ref('excluded.fps'), .values(value)
}, .onConflict((oc) =>
value, oc.column('assetId').doUpdateSet((eb) =>
removeUndefinedKeys(
{
description: eb.ref('excluded.description'),
exifImageWidth: eb.ref('excluded.exifImageWidth'),
exifImageHeight: eb.ref('excluded.exifImageHeight'),
fileSizeInByte: eb.ref('excluded.fileSizeInByte'),
orientation: eb.ref('excluded.orientation'),
dateTimeOriginal: eb.ref('excluded.dateTimeOriginal'),
modifyDate: eb.ref('excluded.modifyDate'),
timeZone: eb.ref('excluded.timeZone'),
latitude: eb.ref('excluded.latitude'),
longitude: eb.ref('excluded.longitude'),
projectionType: eb.ref('excluded.projectionType'),
city: eb.ref('excluded.city'),
livePhotoCID: eb.ref('excluded.livePhotoCID'),
autoStackId: eb.ref('excluded.autoStackId'),
state: eb.ref('excluded.state'),
country: eb.ref('excluded.country'),
make: eb.ref('excluded.make'),
model: eb.ref('excluded.model'),
lensModel: eb.ref('excluded.lensModel'),
fNumber: eb.ref('excluded.fNumber'),
focalLength: eb.ref('excluded.focalLength'),
iso: eb.ref('excluded.iso'),
exposureTime: eb.ref('excluded.exposureTime'),
profileDescription: eb.ref('excluded.profileDescription'),
colorspace: eb.ref('excluded.colorspace'),
bitsPerSample: eb.ref('excluded.bitsPerSample'),
rating: eb.ref('excluded.rating'),
fps: eb.ref('excluded.fps'),
lockedProperties: eb.ref('excluded.lockedProperties'),
},
value,
),
), ),
), )
) .execute();
.execute(); });
} }
@GenerateSql({ params: [[DummyValue.UUID], { model: DummyValue.STRING }] }) @GenerateSql({ params: [[DummyValue.UUID], { model: DummyValue.STRING }] })
@@ -169,7 +203,18 @@ export class AssetRepository {
return; return;
} }
await this.db.updateTable('asset_exif').set(options).where('assetId', 'in', ids).execute(); await this.db
.updateTable('asset_exif')
.set((eb) => ({
...options,
lockedProperties: eb
.fn<
LockableProperty[]
>('array', [sql`select distinct unnest(${eb.fn('array_cat', ['lockedProperties', eb.val(Object.keys(options))])})`])
.as('lockedProperties').expression,
}))
.where('assetId', 'in', ids)
.execute();
} }
@GenerateSql({ params: [[DummyValue.UUID], DummyValue.NUMBER, DummyValue.STRING] }) @GenerateSql({ params: [[DummyValue.UUID], DummyValue.NUMBER, DummyValue.STRING] })
@@ -181,7 +226,15 @@ export class AssetRepository {
): Promise<{ assetId: string; dateTimeOriginal: Date | null; timeZone: string | null }[]> { ): Promise<{ assetId: string; dateTimeOriginal: Date | null; timeZone: string | null }[]> {
return await this.db return await this.db
.updateTable('asset_exif') .updateTable('asset_exif')
.set({ dateTimeOriginal: sql`"dateTimeOriginal" + ${(delta ?? 0) + ' minute'}::interval`, timeZone }) .set((eb) => ({
dateTimeOriginal: sql`"dateTimeOriginal" + ${(delta ?? 0) + ' minute'}::interval`,
timeZone,
lockedProperties: eb
.fn<
LockableProperty[]
>('array', [sql`select distinct unnest(${eb.fn('array_cat', ['lockedProperties', eb.val(['dateTimeOriginal', 'timeZone'])])})`])
.as('lockedProperties').expression,
}))
.where('assetId', 'in', ids) .where('assetId', 'in', ids)
.returning(['assetId', 'dateTimeOriginal', 'timeZone']) .returning(['assetId', 'dateTimeOriginal', 'timeZone'])
.execute(); .execute();

View File

@@ -0,0 +1,9 @@
import { Kysely, sql } from 'kysely';
export async function up(db: Kysely<any>): Promise<void> {
await sql`ALTER TABLE "asset_exif" ADD "lockedProperties" character varying[];`.execute(db);
}
export async function down(db: Kysely<any>): Promise<void> {
await sql`ALTER TABLE "asset_exif" DROP COLUMN "lockedProperties";`.execute(db);
}

View File

@@ -2,6 +2,9 @@ import { UpdatedAtTrigger, UpdateIdColumn } from 'src/decorators';
import { AssetTable } from 'src/schema/tables/asset.table'; import { AssetTable } from 'src/schema/tables/asset.table';
import { Column, ForeignKeyColumn, Generated, Int8, Table, Timestamp, UpdateDateColumn } from 'src/sql-tools'; import { Column, ForeignKeyColumn, Generated, Int8, Table, Timestamp, UpdateDateColumn } from 'src/sql-tools';
export type LockableProperty = (typeof lockableProperties)[number];
export const lockableProperties = ['description', 'dateTimeOriginal', 'latitude', 'longitude', 'rating'] as const;
@Table('asset_exif') @Table('asset_exif')
@UpdatedAtTrigger('asset_exif_updatedAt') @UpdatedAtTrigger('asset_exif_updatedAt')
export class AssetExifTable { export class AssetExifTable {
@@ -97,4 +100,7 @@ export class AssetExifTable {
@UpdateIdColumn({ index: true }) @UpdateIdColumn({ index: true })
updateId!: Generated<string>; updateId!: Generated<string>;
@Column({ type: 'character varying', array: true, nullable: true })
lockedProperties!: Array<LockableProperty> | null;
} }

View File

@@ -370,7 +370,7 @@ export class AssetMediaService extends BaseService {
: this.assetRepository.deleteFile({ assetId, type: AssetFileType.Sidecar })); : this.assetRepository.deleteFile({ assetId, type: AssetFileType.Sidecar }));
await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt)); await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt));
await this.assetRepository.upsertExif({ assetId, fileSizeInByte: file.size }); await this.assetRepository.upsertExif({ assetId, fileSizeInByte: file.size }, { lockedPropertiesBehavior: 'none' });
await this.jobRepository.queue({ await this.jobRepository.queue({
name: JobName.AssetExtractMetadata, name: JobName.AssetExtractMetadata,
data: { id: assetId, source: 'upload' }, data: { id: assetId, source: 'upload' },
@@ -399,7 +399,10 @@ export class AssetMediaService extends BaseService {
}); });
const { size } = await this.storageRepository.stat(created.originalPath); const { size } = await this.storageRepository.stat(created.originalPath);
await this.assetRepository.upsertExif({ assetId: created.id, fileSizeInByte: size }); await this.assetRepository.upsertExif(
{ assetId: created.id, fileSizeInByte: size },
{ lockedPropertiesBehavior: 'none' },
);
await this.jobRepository.queue({ name: JobName.AssetExtractMetadata, data: { id: created.id, source: 'copy' } }); await this.jobRepository.queue({ name: JobName.AssetExtractMetadata, data: { id: created.id, source: 'copy' } });
return created; return created;
} }
@@ -440,7 +443,10 @@ export class AssetMediaService extends BaseService {
await this.storageRepository.utimes(sidecarFile.originalPath, new Date(), new Date(dto.fileModifiedAt)); await this.storageRepository.utimes(sidecarFile.originalPath, new Date(), new Date(dto.fileModifiedAt));
} }
await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt)); await this.storageRepository.utimes(file.originalPath, new Date(), new Date(dto.fileModifiedAt));
await this.assetRepository.upsertExif({ assetId: asset.id, fileSizeInByte: file.size }); await this.assetRepository.upsertExif(
{ assetId: asset.id, fileSizeInByte: file.size },
{ lockedPropertiesBehavior: 'none' },
);
await this.eventRepository.emit('AssetCreate', { asset }); await this.eventRepository.emit('AssetCreate', { asset });

View File

@@ -225,7 +225,10 @@ describe(AssetService.name, () => {
await sut.update(authStub.admin, 'asset-1', { description: 'Test description' }); await sut.update(authStub.admin, 'asset-1', { description: 'Test description' });
expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ assetId: 'asset-1', description: 'Test description' }); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
{ assetId: 'asset-1', description: 'Test description' },
{ lockedPropertiesBehavior: 'update' },
);
}); });
it('should update the exif rating', async () => { it('should update the exif rating', async () => {
@@ -235,7 +238,13 @@ describe(AssetService.name, () => {
await sut.update(authStub.admin, 'asset-1', { rating: 3 }); await sut.update(authStub.admin, 'asset-1', { rating: 3 });
expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ assetId: 'asset-1', rating: 3 }); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
{
assetId: 'asset-1',
rating: 3,
},
{ lockedPropertiesBehavior: 'update' },
);
}); });
it('should fail linking a live video if the motion part could not be found', async () => { it('should fail linking a live video if the motion part could not be found', async () => {

View File

@@ -30,7 +30,7 @@ import {
QueueName, QueueName,
} from 'src/enum'; } from 'src/enum';
import { BaseService } from 'src/services/base.service'; import { BaseService } from 'src/services/base.service';
import { ISidecarWriteJob, JobItem, JobOf } from 'src/types'; import { JobItem, JobOf } from 'src/types';
import { requireElevatedPermission } from 'src/utils/access'; import { requireElevatedPermission } from 'src/utils/access';
import { getAssetFiles, getMyPartnerIds, onAfterUnlink, onBeforeLink, onBeforeUnlink } from 'src/utils/asset.util'; import { getAssetFiles, getMyPartnerIds, onAfterUnlink, onBeforeLink, onBeforeUnlink } from 'src/utils/asset.util';
@@ -143,9 +143,9 @@ export class AssetService extends BaseService {
await this.requireAccess({ auth, permission: Permission.AssetUpdate, ids }); await this.requireAccess({ auth, permission: Permission.AssetUpdate, ids });
const assetDto = { isFavorite, visibility, duplicateId }; const assetDto = { isFavorite, visibility, duplicateId };
const exifDto = { latitude, longitude, rating, description, dateTimeOriginal }; const exifDto = _.omitBy({ latitude, longitude, rating, description, dateTimeOriginal }, _.isUndefined);
const isExifChanged = Object.values(exifDto).some((v) => v !== undefined); const isExifChanged = Object.keys(exifDto).length > 0;
if (isExifChanged) { if (isExifChanged) {
await this.assetRepository.updateAllExif(ids, exifDto); await this.assetRepository.updateAllExif(ids, exifDto);
} }
@@ -456,12 +456,25 @@ export class AssetService extends BaseService {
return asset; return asset;
} }
private async updateExif(dto: ISidecarWriteJob) { private async updateExif(dto: {
id: string;
description?: string;
dateTimeOriginal?: string;
latitude?: number;
longitude?: number;
rating?: number;
}) {
const { id, description, dateTimeOriginal, latitude, longitude, rating } = dto; const { id, description, dateTimeOriginal, latitude, longitude, rating } = dto;
const writes = _.omitBy({ description, dateTimeOriginal, latitude, longitude, rating }, _.isUndefined); const writes = _.omitBy({ description, dateTimeOriginal, latitude, longitude, rating }, _.isUndefined);
if (Object.keys(writes).length > 0) { if (Object.keys(writes).length > 0) {
await this.assetRepository.upsertExif({ assetId: id, ...writes }); await this.assetRepository.upsertExif(
await this.jobRepository.queue({ name: JobName.SidecarWrite, data: { id, ...writes } }); {
assetId: id,
...writes,
},
{ lockedPropertiesBehavior: 'update' },
);
await this.jobRepository.queue({ name: JobName.SidecarWrite, data: { id } });
} }
} }
} }

View File

@@ -187,7 +187,9 @@ describe(MetadataService.name, () => {
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.sidecar.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.sidecar.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ dateTimeOriginal: sidecarDate })); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ dateTimeOriginal: sidecarDate }), {
lockedPropertiesBehavior: 'skip',
});
expect(mocks.asset.update).toHaveBeenCalledWith( expect(mocks.asset.update).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
id: assetStub.image.id, id: assetStub.image.id,
@@ -214,6 +216,7 @@ describe(MetadataService.name, () => {
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith( expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ dateTimeOriginal: fileModifiedAt }), expect.objectContaining({ dateTimeOriginal: fileModifiedAt }),
{ lockedPropertiesBehavior: 'skip' },
); );
expect(mocks.asset.update).toHaveBeenCalledWith({ expect(mocks.asset.update).toHaveBeenCalledWith({
id: assetStub.image.id, id: assetStub.image.id,
@@ -238,7 +241,10 @@ describe(MetadataService.name, () => {
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ dateTimeOriginal: fileCreatedAt })); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ dateTimeOriginal: fileCreatedAt }),
{ lockedPropertiesBehavior: 'skip' },
);
expect(mocks.asset.update).toHaveBeenCalledWith({ expect(mocks.asset.update).toHaveBeenCalledWith({
id: assetStub.image.id, id: assetStub.image.id,
duration: null, duration: null,
@@ -258,6 +264,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
dateTimeOriginal: new Date('2022-01-01T00:00:00.000Z'), dateTimeOriginal: new Date('2022-01-01T00:00:00.000Z'),
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
expect(mocks.asset.update).toHaveBeenCalledWith( expect(mocks.asset.update).toHaveBeenCalledWith(
@@ -281,7 +288,9 @@ describe(MetadataService.name, () => {
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ iso: 160 })); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ iso: 160 }), {
lockedPropertiesBehavior: 'skip',
});
expect(mocks.asset.update).toHaveBeenCalledWith({ expect(mocks.asset.update).toHaveBeenCalledWith({
id: assetStub.image.id, id: assetStub.image.id,
duration: null, duration: null,
@@ -310,6 +319,7 @@ describe(MetadataService.name, () => {
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith( expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ city: null, state: null, country: null }), expect.objectContaining({ city: null, state: null, country: null }),
{ lockedPropertiesBehavior: 'skip' },
); );
expect(mocks.asset.update).toHaveBeenCalledWith({ expect(mocks.asset.update).toHaveBeenCalledWith({
id: assetStub.withLocation.id, id: assetStub.withLocation.id,
@@ -339,6 +349,7 @@ describe(MetadataService.name, () => {
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith( expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ city: 'City', state: 'State', country: 'Country' }), expect.objectContaining({ city: 'City', state: 'State', country: 'Country' }),
{ lockedPropertiesBehavior: 'skip' },
); );
expect(mocks.asset.update).toHaveBeenCalledWith({ expect(mocks.asset.update).toHaveBeenCalledWith({
id: assetStub.withLocation.id, id: assetStub.withLocation.id,
@@ -358,7 +369,10 @@ describe(MetadataService.name, () => {
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining({ latitude: null, longitude: null })); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ latitude: null, longitude: null }),
{ lockedPropertiesBehavior: 'skip' },
);
}); });
it('should extract tags from TagsList', async () => { it('should extract tags from TagsList', async () => {
@@ -571,6 +585,7 @@ describe(MetadataService.name, () => {
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.video.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.video.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith( expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
expect.objectContaining({ orientation: ExifOrientation.Rotate270CW.toString() }), expect.objectContaining({ orientation: ExifOrientation.Rotate270CW.toString() }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -879,37 +894,40 @@ describe(MetadataService.name, () => {
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id); expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.image.id);
expect(mocks.asset.upsertExif).toHaveBeenCalledWith({ expect(mocks.asset.upsertExif).toHaveBeenCalledWith(
assetId: assetStub.image.id, {
bitsPerSample: expect.any(Number), assetId: assetStub.image.id,
autoStackId: null, bitsPerSample: expect.any(Number),
colorspace: tags.ColorSpace, autoStackId: null,
dateTimeOriginal: dateForTest, colorspace: tags.ColorSpace,
description: tags.ImageDescription, dateTimeOriginal: dateForTest,
exifImageHeight: null, description: tags.ImageDescription,
exifImageWidth: null, exifImageHeight: null,
exposureTime: tags.ExposureTime, exifImageWidth: null,
fNumber: null, exposureTime: tags.ExposureTime,
fileSizeInByte: 123_456, fNumber: null,
focalLength: tags.FocalLength, fileSizeInByte: 123_456,
fps: null, focalLength: tags.FocalLength,
iso: tags.ISO, fps: null,
latitude: null, iso: tags.ISO,
lensModel: tags.LensModel, latitude: null,
livePhotoCID: tags.MediaGroupUUID, lensModel: tags.LensModel,
longitude: null, livePhotoCID: tags.MediaGroupUUID,
make: tags.Make, longitude: null,
model: tags.Model, make: tags.Make,
modifyDate: expect.any(Date), model: tags.Model,
orientation: tags.Orientation?.toString(), modifyDate: expect.any(Date),
profileDescription: tags.ProfileDescription, orientation: tags.Orientation?.toString(),
projectionType: 'EQUIRECTANGULAR', profileDescription: tags.ProfileDescription,
timeZone: tags.tz, projectionType: 'EQUIRECTANGULAR',
rating: tags.Rating, timeZone: tags.tz,
country: null, rating: tags.Rating,
state: null, country: null,
city: null, state: null,
}); city: null,
},
{ lockedPropertiesBehavior: 'skip' },
);
expect(mocks.asset.update).toHaveBeenCalledWith( expect(mocks.asset.update).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
id: assetStub.image.id, id: assetStub.image.id,
@@ -943,6 +961,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
timeZone: 'UTC+0', timeZone: 'UTC+0',
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1089,6 +1108,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
description: '', description: '',
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
mockReadTags({ ImageDescription: ' my\n description' }); mockReadTags({ ImageDescription: ' my\n description' });
@@ -1097,6 +1117,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
description: 'my\n description', description: 'my\n description',
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1109,6 +1130,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
description: '1000', description: '1000',
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1332,6 +1354,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
modifyDate: expect.any(Date), modifyDate: expect.any(Date),
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1344,6 +1367,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
rating: null, rating: null,
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1356,6 +1380,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
rating: 5, rating: 5,
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1368,6 +1393,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
rating: -1, rating: -1,
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
@@ -1489,7 +1515,9 @@ describe(MetadataService.name, () => {
mockReadTags(exif); mockReadTags(exif);
await sut.handleMetadataExtraction({ id: assetStub.image.id }); await sut.handleMetadataExtraction({ id: assetStub.image.id });
expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining(expected)); expect(mocks.asset.upsertExif).toHaveBeenCalledWith(expect.objectContaining(expected), {
lockedPropertiesBehavior: 'skip',
});
}); });
it.each([ it.each([
@@ -1515,6 +1543,7 @@ describe(MetadataService.name, () => {
expect.objectContaining({ expect.objectContaining({
lensModel: expected, lensModel: expected,
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
}); });
}); });
@@ -1623,12 +1652,14 @@ describe(MetadataService.name, () => {
describe('handleSidecarWrite', () => { describe('handleSidecarWrite', () => {
it('should skip assets that no longer exist', async () => { it('should skip assets that no longer exist', async () => {
mocks.assetJob.getLockedPropertiesForMetadataExtraction.mockResolvedValue([]);
mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(void 0); mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(void 0);
await expect(sut.handleSidecarWrite({ id: 'asset-123' })).resolves.toBe(JobStatus.Failed); await expect(sut.handleSidecarWrite({ id: 'asset-123' })).resolves.toBe(JobStatus.Failed);
expect(mocks.metadata.writeTags).not.toHaveBeenCalled(); expect(mocks.metadata.writeTags).not.toHaveBeenCalled();
}); });
it('should skip jobs with no metadata', async () => { it('should skip jobs with no metadata', async () => {
mocks.assetJob.getLockedPropertiesForMetadataExtraction.mockResolvedValue([]);
const asset = factory.jobAssets.sidecarWrite(); const asset = factory.jobAssets.sidecarWrite();
mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset); mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset);
await expect(sut.handleSidecarWrite({ id: asset.id })).resolves.toBe(JobStatus.Skipped); await expect(sut.handleSidecarWrite({ id: asset.id })).resolves.toBe(JobStatus.Skipped);
@@ -1641,20 +1672,22 @@ describe(MetadataService.name, () => {
const gps = 12; const gps = 12;
const date = '2023-11-22T04:56:12.196Z'; const date = '2023-11-22T04:56:12.196Z';
mocks.assetJob.getLockedPropertiesForMetadataExtraction.mockResolvedValue([
'description',
'latitude',
'longitude',
'dateTimeOriginal',
]);
mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset); mocks.assetJob.getForSidecarWriteJob.mockResolvedValue(asset);
await expect( await expect(
sut.handleSidecarWrite({ sut.handleSidecarWrite({
id: asset.id, id: asset.id,
description,
latitude: gps,
longitude: gps,
dateTimeOriginal: date,
}), }),
).resolves.toBe(JobStatus.Success); ).resolves.toBe(JobStatus.Success);
expect(mocks.metadata.writeTags).toHaveBeenCalledWith(asset.files[0].path, { expect(mocks.metadata.writeTags).toHaveBeenCalledWith(asset.files[0].path, {
DateTimeOriginal: date,
Description: description, Description: description,
ImageDescription: description, ImageDescription: description,
DateTimeOriginal: date,
GPSLatitude: gps, GPSLatitude: gps,
GPSLongitude: gps, GPSLongitude: gps,
}); });

View File

@@ -289,7 +289,7 @@ export class MetadataService extends BaseService {
}; };
const promises: Promise<unknown>[] = [ const promises: Promise<unknown>[] = [
this.assetRepository.upsertExif(exifData), this.assetRepository.upsertExif(exifData, { lockedPropertiesBehavior: 'skip' }),
this.assetRepository.update({ this.assetRepository.update({
id: asset.id, id: asset.id,
duration: this.getDuration(exifTags), duration: this.getDuration(exifTags),
@@ -392,22 +392,34 @@ export class MetadataService extends BaseService {
@OnJob({ name: JobName.SidecarWrite, queue: QueueName.Sidecar }) @OnJob({ name: JobName.SidecarWrite, queue: QueueName.Sidecar })
async handleSidecarWrite(job: JobOf<JobName.SidecarWrite>): Promise<JobStatus> { async handleSidecarWrite(job: JobOf<JobName.SidecarWrite>): Promise<JobStatus> {
const { id, description, dateTimeOriginal, latitude, longitude, rating, tags } = job; const { id, tags } = job;
const asset = await this.assetJobRepository.getForSidecarWriteJob(id); const asset = await this.assetJobRepository.getForSidecarWriteJob(id);
if (!asset) { if (!asset) {
return JobStatus.Failed; return JobStatus.Failed;
} }
const lockedProperties = await this.assetJobRepository.getLockedPropertiesForMetadataExtraction(id);
const tagsList = (asset.tags || []).map((tag) => tag.value); const tagsList = (asset.tags || []).map((tag) => tag.value);
const { sidecarFile } = getAssetFiles(asset.files); const { sidecarFile } = getAssetFiles(asset.files);
const sidecarPath = sidecarFile?.path || `${asset.originalPath}.xmp`; const sidecarPath = sidecarFile?.path || `${asset.originalPath}.xmp`;
const { description, dateTimeOriginal, latitude, longitude, rating } = _.pick(
{
description: asset.exifInfo.description,
dateTimeOriginal: asset.exifInfo.dateTimeOriginal,
latitude: asset.exifInfo.latitude,
longitude: asset.exifInfo.longitude,
rating: asset.exifInfo.rating,
},
lockedProperties,
);
const exif = _.omitBy( const exif = _.omitBy(
<Tags>{ <Tags>{
Description: description, Description: description,
ImageDescription: description, ImageDescription: description,
DateTimeOriginal: dateTimeOriginal, DateTimeOriginal: dateTimeOriginal?.toISOString(),
GPSLatitude: latitude, GPSLatitude: latitude,
GPSLongitude: longitude, GPSLongitude: longitude,
Rating: rating, Rating: rating,

View File

@@ -222,11 +222,6 @@ export interface IDeleteFilesJob extends IBaseJob {
} }
export interface ISidecarWriteJob extends IEntityJob { export interface ISidecarWriteJob extends IEntityJob {
description?: string;
dateTimeOriginal?: string;
latitude?: number;
longitude?: number;
rating?: number;
tags?: true; tags?: true;
} }

View File

@@ -202,7 +202,7 @@ export class MediumTestContext<S extends BaseService = BaseService> {
} }
async newExif(dto: Insertable<AssetExifTable>) { async newExif(dto: Insertable<AssetExifTable>) {
const result = await this.get(AssetRepository).upsertExif(dto); const result = await this.get(AssetRepository).upsertExif(dto, { lockedPropertiesBehavior: 'none' });
return { result }; return { result };
} }

View File

@@ -95,6 +95,7 @@ describe(MetadataService.name, () => {
dateTimeOriginal: new Date(expected.dateTimeOriginal), dateTimeOriginal: new Date(expected.dateTimeOriginal),
timeZone: expected.timeZone, timeZone: expected.timeZone,
}), }),
{ lockedPropertiesBehavior: 'skip' },
); );
expect(mocks.asset.update).toHaveBeenCalledWith( expect(mocks.asset.update).toHaveBeenCalledWith(

View File

@@ -288,10 +288,13 @@ describe(SyncRequestType.AlbumAssetExifsV1, () => {
// update the asset // update the asset
const assetRepository = ctx.get(AssetRepository); const assetRepository = ctx.get(AssetRepository);
await assetRepository.upsertExif({ await assetRepository.upsertExif(
assetId: asset.id, {
city: 'New City', assetId: asset.id,
}); city: 'New City',
},
{ lockedPropertiesBehavior: 'update' },
);
await expect(ctx.syncStream(auth, [SyncRequestType.AlbumAssetExifsV1])).resolves.toEqual([ await expect(ctx.syncStream(auth, [SyncRequestType.AlbumAssetExifsV1])).resolves.toEqual([
{ {
@@ -346,10 +349,13 @@ describe(SyncRequestType.AlbumAssetExifsV1, () => {
// update the asset // update the asset
const assetRepository = ctx.get(AssetRepository); const assetRepository = ctx.get(AssetRepository);
await assetRepository.upsertExif({ await assetRepository.upsertExif(
assetId: assetDelayedExif.id, {
city: 'Delayed Exif', assetId: assetDelayedExif.id,
}); city: 'Delayed Exif',
},
{ lockedPropertiesBehavior: 'update' },
);
await expect(ctx.syncStream(auth, [SyncRequestType.AlbumAssetExifsV1])).resolves.toEqual([ await expect(ctx.syncStream(auth, [SyncRequestType.AlbumAssetExifsV1])).resolves.toEqual([
{ {

View File

@@ -4,6 +4,7 @@ import {
AuthApiKey, AuthApiKey,
AuthSharedLink, AuthSharedLink,
AuthUser, AuthUser,
Exif,
Library, Library,
Memory, Memory,
Partner, Partner,
@@ -319,18 +320,28 @@ const versionHistoryFactory = () => ({
version: '1.123.45', version: '1.123.45',
}); });
const assetSidecarWriteFactory = () => ({ const assetSidecarWriteFactory = () => {
id: newUuid(), const id = newUuid();
originalPath: '/path/to/original-path.jpg.xmp', return {
tags: [], id,
files: [ originalPath: '/path/to/original-path.jpg.xmp',
{ tags: [],
id: newUuid(), files: [
path: '/path/to/original-path.jpg.xmp', {
type: AssetFileType.Sidecar, id: newUuid(),
}, path: '/path/to/original-path.jpg.xmp',
], type: AssetFileType.Sidecar,
}); },
],
exifInfo: {
assetId: id,
description: 'this is a description',
latitude: 12,
longitude: 12,
dateTimeOriginal: '2023-11-22T04:56:12.196Z',
} as unknown as Exif,
};
};
const assetOcrFactory = ( const assetOcrFactory = (
ocr: { ocr: {