Support Database Custom Data Merging (#3002)

* Introduce _LAST_MODIFIED custom data entry that stores the last modified datetime of the database's custom data entries
* Merge custom data from source database to target
* Modify tests to be aware of _LAST_MODIFIED entry

src/core/CustomData.cpp

@@ -16,9 +16,12 @@
  */
 
 #include "CustomData.h"
+#include "Clock.h"
 
 #include "core/Global.h"
 
+const QString CustomData::LastModified = "_LAST_MODIFIED";
+
 CustomData::CustomData(QObject* parent)
     : QObject(parent)
 {
@@ -60,6 +63,7 @@ void CustomData::set(const QString& key, const QString& value)
 
     if (addAttribute || changeValue) {
         m_data.insert(key, value);
+        updateLastModified();
         emit customDataModified();
     }
 
@@ -74,6 +78,7 @@ void CustomData::remove(const QString& key)
 
     m_data.remove(key);
 
+    updateLastModified();
     emit removed(key);
     emit customDataModified();
 }
@@ -94,6 +99,7 @@ void CustomData::rename(const QString& oldKey, const QString& newKey)
     m_data.remove(oldKey);
     m_data.insert(newKey, data);
 
+    updateLastModified();
     emit customDataModified();
     emit renamed(oldKey, newKey);
 }
@@ -108,9 +114,19 @@ void CustomData::copyDataFrom(const CustomData* other)
 
     m_data = other->m_data;
 
+    updateLastModified();
     emit reset();
     emit customDataModified();
 }
+
+QDateTime CustomData::getLastModified() const
+{
+    if (m_data.contains(LastModified)) {
+        return Clock::parse(m_data.value(LastModified));
+    }
+    return {};
+}
+
 bool CustomData::operator==(const CustomData& other) const
 {
     return (m_data == other.m_data);
@@ -152,3 +168,13 @@ int CustomData::dataSize() const
     }
     return size;
 }
+
+void CustomData::updateLastModified()
+{
+    if (m_data.size() == 1 && m_data.contains(LastModified)) {
+        m_data.remove(LastModified);
+        return;
+    }
+
+    m_data.insert(LastModified, Clock::currentDateTimeUtc().toString());
+}

src/core/CustomData.h

@@ -42,9 +42,12 @@ public:
     int size() const;
     int dataSize() const;
     void copyDataFrom(const CustomData* other);
+    QDateTime getLastModified() const;
     bool operator==(const CustomData& other) const;
     bool operator!=(const CustomData& other) const;
 
+    static const QString LastModified;
+
 signals:
     void customDataModified();
     void aboutToBeAdded(const QString& key);
@@ -55,6 +58,10 @@ signals:
     void renamed(const QString& oldKey, const QString& newKey);
     void aboutToBeReset();
     void reset();
+    void lastModified();
+
+private slots:
+    void updateLastModified();
 
 private:
     QHash<QString, QString> m_data;

src/core/Merger.cpp

@@ -609,9 +609,6 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
     // TODO HNH: missing handling of recycle bin, names, templates for groups and entries,
     //           public data (entries of newer dict override keys of older dict - ignoring
     //           their own age - it is enough if one entry of the whole dict is newer) => possible lost update
-    // TODO HNH: CustomData is merged with entries of the new customData overwrite entries
-    //           of the older CustomData - the dict with the newest entry is considered
-    //           newer regardless of the age of the other entries => possible lost update
     ChangeList changes;
     auto* sourceMetadata = context.m_sourceDb->metadata();
     auto* targetMetadata = context.m_targetDb->metadata();
@@ -624,5 +621,32 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
             changes << tr("Adding missing icon %1").arg(QString::fromLatin1(customIconId.toRfc4122().toHex()));
         }
     }
+
+    // Merge Custom Data if source is newer
+    const auto targetCustomDataModificationTime = sourceMetadata->customData()->getLastModified();
+    const auto sourceCustomDataModificationTime = targetMetadata->customData()->getLastModified();
+    if (!targetMetadata->customData()->contains(CustomData::LastModified) ||
+        (targetCustomDataModificationTime.isValid() && sourceCustomDataModificationTime.isValid() && 
+         targetCustomDataModificationTime > sourceCustomDataModificationTime)) {
+        const auto sourceCustomDataKeys = sourceMetadata->customData()->keys();
+        const auto targetCustomDataKeys = targetMetadata->customData()->keys();
+
+        // Check missing keys from source. Remove those from target
+        for (const auto& key : targetCustomDataKeys) {
+            if (!sourceMetadata->customData()->contains(key)) {
+                auto value = targetMetadata->customData()->value(key);
+                targetMetadata->customData()->remove(key);
+                changes << tr("Removed custom data %1 [%2]").arg(key, value);
+            }
+        }
+
+        // Transfer new/existing keys
+        for (const auto& key : sourceCustomDataKeys) {
+            auto value = sourceMetadata->customData()->value(key);
+            targetMetadata->customData()->set(key, value);
+            changes << tr("Adding custom data %1 [%2]").arg(key, value);
+        }
+    }
+
     return changes;
 }

tests/TestKdbx4.cpp

@@ -186,8 +186,10 @@ void TestKdbx4::testFormat400Upgrade()
 
     QCOMPARE(reader.version(), expectedVersion);
     QCOMPARE(targetDb->cipher(), cipherUuid);
-    QCOMPARE(*targetDb->metadata()->customData(), *sourceDb->metadata()->customData());
+    QCOMPARE(targetDb->metadata()->customData()->value("CustomPublicData"),
-    QCOMPARE(*targetDb->rootGroup()->customData(), *sourceDb->rootGroup()->customData());
+             sourceDb->metadata()->customData()->value("CustomPublicData"));
+    QCOMPARE(targetDb->rootGroup()->customData()->value("CustomGroupData"),
+             sourceDb->rootGroup()->customData()->value("CustomGroupData"));
 }
 
 // clang-format off
@@ -346,20 +348,22 @@ void TestKdbx4::testCustomData()
     const QString customDataKey2 = "CD2";
     const QString customData1 = "abcäöü";
     const QString customData2 = "Hello World";
-    const int dataSize = customDataKey1.toUtf8().size() + customDataKey1.toUtf8().size() + customData1.toUtf8().size()
-                         + customData2.toUtf8().size();
 
     // test custom database data
     db.metadata()->customData()->set(customDataKey1, customData1);
     db.metadata()->customData()->set(customDataKey2, customData2);
-    QCOMPARE(db.metadata()->customData()->size(), 2);
+    auto lastModified = db.metadata()->customData()->value(CustomData::LastModified);
+    const int dataSize = customDataKey1.toUtf8().size() + customDataKey1.toUtf8().size() + customData1.toUtf8().size()
+                         + customData2.toUtf8().size() + lastModified.toUtf8().size()
+                         + CustomData::LastModified.toUtf8().size();
+    QCOMPARE(db.metadata()->customData()->size(), 3);
     QCOMPARE(db.metadata()->customData()->dataSize(), dataSize);
 
     // test custom root group data
     Group* root = db.rootGroup();
     root->customData()->set(customDataKey1, customData1);
     root->customData()->set(customDataKey2, customData2);
-    QCOMPARE(root->customData()->size(), 2);
+    QCOMPARE(root->customData()->size(), 3);
     QCOMPARE(root->customData()->dataSize(), dataSize);
 
     // test copied custom group data
@@ -378,9 +382,9 @@ void TestKdbx4::testCustomData()
 
     // test custom data deletion
     entry->customData()->set("additional item", "foobar");
-    QCOMPARE(entry->customData()->size(), 3);
+    QCOMPARE(entry->customData()->size(), 4);
     entry->customData()->remove("additional item");
-    QCOMPARE(entry->customData()->size(), 2);
+    QCOMPARE(entry->customData()->size(), 3);
     QCOMPARE(entry->customData()->dataSize(), dataSize);
 
     // test custom data on cloned groups

tests/TestMerge.cpp

@@ -1164,6 +1164,65 @@ void TestMerge::testMetadata()
     //           will be used - exception is the target has no recycle bin activated
 }
 
+void TestMerge::testCustomdata()
+{
+    QScopedPointer<Database> dbDestination(new Database());
+    QScopedPointer<Database> dbSource(createTestDatabase());
+    QScopedPointer<Database> dbDestination2(new Database());
+    QScopedPointer<Database> dbSource2(createTestDatabase());
+
+    m_clock->advanceSecond(1);
+    
+    dbDestination->metadata()->customData()->set("toBeDeleted", "value");
+    dbDestination->metadata()->customData()->set("key3", "oldValue");
+
+    dbSource2->metadata()->customData()->set("key1", "value1");
+    dbSource2->metadata()->customData()->set("key2", "value2");
+    dbSource2->metadata()->customData()->set("key3", "newValue");
+    dbSource2->metadata()->customData()->set("Browser", "n'8=3W@L^6d->d.]St_>]");
+
+    m_clock->advanceSecond(1);
+
+    dbSource->metadata()->customData()->set("key1", "value1");
+    dbSource->metadata()->customData()->set("key2", "value2");
+    dbSource->metadata()->customData()->set("key3", "newValue");
+    dbSource->metadata()->customData()->set("Browser", "n'8=3W@L^6d->d.]St_>]");
+
+    dbDestination2->metadata()->customData()->set("notToBeDeleted", "value");
+    dbDestination2->metadata()->customData()->set("key3", "oldValue");
+
+    // Sanity check.
+    QVERIFY(!dbSource->metadata()->customData()->isEmpty());
+    QVERIFY(!dbSource2->metadata()->customData()->isEmpty());
+
+    m_clock->advanceSecond(1);
+
+    Merger merger(dbSource.data(), dbDestination.data());
+    merger.merge();
+
+    Merger merger2(dbSource2.data(), dbDestination2.data());
+    merger2.merge();
+
+    // Source is newer, data should be merged
+    QVERIFY(!dbDestination->metadata()->customData()->isEmpty());
+    QVERIFY(dbDestination->metadata()->customData()->contains("key1"));
+    QVERIFY(dbDestination->metadata()->customData()->contains("key2"));
+    QVERIFY(dbDestination->metadata()->customData()->contains("Browser"));
+    QVERIFY(!dbDestination->metadata()->customData()->contains("toBeDeleted"));
+    QCOMPARE(dbDestination->metadata()->customData()->value("key1"), QString("value1"));
+    QCOMPARE(dbDestination->metadata()->customData()->value("key2"), QString("value2"));
+    QCOMPARE(dbDestination->metadata()->customData()->value("Browser"), QString("n'8=3W@L^6d->d.]St_>]"));
+    QCOMPARE(dbDestination->metadata()->customData()->value("key3"), QString("newValue")); // Old value should be replaced
+
+    // Target is newer, no data is merged
+    QVERIFY(!dbDestination2->metadata()->customData()->isEmpty());
+    QVERIFY(!dbDestination2->metadata()->customData()->contains("key1"));
+    QVERIFY(!dbDestination2->metadata()->customData()->contains("key2"));
+    QVERIFY(!dbDestination2->metadata()->customData()->contains("Browser"));
+    QVERIFY(dbDestination2->metadata()->customData()->contains("notToBeDeleted"));
+    QCOMPARE(dbDestination2->metadata()->customData()->value("key3"), QString("oldValue")); // Old value should not be replaced
+}
+
 void TestMerge::testDeletedEntry()
 {
     QScopedPointer<Database> dbDestination(createTestDatabase());

tests/TestMerge.h

@@ -59,6 +59,7 @@ private slots:
     void testMergeCustomIcons();
     void testMergeDuplicateCustomIcons();
     void testMetadata();
+    void testCustomdata();
     void testDeletedEntry();
     void testDeletedGroup();
     void testDeletedRevertedEntry();