From e4eee897f9fb931d8081f9b70da0b0f1e6a8b9b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20V=C3=A4nttinen?= Date: Thu, 2 May 2019 01:35:08 +0300 Subject: [PATCH] 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 | 26 ++++++++++++++++++ src/core/CustomData.h | 7 +++++ src/core/Merger.cpp | 30 ++++++++++++++++++--- tests/TestKdbx4.cpp | 20 ++++++++------ tests/TestMerge.cpp | 59 +++++++++++++++++++++++++++++++++++++++++ tests/TestMerge.h | 1 + 6 files changed, 132 insertions(+), 11 deletions(-) diff --git a/src/core/CustomData.cpp b/src/core/CustomData.cpp index 86adae158..f009176a0 100644 --- a/src/core/CustomData.cpp +++ b/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()); +} diff --git a/src/core/CustomData.h b/src/core/CustomData.h index d085c9409..126d4d84e 100644 --- a/src/core/CustomData.h +++ b/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 m_data; diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index c73248388..dcbed250f 100644 --- a/src/core/Merger.cpp +++ b/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; } diff --git a/tests/TestKdbx4.cpp b/tests/TestKdbx4.cpp index fe4679da5..88352d825 100644 --- a/tests/TestKdbx4.cpp +++ b/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->rootGroup()->customData(), *sourceDb->rootGroup()->customData()); + QCOMPARE(targetDb->metadata()->customData()->value("CustomPublicData"), + 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 diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index 03eae32ef..4d9aef211 100644 --- a/tests/TestMerge.cpp +++ b/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 dbDestination(new Database()); + QScopedPointer dbSource(createTestDatabase()); + QScopedPointer dbDestination2(new Database()); + QScopedPointer 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 dbDestination(createTestDatabase()); diff --git a/tests/TestMerge.h b/tests/TestMerge.h index 357f85262..15f67ca79 100644 --- a/tests/TestMerge.h +++ b/tests/TestMerge.h @@ -59,6 +59,7 @@ private slots: void testMergeCustomIcons(); void testMergeDuplicateCustomIcons(); void testMetadata(); + void testCustomdata(); void testDeletedEntry(); void testDeletedGroup(); void testDeletedRevertedEntry();