Merger: prevent duplicate entry when merging histories

If the alien database (source) entry is newer, a copy of the entry is
made. But before moving the merged entry to the target group, it must be
removed. Otherwise there will be briefly two entries with the same UUID
in the same group/database.

Even though this is only the case during the transaction, it can still
be observed because the operations emit signals. A notable problem is
the fdosecrets feature that relies on the uniqueness of the UUID or will
otherwise run into problems because the UUID is used as part of the DBus
path.

src/core/Merger.cpp

@@ -261,6 +261,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
     const int comparison = compare(targetEntry->timeInfo().lastModificationTime(),
                                    sourceEntry->timeInfo().lastModificationTime(),
                                    CompareItemIgnoreMilliseconds);
+    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     if (comparison < 0) {
         Group* currentGroup = targetEntry->group();
         Entry* clonedEntry = sourceEntry->clone(Entry::CloneIncludeHistory);
@@ -269,15 +270,15 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
                qPrintable(sourceEntry->title()),
                qPrintable(currentGroup->name()));
         changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
-        moveEntry(clonedEntry, currentGroup);
-        mergeHistory(targetEntry, clonedEntry, mergeMethod);
+        mergeHistory(targetEntry, clonedEntry, mergeMethod, maxItems);
         eraseEntry(targetEntry);
+        moveEntry(clonedEntry, currentGroup);
     } else {
         qDebug("Merge %s/%s with local on top/under %s",
                qPrintable(targetEntry->title()),
                qPrintable(sourceEntry->title()),
                qPrintable(targetEntry->group()->name()));
-        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod);
+        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod, maxItems);
         if (changed) {
             changes
                 << tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
@@ -297,7 +298,10 @@ Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEnt
     return resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode);
 }
 
-bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod)
+bool Merger::mergeHistory(const Entry* sourceEntry,
+                          Entry* targetEntry,
+                          Group::MergeMode mergeMethod,
+                          const int maxItems)
 {
     Q_UNUSED(mergeMethod);
     const auto targetHistoryItems = targetEntry->historyItems();
@@ -370,7 +374,6 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M
     }
 
     bool changed = false;
-    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     const auto updatedHistoryItems = merged.values();
     for (int i = 0; i < maxItems; ++i) {
         const Entry* oldEntry = targetHistoryItems.value(targetHistoryItems.count() - i);

src/core/Merger.h

@@ -49,7 +49,7 @@ class Merger : public QObject
     ChangeList mergeGroup(const MergeContext& context);
     ChangeList mergeDeletions(const MergeContext& context);
     ChangeList mergeMetadata(const MergeContext& context);
-    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod);
+    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod, const int maxItems);
     void moveEntry(Entry* entry, Group* targetGroup);
     void moveGroup(Group* group, Group* targetGroup);
     // remove an entry without a trace in the deletedObjects - needed for elemination cloned entries