sevenhill/Immich
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

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

* Update server/src/services/duplicate.service.spec.ts

Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>

---------

Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>
This commit is contained in:
Timon
2026-06-15 16:05:04 -04:00
committed by GitHub
co-authored by Daniel Dietzler
parent 622a330d82
commit cc8d3b4107
2 changed files with 57 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/services/duplicate.service.spec.ts
+20
View File
@@ -369,6 +369,26 @@ describe(DuplicateService.name, () => {
expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.SidecarWrite, data: { id: asset1.id } }]); 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();
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 // NOTE: The following integration-style tests are covered by E2E tests instead
// to avoid complex mock setup. The validation and error-handling logic above // to avoid complex mock setup. The validation and error-handling logic above
// is thoroughly unit tested. // is thoroughly unit tested.
server/src/services/duplicate.service.ts
+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( const { assetUpdate, exifUpdate, mergedAlbumIds, mergedTagIds, mergedTagValues } = this.getSyncMergeResult(
duplicateGroup.assets, duplicateGroup.assets,
assetAlbumMap, assetAlbumMap,
); );
if (mergedAlbumIds.length > 0) { if (mergedAlbumIds.length > 0) {
const allowedAlbumIds = await this.checkAccess({ const allowedAlbumIds = await this.checkAccess({
auth, auth,
permission: Permission.AlbumAssetCreate, permission: Permission.AlbumAssetCreate,
ids: mergedAlbumIds, ids: mergedAlbumIds,
}); });
const allowedShareIds = await this.checkAccess({ const allowedShareIds = await this.checkAccess({
auth, auth,
permission: Permission.AssetShare, permission: Permission.AssetShare,
ids: idsToKeep, ids: idsToKeep,
}); });
if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) { if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) {
await this.albumRepository.addAssetIdsToAlbums( await this.albumRepository.addAssetIdsToAlbums(
[...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))), [...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))),
); );
}
} }
}
if (mergedTagIds.length > 0) { if (mergedTagIds.length > 0) {
const allowedTagIds = await this.checkAccess({ const allowedTagIds = await this.checkAccess({
auth, auth,
permission: Permission.TagAsset, permission: Permission.TagAsset,
ids: mergedTagIds, ids: mergedTagIds,
}); });
if (allowedTagIds.size > 0) { if (allowedTagIds.size > 0) {
// Replace tags for each keeper asset to ensure all merged tags are applied await Promise.all(
await Promise.all(idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds]))); idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds])),
);
// Update asset_exif.tags so the subsequent SidecarWrite + MetadataExtraction await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
// cycle preserves the merged tags (updateAllExif locks the property automatically) }
await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
} }
}
if (idsToKeep.length > 0) {
const hasExifUpdate = Object.keys(exifUpdate).length > 0; const hasExifUpdate = Object.keys(exifUpdate).length > 0;
const hasTagUpdate = mergedTagIds.length > 0; const hasTagUpdate = mergedTagIds.length > 0;
@@ -213,6 +213,8 @@ export class DuplicateService extends BaseService {
} }
await this.assetRepository.updateAll(idsToKeep, { duplicateId: null, ...assetUpdate }); await this.assetRepository.updateAll(idsToKeep, { duplicateId: null, ...assetUpdate });
} else if (idsToKeep.length > 0) {
await this.assetRepository.updateAll(idsToKeep, { duplicateId: null });
} }
if (idsToTrash.length > 0) { if (idsToTrash.length > 0) {