diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index b7d5892882..050b50ed5f 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -5615,22 +5615,6 @@ We recommend you use the AppImage available on our downloads page. older entry merged from database "%1" - - Adding backup for older target %1 [%2] - - - - Adding backup for older source %1 [%2] - - - - Reapplying older target entry on top of newer source %1 [%2] - - - - Reapplying older source entry on top of newer target %1 [%2] - - Synchronizing from newer source %1 [%2] diff --git a/src/core/Group.h b/src/core/Group.h index ac28f73e0e..67f14d7762 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -39,9 +39,6 @@ class Group : public ModifiableObject enum MergeMode { Default, // Determine merge strategy from parent or fallback (Synchronize) - Duplicate, // lossy strategy regarding deletions, duplicate older changes in a new entry - KeepLocal, // merge history forcing local as top regardless of age - KeepRemote, // merge history forcing remote as top regardless of age KeepNewer, // merge history Synchronize, // merge history keeping most recent as top entry and applying deletions }; diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index fd30da7aa4..3f9ae4c0c4 100644 --- a/src/core/Merger.cpp +++ b/src/core/Merger.cpp @@ -257,76 +257,6 @@ void Merger::eraseGroup(Group* group) database->setDeletedObjects(deletions); } -Merger::ChangeList -Merger::resolveEntryConflict_Duplicate(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry) -{ - ChangeList changes; - const int comparison = compare(targetEntry->timeInfo().lastModificationTime(), - sourceEntry->timeInfo().lastModificationTime(), - CompareItemIgnoreMilliseconds); - // if one entry is newer, create a clone and add it to the group - if (comparison < 0) { - Entry* clonedEntry = sourceEntry->clone(Entry::CloneNewUuid | Entry::CloneIncludeHistory); - moveEntry(clonedEntry, context.m_targetGroup); - markOlderEntry(targetEntry); - changes << tr("Adding backup for older target %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); - } else if (comparison > 0) { - Entry* clonedEntry = sourceEntry->clone(Entry::CloneNewUuid | Entry::CloneIncludeHistory); - moveEntry(clonedEntry, context.m_targetGroup); - markOlderEntry(clonedEntry); - changes << tr("Adding backup for older source %1 [%2]").arg(sourceEntry->title(), sourceEntry->uuidToHex()); - } - return changes; -} - -Merger::ChangeList -Merger::resolveEntryConflict_KeepLocal(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry) -{ - Q_UNUSED(context); - ChangeList changes; - const int comparison = compare(targetEntry->timeInfo().lastModificationTime(), - sourceEntry->timeInfo().lastModificationTime(), - CompareItemIgnoreMilliseconds); - if (comparison < 0) { - // we need to make our older entry "newer" than the new entry - therefore - // we just create a new history entry without any changes - this preserves - // the old state before merging the new state and updates the timestamp - // the merge takes care, that the newer entry is sorted inbetween both entries - // this type of merge changes the database timestamp since reapplying the - // old entry is an active change of the database! - changes << tr("Reapplying older target entry on top of newer source %1 [%2]") - .arg(targetEntry->title(), targetEntry->uuidToHex()); - Entry* agedTargetEntry = targetEntry->clone(Entry::CloneNoFlags); - targetEntry->addHistoryItem(agedTargetEntry); - } - return changes; -} - -Merger::ChangeList -Merger::resolveEntryConflict_KeepRemote(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry) -{ - Q_UNUSED(context); - ChangeList changes; - const int comparison = compare(targetEntry->timeInfo().lastModificationTime(), - sourceEntry->timeInfo().lastModificationTime(), - CompareItemIgnoreMilliseconds); - if (comparison > 0) { - // we need to make our older entry "newer" than the new entry - therefore - // we just create a new history entry without any changes - this preserves - // the old state before merging the new state and updates the timestamp - // the merge takes care, that the newer entry is sorted inbetween both entries - // this type of merge changes the database timestamp since reapplying the - // old entry is an active change of the database! - changes << tr("Reapplying older source entry on top of newer target %1 [%2]") - .arg(targetEntry->title(), targetEntry->uuidToHex()); - targetEntry->beginUpdate(); - targetEntry->copyDataFrom(sourceEntry); - targetEntry->endUpdate(); - // History item is created by endUpdate since we should have changes - } - return changes; -} - Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry, @@ -366,38 +296,12 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex Merger::ChangeList Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry) { - ChangeList changes; // We need to cut off the milliseconds since the persistent format only supports times down to seconds // so when we import data from a remote source, it may represent the (or even some msec newer) data // which may be discarded due to higher runtime precision Group::MergeMode mergeMode = m_mode == Group::Default ? context.m_targetGroup->mergeMode() : m_mode; - switch (mergeMode) { - case Group::Duplicate: - changes << resolveEntryConflict_Duplicate(context, sourceEntry, targetEntry); - break; - - case Group::KeepLocal: - changes << resolveEntryConflict_KeepLocal(context, sourceEntry, targetEntry); - changes << resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode); - break; - - case Group::KeepRemote: - changes << resolveEntryConflict_KeepRemote(context, sourceEntry, targetEntry); - changes << resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode); - break; - - case Group::Synchronize: - case Group::KeepNewer: - // nothing special to do since resolveEntryConflictMergeHistories takes care to use the newest entry - changes << resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode); - break; - - default: - // do nothing - break; - } - return changes; + return resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode); } bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod) @@ -408,8 +312,8 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M const int comparison = compare(sourceEntry->timeInfo().lastModificationTime(), targetEntry->timeInfo().lastModificationTime(), CompareItemIgnoreMilliseconds); - const bool preferLocal = mergeMethod == Group::KeepLocal || comparison < 0; - const bool preferRemote = mergeMethod == Group::KeepRemote || comparison > 0; + const bool preferLocal = comparison < 0; + const bool preferRemote = comparison > 0; QMap merged; for (Entry* historyItem : targetHistoryItems) { diff --git a/src/core/Merger.h b/src/core/Merger.h index 4b277f9561..294eb82aca 100644 --- a/src/core/Merger.h +++ b/src/core/Merger.h @@ -59,12 +59,6 @@ class Merger : public QObject void eraseGroup(Group* group); ChangeList resolveEntryConflict(const MergeContext& context, const Entry* existingEntry, Entry* otherEntry); ChangeList resolveGroupConflict(const MergeContext& context, const Group* existingGroup, Group* otherGroup); - Merger::ChangeList - resolveEntryConflict_Duplicate(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry); - Merger::ChangeList - resolveEntryConflict_KeepLocal(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry); - Merger::ChangeList - resolveEntryConflict_KeepRemote(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry); Merger::ChangeList resolveEntryConflict_MergeHistories(const MergeContext& context, const Entry* sourceEntry, Entry* targetEntry, diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index ecb37d5097..1180279567 100644 --- a/tests/TestMerge.cpp +++ b/tests/TestMerge.cpp @@ -29,13 +29,6 @@ QTEST_GUILESS_MAIN(TestMerge) namespace { - TimeInfo modificationTime(TimeInfo timeInfo, int years, int months, int days) - { - const QDateTime time = timeInfo.lastModificationTime(); - timeInfo.setLastModificationTime(time.addYears(years).addMonths(months).addDays(days)); - return timeInfo; - } - MockClock* m_clock = nullptr; } // namespace @@ -258,50 +251,6 @@ void TestMerge::testResolveConflictExisting() } } -/** - * Tests the KeepBoth merge mode. - */ -void TestMerge::testResolveConflictDuplicate() -{ - QScopedPointer dbDestination(createTestDatabase()); - QScopedPointer dbSource( - createTestDatabaseStructureClone(dbDestination.data(), Entry::CloneIncludeHistory, Group::CloneIncludeEntries)); - - // sanity check - QCOMPARE(dbDestination->rootGroup()->children().at(0)->entries().size(), 2); - - // make this entry newer than in original db - QPointer updatedDestinationEntry = dbDestination->rootGroup()->children().at(0)->entries().at(0); - const TimeInfo initialEntryTimeInfo = updatedDestinationEntry->timeInfo(); - const TimeInfo updatedEntryTimeInfo = modificationTime(initialEntryTimeInfo, 1, 0, 0); - - updatedDestinationEntry->setTimeInfo(updatedEntryTimeInfo); - - dbDestination->rootGroup()->setMergeMode(Group::MergeMode::Duplicate); - - // Make sure the merge changes have a different timestamp. - m_clock->advanceSecond(1); - - Merger merger(dbSource.data(), dbDestination.data()); - merger.merge(); - - // one entry is duplicated because of mode - QCOMPARE(dbDestination->rootGroup()->children().at(0)->entries().size(), 3); - QCOMPARE(dbDestination->rootGroup()->children().at(0)->entries().at(0)->historyItems().isEmpty(), false); - // the older entry was merged from the other db as last in the group - QPointer newerEntry = dbDestination->rootGroup()->children().at(0)->entries().at(0); - QPointer olderEntry = dbDestination->rootGroup()->children().at(0)->entries().at(2); - QVERIFY(newerEntry->title() == olderEntry->title()); - QVERIFY2(!newerEntry->attributes()->hasKey("merged"), "newer entry is not marked with an attribute \"merged\""); - QVERIFY2(olderEntry->attributes()->hasKey("merged"), "older entry is marked with an attribute \"merged\""); - QCOMPARE(olderEntry->historyItems().isEmpty(), false); - QCOMPARE(newerEntry->timeInfo(), updatedEntryTimeInfo); - // TODO HNH: this may be subject to discussions since the entry itself is newer but represents an older one - // QCOMPARE(olderEntry->timeInfo(), initialEntryTimeInfo); - QVERIFY2(olderEntry->uuidToHex() != updatedDestinationEntry->uuidToHex(), - "KeepBoth should not reuse the UUIDs when cloning."); -} - void TestMerge::testResolveConflictTemplate( int mergeMode, std::function&)> verification) @@ -733,26 +682,11 @@ void TestMerge::testDeletionConflictEntry_Synchronized() testDeletionConflictTemplate(Group::Synchronize, &TestMerge::assertDeletionNewerOnly); } -void TestMerge::testDeletionConflictEntry_KeepLocal() -{ - testDeletionConflictTemplate(Group::KeepLocal, &TestMerge::assertDeletionLocalOnly); -} - -void TestMerge::testDeletionConflictEntry_KeepRemote() -{ - testDeletionConflictTemplate(Group::KeepRemote, &TestMerge::assertDeletionLocalOnly); -} - void TestMerge::testDeletionConflictEntry_KeepNewer() { testDeletionConflictTemplate(Group::KeepNewer, &TestMerge::assertDeletionLocalOnly); } -void TestMerge::testDeletionConflictEntry_Duplicate() -{ - testDeletionConflictTemplate(Group::Duplicate, &TestMerge::assertDeletionLocalOnly); -} - /** * Tests the KeepNewer mode concerning history. */ @@ -766,29 +700,6 @@ void TestMerge::testResolveConflictEntry_Synchronize() }); } -/** - * Tests the KeepExisting mode concerning history. - */ -void TestMerge::testResolveConflictEntry_KeepLocal() -{ - testResolveConflictTemplate(Group::KeepLocal, [](Database* db, const QMap& timestamps) { - QPointer mergedRootGroup = db->rootGroup(); - QPointer mergedGroup1 = mergedRootGroup->children().at(0); - TestMerge::assertUpdateMergedEntry1(mergedGroup1->entries().at(0), timestamps); - TestMerge::assertUpdateReappliedEntry2(mergedGroup1->entries().at(1), timestamps); - }); -} - -void TestMerge::testResolveConflictEntry_KeepRemote() -{ - testResolveConflictTemplate(Group::KeepRemote, [](Database* db, const QMap& timestamps) { - QPointer mergedRootGroup = db->rootGroup(); - QPointer mergedGroup1 = mergedRootGroup->children().at(0); - TestMerge::assertUpdateReappliedEntry1(mergedGroup1->entries().at(0), timestamps); - TestMerge::assertUpdateMergedEntry2(mergedGroup1->entries().at(1), timestamps); - }); -} - void TestMerge::testResolveConflictEntry_KeepNewer() { testResolveConflictTemplate(Group::KeepNewer, [](Database* db, const QMap& timestamps) { diff --git a/tests/TestMerge.h b/tests/TestMerge.h index cb17ac8fb5..2f4e4a224f 100644 --- a/tests/TestMerge.h +++ b/tests/TestMerge.h @@ -35,15 +35,9 @@ private slots: void testResolveGroupConflictOlder(); void testMergeNotModified(); void testMergeModified(); - void testResolveConflictDuplicate(); void testResolveConflictEntry_Synchronize(); - void testResolveConflictEntry_KeepLocal(); - void testResolveConflictEntry_KeepRemote(); void testResolveConflictEntry_KeepNewer(); - void testDeletionConflictEntry_Duplicate(); void testDeletionConflictEntry_Synchronized(); - void testDeletionConflictEntry_KeepLocal(); - void testDeletionConflictEntry_KeepRemote(); void testDeletionConflictEntry_KeepNewer(); void testMoveEntry(); void testMoveEntryPreserveChanges();