Merge pull request #1165 from louib/feature_merge_enhancements

Merge : Synchronising groups.
2025-02-14 13:41:44 -05:00 · 2017-11-21 23:17:26 +01:00 · 2017-11-21 23:17:26 +01:00 · b0e6bcfaa0
commit b0e6bcfaa0
parent feba525e7c f25c23c051
5 changed files with 240 additions and 35 deletions
--- a/src/core/Group.cpp
+++ b/src/core/Group.cpp
@ -26,6 +26,11 @@
 const int Group::DefaultIconNumber = 48;
 const int Group::RecycleBinIconNumber = 43;

+Group::CloneFlags Group::DefaultCloneFlags = static_cast<Group::CloneFlags>(
+    Group::CloneNewUuid | Group::CloneResetTimeInfo | Group::CloneIncludeEntries);
+Entry::CloneFlags Group::DefaultEntryCloneFlags = static_cast<Entry::CloneFlags>(
+    Entry::CloneNewUuid | Entry::CloneResetTimeInfo);
+
 Group::Group()
    : m_updateTimeinfo(true)
 {
@ -77,8 +82,7 @@ template <class P, class V> inline bool Group::set(P& property, const V& value)
        updateTimeinfo();
        emit modified();
        return true;
-    }
-    else {
+    } else {
        return false;
    }
 }
@ -682,38 +686,59 @@ void Group::merge(const Group* other)

        Entry* existingEntry = rootGroup->findEntryByUuid(entry->uuid());

-        // This entry does not exist at all. Create it.
        if (!existingEntry) {
+            // This entry does not exist at all. Create it.
            qDebug("New entry %s detected. Creating it.", qPrintable(entry->title()));
            entry->clone(Entry::CloneIncludeHistory)->setGroup(this);
-        // Entry is already present in the database. Update it.
        } else {
+            // Entry is already present in the database. Update it.
            bool locationChanged = existingEntry->timeInfo().locationChanged() < entry->timeInfo().locationChanged();
            if (locationChanged && existingEntry->group() != this) {
                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<Group*> 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 +750,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<Entry*> entryList = entries();
        for (Entry* entry : entryList) {
            Entry* clonedEntry = entry->clone(entryFlags);
@ -743,18 +773,20 @@ Group* Group::clone(Entry::CloneFlags entryFlags, bool shallow) const

        const QList<Group*> 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 +940,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 +978,26 @@ 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 is newer, update the existing one.
+    if (timeExisting < timeOther) {
+        qDebug("Updating group %s.", qPrintable(existingGroup->name()));
+        existingGroup->setName(otherGroup->name());
+        existingGroup->setNotes(otherGroup->notes());
+        if (otherGroup->iconNumber() == 0) {
+          existingGroup->setIcon(otherGroup->iconUuid());
+        } else {
+          existingGroup->setIcon(otherGroup->iconNumber());
+        }
+        existingGroup->setExpiryTime(otherGroup->timeInfo().expiryTime());
+    }
+
+}
+
 QStringList Group::locate(QString locateTerm, QString currentPath)
 {
    Q_ASSERT(!locateTerm.isNull());
--- 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;
@ -78,8 +86,11 @@ public:

    static const int DefaultIconNumber;
    static const int RecycleBinIconNumber;
+    static CloneFlags DefaultCloneFlags;
+    static Entry::CloneFlags DefaultEntryCloneFlags;

    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(""));
@ -119,14 +130,13 @@ public:
    QList<Group*> groupsRecursive(bool includeSelf);
    QSet<Uuid> customIconsRecursive() const;
    /**
-     * Creates a duplicate of this group including all child entries and groups (if not shallow).
-     * The exceptions are that the returned group doesn't have a parent group
-     * and all TimeInfo attributes are set to the current time.
+     * Creates a duplicate of this group.
     * Note that you need to copy the custom icons manually when inserting the
     * new group into another database.
     */
-    Group* clone(Entry::CloneFlags entryFlags = Entry::CloneNewUuid | Entry::CloneResetTimeInfo,
-                 bool shallow = false) const;
+    Group* clone(Entry::CloneFlags entryFlags = DefaultEntryCloneFlags,
+                 CloneFlags groupFlags = DefaultCloneFlags) 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
--- a/tests/TestGroup.cpp
+++ b/tests/TestGroup.cpp
@ -391,6 +391,22 @@ void TestGroup::testClone()
    QCOMPARE(clonedGroupKeepUuid->entries().at(0)->uuid(), originalGroupEntry->uuid());
    QCOMPARE(clonedGroupKeepUuid->children().at(0)->entries().at(0)->uuid(), subGroupEntry->uuid());

+    Group* clonedGroupNoFlags = originalGroup->clone(Entry::CloneNoFlags, Group::CloneNoFlags);
+    QCOMPARE(clonedGroupNoFlags->entries().size(), 0);
+    QVERIFY(clonedGroupNoFlags->uuid() == originalGroup->uuid());
+
+    Group* clonedGroupNewUuid = originalGroup->clone(Entry::CloneNoFlags, Group::CloneNewUuid);
+    QCOMPARE(clonedGroupNewUuid->entries().size(), 0);
+    QVERIFY(clonedGroupNewUuid->uuid() != originalGroup->uuid());
+
+    // Making sure the new modification date is not the same.
+    QTest::qSleep(1);
+
+    Group* clonedGroupResetTimeInfo = originalGroup->clone(Entry::CloneNoFlags, Group::CloneNewUuid | Group::CloneResetTimeInfo);
+    QCOMPARE(clonedGroupResetTimeInfo->entries().size(), 0);
+    QVERIFY(clonedGroupResetTimeInfo->uuid() != originalGroup->uuid());
+    QVERIFY(clonedGroupResetTimeInfo->timeInfo().lastModificationTime() != originalGroup->timeInfo().lastModificationTime());
+
    delete clonedGroup;
    delete clonedGroupKeepUuid;
    delete db;
--- 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();
--- 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();