From 86695bd8e3274673de39e8571dc17639c7ba6066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 29 Dec 2023 13:59:10 +0100 Subject: [PATCH 1/3] Entry: re-parent before adding to new group Adding the Entry to the Group will emit signals about the action. Present the object with the correct parent already. --- src/core/Entry.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 4ae6b091c8..ecd4228e6f 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -1277,11 +1277,11 @@ void Entry::setGroup(Group* group, bool trackPrevious) } } + QObject::setParent(group); + m_group = group; group->addEntry(this); - QObject::setParent(group); - if (m_updateTimeinfo) { m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc()); } From 33bf3771987ddc3edddc24ff657f9a56b5cf2495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 29 Dec 2023 14:03:45 +0100 Subject: [PATCH 2/3] fdosecrets: Item::Create() can fail If an entry cannot be registered on DBus, Item::Create() will return a nullptr. Basically, this can only happen if there is already an item with the same UUID in the collection. The only viable option here is to ignore the new entry. --- src/fdosecrets/objects/Collection.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp index 4cc6ca537d..be452d4299 100644 --- a/src/fdosecrets/objects/Collection.cpp +++ b/src/fdosecrets/objects/Collection.cpp @@ -495,6 +495,10 @@ namespace FdoSecrets } auto item = Item::Create(this, entry); + if (!item) { + return; + } + m_items << item; m_entryToItem[entry] = item; From e18a18619fe0af63ff57e01b5d508ddfb110fc48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 29 Dec 2023 14:08:04 +0100 Subject: [PATCH 3/3] Merger: prevent duplicate entry when merging histories If the alien database (source) entry is newer, a copy of the entry is made. But before moving the merged entry to the target group, it must be removed. Otherwise there will be briefly two entries with the same UUID in the same group/database. Even though this is only the case during the transaction, it can still be observed because the operations emit signals. A notable problem is the fdosecrets feature that relies on the uniqueness of the UUID or will otherwise run into problems because the UUID is used as part of the DBus path. --- src/core/Merger.cpp | 13 ++++++++----- src/core/Merger.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index 4cfe4b5e4e..5e14e8452e 100644 --- a/src/core/Merger.cpp +++ b/src/core/Merger.cpp @@ -261,6 +261,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex const int comparison = compare(targetEntry->timeInfo().lastModificationTime(), sourceEntry->timeInfo().lastModificationTime(), CompareItemIgnoreMilliseconds); + const int maxItems = targetEntry->database()->metadata()->historyMaxItems(); if (comparison < 0) { Group* currentGroup = targetEntry->group(); Entry* clonedEntry = sourceEntry->clone(Entry::CloneIncludeHistory); @@ -269,15 +270,15 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex qPrintable(sourceEntry->title()), qPrintable(currentGroup->name())); changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); - moveEntry(clonedEntry, currentGroup); - mergeHistory(targetEntry, clonedEntry, mergeMethod); + mergeHistory(targetEntry, clonedEntry, mergeMethod, maxItems); eraseEntry(targetEntry); + moveEntry(clonedEntry, currentGroup); } else { qDebug("Merge %s/%s with local on top/under %s", qPrintable(targetEntry->title()), qPrintable(sourceEntry->title()), qPrintable(targetEntry->group()->name())); - const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod); + const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod, maxItems); if (changed) { changes << tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); @@ -297,7 +298,10 @@ Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEnt return resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode); } -bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod) +bool Merger::mergeHistory(const Entry* sourceEntry, + Entry* targetEntry, + Group::MergeMode mergeMethod, + const int maxItems) { Q_UNUSED(mergeMethod); const auto targetHistoryItems = targetEntry->historyItems(); @@ -370,7 +374,6 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M } bool changed = false; - const int maxItems = targetEntry->database()->metadata()->historyMaxItems(); const auto updatedHistoryItems = merged.values(); for (int i = 0; i < maxItems; ++i) { const Entry* oldEntry = targetHistoryItems.value(targetHistoryItems.count() - i); diff --git a/src/core/Merger.h b/src/core/Merger.h index d2d1da4fdb..e98f7a062f 100644 --- a/src/core/Merger.h +++ b/src/core/Merger.h @@ -49,7 +49,7 @@ class Merger : public QObject ChangeList mergeGroup(const MergeContext& context); ChangeList mergeDeletions(const MergeContext& context); ChangeList mergeMetadata(const MergeContext& context); - bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod); + 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); // remove an entry without a trace in the deletedObjects - needed for elemination cloned entries