fix(server): do not merge metadata when multiple duplicates are kept

This commit is contained in:
timonrieger
2026-06-12 18:41:49 +02:00
parent 43b2d04e2c
commit 3087a1268d
2 changed files with 57 additions and 35 deletions
@@ -369,6 +369,26 @@ describe(DuplicateService.name, () => {
expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.SidecarWrite, data: { id: asset1.id } }]);
});
it('should not merge metadata when multiple assets are kept', async () => {
const asset1 = { ...AssetFactory.create(), isFavorite: true };
const asset2 = { ...AssetFactory.create(), isFavorite: false };
mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));
mocks.duplicateRepository.get.mockResolvedValue({
duplicateId: 'group-1',
assets: [asset1 as unknown as MapAsset, asset2 as unknown as MapAsset],
});
const result = await sut.resolve(authStub.admin, {
groups: [{ duplicateId: 'group-1', keepAssetIds: [asset1.id, asset2.id], trashAssetIds: [] }],
});
expect(result[0].success).toBe(true);
expect(mocks.album.addAssetIdsToAlbums).not.toHaveBeenCalled();
expect(mocks.tag.replaceAssetTags).not.toHaveBeenCalled();
expect(mocks.asset.updateAllExif).not.toHaveBeenCalled();
expect(mocks.asset.updateAll).toHaveBeenCalledWith([asset1.id, asset2.id], { duplicateId: null });
});
// NOTE: The following integration-style tests are covered by E2E tests instead
// to avoid complex mock setup. The validation and error-handling logic above
// is thoroughly unit tested.
+37 -35
View File
@@ -156,51 +156,51 @@ export class DuplicateService extends BaseService {
}
}
const assetAlbumMap = await this.albumRepository.getByAssetIds(auth.user.id, [...groupAssetIds]);
// Only merge metadata into the keeper when exactly one asset can absorb trashed duplicates.
if (idsToKeep.length === 1 && idsToTrash.length > 0) {
const assetAlbumMap = await this.albumRepository.getByAssetIds(auth.user.id, [...groupAssetIds]);
const { assetUpdate, exifUpdate, mergedAlbumIds, mergedTagIds, mergedTagValues } = this.getSyncMergeResult(
duplicateGroup.assets,
assetAlbumMap,
);
const { assetUpdate, exifUpdate, mergedAlbumIds, mergedTagIds, mergedTagValues } = this.getSyncMergeResult(
duplicateGroup.assets,
assetAlbumMap,
);
if (mergedAlbumIds.length > 0) {
const allowedAlbumIds = await this.checkAccess({
auth,
permission: Permission.AlbumAssetCreate,
ids: mergedAlbumIds,
});
if (mergedAlbumIds.length > 0) {
const allowedAlbumIds = await this.checkAccess({
auth,
permission: Permission.AlbumAssetCreate,
ids: mergedAlbumIds,
});
const allowedShareIds = await this.checkAccess({
auth,
permission: Permission.AssetShare,
ids: idsToKeep,
});
const allowedShareIds = await this.checkAccess({
auth,
permission: Permission.AssetShare,
ids: idsToKeep,
});
if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) {
await this.albumRepository.addAssetIdsToAlbums(
[...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))),
);
if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) {
await this.albumRepository.addAssetIdsToAlbums(
[...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))),
);
}
}
}
if (mergedTagIds.length > 0) {
const allowedTagIds = await this.checkAccess({
auth,
permission: Permission.TagAsset,
ids: mergedTagIds,
});
if (mergedTagIds.length > 0) {
const allowedTagIds = await this.checkAccess({
auth,
permission: Permission.TagAsset,
ids: mergedTagIds,
});
if (allowedTagIds.size > 0) {
// Replace tags for each keeper asset to ensure all merged tags are applied
await Promise.all(idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds])));
if (allowedTagIds.size > 0) {
await Promise.all(
idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds])),
);
// Update asset_exif.tags so the subsequent SidecarWrite + MetadataExtraction
// cycle preserves the merged tags (updateAllExif locks the property automatically)
await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
}
}
}
if (idsToKeep.length > 0) {
const hasExifUpdate = Object.keys(exifUpdate).length > 0;
const hasTagUpdate = mergedTagIds.length > 0;
@@ -213,6 +213,8 @@ export class DuplicateService extends BaseService {
}
await this.assetRepository.updateAll(idsToKeep, { duplicateId: null, ...assetUpdate });
} else if (idsToKeep.length > 0) {
await this.assetRepository.updateAll(idsToKeep, { duplicateId: null });
}
if (idsToTrash.length > 0) {