diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 7e8ce9144..cf7e20e24 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -6136,38 +6136,6 @@ Expect some bugs and minor issues, this version is meant for testing purposes.Overwriting %1 [%2] - - 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] - - - - Synchronizing from older source %1 [%2] - - - - Deleting child %1 [%2] - - Deleting orphan %1 [%2] @@ -6188,6 +6156,18 @@ Expect some bugs and minor issues, this version is meant for testing purposes.Adding custom data %1 [%2] + + Synchronizing from newer source %1 [%2] + + + + Synchronizing from older source %1 [%2] + + + + Deleting child %1 [%2] + + NewDatabaseWizard diff --git a/src/core/Group.h b/src/core/Group.h index 9df144e92..74232fb30 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -39,9 +39,6 @@ public: 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 appling deletions }; diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index 747ffba46..f640ff2f8 100644 --- a/src/core/Merger.cpp +++ b/src/core/Merger.cpp @@ -165,13 +165,6 @@ Merger::resolveGroupConflict(const MergeContext& context, const Group* sourceChi return changes; } -bool Merger::markOlderEntry(Entry* entry) -{ - entry->attributes()->set( - "merged", tr("older entry merged from database \"%1\"").arg(entry->group()->database()->metadata()->name())); - return true; -} - void Merger::moveEntry(Entry* entry, Group* targetGroup) { Q_ASSERT(entry); @@ -264,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, @@ -374,7 +297,6 @@ 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 @@ -419,8 +341,8 @@ bool Merger::mergeHistory(const Entry* sourceEntry, 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 b46c296ea..75c8da990 100644 --- a/src/core/Merger.h +++ b/src/core/Merger.h @@ -50,7 +50,6 @@ private: ChangeList mergeGroup(const MergeContext& context); ChangeList mergeDeletions(const MergeContext& context); ChangeList mergeMetadata(const MergeContext& context); - bool markOlderEntry(Entry* entry); bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod, const int maxItems); void moveEntry(Entry* entry, Group* targetGroup); void moveGroup(Group* group, Group* targetGroup); @@ -60,12 +59,6 @@ private: 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 8386191dd..2dcf3a444 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 @@ -294,50 +287,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) @@ -769,26 +718,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. */ @@ -802,29 +736,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 6990a5dbe..b9ea54f72 100644 --- a/tests/TestMerge.h +++ b/tests/TestMerge.h @@ -36,15 +36,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();