From 9603c91877cce828c223310635e036688a5371ab Mon Sep 17 00:00:00 2001 From: Louis-Bertrand Varin Date: Fri, 3 Nov 2017 16:29:42 -0400 Subject: [PATCH] Merge : Synchronising groups. --- src/core/Group.cpp | 80 +++++++++++++++++++------ src/core/Group.h | 17 +++++- tests/TestMerge.cpp | 139 +++++++++++++++++++++++++++++++++++++++++--- tests/TestMerge.h | 3 + 4 files changed, 210 insertions(+), 29 deletions(-) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 6f70db347..443f7cbed 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -77,8 +77,7 @@ template inline bool Group::set(P& property, const V& value) updateTimeinfo(); emit modified(); return true; - } - else { + } else { return false; } } @@ -693,27 +692,48 @@ void Group::merge(const Group* other) existingEntry->setGroup(this); qDebug("Location changed for entry %s. Updating it", qPrintable(existingEntry->title())); } - resolveConflict(existingEntry, entry); + resolveEntryConflict(existingEntry, entry); } } // merge groups recursively const QList dbChildren = other->children(); for (Group* group : dbChildren) { - // groups are searched by name instead of uuid - if (findChildByName(group->name())) { - findChildByName(group->name())->merge(group); - } else { + + Group* existingGroup = rootGroup->findChildByUuid(group->uuid()); + + if (!existingGroup) { qDebug("New group %s detected. Creating it.", qPrintable(group->name())); - Group* newGroup = group->clone(Entry::CloneNoFlags, true); + Group* newGroup = group->clone(Entry::CloneNoFlags, Group::CloneNoFlags); newGroup->setParent(this); newGroup->merge(group); + } else { + bool locationChanged = existingGroup->timeInfo().locationChanged() < group->timeInfo().locationChanged(); + if (locationChanged && existingGroup->parent() != this) { + existingGroup->setParent(this); + qDebug("Location changed for group %s. Updating it", qPrintable(existingGroup->name())); + } + resolveGroupConflict(existingGroup, group); + existingGroup->merge(group); } + } emit modified(); } +Group* Group::findChildByUuid(const Uuid& uuid) +{ + Q_ASSERT(!uuid.isNull()); + for (Group* group : groupsRecursive(true)) { + if (group->uuid() == uuid) { + return group; + } + } + + return nullptr; +} + Group* Group::findChildByName(const QString& name) { for (Group* group : asConst(m_children)) { @@ -725,16 +745,21 @@ Group* Group::findChildByName(const QString& name) return nullptr; } -Group* Group::clone(Entry::CloneFlags entryFlags, bool shallow) const +Group* Group::clone(Entry::CloneFlags entryFlags, Group::CloneFlags groupFlags) const { Group* clonedGroup = new Group(); clonedGroup->setUpdateTimeinfo(false); - clonedGroup->setUuid(Uuid::random()); + if (groupFlags & Group::CloneNewUuid) { + clonedGroup->setUuid(Uuid::random()); + } else { + clonedGroup->setUuid(this->uuid()); + } + clonedGroup->m_data = m_data; - if (!shallow) { + if (groupFlags & Group::CloneIncludeEntries) { const QList entryList = entries(); for (Entry* entry : entryList) { Entry* clonedEntry = entry->clone(entryFlags); @@ -743,18 +768,20 @@ Group* Group::clone(Entry::CloneFlags entryFlags, bool shallow) const const QList childrenGroups = children(); for (Group* groupChild : childrenGroups) { - Group* clonedGroupChild = groupChild->clone(entryFlags); + Group* clonedGroupChild = groupChild->clone(entryFlags, groupFlags); clonedGroupChild->setParent(clonedGroup); } } clonedGroup->setUpdateTimeinfo(true); + if (groupFlags & Group::CloneResetTimeInfo) { - QDateTime now = QDateTime::currentDateTimeUtc(); - clonedGroup->m_data.timeInfo.setCreationTime(now); - clonedGroup->m_data.timeInfo.setLastModificationTime(now); - clonedGroup->m_data.timeInfo.setLastAccessTime(now); - clonedGroup->m_data.timeInfo.setLocationChanged(now); + QDateTime now = QDateTime::currentDateTimeUtc(); + clonedGroup->m_data.timeInfo.setCreationTime(now); + clonedGroup->m_data.timeInfo.setLastModificationTime(now); + clonedGroup->m_data.timeInfo.setLastAccessTime(now); + clonedGroup->m_data.timeInfo.setLocationChanged(now); + } return clonedGroup; } @@ -908,7 +935,7 @@ bool Group::resolveAutoTypeEnabled() const } } -void Group::resolveConflict(Entry* existingEntry, Entry* otherEntry) +void Group::resolveEntryConflict(Entry* existingEntry, Entry* otherEntry) { const QDateTime timeExisting = existingEntry->timeInfo().lastModificationTime(); const QDateTime timeOther = otherEntry->timeInfo().lastModificationTime(); @@ -946,6 +973,23 @@ void Group::resolveConflict(Entry* existingEntry, Entry* otherEntry) } } +void Group::resolveGroupConflict(Group* existingGroup, Group* otherGroup) +{ + const QDateTime timeExisting = existingGroup->timeInfo().lastModificationTime(); + const QDateTime timeOther = otherGroup->timeInfo().lastModificationTime(); + + // only if the other group newer, update the existing one. + if (timeExisting < timeOther) { + qDebug("Updating group %s.", qPrintable(existingGroup->name())); + existingGroup->setName(otherGroup->name()); + existingGroup->setNotes(otherGroup->notes()); + existingGroup->setIcon(otherGroup->iconNumber()); + existingGroup->setIcon(otherGroup->iconUuid()); + existingGroup->setExpiryTime(otherGroup->timeInfo().expiryTime()); + } + +} + QStringList Group::locate(QString locateTerm, QString currentPath) { Q_ASSERT(!locateTerm.isNull()); diff --git a/src/core/Group.h b/src/core/Group.h index 2a9d2b18c..6d309bdcf 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -37,6 +37,14 @@ public: enum TriState { Inherit, Enable, Disable }; enum MergeMode { ModeInherit, KeepBoth, KeepNewer, KeepExisting }; + enum CloneFlag { + CloneNoFlags = 0, + CloneNewUuid = 1, // generate a random uuid for the clone + CloneResetTimeInfo = 2, // set all TimeInfo attributes to the current time + CloneIncludeEntries = 4, // clone the group entries + }; + Q_DECLARE_FLAGS(CloneFlags, CloneFlag) + struct GroupData { QString name; @@ -80,6 +88,7 @@ public: static const int RecycleBinIconNumber; Group* findChildByName(const QString& name); + Group* findChildByUuid(const Uuid& uuid); Entry* findEntry(QString entryId); Entry* findEntryByUuid(const Uuid& uuid); Entry* findEntryByPath(QString entryPath, QString basePath = QString("")); @@ -126,7 +135,8 @@ public: * new group into another database. */ Group* clone(Entry::CloneFlags entryFlags = Entry::CloneNewUuid | Entry::CloneResetTimeInfo, - bool shallow = false) const; + CloneFlags groupFlags = static_cast(Group::CloneNewUuid | Group::CloneResetTimeInfo | + Group::CloneIncludeEntries)) const; void copyDataFrom(const Group* other); void merge(const Group* other); QString print(bool recursive = false, int depth = 0); @@ -160,7 +170,8 @@ private: void removeEntry(Entry* entry); void setParent(Database* db); void markOlderEntry(Entry* entry); - void resolveConflict(Entry* existingEntry, Entry* otherEntry); + void resolveEntryConflict(Entry* existingEntry, Entry* otherEntry); + void resolveGroupConflict(Group* existingGroup, Group* otherGroup); void recSetDatabase(Database* db); void cleanupParent(); @@ -183,4 +194,6 @@ private: friend void Entry::setGroup(Group* group); }; +Q_DECLARE_OPERATORS_FOR_FLAGS(Group::CloneFlags) + #endif // KEEPASSX_GROUP_H diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index 4f8daa068..d68c4f102 100644 --- a/tests/TestMerge.cpp +++ b/tests/TestMerge.cpp @@ -64,7 +64,7 @@ void TestMerge::testMergeNoChanges() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); QCOMPARE(dbDestination->rootGroup()->entriesRecursive().size(), 2); QCOMPARE(dbSource->rootGroup()->entriesRecursive().size(), 2); @@ -92,7 +92,7 @@ void TestMerge::testResolveConflictNewer() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); // sanity check Group* group1 = dbSource->rootGroup()->findChildByName("group1"); @@ -141,7 +141,7 @@ void TestMerge::testResolveConflictOlder() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); // sanity check Group* group1 = dbSource->rootGroup()->findChildByName("group1"); @@ -197,7 +197,7 @@ void TestMerge::testResolveConflictKeepBoth() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneIncludeHistory)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneIncludeHistory, Group::CloneIncludeEntries)); // sanity check QCOMPARE(dbDestination->rootGroup()->children().at(0)->entries().size(), 2); @@ -236,7 +236,7 @@ void TestMerge::testMoveEntry() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); Entry* entry1 = dbSource->rootGroup()->findEntry("entry1"); QVERIFY(entry1 != nullptr); @@ -270,7 +270,7 @@ void TestMerge::testMoveEntryPreserveChanges() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); Entry* entry1 = dbSource->rootGroup()->findEntry("entry1"); QVERIFY(entry1 != nullptr); @@ -307,11 +307,12 @@ void TestMerge::testCreateNewGroups() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); QTest::qSleep(1); Group* group3 = new Group(); group3->setName("group3"); + group3->setUuid(Uuid::random()); group3->setParent(dbSource->rootGroup()); dbDestination->merge(dbSource); @@ -329,11 +330,12 @@ void TestMerge::testMoveEntryIntoNewGroup() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); QTest::qSleep(1); Group* group3 = new Group(); group3->setName("group3"); + group3->setUuid(Uuid::random()); group3->setParent(dbSource->rootGroup()); Entry* entry1 = dbSource->rootGroup()->findEntry("entry1"); @@ -365,10 +367,11 @@ void TestMerge::testUpdateEntryDifferentLocation() Database* dbDestination = createTestDatabase(); Database* dbSource = new Database(); - dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags)); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); Group* group3 = new Group(); group3->setName("group3"); + group3->setUuid(Uuid::random()); group3->setParent(dbDestination->rootGroup()); Entry* entry1 = dbDestination->rootGroup()->findEntry("entry1"); @@ -399,6 +402,84 @@ void TestMerge::testUpdateEntryDifferentLocation() delete dbSource; } +/** + * Groups should be updated using the uuids. + */ +void TestMerge::testUpdateGroup() +{ + Database* dbDestination = createTestDatabase(); + + Database* dbSource = new Database(); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); + + QTest::qSleep(1); + + Group* group2 = dbSource->rootGroup()->findChildByName("group2"); + group2->setName("group2 renamed"); + group2->setNotes("updated notes"); + Uuid customIconId = Uuid::random(); + QImage customIcon; + dbSource->metadata()->addCustomIcon(customIconId, customIcon); + group2->setIcon(customIconId); + + Entry* entry1 = dbSource->rootGroup()->findEntry("entry1"); + QVERIFY(entry1 != nullptr); + entry1->setGroup(group2); + entry1->setTitle("entry1 renamed"); + Uuid uuidBeforeSyncing = entry1->uuid(); + + dbDestination->merge(dbSource); + + QCOMPARE(dbDestination->rootGroup()->entriesRecursive().size(), 2); + + entry1 = dbDestination->rootGroup()->findEntry("entry1 renamed"); + QVERIFY(entry1 != nullptr); + QVERIFY(entry1->group() != nullptr); + QCOMPARE(entry1->group()->name(), QString("group2 renamed")); + QCOMPARE(uuidBeforeSyncing, entry1->uuid()); + + group2 = dbDestination->rootGroup()->findChildByName("group2 renamed"); + QCOMPARE(group2->notes(), QString("updated notes")); + QCOMPARE(group2->iconUuid(), customIconId); + + delete dbDestination; + delete dbSource; +} + +void TestMerge::testUpdateGroupLocation() +{ + Database* dbDestination = createTestDatabase(); + Group* group3 = new Group(); + Uuid group3Uuid = Uuid::random(); + group3->setUuid(group3Uuid); + group3->setName("group3"); + group3->setParent(dbDestination->rootGroup()->findChildByName("group1")); + + Database* dbSource = new Database(); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); + + // Sanity check + group3 = dbSource->rootGroup()->findChildByUuid(group3Uuid); + QVERIFY(group3 != nullptr); + + QTest::qSleep(1); + + group3->setParent(dbSource->rootGroup()->findChildByName("group2")); + + dbDestination->merge(dbSource); + group3 = dbDestination->rootGroup()->findChildByUuid(group3Uuid); + QVERIFY(group3 != nullptr); + QCOMPARE(group3->parent(), dbDestination->rootGroup()->findChildByName("group2")); + + dbDestination->merge(dbSource); + group3 = dbDestination->rootGroup()->findChildByUuid(group3Uuid); + QVERIFY(group3 != nullptr); + QCOMPARE(group3->parent(), dbDestination->rootGroup()->findChildByName("group2")); + + delete dbDestination; + delete dbSource; +} + /** * The first merge should create new entries, the * second should only sync them, since they have @@ -447,14 +528,54 @@ void TestMerge::testMergeCustomIcons() delete dbSource; } +/** + * If the group is updated in the source database, and the + * destination database after, the group should remain the + * same. + */ +void TestMerge::testResolveGroupConflictOlder() +{ + Database* dbDestination = createTestDatabase(); + + Database* dbSource = new Database(); + dbSource->setRootGroup(dbDestination->rootGroup()->clone(Entry::CloneNoFlags, Group::CloneIncludeEntries)); + + // sanity check + Group* group1 = dbSource->rootGroup()->findChildByName("group1"); + QVERIFY(group1 != nullptr); + + // Make sure the two changes have a different timestamp. + QTest::qSleep(1); + group1->setName("group1 updated in source"); + + // Make sure the two changes have a different timestamp. + QTest::qSleep(1); + + group1 = dbDestination->rootGroup()->findChildByName("group1"); + group1->setName("group1 updated in destination"); + + dbDestination->merge(dbSource); + + // sanity check + group1 = dbDestination->rootGroup()->findChildByName("group1 updated in destination"); + QVERIFY(group1 != nullptr); + + delete dbDestination; + delete dbSource; +} + + Database* TestMerge::createTestDatabase() { Database* db = new Database(); Group* group1 = new Group(); group1->setName("group1"); + group1->setUuid(Uuid::random()); + Group* group2 = new Group(); group2->setName("group2"); + group2->setUuid(Uuid::random()); Entry* entry1 = new Entry(); Entry* entry2 = new Entry(); diff --git a/tests/TestMerge.h b/tests/TestMerge.h index 0b3ec618e..3588cfd53 100644 --- a/tests/TestMerge.h +++ b/tests/TestMerge.h @@ -31,12 +31,15 @@ private slots: void testMergeNoChanges(); void testResolveConflictNewer(); void testResolveConflictOlder(); + void testResolveGroupConflictOlder(); void testResolveConflictKeepBoth(); void testMoveEntry(); void testMoveEntryPreserveChanges(); void testMoveEntryIntoNewGroup(); void testCreateNewGroups(); void testUpdateEntryDifferentLocation(); + void testUpdateGroup(); + void testUpdateGroupLocation(); void testMergeAndSync(); void testMergeCustomIcons();