From ff9f6391abbbfea9e278aac6b4973fc62e33495d Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Tue, 19 Mar 2024 13:13:10 +0100 Subject: [PATCH 01/12] Preserve the LastModificationTime when setting a group/entry previous parent Partially fixes #8170 (for inter-db moves) --- src/core/Entry.cpp | 4 ++++ src/core/Group.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index b57fd78f15..78fcfe88a3 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -1487,7 +1487,11 @@ QUuid Entry::previousParentGroupUuid() const void Entry::setPreviousParentGroupUuid(const QUuid& uuid) { + // prevent set from changing the LastModificationTime + bool prevUpdateTimeinfo = m_updateTimeinfo; + m_updateTimeinfo = false; set(m_data.previousParentGroupUuid, uuid); + m_updateTimeinfo = prevUpdateTimeinfo; } void Entry::setPreviousParentGroup(const Group* group) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 6991af55da..b1d2a317a7 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -1240,7 +1240,11 @@ QUuid Group::previousParentGroupUuid() const void Group::setPreviousParentGroupUuid(const QUuid& uuid) { + // prevent set from changing the LastModificationTime + bool prevUpdateTimeinfo = m_updateTimeinfo; + m_updateTimeinfo = false; set(m_data.previousParentGroupUuid, uuid); + m_updateTimeinfo = prevUpdateTimeinfo; } void Group::setPreviousParentGroup(const Group* group) From 0665a6c99b0754de9f964ea188bcabf5f0b3be70 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Tue, 19 Mar 2024 13:17:01 +0100 Subject: [PATCH 02/12] Improve drag & drop behavior - Preserve the LastModificationTime for a group/entry cloned with the CloneResetTimeInfo flag - Retain UUID's and timeInfo when moving group(s)/entry(s) between db's - Prevent the Recycle Bin group from being moved to another db - Cross-db moves that might require merge logic are not (yet) supported and show an error message instead Fixes #6202 Fixes #8170 --- src/core/Entry.cpp | 8 +--- src/core/Entry.h | 8 ++-- src/core/Group.cpp | 7 +-- src/core/Group.h | 7 ++- src/core/Metadata.cpp | 17 +++++-- src/core/Metadata.h | 1 + src/gui/MainWindow.cpp | 4 ++ src/gui/MainWindow.h | 1 + src/gui/group/GroupModel.cpp | 91 +++++++++++++++++++++++++++++------- 9 files changed, 107 insertions(+), 37 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 78fcfe88a3..8fee335879 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -947,9 +947,9 @@ Entry* Entry::clone(CloneFlags flags) const if (flags & CloneResetTimeInfo) { QDateTime now = Clock::currentDateTimeUtc(); entry->m_data.timeInfo.setCreationTime(now); - entry->m_data.timeInfo.setLastModificationTime(now); entry->m_data.timeInfo.setLastAccessTime(now); entry->m_data.timeInfo.setLocationChanged(now); + // preserve LastModificationTime } if (flags & CloneRenameTitle) { @@ -1267,11 +1267,7 @@ void Entry::setGroup(Group* group, bool trackPrevious) m_group->database()->addDeletedObject(m_uuid); // copy custom icon to the new database - if (!iconUuid().isNull() && group->database() && m_group->database()->metadata()->hasCustomIcon(iconUuid()) - && !group->database()->metadata()->hasCustomIcon(iconUuid())) { - group->database()->metadata()->addCustomIcon(iconUuid(), - m_group->database()->metadata()->customIcon(iconUuid())); - } + group->database()->metadata()->copyCustomIcon(iconUuid(), m_group->database()->metadata()); } else if (trackPrevious && m_group->database() && group != m_group) { setPreviousParentGroup(m_group); } diff --git a/src/core/Entry.h b/src/core/Entry.h index 3a03892d0a..9f5e462e9d 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -175,13 +175,15 @@ class Entry : public ModifiableObject { CloneNoFlags = 0, CloneNewUuid = 1, // generate a random uuid for the clone - CloneResetTimeInfo = 2, // set all TimeInfo attributes to the current time + CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time CloneIncludeHistory = 4, // clone the history items - CloneDefault = CloneNewUuid | CloneResetTimeInfo, - CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory, CloneRenameTitle = 8, // add "-Clone" after the original title CloneUserAsRef = 16, // Add the user as a reference to the original entry ClonePassAsRef = 32, // Add the password as a reference to the original entry + + CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory, + CloneExactCopy = CloneIncludeHistory, + CloneDefault = CloneNewUuid | CloneResetTimeInfo, }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index b1d2a317a7..f6b7f03fe9 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -469,10 +469,7 @@ void Group::setParent(Group* parent, int index, bool trackPrevious) recCreateDelObjects(); // copy custom icon to the new database - if (!iconUuid().isNull() && parent->m_db && m_db->metadata()->hasCustomIcon(iconUuid()) - && !parent->m_db->metadata()->hasCustomIcon(iconUuid())) { - parent->m_db->metadata()->addCustomIcon(iconUuid(), m_db->metadata()->customIcon(iconUuid())); - } + parent->m_db->metadata()->copyCustomIcon(iconUuid(), m_db->metadata()); } if (m_db != parent->m_db) { connectDatabaseSignalsRecursive(parent->m_db); @@ -949,9 +946,9 @@ Group* Group::clone(Entry::CloneFlags entryFlags, Group::CloneFlags groupFlags) QDateTime now = Clock::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); + // preserve LastModificationTime } if (groupFlags & Group::CloneRenameTitle) { diff --git a/src/core/Group.h b/src/core/Group.h index 85a4317a3f..1552aabd33 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -47,10 +47,13 @@ class Group : public ModifiableObject { CloneNoFlags = 0, CloneNewUuid = 1, // generate a random uuid for the clone - CloneResetTimeInfo = 2, // set all TimeInfo attributes to the current time + CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time CloneIncludeEntries = 4, // clone the group entries - CloneDefault = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries, CloneRenameTitle = 8, // add "- Clone" after the original title + + CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries, + CloneExactCopy = CloneIncludeEntries, + CloneDefault = CloneCopy, }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/core/Metadata.cpp b/src/core/Metadata.cpp index 8e714e0f22..62a34ff409 100644 --- a/src/core/Metadata.cpp +++ b/src/core/Metadata.cpp @@ -419,14 +419,21 @@ QUuid Metadata::findCustomIcon(const QByteArray& candidate) return m_customIconsHashes.value(hash, QUuid()); } +void Metadata::copyCustomIcon(const QUuid& iconUuid, const Metadata* otherMetadata) +{ + if (iconUuid.isNull()) { + return; + } + Q_ASSERT(otherMetadata->hasCustomIcon(iconUuid)); + if (!hasCustomIcon(iconUuid) && otherMetadata->hasCustomIcon(iconUuid)) { + addCustomIcon(iconUuid, otherMetadata->customIcon(iconUuid)); + } +} + void Metadata::copyCustomIcons(const QSet& iconList, const Metadata* otherMetadata) { for (const QUuid& uuid : iconList) { - Q_ASSERT(otherMetadata->hasCustomIcon(uuid)); - - if (!hasCustomIcon(uuid) && otherMetadata->hasCustomIcon(uuid)) { - addCustomIcon(uuid, otherMetadata->customIcon(uuid)); - } + copyCustomIcon(uuid, otherMetadata); } } diff --git a/src/core/Metadata.h b/src/core/Metadata.h index 6e80ebc099..fbb9709f01 100644 --- a/src/core/Metadata.h +++ b/src/core/Metadata.h @@ -138,6 +138,7 @@ class Metadata : public ModifiableObject const QString& name = {}, const QDateTime& lastModified = {}); void removeCustomIcon(const QUuid& uuid); + void copyCustomIcon(const QUuid& iconUuid, const Metadata* otherMetadata); void copyCustomIcons(const QSet& iconList, const Metadata* otherMetadata); QUuid findCustomIcon(const QByteArray& candidate); void setRecycleBinEnabled(bool value); diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index c97e049365..72417b7814 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -708,6 +708,10 @@ QList MainWindow::getOpenDatabases() return dbWidgets; } +DatabaseWidget* MainWindow::currentDatabaseWidget() { + return m_ui->tabWidget->currentDatabaseWidget(); +} + void MainWindow::showErrorMessage(const QString& message) { m_ui->globalMessageWidget->showMessage(message, MessageWidget::Error); diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index c47c0d205c..5c727e5a6d 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -52,6 +52,7 @@ class MainWindow : public QMainWindow ~MainWindow() override; QList getOpenDatabases(); + DatabaseWidget* currentDatabaseWidget(); void restoreConfigState(); void setAllowScreenCapture(bool state); diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index 18b926dc20..390b618432 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -25,6 +25,7 @@ #include "core/Tools.h" #include "gui/DatabaseIcons.h" #include "gui/Icons.h" +#include "gui/MainWindow.h" #include "keeshare/KeeShare.h" GroupModel::GroupModel(Database* db, QObject* parent) @@ -229,6 +230,13 @@ bool GroupModel::dropMimeData(const QMimeData* data, Group* parentGroup = groupFromIndex(parent); + auto showErrorMessage = [](const QString& errorMessage){ + auto dbWidget = getMainWindow()->currentDatabaseWidget(); + if(dbWidget) { + dbWidget->showErrorMessage(errorMessage); + } + }; + if (isGroup) { QUuid dbUuid; QUuid groupUuid; @@ -258,17 +266,49 @@ bool GroupModel::dropMimeData(const QMimeData* data, Group* group = dragGroup; if (sourceDb != targetDb) { - QSet customIcons = group->customIconsRecursive(); - targetDb->metadata()->copyCustomIcons(customIcons, sourceDb->metadata()); - - // Always clone the group across db's to reset UUIDs - group = dragGroup->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); if (action == Qt::MoveAction) { - // Remove the original group from the sourceDb + Group* binGroup = sourceDb->metadata()->recycleBin(); + if(binGroup && binGroup->uuid() == dragGroup->uuid()) { + showErrorMessage(tr("Move Error: %1 group cannot be moved").arg(tr("Recycle bin"))); + return true; + } + + // the move is complex when any group or entry is known in the other db + bool complexMove = false; + for (const Group* g: group->groupsRecursive(true)) { + if (targetDb->containsDeletedObject(g->uuid()) || targetDb->rootGroup()->findGroupByUuid(g->uuid())) { + complexMove = true; + break; + } + } + for (const Entry* e: group->entriesRecursive(false)) { + if (targetDb->containsDeletedObject(e->uuid()) || targetDb->rootGroup()->findEntryByUuid(e->uuid())) { + complexMove = true; + break; + } + } + // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging. + if (complexMove) { + showErrorMessage(tr("Move Error: (sub-)group(s) and/or entry(s) already present in the other database")); + return true; + } + + group = dragGroup->clone(Entry::CloneExactCopy, Group::CloneExactCopy); + } + + targetDb->metadata()->copyCustomIcons(dragGroup->customIconsRecursive(), sourceDb->metadata()); + } + + if (action == Qt::MoveAction) { + // remove the original group if it moved a clone + if (group != dragGroup) { + QList delObjects(sourceDb->deletedObjects()); delete dragGroup; + // prevent group, sub-group(s) & entry(s) from ending up on the deleted object list + sourceDb->setDeletedObjects(delObjects); } - } else if (action == Qt::CopyAction) { - group = dragGroup->clone(Entry::CloneCopy); + } else { // Action == Qt::CopyAction + group = dragGroup->clone(Entry::CloneCopy, Group::CloneCopy); } group->setParent(parentGroup, row); @@ -277,11 +317,13 @@ bool GroupModel::dropMimeData(const QMimeData* data, return false; } + int numEntries = 0, numEntriesNotMoved = 0; while (!stream.atEnd()) { QUuid dbUuid; QUuid entryUuid; stream >> dbUuid >> entryUuid; - + ++numEntries; + Database* db = Database::databaseByUuid(dbUuid); if (!db) { continue; @@ -298,22 +340,39 @@ bool GroupModel::dropMimeData(const QMimeData* data, Entry* entry = dragEntry; if (sourceDb != targetDb) { - QUuid customIcon = entry->iconUuid(); - if (!customIcon.isNull() && !targetDb->metadata()->hasCustomIcon(customIcon)) { - targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon).data); + if (action == Qt::MoveAction) { + // the move is complex when the entry is known in the other db + bool complexMove = targetDb->containsDeletedObject(entry->uuid()) + || targetDb->rootGroup()->findEntryByUuid(entry->uuid()); + // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging. + if (complexMove) { + ++numEntriesNotMoved; + continue; + } + entry = dragEntry->clone(Entry::CloneExactCopy); } - // Reset the UUID when moving across db boundary - entry = dragEntry->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); - if (action == Qt::MoveAction) { + targetDb->metadata()->copyCustomIcon(entry->iconUuid(), sourceDb->metadata()); + } + + if (action == Qt::MoveAction) { + // remove the original entry if it moved a clone + if (entry != dragEntry) { + QList delObjects(sourceDb->deletedObjects()); delete dragEntry; + // prevent entry from ending up on the deleted object list + sourceDb->setDeletedObjects(delObjects); } - } else if (action == Qt::CopyAction) { + } else { // Action == Qt::CopyAction entry = dragEntry->clone(Entry::CloneCopy); } entry->setGroup(parentGroup); } + + if (numEntriesNotMoved) { + showErrorMessage(tr("Move Error: %1 of %2 entry(s) already present in the other database").arg(numEntriesNotMoved).arg(numEntries)); + } } return true; From d644866357766b30712540321f537309eb5c5f7a Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Wed, 20 Mar 2024 22:50:57 +0100 Subject: [PATCH 03/12] Tweak error messages --- src/gui/group/GroupModel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index 390b618432..8eb6e1f51a 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -289,7 +289,7 @@ bool GroupModel::dropMimeData(const QMimeData* data, } // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging. if (complexMove) { - showErrorMessage(tr("Move Error: (sub-)group(s) and/or entry(s) already present in the other database")); + showErrorMessage(tr("Move Error: (sub-)group(s) and/or entry(s) already present in this database")); return true; } @@ -371,7 +371,7 @@ bool GroupModel::dropMimeData(const QMimeData* data, } if (numEntriesNotMoved) { - showErrorMessage(tr("Move Error: %1 of %2 entry(s) already present in the other database").arg(numEntriesNotMoved).arg(numEntries)); + showErrorMessage(tr("Move Error: %1 of %2 entry(s) already present in this database").arg(numEntriesNotMoved).arg(numEntries)); } } From 9b6c36b517b3e23750df73f082d527aa1253d471 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Wed, 20 Mar 2024 22:57:56 +0100 Subject: [PATCH 04/12] Tweak comments --- src/gui/group/GroupModel.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index 8eb6e1f51a..3e36e32545 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -300,11 +300,11 @@ bool GroupModel::dropMimeData(const QMimeData* data, } if (action == Qt::MoveAction) { - // remove the original group if it moved a clone + // delete the original group when moving a clone if (group != dragGroup) { QList delObjects(sourceDb->deletedObjects()); delete dragGroup; - // prevent group, sub-group(s) & entry(s) from ending up on the deleted object list + // prevent group(s)/entry(s) from ending up on the deleted object list by restoring the previous list. sourceDb->setDeletedObjects(delObjects); } } else { // Action == Qt::CopyAction @@ -356,11 +356,11 @@ bool GroupModel::dropMimeData(const QMimeData* data, } if (action == Qt::MoveAction) { - // remove the original entry if it moved a clone + // delete the original entry when moving a clone if (entry != dragEntry) { QList delObjects(sourceDb->deletedObjects()); delete dragEntry; - // prevent entry from ending up on the deleted object list + // prevent entry from ending up on the deleted object list by restoring the previous list. sourceDb->setDeletedObjects(delObjects); } } else { // Action == Qt::CopyAction From ebb2d88bc0dda231b3b51ff22928a56450ad45f1 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Mon, 25 Mar 2024 13:09:15 +0100 Subject: [PATCH 05/12] Restore previous drag & drop logic --- src/core/Entry.h | 6 +-- src/core/Group.h | 5 +- src/gui/MainWindow.cpp | 4 -- src/gui/MainWindow.h | 1 - src/gui/group/GroupModel.cpp | 91 +++++++----------------------------- 5 files changed, 19 insertions(+), 88 deletions(-) diff --git a/src/core/Entry.h b/src/core/Entry.h index 9f5e462e9d..2e42b50d4f 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -177,13 +177,11 @@ class Entry : public ModifiableObject CloneNewUuid = 1, // generate a random uuid for the clone CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time CloneIncludeHistory = 4, // clone the history items + CloneDefault = CloneNewUuid | CloneResetTimeInfo, + CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory, CloneRenameTitle = 8, // add "-Clone" after the original title CloneUserAsRef = 16, // Add the user as a reference to the original entry ClonePassAsRef = 32, // Add the password as a reference to the original entry - - CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory, - CloneExactCopy = CloneIncludeHistory, - CloneDefault = CloneNewUuid | CloneResetTimeInfo, }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/core/Group.h b/src/core/Group.h index 1552aabd33..a121922f56 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -49,11 +49,8 @@ class Group : public ModifiableObject CloneNewUuid = 1, // generate a random uuid for the clone CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time CloneIncludeEntries = 4, // clone the group entries + CloneDefault = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries, CloneRenameTitle = 8, // add "- Clone" after the original title - - CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries, - CloneExactCopy = CloneIncludeEntries, - CloneDefault = CloneCopy, }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 72417b7814..c97e049365 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -708,10 +708,6 @@ QList MainWindow::getOpenDatabases() return dbWidgets; } -DatabaseWidget* MainWindow::currentDatabaseWidget() { - return m_ui->tabWidget->currentDatabaseWidget(); -} - void MainWindow::showErrorMessage(const QString& message) { m_ui->globalMessageWidget->showMessage(message, MessageWidget::Error); diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index 5c727e5a6d..c47c0d205c 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -52,7 +52,6 @@ class MainWindow : public QMainWindow ~MainWindow() override; QList getOpenDatabases(); - DatabaseWidget* currentDatabaseWidget(); void restoreConfigState(); void setAllowScreenCapture(bool state); diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index 3e36e32545..18b926dc20 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -25,7 +25,6 @@ #include "core/Tools.h" #include "gui/DatabaseIcons.h" #include "gui/Icons.h" -#include "gui/MainWindow.h" #include "keeshare/KeeShare.h" GroupModel::GroupModel(Database* db, QObject* parent) @@ -230,13 +229,6 @@ bool GroupModel::dropMimeData(const QMimeData* data, Group* parentGroup = groupFromIndex(parent); - auto showErrorMessage = [](const QString& errorMessage){ - auto dbWidget = getMainWindow()->currentDatabaseWidget(); - if(dbWidget) { - dbWidget->showErrorMessage(errorMessage); - } - }; - if (isGroup) { QUuid dbUuid; QUuid groupUuid; @@ -266,49 +258,17 @@ bool GroupModel::dropMimeData(const QMimeData* data, Group* group = dragGroup; if (sourceDb != targetDb) { - if (action == Qt::MoveAction) { - Group* binGroup = sourceDb->metadata()->recycleBin(); - if(binGroup && binGroup->uuid() == dragGroup->uuid()) { - showErrorMessage(tr("Move Error: %1 group cannot be moved").arg(tr("Recycle bin"))); - return true; - } + QSet customIcons = group->customIconsRecursive(); + targetDb->metadata()->copyCustomIcons(customIcons, sourceDb->metadata()); - // the move is complex when any group or entry is known in the other db - bool complexMove = false; - for (const Group* g: group->groupsRecursive(true)) { - if (targetDb->containsDeletedObject(g->uuid()) || targetDb->rootGroup()->findGroupByUuid(g->uuid())) { - complexMove = true; - break; - } - } - for (const Entry* e: group->entriesRecursive(false)) { - if (targetDb->containsDeletedObject(e->uuid()) || targetDb->rootGroup()->findEntryByUuid(e->uuid())) { - complexMove = true; - break; - } - } - // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging. - if (complexMove) { - showErrorMessage(tr("Move Error: (sub-)group(s) and/or entry(s) already present in this database")); - return true; - } - - group = dragGroup->clone(Entry::CloneExactCopy, Group::CloneExactCopy); - } - - targetDb->metadata()->copyCustomIcons(dragGroup->customIconsRecursive(), sourceDb->metadata()); - } - - if (action == Qt::MoveAction) { - // delete the original group when moving a clone - if (group != dragGroup) { - QList delObjects(sourceDb->deletedObjects()); + // Always clone the group across db's to reset UUIDs + group = dragGroup->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); + if (action == Qt::MoveAction) { + // Remove the original group from the sourceDb delete dragGroup; - // prevent group(s)/entry(s) from ending up on the deleted object list by restoring the previous list. - sourceDb->setDeletedObjects(delObjects); } - } else { // Action == Qt::CopyAction - group = dragGroup->clone(Entry::CloneCopy, Group::CloneCopy); + } else if (action == Qt::CopyAction) { + group = dragGroup->clone(Entry::CloneCopy); } group->setParent(parentGroup, row); @@ -317,13 +277,11 @@ bool GroupModel::dropMimeData(const QMimeData* data, return false; } - int numEntries = 0, numEntriesNotMoved = 0; while (!stream.atEnd()) { QUuid dbUuid; QUuid entryUuid; stream >> dbUuid >> entryUuid; - ++numEntries; - + Database* db = Database::databaseByUuid(dbUuid); if (!db) { continue; @@ -340,39 +298,22 @@ bool GroupModel::dropMimeData(const QMimeData* data, Entry* entry = dragEntry; if (sourceDb != targetDb) { - if (action == Qt::MoveAction) { - // the move is complex when the entry is known in the other db - bool complexMove = targetDb->containsDeletedObject(entry->uuid()) - || targetDb->rootGroup()->findEntryByUuid(entry->uuid()); - // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging. - if (complexMove) { - ++numEntriesNotMoved; - continue; - } - entry = dragEntry->clone(Entry::CloneExactCopy); + QUuid customIcon = entry->iconUuid(); + if (!customIcon.isNull() && !targetDb->metadata()->hasCustomIcon(customIcon)) { + targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon).data); } - targetDb->metadata()->copyCustomIcon(entry->iconUuid(), sourceDb->metadata()); - } - - if (action == Qt::MoveAction) { - // delete the original entry when moving a clone - if (entry != dragEntry) { - QList delObjects(sourceDb->deletedObjects()); + // Reset the UUID when moving across db boundary + entry = dragEntry->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); + if (action == Qt::MoveAction) { delete dragEntry; - // prevent entry from ending up on the deleted object list by restoring the previous list. - sourceDb->setDeletedObjects(delObjects); } - } else { // Action == Qt::CopyAction + } else if (action == Qt::CopyAction) { entry = dragEntry->clone(Entry::CloneCopy); } entry->setGroup(parentGroup); } - - if (numEntriesNotMoved) { - showErrorMessage(tr("Move Error: %1 of %2 entry(s) already present in this database").arg(numEntriesNotMoved).arg(numEntries)); - } } return true; From 6a0a78b870b13cc180cf88674f78df761618bdb8 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Mon, 25 Mar 2024 14:46:29 +0100 Subject: [PATCH 06/12] Fix missed case of incorrect update to a Group's LastModificationTime --- src/core/Entry.cpp | 3 +-- src/core/Group.cpp | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 8fee335879..cf1c504a0e 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -1483,9 +1483,8 @@ QUuid Entry::previousParentGroupUuid() const void Entry::setPreviousParentGroupUuid(const QUuid& uuid) { - // prevent set from changing the LastModificationTime bool prevUpdateTimeinfo = m_updateTimeinfo; - m_updateTimeinfo = false; + m_updateTimeinfo = false; // prevent update of LastModificationTime set(m_data.previousParentGroupUuid, uuid); m_updateTimeinfo = prevUpdateTimeinfo; } diff --git a/src/core/Group.cpp b/src/core/Group.cpp index f6b7f03fe9..d1df6e94c3 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -494,7 +494,11 @@ void Group::setParent(Group* parent, int index, bool trackPrevious) m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc()); } + bool prevUpdateTimeInfo = m_updateTimeinfo; + m_updateTimeinfo = false; // prevent update of LastModificationTime emitModified(); + m_updateTimeinfo = prevUpdateTimeInfo; + if (!moveWithinDatabase) { emit groupAdded(); @@ -1237,9 +1241,8 @@ QUuid Group::previousParentGroupUuid() const void Group::setPreviousParentGroupUuid(const QUuid& uuid) { - // prevent set from changing the LastModificationTime bool prevUpdateTimeinfo = m_updateTimeinfo; - m_updateTimeinfo = false; + m_updateTimeinfo = false; // prevent update of LastModificationTime set(m_data.previousParentGroupUuid, uuid); m_updateTimeinfo = prevUpdateTimeinfo; } From 0d3a3e9df836b1b7405d39cfdb2dc3eae4ab662a Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Mon, 25 Mar 2024 15:08:13 +0100 Subject: [PATCH 07/12] Correct overzealous refactor --- src/core/Entry.cpp | 4 +++- src/core/Group.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index cf1c504a0e..3ab2839d8b 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -1267,7 +1267,9 @@ void Entry::setGroup(Group* group, bool trackPrevious) m_group->database()->addDeletedObject(m_uuid); // copy custom icon to the new database - group->database()->metadata()->copyCustomIcon(iconUuid(), m_group->database()->metadata()); + if (group->database()) { + group->database()->metadata()->copyCustomIcon(iconUuid(), m_group->database()->metadata()); + } } else if (trackPrevious && m_group->database() && group != m_group) { setPreviousParentGroup(m_group); } diff --git a/src/core/Group.cpp b/src/core/Group.cpp index d1df6e94c3..c217a0e829 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -469,7 +469,9 @@ void Group::setParent(Group* parent, int index, bool trackPrevious) recCreateDelObjects(); // copy custom icon to the new database - parent->m_db->metadata()->copyCustomIcon(iconUuid(), m_db->metadata()); + if (parent->m_db) { + parent->m_db->metadata()->copyCustomIcon(iconUuid(), m_db->metadata()); + } } if (m_db != parent->m_db) { connectDatabaseSignalsRecursive(parent->m_db); From 2a287f6884d7129203e3287ece20a1a5c7779b14 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Mon, 25 Mar 2024 21:54:23 +0100 Subject: [PATCH 08/12] Fix case of incorrect update to a Group's LastModificationTime --- src/core/Group.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index c217a0e829..e9f875cb5d 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -1074,7 +1074,12 @@ void Group::cleanupParent() if (m_parent) { emit groupAboutToRemove(this); m_parent->m_children.removeAll(this); + + bool prevUpdateTimeinfo = m_updateTimeinfo; + m_updateTimeinfo = false; // prevent update of LastModificationTime emitModified(); + m_updateTimeinfo = prevUpdateTimeinfo; + emit groupRemoved(); } } From ffb9b111fdeedf5fe908038bcf196c8409421c35 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Mon, 25 Mar 2024 21:56:06 +0100 Subject: [PATCH 09/12] Add check to test case --- tests/TestEntry.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/TestEntry.cpp b/tests/TestEntry.cpp index 6566b4d1e5..eac09f832a 100644 --- a/tests/TestEntry.cpp +++ b/tests/TestEntry.cpp @@ -123,6 +123,8 @@ void TestEntry::testClone() QCOMPARE(entryCloneResetTime->title(), QString("New Title")); QCOMPARE(entryCloneResetTime->historyItems().size(), 0); QVERIFY(entryCloneResetTime->timeInfo().creationTime() != entryOrg->timeInfo().creationTime()); + // Cloning with CloneResetTimeInfo should not affect the LastModificationTime + QCOMPARE(entryCloneResetTime->timeInfo().lastModificationTime(), entryOrg->timeInfo().lastModificationTime()); // Date back history of original entry Entry* firstHistoryItem = entryOrg->historyItems()[0]; From 9a7db370bf9d1e58bb2435c37cbeb064242892c3 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Tue, 26 Mar 2024 16:52:34 +0100 Subject: [PATCH 10/12] Add and adjust test-cases --- tests/CMakeLists.txt | 2 +- tests/TestEntry.cpp | 56 ++++++++++++++++++++++++++++++++++++++++++++ tests/TestEntry.h | 3 +++ tests/TestGroup.cpp | 55 ++++++++++++++++++++++++++++++++++++++++--- tests/TestGroup.h | 1 + 5 files changed, 113 insertions(+), 4 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4b26d38bc0..19761378d9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -168,7 +168,7 @@ if(WITH_XC_SSHAGENT) endif() add_unit_test(NAME testentry SOURCES TestEntry.cpp - LIBS ${TEST_LIBRARIES}) + LIBS testsupport ${TEST_LIBRARIES}) add_unit_test(NAME testmerge SOURCES TestMerge.cpp LIBS testsupport ${TEST_LIBRARIES}) diff --git a/tests/TestEntry.cpp b/tests/TestEntry.cpp index eac09f832a..ec7969d808 100644 --- a/tests/TestEntry.cpp +++ b/tests/TestEntry.cpp @@ -25,13 +25,34 @@ #include "core/TimeInfo.h" #include "crypto/Crypto.h" +#include "mock/MockClock.h" + QTEST_GUILESS_MAIN(TestEntry) +namespace +{ + MockClock* m_clock = nullptr; +} + void TestEntry::initTestCase() { QVERIFY(Crypto::init()); } +void TestEntry::init() +{ + Q_ASSERT(m_clock == nullptr); + m_clock = new MockClock(2010, 5, 5, 10, 30, 10); + MockClock::setup(m_clock); +} + +void TestEntry::cleanup() +{ + MockClock::teardown(); + m_clock = nullptr; +} + + void TestEntry::testHistoryItemDeletion() { QScopedPointer entry(new Entry()); @@ -109,6 +130,8 @@ void TestEntry::testClone() QCOMPARE(entryCloneNewUuid->timeInfo().creationTime(), entryOrg->timeInfo().creationTime()); // Reset modification time + entryOrgTime.setLastAccessTime(Clock::datetimeUtc(60)); + entryOrgTime.setLocationChanged(Clock::datetimeUtc(60)); entryOrgTime.setLastModificationTime(Clock::datetimeUtc(60)); entryOrg->setTimeInfo(entryOrgTime); @@ -122,7 +145,10 @@ void TestEntry::testClone() QCOMPARE(entryCloneResetTime->uuid(), entryOrg->uuid()); QCOMPARE(entryCloneResetTime->title(), QString("New Title")); QCOMPARE(entryCloneResetTime->historyItems().size(), 0); + // Cloning with CloneResetTimeInfo should affect the CreationTime, LocationChanged, LastAccessTime QVERIFY(entryCloneResetTime->timeInfo().creationTime() != entryOrg->timeInfo().creationTime()); + QVERIFY(entryCloneResetTime->timeInfo().locationChanged() != entryOrg->timeInfo().locationChanged()); + QVERIFY(entryCloneResetTime->timeInfo().lastAccessTime() != entryOrg->timeInfo().lastAccessTime()); // Cloning with CloneResetTimeInfo should not affect the LastModificationTime QCOMPARE(entryCloneResetTime->timeInfo().lastModificationTime(), entryOrg->timeInfo().lastModificationTime()); @@ -770,3 +796,33 @@ void TestEntry::testPreviousParentGroup() QVERIFY(entry->previousParentGroupUuid() == group1->uuid()); QVERIFY(entry->previousParentGroup() == group1); } + +void TestEntry::testTimeinfoChanges() +{ + Database db; + auto* root = db.rootGroup(); + auto* subgroup = new Group(); + subgroup->setUuid(QUuid::createUuid()); + subgroup->setParent(root); + QDateTime startTime = Clock::currentDateTimeUtc(); + TimeInfo startTimeinfo; + startTimeinfo.setCreationTime(startTime); + startTimeinfo.setLastModificationTime(startTime); + startTimeinfo.setLocationChanged(startTime); + startTimeinfo.setLastAccessTime(startTime); + m_clock->advanceMinute(1); + + QScopedPointer entry(new Entry()); + entry->setUuid(QUuid::createUuid()); + entry->setGroup(root); + entry->setTimeInfo(startTimeinfo); + entry->setPreviousParentGroup(subgroup); + // setting previous parent group should not affect the LastModificationTime + QCOMPARE(entry->timeInfo().lastModificationTime(), startTime); + entry->setGroup(subgroup); + // changing group should not affect LastModicationTime, CreationTime + QCOMPARE(entry->timeInfo().creationTime(), startTime); + QCOMPARE(entry->timeInfo().lastModificationTime(), startTime); + // changing group should affect the LocationChanged time + QCOMPARE(entry->timeInfo().locationChanged(), Clock::currentDateTimeUtc()); +} diff --git a/tests/TestEntry.h b/tests/TestEntry.h index 3bfd8f52dc..ac2259b6dc 100644 --- a/tests/TestEntry.h +++ b/tests/TestEntry.h @@ -28,6 +28,8 @@ class TestEntry : public QObject private slots: void initTestCase(); + void init(); + void cleanup(); void testHistoryItemDeletion(); void testCopyDataFrom(); void testClone(); @@ -40,6 +42,7 @@ private slots: void testIsRecycled(); void testMoveUpDown(); void testPreviousParentGroup(); + void testTimeinfoChanges(); }; #endif // KEEPASSX_TESTENTRY_H diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 22807c8781..cd1dcd7d6f 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -382,18 +382,21 @@ void TestGroup::testClone() QCOMPARE(clonedGroup->iconNumber(), 42); QCOMPARE(clonedGroup->children().size(), 1); QCOMPARE(clonedGroup->entries().size(), 1); + QCOMPARE(clonedGroup->timeInfo(), originalGroup->timeInfo()); Entry* clonedGroupEntry = clonedGroup->entries().at(0); QVERIFY(clonedGroupEntry->uuid() != originalGroupEntry->uuid()); QCOMPARE(clonedGroupEntry->title(), QString("GroupEntry")); QCOMPARE(clonedGroupEntry->iconNumber(), 43); QCOMPARE(clonedGroupEntry->historyItems().size(), 0); + QCOMPARE(clonedGroupEntry->timeInfo(), originalGroupEntry->timeInfo()); Group* clonedSubGroup = clonedGroup->children().at(0); QVERIFY(clonedSubGroup->uuid() != subGroup->uuid()); QCOMPARE(clonedSubGroup->name(), QString("SubGroup")); QCOMPARE(clonedSubGroup->children().size(), 0); QCOMPARE(clonedSubGroup->entries().size(), 1); + QCOMPARE(clonedSubGroup->timeInfo(), subGroup->timeInfo()); Entry* clonedSubGroupEntry = clonedSubGroup->entries().at(0); QVERIFY(clonedSubGroupEntry->uuid() != subGroupEntry->uuid()); @@ -411,15 +414,17 @@ void TestGroup::testClone() QCOMPARE(clonedGroupNewUuid->entries().size(), 0); QVERIFY(clonedGroupNewUuid->uuid() != originalGroup->uuid()); - // Making sure the new modification date is not the same. + // Verify Timeinfo modifications for CloneResetTimeInfo m_clock->advanceSecond(1); QScopedPointer 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()); + QVERIFY(clonedGroupResetTimeInfo->timeInfo().creationTime() != originalGroup->timeInfo().creationTime()); + QVERIFY(clonedGroupResetTimeInfo->timeInfo().lastAccessTime() != originalGroup->timeInfo().lastAccessTime()); + QVERIFY(clonedGroupResetTimeInfo->timeInfo().locationChanged() != originalGroup->timeInfo().locationChanged()); + QCOMPARE(clonedGroupResetTimeInfo->timeInfo().lastModificationTime(), originalGroup->timeInfo().lastModificationTime()); } void TestGroup::testCopyCustomIcons() @@ -1319,3 +1324,47 @@ void TestGroup::testAutoTypeState() QVERIFY(!entry1->groupAutoTypeEnabled()); QVERIFY(entry2->groupAutoTypeEnabled()); } + +void TestGroup::testTimeinfoChanges() +{ + Database db, db2; + auto* root = db.rootGroup(); + auto* subgroup1 = new Group(); + auto* subgroup2 = new Group(); + subgroup1->setUuid(QUuid::createUuid()); + subgroup1->setParent(root); + subgroup2->setUuid(QUuid::createUuid()); + subgroup2->setParent(root); + QDateTime startTime = Clock::currentDateTimeUtc(); + TimeInfo startTimeinfo; + startTimeinfo.setCreationTime(startTime); + startTimeinfo.setLastModificationTime(startTime); + startTimeinfo.setLocationChanged(startTime); + startTimeinfo.setLastAccessTime(startTime); + m_clock->advanceMinute(1); + root->setTimeInfo(startTimeinfo); + subgroup1->setTimeInfo(startTimeinfo); + subgroup2->setTimeInfo(startTimeinfo); + + subgroup2->setPreviousParentGroup(subgroup1); + // setting previous parent group should not affect the LastModificationTime + QCOMPARE(subgroup2->timeInfo().lastModificationTime(), startTime); + subgroup2->setPreviousParentGroup(nullptr); + subgroup2->setParent(subgroup1); + QCOMPARE(root->timeInfo(), startTimeinfo); + QCOMPARE(subgroup1->timeInfo(), startTimeinfo); + // changing group should not affect LastModificationTime, CreationTime + QCOMPARE(subgroup2->timeInfo().creationTime(), startTime); + QCOMPARE(subgroup2->timeInfo().lastModificationTime(), startTime); + // changing group should affect the LocationChanged time + QCOMPARE(subgroup2->timeInfo().locationChanged(), Clock::currentDateTimeUtc()); + + // cross-db move + db2.rootGroup()->setTimeInfo(startTimeinfo); + m_clock->advanceMinute(1); + subgroup2->setParent(db2.rootGroup()); + QCOMPARE(subgroup2->timeInfo().creationTime(), startTime); + QCOMPARE(subgroup2->timeInfo().lastModificationTime(), startTime); + QCOMPARE(subgroup2->timeInfo().locationChanged(), Clock::currentDateTimeUtc()); + QCOMPARE(db2.rootGroup()->timeInfo(), startTimeinfo); +} diff --git a/tests/TestGroup.h b/tests/TestGroup.h index d3326e4648..1f01d94912 100644 --- a/tests/TestGroup.h +++ b/tests/TestGroup.h @@ -50,6 +50,7 @@ private slots: void testMoveUpDown(); void testPreviousParentGroup(); void testAutoTypeState(); + void testTimeinfoChanges(); }; #endif // KEEPASSX_TESTGROUP_H From ca4ae86eb640407edae84734f7ce6d9335517665 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Thu, 28 Mar 2024 12:26:33 +0100 Subject: [PATCH 11/12] Fix moving a group/entry across dbs resets the CreationTime --- src/core/Entry.cpp | 13 +++++++++---- src/core/Entry.h | 17 +++++++++++------ src/core/Group.cpp | 14 +++++++++----- src/core/Group.h | 14 ++++++++++---- src/gui/group/GroupModel.cpp | 23 +++++++++++++---------- 5 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 3ab2839d8b..ab6fd2a28b 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -946,10 +946,15 @@ Entry* Entry::clone(CloneFlags flags) const if (flags & CloneResetTimeInfo) { QDateTime now = Clock::currentDateTimeUtc(); - entry->m_data.timeInfo.setCreationTime(now); - entry->m_data.timeInfo.setLastAccessTime(now); - entry->m_data.timeInfo.setLocationChanged(now); - // preserve LastModificationTime + if (flags & CloneResetCreationTime) { + entry->m_data.timeInfo.setCreationTime(now); + } + if (flags & CloneResetLastAccessTime) { + entry->m_data.timeInfo.setLastAccessTime(now); + } + if (flags & CloneResetLocationChangedTime) { + entry->m_data.timeInfo.setLocationChanged(now); + } } if (flags & CloneRenameTitle) { diff --git a/src/core/Entry.h b/src/core/Entry.h index 2e42b50d4f..6c739d78f4 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -175,13 +175,18 @@ class Entry : public ModifiableObject { CloneNoFlags = 0, CloneNewUuid = 1, // generate a random uuid for the clone - CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time - CloneIncludeHistory = 4, // clone the history items + CloneResetCreationTime = 2, // set timeInfo.CreationTime to the current time + CloneResetLastAccessTime = 4, // set timeInfo.LastAccessTime to the current time + CloneResetLocationChangedTime = 8, // set timeInfo.LocationChangedTime to the current time + CloneIncludeHistory = 16, // clone the history items + CloneRenameTitle = 32, // add "-Clone" after the original title + CloneUserAsRef = 64, // Add the user as a reference to the original entry + ClonePassAsRef = 128, // Add the password as a reference to the original entry + + CloneResetTimeInfo = CloneResetCreationTime | CloneResetLastAccessTime | CloneResetLocationChangedTime, + CloneExactCopy = CloneIncludeHistory, + CloneCopy = CloneExactCopy | CloneNewUuid | CloneResetTimeInfo, CloneDefault = CloneNewUuid | CloneResetTimeInfo, - CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory, - CloneRenameTitle = 8, // add "-Clone" after the original title - CloneUserAsRef = 16, // Add the user as a reference to the original entry - ClonePassAsRef = 32, // Add the password as a reference to the original entry }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index e9f875cb5d..0e652878c1 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -949,12 +949,16 @@ Group* Group::clone(Entry::CloneFlags entryFlags, Group::CloneFlags groupFlags) clonedGroup->setUpdateTimeinfo(true); if (groupFlags & Group::CloneResetTimeInfo) { - QDateTime now = Clock::currentDateTimeUtc(); - clonedGroup->m_data.timeInfo.setCreationTime(now); - clonedGroup->m_data.timeInfo.setLastAccessTime(now); - clonedGroup->m_data.timeInfo.setLocationChanged(now); - // preserve LastModificationTime + if (groupFlags & Group::CloneResetCreationTime) { + clonedGroup->m_data.timeInfo.setCreationTime(now); + } + if (groupFlags & Group::CloneResetLastAccessTime) { + clonedGroup->m_data.timeInfo.setLastAccessTime(now); + } + if (groupFlags & Group::CloneResetLocationChangedTime) { + clonedGroup->m_data.timeInfo.setLocationChanged(now); + } } if (groupFlags & Group::CloneRenameTitle) { diff --git a/src/core/Group.h b/src/core/Group.h index a121922f56..f1d93825dc 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -47,10 +47,16 @@ class Group : public ModifiableObject { CloneNoFlags = 0, CloneNewUuid = 1, // generate a random uuid for the clone - CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time - CloneIncludeEntries = 4, // clone the group entries - CloneDefault = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries, - CloneRenameTitle = 8, // add "- Clone" after the original title + CloneResetCreationTime = 2, // set timeInfo.CreationTime to the current time + CloneResetLastAccessTime = 4, // set timeInfo.LastAccessTime to the current time + CloneResetLocationChangedTime = 8, // set timeInfo.LocationChangedTime to the current time + CloneIncludeEntries = 16, // clone the group entries + CloneRenameTitle = 32, // add "- Clone" after the original title + + CloneResetTimeInfo = CloneResetCreationTime | CloneResetLastAccessTime | CloneResetLocationChangedTime, + CloneExactCopy = CloneIncludeEntries, + CloneCopy = CloneExactCopy | CloneNewUuid | CloneResetTimeInfo, + CloneDefault = CloneCopy, }; Q_DECLARE_FLAGS(CloneFlags, CloneFlag) diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index 18b926dc20..436660f918 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -258,14 +258,16 @@ bool GroupModel::dropMimeData(const QMimeData* data, Group* group = dragGroup; if (sourceDb != targetDb) { - QSet customIcons = group->customIconsRecursive(); - targetDb->metadata()->copyCustomIcons(customIcons, sourceDb->metadata()); + targetDb->metadata()->copyCustomIcons(group->customIconsRecursive(), sourceDb->metadata()); - // Always clone the group across db's to reset UUIDs - group = dragGroup->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); if (action == Qt::MoveAction) { - // Remove the original group from the sourceDb + // For cross-db moves use a clone with new UUID but original CreationTime + group = dragGroup->clone(Entry::CloneFlags(Entry::CloneCopy & ~Entry::CloneResetCreationTime), + Group::CloneFlags(Group::CloneCopy & ~Group::CloneResetCreationTime)); + // Remove the original from the sourceDb to allow this change to sync to other dbs delete dragGroup; + } else { + group = dragGroup->clone(Entry::CloneCopy); } } else if (action == Qt::CopyAction) { group = dragGroup->clone(Entry::CloneCopy); @@ -298,15 +300,16 @@ bool GroupModel::dropMimeData(const QMimeData* data, Entry* entry = dragEntry; if (sourceDb != targetDb) { - QUuid customIcon = entry->iconUuid(); - if (!customIcon.isNull() && !targetDb->metadata()->hasCustomIcon(customIcon)) { - targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon).data); - } + targetDb->metadata()->copyCustomIcon(entry->iconUuid(), sourceDb->metadata()); // Reset the UUID when moving across db boundary - entry = dragEntry->clone(Entry::CloneDefault | Entry::CloneIncludeHistory); if (action == Qt::MoveAction) { + // For cross-db moves use a clone with new UUID but original CreationTime + entry = dragEntry->clone(Entry::CloneFlags(Entry::CloneCopy & ~Entry::CloneResetCreationTime)); + // Remove the original from the sourceDb to allow this change to sync to other dbs delete dragEntry; + } else { + entry = dragEntry->clone(Entry::CloneCopy); } } else if (action == Qt::CopyAction) { entry = dragEntry->clone(Entry::CloneCopy); From 22a1a82e5bf13bfc2b7546f9f8d7ac948bb8a958 Mon Sep 17 00:00:00 2001 From: vuurvlieg Date: Sat, 30 Mar 2024 13:36:54 +0100 Subject: [PATCH 12/12] Fix additional cases of incorrectly handling of a Group's LastModificationTime Cases: - Entry is added to a Group - Entry is removed from a Group - The Group list is sorted --- src/core/Group.cpp | 37 +++++++++++++++++-------------------- src/core/Group.h | 3 ++- tests/TestGroup.cpp | 13 +++++++++++++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 0e652878c1..4f8434bab8 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -77,11 +77,11 @@ Group::~Group() cleanupParent(); } -template inline bool Group::set(P& property, const V& value) +template inline bool Group::set(P& property, const V& value, bool preserveTimeinfo) { if (property != value) { property = value; - emitModified(); + emitModifiedEx(preserveTimeinfo); return true; } else { return false; @@ -440,6 +440,15 @@ const Group* Group::parentGroup() const return m_parent; } +void Group::emitModifiedEx(bool preserveTimeinfo) { + bool prevUpdateTimeinfo = m_updateTimeinfo; + if (preserveTimeinfo) { + m_updateTimeinfo = false; // prevent update of LastModificationTime + } + emitModified(); + m_updateTimeinfo = prevUpdateTimeinfo; +} + void Group::setParent(Group* parent, int index, bool trackPrevious) { Q_ASSERT(parent); @@ -496,11 +505,7 @@ void Group::setParent(Group* parent, int index, bool trackPrevious) m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc()); } - bool prevUpdateTimeInfo = m_updateTimeinfo; - m_updateTimeinfo = false; // prevent update of LastModificationTime - emitModified(); - m_updateTimeinfo = prevUpdateTimeInfo; - + emitModifiedEx(true); if (!moveWithinDatabase) { emit groupAdded(); @@ -990,7 +995,7 @@ void Group::addEntry(Entry* entry) connect(entry, &Entry::modified, m_db, &Database::markAsModified); } - emitModified(); + emitModifiedEx(true); emit entryAdded(entry); } @@ -1007,7 +1012,7 @@ void Group::removeEntry(Entry* entry) entry->disconnect(m_db); } m_entries.removeAll(entry); - emitModified(); + emitModifiedEx(true); emit entryRemoved(entry); } @@ -1078,12 +1083,7 @@ void Group::cleanupParent() if (m_parent) { emit groupAboutToRemove(this); m_parent->m_children.removeAll(this); - - bool prevUpdateTimeinfo = m_updateTimeinfo; - m_updateTimeinfo = false; // prevent update of LastModificationTime - emitModified(); - m_updateTimeinfo = prevUpdateTimeinfo; - + emitModifiedEx(true); emit groupRemoved(); } } @@ -1234,7 +1234,7 @@ void Group::sortChildrenRecursively(bool reverse) child->sortChildrenRecursively(reverse); } - emitModified(); + emitModifiedEx(true); } const Group* Group::previousParentGroup() const @@ -1252,10 +1252,7 @@ QUuid Group::previousParentGroupUuid() const void Group::setPreviousParentGroupUuid(const QUuid& uuid) { - bool prevUpdateTimeinfo = m_updateTimeinfo; - m_updateTimeinfo = false; // prevent update of LastModificationTime - set(m_data.previousParentGroupUuid, uuid); - m_updateTimeinfo = prevUpdateTimeinfo; + set(m_data.previousParentGroupUuid, uuid, true); } void Group::setPreviousParentGroup(const Group* group) diff --git a/src/core/Group.h b/src/core/Group.h index f1d93825dc..e10aafd870 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -210,8 +210,9 @@ private slots: void updateTimeinfo(); private: - template bool set(P& property, const V& value); + template bool set(P& property, const V& value, bool preserveTimeinfo = false); + void emitModifiedEx(bool preserveTimeinfo); void setParent(Database* db); void connectDatabaseSignalsRecursive(Database* db); diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index cd1dcd7d6f..7b5daf803a 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -1367,4 +1367,17 @@ void TestGroup::testTimeinfoChanges() QCOMPARE(subgroup2->timeInfo().lastModificationTime(), startTime); QCOMPARE(subgroup2->timeInfo().locationChanged(), Clock::currentDateTimeUtc()); QCOMPARE(db2.rootGroup()->timeInfo(), startTimeinfo); + + QScopedPointer entry1(new Entry()); + entry1->setGroup(subgroup1); + // adding/removing an entry should not affect the LastModificationTime + QCOMPARE(subgroup1->timeInfo().lastModificationTime(), startTime); + entry1.reset(); // delete + QCOMPARE(subgroup1->timeInfo().lastModificationTime(), startTime); + + // sorting should not affect the LastModificationTime + root->sortChildrenRecursively(true); + root->sortChildrenRecursively(false); + QCOMPARE(root->timeInfo().lastModificationTime(), startTime); + QCOMPARE(subgroup1->timeInfo().lastModificationTime(), startTime); }