Fix duplicate icon add in export

SharedObserver did not check for already added icons add during export
leading to duplicate icons in the target db.

src/core/Merger.cpp

@@ -618,8 +618,8 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
 
     const auto keys = sourceMetadata->customIcons().keys();
     for (QUuid customIconId : keys) {
-        QImage customIcon = sourceMetadata->customIcon(customIconId);
         if (!targetMetadata->containsCustomIcon(customIconId)) {
+            QImage customIcon = sourceMetadata->customIcon(customIconId);
             targetMetadata->addCustomIcon(customIconId, customIcon);
             changes << tr("Adding missing icon %1").arg(QString::fromLatin1(customIconId.toRfc4122().toHex()));
         }

src/core/Metadata.cpp

@@ -382,10 +382,12 @@ void Metadata::addCustomIcon(const QUuid& uuid, const QImage& icon)
     Q_ASSERT(!uuid.isNull());
     Q_ASSERT(!m_customIcons.contains(uuid));
 
-    m_customIcons.insert(uuid, icon);
+    m_customIcons[uuid] = icon;
     // reset cache in case there is also an icon with that uuid
     m_customIconCacheKeys[uuid] = QPixmapCache::Key();
     m_customIconScaledCacheKeys[uuid] = QPixmapCache::Key();
+    // remove all uuids to prevent duplicates in release mode
+    m_customIconsOrder.removeAll(uuid);
     m_customIconsOrder.append(uuid);
     // Associate image hash to uuid
     QByteArray hash = hashImage(icon);

src/keeshare/ShareObserver.cpp

@@ -592,7 +592,8 @@ Database* ShareObserver::exportIntoContainer(const KeeShareSettings::Reference&
 {
     const auto* sourceDb = sourceRoot->database();
     auto* targetDb = new Database();
-    targetDb->metadata()->setRecycleBinEnabled(false);
+    auto* targetMetadata = targetDb->metadata();
+    targetMetadata->setRecycleBinEnabled(false);
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create(reference.password));
 
@@ -610,8 +611,8 @@ Database* ShareObserver::exportIntoContainer(const KeeShareSettings::Reference&
         targetEntry->setGroup(targetRoot);
         targetEntry->setUpdateTimeinfo(updateTimeinfo);
         const auto iconUuid = targetEntry->iconUuid();
-        if (!iconUuid.isNull()) {
+        if (!iconUuid.isNull() && !targetMetadata->containsCustomIcon(iconUuid)) {
-            targetDb->metadata()->addCustomIcon(iconUuid, sourceEntry->icon());
+            targetMetadata->addCustomIcon(iconUuid, sourceEntry->icon());
         }
     }
 

tests/TestMerge.cpp

@@ -1127,6 +1127,34 @@ void TestMerge::testMergeCustomIcons()
     QVERIFY(dbDestination->metadata()->containsCustomIcon(customIconId));
 }
 
+/**
+ * No duplicate icons should be created
+ */
+void TestMerge::testMergeDuplicateCustomIcons()
+{
+    QScopedPointer<Database> dbDestination(new Database());
+    QScopedPointer<Database> dbSource(createTestDatabase());
+
+    m_clock->advanceSecond(1);
+
+    QUuid customIconId = QUuid::createUuid();
+    QImage customIcon;
+
+    dbSource->metadata()->addCustomIcon(customIconId, customIcon);
+    dbDestination->metadata()->addCustomIcon(customIconId, customIcon);
+    // Sanity check.
+    QVERIFY(dbSource->metadata()->containsCustomIcon(customIconId));
+    QVERIFY(dbDestination->metadata()->containsCustomIcon(customIconId));
+
+    m_clock->advanceSecond(1);
+
+    Merger merger(dbSource.data(), dbDestination.data());
+    merger.merge();
+
+    QVERIFY(dbDestination->metadata()->containsCustomIcon(customIconId));
+    QCOMPARE(dbDestination->metadata()->customIcons().count(), 1);
+}
+
 void TestMerge::testMetadata()
 {
     QSKIP("Sophisticated merging for Metadata not implemented");

tests/TestMerge.h

@@ -57,6 +57,7 @@ private slots:
     void testUpdateGroupLocation();
     void testMergeAndSync();
     void testMergeCustomIcons();
+    void testMergeDuplicateCustomIcons();
     void testMetadata();
     void testDeletedEntry();
     void testDeletedGroup();