merge confirmation without merge logic refactoring

This commit is contained in:
Tamino Bauknecht 2024-02-29 17:27:27 +01:00
parent 6877b5ecf8
commit 31bc5292a9
No known key found for this signature in database
GPG Key ID: 77837396BE935C6C
7 changed files with 217 additions and 34 deletions

View File

@ -87,10 +87,10 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
}
Merger merger(db2.data(), database.data());
QStringList changeList = merger.merge();
auto changeList = merger.merge();
for (auto& mergeChange : changeList) {
out << "\t" << mergeChange << endl;
out << "\t" << mergeChange.toString() << endl;
}
if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {

View File

@ -19,6 +19,99 @@
#include "core/Metadata.h"
Merger::Change::Change(Type type, const Group& group, QString details)
: m_type{type}
, m_group{group.fullPath()}
, m_uuid{group.uuid()}
, m_details{std::move(details)}
{
}
Merger::Change::Change(Type type, const Entry& entry, QString details)
: m_type{type}
, m_title{entry.title()}
, m_uuid{entry.uuid()}
, m_details{std::move(details)}
{
if (const auto* group = entry.group()) {
m_group = group->fullPath();
}
}
Merger::Change::Change(QString details)
: m_details{std::move(details)}
{
}
Merger::Change::Type Merger::Change::type() const
{
return m_type;
}
const QString& Merger::Change::title() const
{
return m_title;
}
const QString& Merger::Change::group() const
{
return m_group;
}
const QUuid& Merger::Change::uuid() const
{
return m_uuid;
}
const QString& Merger::Change::details() const
{
return m_details;
}
QString Merger::Change::typeString() const
{
switch (m_type) {
case Type::Added:
return tr("Added");
break;
case Type::Modified:
return tr("Modified");
break;
case Type::Moved:
return tr("Moved");
break;
case Type::Deleted:
return tr("Deleted");
break;
case Type::Unspecified:
return "";
break;
default:
return "?";
}
}
QString Merger::Change::toString() const
{
QString result;
if (m_type != Type::Unspecified) {
result += QString("%1: ").arg(typeString());
}
if (!m_group.isEmpty()) {
result += QString("'%1'").arg(m_group);
}
if (!m_title.isEmpty()) {
result += QString("/'%1'").arg(m_title);
}
if (!m_uuid.isNull()) {
result += QString(" [%1]").arg(m_uuid.toString());
}
if (!m_details.isEmpty()) {
result += QString(" (%1)").arg(m_details);
}
return result;
}
bool operator==(const Merger::Change& lhs, const Merger::Change& rhs)
{
return lhs.type() == rhs.type() && lhs.group() == rhs.group() && lhs.title() == rhs.title()
&& lhs.uuid() == rhs.uuid() && lhs.details() == rhs.details();
}
Merger::Merger(const Database* sourceDb, Database* targetDb)
: m_mode(Group::Default)
{
@ -57,7 +150,7 @@ void Merger::resetForcedMergeMode()
m_mode = Group::Default;
}
QStringList Merger::merge()
Merger::ChangeList Merger::merge()
{
// Order of merge steps is important - it is possible that we
// create some items before deleting them afterwards
@ -81,7 +174,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context)
for (Entry* sourceEntry : sourceEntries) {
Entry* targetEntry = context.m_targetRootGroup->findEntryByUuid(sourceEntry->uuid());
if (!targetEntry) {
changes << tr("Creating missing %1 [%2]").arg(sourceEntry->title(), sourceEntry->uuidToHex());
changes << Change(Change::Type::Added, *sourceEntry, tr("Creating missing"));
// This entry does not exist at all. Create it.
targetEntry = sourceEntry->clone(Entry::CloneIncludeHistory);
moveEntry(targetEntry, context.m_targetGroup);
@ -90,7 +183,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context)
const bool locationChanged =
targetEntry->timeInfo().locationChanged() < sourceEntry->timeInfo().locationChanged();
if (locationChanged && targetEntry->group() != context.m_targetGroup) {
changes << tr("Relocating %1 [%2]").arg(sourceEntry->title(), sourceEntry->uuidToHex());
changes << Change(Change::Type::Moved, *sourceEntry, tr("Relocating"));
moveEntry(targetEntry, context.m_targetGroup);
}
changes << resolveEntryConflict(context, sourceEntry, targetEntry);
@ -102,7 +195,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context)
for (Group* sourceChildGroup : sourceChildGroups) {
Group* targetChildGroup = context.m_targetRootGroup->findGroupByUuid(sourceChildGroup->uuid());
if (!targetChildGroup) {
changes << tr("Creating missing %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex());
changes << Change(Change::Type::Added, *sourceChildGroup, tr("Creating missing"));
targetChildGroup = sourceChildGroup->clone(Entry::CloneNoFlags, Group::CloneNoFlags);
moveGroup(targetChildGroup, context.m_targetGroup);
TimeInfo timeinfo = targetChildGroup->timeInfo();
@ -112,7 +205,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context)
bool locationChanged =
targetChildGroup->timeInfo().locationChanged() < sourceChildGroup->timeInfo().locationChanged();
if (locationChanged && targetChildGroup->parent() != context.m_targetGroup) {
changes << tr("Relocating %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex());
changes << Change(Change::Type::Moved, *sourceChildGroup, tr("Relocating"));
moveGroup(targetChildGroup, context.m_targetGroup);
TimeInfo timeinfo = targetChildGroup->timeInfo();
timeinfo.setLocationChanged(sourceChildGroup->timeInfo().locationChanged());
@ -142,7 +235,7 @@ Merger::resolveGroupConflict(const MergeContext& context, const Group* sourceChi
// only if the other group is newer, update the existing one.
if (timeExisting < timeOther) {
changes << tr("Overwriting %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex());
changes << Change(Change::Type::Modified, *sourceChildGroup, tr("Overwriting group properties"));
targetChildGroup->setName(sourceChildGroup->name());
targetChildGroup->setNotes(sourceChildGroup->notes());
if (sourceChildGroup->iconNumber() == 0) {
@ -269,7 +362,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
qPrintable(targetEntry->title()),
qPrintable(sourceEntry->title()),
qPrintable(currentGroup->name()));
changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
changes << Change(Change::Type::Modified, *targetEntry, tr("Synchronizing from newer source"));
mergeHistory(targetEntry, clonedEntry, mergeMethod, maxItems);
eraseEntry(targetEntry);
moveEntry(clonedEntry, currentGroup);
@ -280,8 +373,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
qPrintable(targetEntry->group()->name()));
const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod, maxItems);
if (changed) {
changes
<< tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
changes << Change(Change::Type::Modified, *targetEntry, tr("Synchronizing from older source"));
}
}
return changes;
@ -458,9 +550,9 @@ Merger::ChangeList Merger::mergeDeletions(const MergeContext& context)
}
deletions << object;
if (entry->group()) {
changes << tr("Deleting child %1 [%2]").arg(entry->title(), entry->uuidToHex());
changes << Change(Change::Type::Deleted, *entry, tr("Deleting child"));
} else {
changes << tr("Deleting orphan %1 [%2]").arg(entry->title(), entry->uuidToHex());
changes << Change(Change::Type::Deleted, *entry, tr("Deleting orphan"));
}
// Entry is inserted into deletedObjects after deletions are processed
eraseEntry(entry);
@ -484,15 +576,15 @@ Merger::ChangeList Merger::mergeDeletions(const MergeContext& context)
}
deletions << object;
if (group->parentGroup()) {
changes << tr("Deleting child %1 [%2]").arg(group->name(), group->uuidToHex());
changes << Change(Change::Type::Deleted, *group, tr("Deleting child"));
} else {
changes << tr("Deleting orphan %1 [%2]").arg(group->name(), group->uuidToHex());
changes << Change(Change::Type::Deleted, *group, tr("Deleting orphan"));
}
eraseGroup(group);
}
// Put every deletion to the earliest date of deletion
if (deletions != context.m_targetDb->deletedObjects()) {
changes << tr("Changed deleted objects");
changes << Change(tr("Changed deleted objects"));
}
context.m_targetDb->setDeletedObjects(deletions);
return changes;
@ -510,7 +602,7 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
for (const auto& iconUuid : sourceMetadata->customIconsOrder()) {
if (!targetMetadata->hasCustomIcon(iconUuid)) {
targetMetadata->addCustomIcon(iconUuid, sourceMetadata->customIcon(iconUuid));
changes << tr("Adding missing icon %1").arg(QString::fromLatin1(iconUuid.toRfc4122().toHex()));
changes << Change(tr("Adding missing icon %1").arg(QString::fromLatin1(iconUuid.toRfc4122().toHex())));
}
}
@ -529,7 +621,7 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
if (!sourceMetadata->customData()->contains(key) && !sourceMetadata->customData()->isProtected(key)) {
auto value = targetMetadata->customData()->value(key);
targetMetadata->customData()->remove(key);
changes << tr("Removed custom data %1 [%2]").arg(key, value);
changes << Change(tr("Removed custom data %1 [%2]").arg(key, value));
}
}
@ -545,7 +637,7 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
// Merge only if the values are not the same.
if (sourceValue != targetValue) {
targetMetadata->customData()->set(key, sourceValue);
changes << tr("Adding custom data %1 [%2]").arg(key, sourceValue);
changes << Change(tr("Adding custom data %1 [%2]").arg(key, sourceValue));
}
}
}

View File

@ -27,16 +27,50 @@ class Merger : public QObject
{
Q_OBJECT
public:
class Change;
using ChangeList = QList<Change>;
Merger(const Database* sourceDb, Database* targetDb);
Merger(const Group* sourceGroup, Group* targetGroup);
void setForcedMergeMode(Group::MergeMode mode);
void resetForcedMergeMode();
QStringList merge();
ChangeList merge();
class Change
{
public:
enum class Type
{
Unspecified,
Added,
Modified,
Moved,
Deleted,
};
Change(Type type, const Group& group, QString details = "");
Change(Type type, const Entry& entry, QString details = "");
explicit Change(QString details = "");
[[nodiscard]] Type type() const;
[[nodiscard]] QString typeString() const;
[[nodiscard]] const QString& title() const;
[[nodiscard]] const QString& group() const;
[[nodiscard]] const QUuid& uuid() const;
[[nodiscard]] const QString& details() const;
[[nodiscard]] QString toString() const;
void merge();
private:
Type m_type{Type::Unspecified};
QString m_title;
QString m_group;
QUuid m_uuid;
QString m_details;
};
private:
typedef QString Change;
typedef QStringList ChangeList;
struct MergeContext
{
QPointer<const Database> m_sourceDb;
@ -68,4 +102,6 @@ private:
Group::MergeMode m_mode;
};
bool operator==(const Merger::Change& lhs, const Merger::Change& rhs);
#endif // KEEPASSXC_MERGER_H

View File

@ -1199,6 +1199,20 @@ void DatabaseWidget::mergeDatabase(bool accepted)
showMessage(tr("Database was not modified by merge operation."), MessageWidget::Information);
}
});
connect(mergeDialog,
&MergeDialog::databaseModifiedMerge,
[this](const Merger::ChangeList& actualChanges, const Merger::ChangeList&) {
if (!actualChanges.isEmpty()) {
showMessage(tr("Merged changes do not match displayed changes!"), MessageWidget::Warning);
auto* actualChangesDialog = new MergeDialog(actualChanges, this);
actualChangesDialog->setWindowTitle(tr("Actual Merge Result"));
actualChangesDialog->open();
} else {
showMessage(
tr("Database was not modified by merge operation, displayed changes were not applied!"),
MessageWidget::Warning);
}
});
connect(mergeDialog, &MergeDialog::rejected, [this]() {
showMessage(tr("Merge aborted - database was not modified."), MessageWidget::Information);
});

View File

@ -27,7 +27,6 @@
MergeDialog::MergeDialog(QSharedPointer<Database> source, QSharedPointer<Database> target, QWidget* parent)
: QDialog(parent)
, m_merger(source.data(), target.data())
, m_sourceDatabase{std::move(source)}
, m_targetDatabase{std::move(target)}
{
@ -38,7 +37,7 @@ MergeDialog::MergeDialog(QSharedPointer<Database> source, QSharedPointer<Databas
m_ui.buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Merge"));
m_ui.buttonBox->button(QDialogButtonBox::Ok)->setFocus();
connect(m_ui.buttonBox, SIGNAL(rejected()), SLOT(close()));
connect(m_ui.buttonBox, SIGNAL(rejected()), SLOT(abortMerge()));
connect(m_ui.buttonBox, SIGNAL(accepted()), SLOT(performMerge()));
setupChangeTable();
@ -47,9 +46,32 @@ MergeDialog::MergeDialog(QSharedPointer<Database> source, QSharedPointer<Databas
setWindowModality(Qt::WindowModality::ApplicationModal);
}
MergeDialog::MergeDialog(const Merger::ChangeList& changes, QWidget* parent)
: QDialog(parent)
, m_changes(changes)
{
setAttribute(Qt::WA_DeleteOnClose);
m_ui.setupUi(this);
m_ui.buttonBox->button(QDialogButtonBox::Ok)->setFocus();
m_ui.buttonBox->button(QDialogButtonBox::Abort)->hide();
connect(m_ui.buttonBox, SIGNAL(accepted()), SLOT(close()));
setupChangeTable();
}
void MergeDialog::setupChangeTable()
{
auto changeList = m_merger.changes();
if (m_changes.isEmpty()) {
// deep copy of root group with same uuid
auto tmpRootGroup = m_targetDatabase->rootGroup()->clone(Entry::CloneFlag::CloneIncludeHistory,
Group::CloneFlag::CloneIncludeEntries);
Database tmpDatabase;
tmpDatabase.setRootGroup(tmpRootGroup);
m_changes = Merger(m_sourceDatabase.data(), &tmpDatabase).merge();
}
auto* table = m_ui.changeTable;
auto columns = QVector<QPair<QString, std::function<QString(const Merger::Change&)>>>{
@ -65,13 +87,13 @@ void MergeDialog::setupChangeTable()
qMakePair(tr("Type of change"), [](const auto& change) { return change.typeString(); }),
qMakePair(tr("Details"), [](const auto& change) { return change.details(); })};
table->setColumnCount(columns.size());
table->setRowCount(changeList.size());
table->setRowCount(m_changes.size());
for (int column = 0; column < columns.size(); ++column) {
const auto& columnName = columns[column].first;
table->setHorizontalHeaderItem(column, new QTableWidgetItem(columnName));
}
for (int row = 0; row < changeList.size(); ++row) {
const auto& change = changeList[row];
for (int row = 0; row < m_changes.size(); ++row) {
const auto& change = m_changes[row];
for (int column = 0; column < columns.size(); ++column) {
auto changeMember = columns[column].second;
table->setItem(row, column, new QTableWidgetItem(changeMember(change)));
@ -91,7 +113,16 @@ void MergeDialog::setupChangeTable()
void MergeDialog::performMerge()
{
auto changelist = m_merger.merge(false);
emit databaseMerged(!changelist.isEmpty());
auto changes = Merger(m_sourceDatabase.data(), m_targetDatabase.data()).merge();
if (changes != m_changes) {
emit databaseModifiedMerge(changes, m_changes);
} else {
emit databaseMerged(!changes.isEmpty());
}
done(QDialog::Accepted);
}
void MergeDialog::abortMerge()
{
done(QDialog::Rejected);
}

View File

@ -31,13 +31,23 @@ class MergeDialog : public QDialog
Q_OBJECT
public:
/**
* Merge source into copy of target and display changes.
* On user confirmation, merge source into target.
*/
explicit MergeDialog(QSharedPointer<Database> source, QSharedPointer<Database> target, QWidget* parent = nullptr);
/**
* Display given changes.
*/
explicit MergeDialog(const Merger::ChangeList& changes, QWidget* parent = nullptr);
Q_SIGNALS:
void databaseMerged(bool databaseChanged);
void databaseModifiedMerge(const Merger::ChangeList& actualChanges, const Merger::ChangeList& expectedChanges);
private Q_SLOTS:
void performMerge();
void abortMerge();
private:
void setupChangeTable();
@ -45,7 +55,7 @@ private:
private:
Ui::MergeDialog m_ui;
Merger m_merger;
Merger::ChangeList m_changes;
QSharedPointer<Database> m_sourceDatabase;
QSharedPointer<Database> m_targetDatabase;
};

View File

@ -1110,7 +1110,7 @@ void TestMerge::testCustomData()
m_clock->advanceSecond(1);
Merger merger(dbSource.data(), dbDestination.data());
QStringList changes = merger.merge();
auto changes = merger.merge();
QVERIFY(!changes.isEmpty());
@ -1131,7 +1131,7 @@ void TestMerge::testCustomData()
dbSource->metadata()->customData()->set("key3", "oldValue");
dbSource->metadata()->customData()->set("key3", "newValue");
Merger merger2(dbSource.data(), dbDestination.data());
QStringList changes2 = merger2.merge();
auto changes2 = merger2.merge();
QVERIFY(changes2.isEmpty());
Merger merger3(dbSource2.data(), dbDestination2.data());