From 81a66c439ccacf831e7f4e0cc86d72f950e0e1fc Mon Sep 17 00:00:00 2001 From: Aetf Date: Thu, 27 May 2021 21:50:15 -0400 Subject: [PATCH] Properly block modified signal during Database destruction (#6438) fixes #6393 --- src/CMakeLists.txt | 1 + src/core/AutoTypeAssociations.cpp | 10 +-- src/core/AutoTypeAssociations.h | 5 +- src/core/CustomData.cpp | 12 ++-- src/core/CustomData.h | 6 +- src/core/Database.cpp | 48 +++++++------- src/core/Database.h | 6 +- src/core/Entry.cpp | 32 ++++----- src/core/Entry.h | 4 +- src/core/EntryAttachments.cpp | 18 ++--- src/core/EntryAttachments.h | 5 +- src/core/EntryAttributes.cpp | 26 ++++---- src/core/EntryAttributes.h | 5 +- src/core/Group.cpp | 50 +++++++------- src/core/Group.h | 4 +- src/core/Metadata.cpp | 12 ++-- src/core/Metadata.h | 6 +- src/core/ModifiableObject.cpp | 65 +++++++++++++++++++ src/core/ModifiableObject.h | 59 +++++++++++++++++ src/fdosecrets/objects/Collection.cpp | 10 +-- src/fdosecrets/objects/Item.cpp | 2 +- src/fdosecrets/objects/Service.cpp | 11 ++-- src/gui/DatabaseWidget.cpp | 22 ++++--- src/gui/DatabaseWidget.h | 2 +- src/gui/EditWidgetProperties.cpp | 2 +- src/gui/entry/EditEntryWidget.cpp | 30 +++++---- src/gui/group/EditGroupWidget.cpp | 2 +- src/keeshare/ShareObserver.cpp | 12 ++-- .../group/EditGroupWidgetKeeShare.cpp | 2 +- tests/TestDatabase.cpp | 8 +-- tests/TestGroup.cpp | 2 +- tests/TestMerge.cpp | 4 +- tests/TestModified.cpp | 65 +++++++++++++++++-- tests/TestModified.h | 1 + 34 files changed, 370 insertions(+), 179 deletions(-) create mode 100644 src/core/ModifiableObject.cpp create mode 100644 src/core/ModifiableObject.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index eba85a947..c4b09e160 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -52,6 +52,7 @@ set(keepassx_SOURCES core/InactivityTimer.cpp core/Merger.cpp core/Metadata.cpp + core/ModifiableObject.cpp core/PasswordGenerator.cpp core/PasswordHealth.cpp core/PassphraseGenerator.cpp diff --git a/src/core/AutoTypeAssociations.cpp b/src/core/AutoTypeAssociations.cpp index a9ecc0db1..e3fcca04a 100644 --- a/src/core/AutoTypeAssociations.cpp +++ b/src/core/AutoTypeAssociations.cpp @@ -28,7 +28,7 @@ bool AutoTypeAssociations::Association::operator!=(const AutoTypeAssociations::A } AutoTypeAssociations::AutoTypeAssociations(QObject* parent) - : QObject(parent) + : ModifiableObject(parent) { } @@ -41,7 +41,7 @@ void AutoTypeAssociations::copyDataFrom(const AutoTypeAssociations* other) emit aboutToReset(); m_associations = other->m_associations; emit reset(); - emit modified(); + emitModified(); } void AutoTypeAssociations::add(const AutoTypeAssociations::Association& association) @@ -50,7 +50,7 @@ void AutoTypeAssociations::add(const AutoTypeAssociations::Association& associat emit aboutToAdd(index); m_associations.append(association); emit added(index); - emit modified(); + emitModified(); } void AutoTypeAssociations::remove(int index) @@ -60,7 +60,7 @@ void AutoTypeAssociations::remove(int index) emit aboutToRemove(index); m_associations.removeAt(index); emit removed(index); - emit modified(); + emitModified(); } void AutoTypeAssociations::removeEmpty() @@ -81,7 +81,7 @@ void AutoTypeAssociations::update(int index, const AutoTypeAssociations::Associa if (m_associations.at(index) != association) { m_associations[index] = association; emit dataChanged(index); - emit modified(); + emitModified(); } } diff --git a/src/core/AutoTypeAssociations.h b/src/core/AutoTypeAssociations.h index 17d5c3bcd..4457eafdf 100644 --- a/src/core/AutoTypeAssociations.h +++ b/src/core/AutoTypeAssociations.h @@ -18,9 +18,9 @@ #ifndef KEEPASSX_AUTOTYPEASSOCIATIONS_H #define KEEPASSX_AUTOTYPEASSOCIATIONS_H -#include +#include "core/ModifiableObject.h" -class AutoTypeAssociations : public QObject +class AutoTypeAssociations : public ModifiableObject { Q_OBJECT @@ -53,7 +53,6 @@ private: QList m_associations; signals: - void modified(); void dataChanged(int index); void aboutToAdd(int index); void added(int index); diff --git a/src/core/CustomData.cpp b/src/core/CustomData.cpp index bb287ec45..6e1dbcb70 100644 --- a/src/core/CustomData.cpp +++ b/src/core/CustomData.cpp @@ -27,7 +27,7 @@ const QString CustomData::BrowserLegacyKeyPrefix = QStringLiteral("Public Key: " const QString CustomData::ExcludeFromReports = QStringLiteral("KnownBad"); CustomData::CustomData(QObject* parent) - : QObject(parent) + : ModifiableObject(parent) { } @@ -68,7 +68,7 @@ void CustomData::set(const QString& key, const QString& value) if (addAttribute || changeValue) { m_data.insert(key, value); updateLastModified(); - emit customDataModified(); + emitModified(); } if (addAttribute) { @@ -84,7 +84,7 @@ void CustomData::remove(const QString& key) updateLastModified(); emit removed(key); - emit customDataModified(); + emitModified(); } void CustomData::rename(const QString& oldKey, const QString& newKey) @@ -104,7 +104,7 @@ void CustomData::rename(const QString& oldKey, const QString& newKey) m_data.insert(newKey, data); updateLastModified(); - emit customDataModified(); + emitModified(); emit renamed(oldKey, newKey); } @@ -120,7 +120,7 @@ void CustomData::copyDataFrom(const CustomData* other) updateLastModified(); emit reset(); - emit customDataModified(); + emitModified(); } QDateTime CustomData::getLastModified() const @@ -153,7 +153,7 @@ void CustomData::clear() m_data.clear(); emit reset(); - emit customDataModified(); + emitModified(); } bool CustomData::isEmpty() const diff --git a/src/core/CustomData.h b/src/core/CustomData.h index aeaa136ef..3879daffc 100644 --- a/src/core/CustomData.h +++ b/src/core/CustomData.h @@ -23,7 +23,9 @@ #include #include -class CustomData : public QObject +#include "core/ModifiableObject.h" + +class CustomData : public ModifiableObject { Q_OBJECT @@ -54,7 +56,6 @@ public: static const QString ExcludeFromReports; signals: - void customDataModified(); void aboutToBeAdded(const QString& key); void added(const QString& key); void aboutToBeRemoved(const QString& key); @@ -63,7 +64,6 @@ signals: void renamed(const QString& oldKey, const QString& newKey); void aboutToBeReset(); void reset(); - void lastModified(); private slots: void updateLastModified(); diff --git a/src/core/Database.cpp b/src/core/Database.cpp index b5222b559..891e4c755 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -44,24 +44,35 @@ Database::Database() , m_data() , m_rootGroup(nullptr) , m_fileWatcher(new FileWatcher(this)) - , m_emitModified(false) , m_uuid(QUuid::createUuid()) { + // setup modified timer + m_modifiedTimer.setSingleShot(true); + connect(this, &Database::emitModifiedChanged, this, [this](bool value) { + if (!value) { + stopModifiedTimer(); + } + }); + connect(&m_modifiedTimer, &QTimer::timeout, this, &Database::emitModified); + + // other signals + connect(m_metadata, &Metadata::modified, this, &Database::markAsModified); + connect(this, &Database::databaseOpened, this, [this]() { updateCommonUsernames(); }); + connect(this, &Database::databaseSaved, this, [this]() { updateCommonUsernames(); }); + connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged); + + // static uuid map + s_uuidMap.insert(m_uuid, this); + + // block modified signal and set root group + setEmitModified(false); + setRootGroup(new Group()); rootGroup()->setUuid(QUuid::createUuid()); rootGroup()->setName(tr("Passwords", "Root group name")); - m_modifiedTimer.setSingleShot(true); - - s_uuidMap.insert(m_uuid, this); - - connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified())); - connect(&m_modifiedTimer, SIGNAL(timeout()), SIGNAL(databaseModified())); - connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames())); - connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames())); - connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged); m_modified = false; - m_emitModified = true; + setEmitModified(true); } Database::Database(const QString& filePath) @@ -431,7 +442,6 @@ void Database::releaseData() setEmitModified(false); m_modified = false; - stopModifiedTimer(); s_uuidMap.remove(m_uuid); m_uuid = QUuid(); @@ -439,7 +449,10 @@ void Database::releaseData() m_data.clear(); m_metadata->clear(); + auto oldGroup = rootGroup(); setRootGroup(new Group()); + // explicitly delete old group, otherwise it is only deleted when the database object is destructed + delete oldGroup; m_fileWatcher->stop(); @@ -840,15 +853,6 @@ void Database::emptyRecycleBin() } } -void Database::setEmitModified(bool value) -{ - if (m_emitModified && !value) { - stopModifiedTimer(); - } - - m_emitModified = value; -} - bool Database::isModified() const { return m_modified; @@ -862,7 +866,7 @@ bool Database::hasNonDataChanges() const void Database::markAsModified() { m_modified = true; - if (m_emitModified && !m_modifiedTimer.isActive()) { + if (!m_modifiedTimer.isActive()) { // Small time delay prevents numerous consecutive saves due to repeated signals startModifiedTimer(); } diff --git a/src/core/Database.h b/src/core/Database.h index fa5c145cb..d51b4f230 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -27,6 +27,7 @@ #include #include "config-keepassx.h" +#include "core/ModifiableObject.h" #include "crypto/kdf/AesKdf.h" #include "crypto/kdf/Kdf.h" #include "format/KeePass2.h" @@ -52,7 +53,7 @@ struct DeletedObject Q_DECLARE_TYPEINFO(DeletedObject, Q_MOVABLE_TYPE); -class Database : public QObject +class Database : public ModifiableObject { Q_OBJECT @@ -83,7 +84,6 @@ public: bool isInitialized() const; bool isModified() const; bool hasNonDataChanges() const; - void setEmitModified(bool value); bool isReadOnly() const; void setReadOnly(bool readOnly); bool isSaving(); @@ -151,7 +151,6 @@ signals: void groupAboutToMove(Group* group, Group* toGroup, int index); void groupMoved(); void databaseOpened(); - void databaseModified(); void databaseSaved(); void databaseDiscarded(); void databaseFileChanged(); @@ -213,7 +212,6 @@ private: QMutex m_saveMutex; QPointer m_fileWatcher; bool m_modified = false; - bool m_emitModified; bool m_hasNonDataChange = false; QString m_keyError; diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 49beb1e9e..c56c5f74e 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -47,15 +47,15 @@ Entry::Entry() m_data.autoTypeEnabled = true; m_data.autoTypeObfuscation = 0; - connect(m_attributes, SIGNAL(entryAttributesModified()), SLOT(updateTotp())); - connect(m_attributes, SIGNAL(entryAttributesModified()), this, SIGNAL(entryModified())); - connect(m_attributes, SIGNAL(defaultKeyModified()), SLOT(emitDataChanged())); - connect(m_attachments, SIGNAL(entryAttachmentsModified()), this, SIGNAL(entryModified())); - connect(m_autoTypeAssociations, SIGNAL(modified()), SIGNAL(entryModified())); - connect(m_customData, SIGNAL(customDataModified()), this, SIGNAL(entryModified())); + connect(m_attributes, &EntryAttributes::modified, this, &Entry::updateTotp); + connect(m_attributes, &EntryAttributes::modified, this, &Entry::modified); + connect(m_attributes, &EntryAttributes::defaultKeyModified, this, &Entry::emitDataChanged); + connect(m_attachments, &EntryAttachments::modified, this, &Entry::modified); + connect(m_autoTypeAssociations, &AutoTypeAssociations::modified, this, &Entry::modified); + connect(m_customData, &CustomData::modified, this, &Entry::modified); - connect(this, SIGNAL(entryModified()), SLOT(updateTimeinfo())); - connect(this, SIGNAL(entryModified()), SLOT(updateModifiedSinceBegin())); + connect(this, &Entry::modified, this, &Entry::updateTimeinfo); + connect(this, &Entry::modified, this, &Entry::updateModifiedSinceBegin); } Entry::~Entry() @@ -76,7 +76,7 @@ template inline bool Entry::set(T& property, const T& value) { if (property != value) { property = value; - emit entryModified(); + emitModified(); return true; } return false; @@ -609,7 +609,7 @@ void Entry::setIcon(int iconNumber) m_data.iconNumber = iconNumber; m_data.customIcon = QUuid(); - emit entryModified(); + emitModified(); emitDataChanged(); } } @@ -622,7 +622,7 @@ void Entry::setIcon(const QUuid& uuid) m_data.customIcon = uuid; m_data.iconNumber = 0; - emit entryModified(); + emitModified(); emitDataChanged(); } } @@ -715,7 +715,7 @@ void Entry::setExpires(const bool& value) { if (m_data.timeInfo.expires() != value) { m_data.timeInfo.setExpires(value); - emit entryModified(); + emitModified(); } } @@ -723,7 +723,7 @@ void Entry::setExpiryTime(const QDateTime& dateTime) { if (m_data.timeInfo.expiryTime() != dateTime) { m_data.timeInfo.setExpiryTime(dateTime); - emit entryModified(); + emitModified(); } } @@ -742,7 +742,7 @@ void Entry::addHistoryItem(Entry* entry) Q_ASSERT(!entry->parent()); m_history.append(entry); - emit entryModified(); + emitModified(); } void Entry::removeHistoryItems(const QList& historyEntries) @@ -760,7 +760,7 @@ void Entry::removeHistoryItems(const QList& historyEntries) delete entry; } - emit entryModified(); + emitModified(); } void Entry::truncateHistory() @@ -813,7 +813,7 @@ void Entry::truncateHistory() } if (changed) { - emit entryModified(); + emitModified(); } } diff --git a/src/core/Entry.h b/src/core/Entry.h index 88a89761a..9dd58e5a7 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -32,6 +32,7 @@ #include "core/EntryAttachments.h" #include "core/EntryAttributes.h" #include "core/Global.h" +#include "core/ModifiableObject.h" #include "core/TimeInfo.h" class Database; @@ -75,7 +76,7 @@ struct EntryData bool equals(const EntryData& other, CompareItemOptions options) const; }; -class Entry : public QObject +class Entry : public ModifiableObject { Q_OBJECT @@ -260,7 +261,6 @@ signals: * Emitted when a default attribute has been changed. */ void entryDataChanged(Entry* entry); - void entryModified(); private slots: void emitDataChanged(); diff --git a/src/core/EntryAttachments.cpp b/src/core/EntryAttachments.cpp index 23aef01f9..fea7a1bd4 100644 --- a/src/core/EntryAttachments.cpp +++ b/src/core/EntryAttachments.cpp @@ -23,7 +23,7 @@ #include EntryAttachments::EntryAttachments(QObject* parent) - : QObject(parent) + : ModifiableObject(parent) { } @@ -49,7 +49,7 @@ QByteArray EntryAttachments::value(const QString& key) const void EntryAttachments::set(const QString& key, const QByteArray& value) { - bool emitModified = false; + bool shouldEmitModified = false; bool addAttachment = !m_attachments.contains(key); if (addAttachment) { @@ -58,7 +58,7 @@ void EntryAttachments::set(const QString& key, const QByteArray& value) if (addAttachment || m_attachments.value(key) != value) { m_attachments.insert(key, value); - emitModified = true; + shouldEmitModified = true; } if (addAttachment) { @@ -67,8 +67,8 @@ void EntryAttachments::set(const QString& key, const QByteArray& value) emit keyModified(key); } - if (emitModified) { - emit entryAttachmentsModified(); + if (shouldEmitModified) { + emitModified(); } } @@ -84,7 +84,7 @@ void EntryAttachments::remove(const QString& key) m_attachments.remove(key); emit removed(key); - emit entryAttachmentsModified(); + emitModified(); } void EntryAttachments::remove(const QStringList& keys) @@ -108,7 +108,7 @@ void EntryAttachments::remove(const QStringList& keys) } if (isModified) { - emit entryAttachmentsModified(); + emitModified(); } } @@ -135,7 +135,7 @@ void EntryAttachments::clear() m_attachments.clear(); emit reset(); - emit entryAttachmentsModified(); + emitModified(); } void EntryAttachments::copyDataFrom(const EntryAttachments* other) @@ -146,7 +146,7 @@ void EntryAttachments::copyDataFrom(const EntryAttachments* other) m_attachments = other->m_attachments; emit reset(); - emit entryAttachmentsModified(); + emitModified(); } } diff --git a/src/core/EntryAttachments.h b/src/core/EntryAttachments.h index 923376f3b..d23681b90 100644 --- a/src/core/EntryAttachments.h +++ b/src/core/EntryAttachments.h @@ -21,9 +21,11 @@ #include #include +#include "core/ModifiableObject.h" + class QStringList; -class EntryAttachments : public QObject +class EntryAttachments : public ModifiableObject { Q_OBJECT @@ -45,7 +47,6 @@ public: int attachmentsSize() const; signals: - void entryAttachmentsModified(); void keyModified(const QString& key); void aboutToBeAdded(const QString& key); void added(const QString& key); diff --git a/src/core/EntryAttributes.cpp b/src/core/EntryAttributes.cpp index 80067c563..aaa89d7d2 100644 --- a/src/core/EntryAttributes.cpp +++ b/src/core/EntryAttributes.cpp @@ -35,7 +35,7 @@ const QString EntryAttributes::SearchTextGroupName = "SearchText"; const QString EntryAttributes::RememberCmdExecAttr = "_EXEC_CMD"; EntryAttributes::EntryAttributes(QObject* parent) - : QObject(parent) + : ModifiableObject(parent) { clear(); } @@ -104,7 +104,7 @@ bool EntryAttributes::isReference(const QString& key) const void EntryAttributes::set(const QString& key, const QString& value, bool protect) { - bool emitModified = false; + bool shouldEmitModified = false; bool addAttribute = !m_attributes.contains(key); bool changeValue = !addAttribute && (m_attributes.value(key) != value); @@ -116,27 +116,27 @@ void EntryAttributes::set(const QString& key, const QString& value, bool protect if (addAttribute || changeValue) { m_attributes.insert(key, value); - emitModified = true; + shouldEmitModified = true; } if (protect) { if (!m_protectedAttributes.contains(key)) { - emitModified = true; + shouldEmitModified = true; } m_protectedAttributes.insert(key); } else if (m_protectedAttributes.remove(key)) { - emitModified = true; + shouldEmitModified = true; } - if (emitModified) { - emit entryAttributesModified(); + if (shouldEmitModified) { + emitModified(); } if (defaultAttribute && changeValue) { emit defaultKeyModified(); } else if (addAttribute) { emit added(key); - } else if (emitModified) { + } else if (shouldEmitModified) { emit customKeyModified(key); } } @@ -155,7 +155,7 @@ void EntryAttributes::remove(const QString& key) m_protectedAttributes.remove(key); emit removed(key); - emit entryAttributesModified(); + emitModified(); } void EntryAttributes::rename(const QString& oldKey, const QString& newKey) @@ -185,7 +185,7 @@ void EntryAttributes::rename(const QString& oldKey, const QString& newKey) m_protectedAttributes.insert(newKey); } - emit entryAttributesModified(); + emitModified(); emit renamed(oldKey, newKey); } @@ -217,7 +217,7 @@ void EntryAttributes::copyCustomKeysFrom(const EntryAttributes* other) } emit reset(); - emit entryAttributesModified(); + emitModified(); } bool EntryAttributes::areCustomKeysDifferent(const EntryAttributes* other) @@ -250,7 +250,7 @@ void EntryAttributes::copyDataFrom(const EntryAttributes* other) m_protectedAttributes = other->m_protectedAttributes; emit reset(); - emit entryAttributesModified(); + emitModified(); } } @@ -303,7 +303,7 @@ void EntryAttributes::clear() } emit reset(); - emit entryAttributesModified(); + emitModified(); } int EntryAttributes::attributesSize() const diff --git a/src/core/EntryAttributes.h b/src/core/EntryAttributes.h index 2cba13c64..ee73f89b3 100644 --- a/src/core/EntryAttributes.h +++ b/src/core/EntryAttributes.h @@ -26,7 +26,9 @@ #include #include -class EntryAttributes : public QObject +#include "core/ModifiableObject.h" + +class EntryAttributes : public ModifiableObject { Q_OBJECT @@ -69,7 +71,6 @@ public: static const QString SearchTextGroupName; signals: - void entryAttributesModified(); void defaultKeyModified(); void customKeyModified(const QString& key); void aboutToBeAdded(const QString& key); diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 45e44bff0..0564df82c 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -46,9 +46,9 @@ Group::Group() m_data.searchingEnabled = Inherit; m_data.mergeMode = Default; - connect(m_customData, SIGNAL(customDataModified()), this, SIGNAL(groupModified())); - connect(this, SIGNAL(groupModified()), SLOT(updateTimeinfo())); - connect(this, SIGNAL(groupNonDataChange()), SLOT(updateTimeinfo())); + connect(m_customData, &CustomData::modified, this, &Group::modified); + connect(this, &Group::modified, this, &Group::updateTimeinfo); + connect(this, &Group::groupNonDataChange, this, &Group::updateTimeinfo); } Group::~Group() @@ -80,7 +80,7 @@ template inline bool Group::set(P& property, const V& value) { if (property != value) { property = value; - emit groupModified(); + emitModified(); return true; } else { return false; @@ -333,7 +333,7 @@ void Group::setIcon(int iconNumber) if (iconNumber >= 0 && (m_data.iconNumber != iconNumber || !m_data.customIcon.isNull())) { m_data.iconNumber = iconNumber; m_data.customIcon = QUuid(); - emit groupModified(); + emitModified(); emit groupDataChanged(this); } } @@ -343,7 +343,7 @@ void Group::setIcon(const QUuid& uuid) if (!uuid.isNull() && m_data.customIcon != uuid) { m_data.customIcon = uuid; m_data.iconNumber = 0; - emit groupModified(); + emitModified(); emit groupDataChanged(this); } } @@ -385,7 +385,7 @@ void Group::setExpires(bool value) { if (m_data.timeInfo.expires() != value) { m_data.timeInfo.setExpires(value); - emit groupModified(); + emitModified(); } } @@ -393,7 +393,7 @@ void Group::setExpiryTime(const QDateTime& dateTime) { if (m_data.timeInfo.expiryTime() != dateTime) { m_data.timeInfo.setExpiryTime(dateTime); - emit groupModified(); + emitModified(); } } @@ -465,7 +465,7 @@ void Group::setParent(Group* parent, int index) m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc()); } - emit groupModified(); + emitModified(); if (!moveWithinDatabase) { emit groupAdded(); @@ -927,12 +927,12 @@ void Group::addEntry(Entry* entry) emit entryAboutToAdd(entry); m_entries << entry; - connect(entry, SIGNAL(entryDataChanged(Entry*)), SIGNAL(entryDataChanged(Entry*))); + connect(entry, &Entry::entryDataChanged, this, &Group::entryDataChanged); if (m_db) { - connect(entry, SIGNAL(entryModified()), m_db, SLOT(markAsModified())); + connect(entry, &Entry::modified, m_db, &Database::markAsModified); } - emit groupModified(); + emitModified(); emit entryAdded(entry); } @@ -949,7 +949,7 @@ void Group::removeEntry(Entry* entry) entry->disconnect(m_db); } m_entries.removeAll(entry); - emit groupModified(); + emitModified(); emit entryRemoved(entry); } @@ -990,21 +990,21 @@ void Group::connectDatabaseSignalsRecursive(Database* db) entry->disconnect(m_db); } if (db) { - connect(entry, SIGNAL(entryModified()), db, SLOT(markAsModified())); + connect(entry, &Entry::modified, db, &Database::markAsModified); } } if (db) { // clang-format off - connect(this, SIGNAL(groupDataChanged(Group*)), db, SIGNAL(groupDataChanged(Group*))); - connect(this, SIGNAL(groupAboutToRemove(Group*)), db, SIGNAL(groupAboutToRemove(Group*))); - connect(this, SIGNAL(groupRemoved()), db, SIGNAL(groupRemoved())); - connect(this, SIGNAL(groupAboutToAdd(Group*, int)), db, SIGNAL(groupAboutToAdd(Group*,int))); - connect(this, SIGNAL(groupAdded()), db, SIGNAL(groupAdded())); - connect(this, SIGNAL(aboutToMove(Group*,Group*,int)), db, SIGNAL(groupAboutToMove(Group*,Group*,int))); - connect(this, SIGNAL(groupMoved()), db, SIGNAL(groupMoved())); - connect(this, SIGNAL(groupModified()), db, SLOT(markAsModified())); - connect(this, SIGNAL(groupNonDataChange()), db, SLOT(markNonDataChange())); + connect(this, &Group::groupDataChanged, db, &Database::groupDataChanged); + connect(this, &Group::groupAboutToRemove, db, &Database::groupAboutToRemove); + connect(this, &Group::groupRemoved, db, &Database::groupRemoved); + connect(this, &Group::groupAboutToAdd, db, &Database::groupAboutToAdd); + connect(this, &Group::groupAdded, db, &Database::groupAdded); + connect(this, &Group::aboutToMove, db, &Database::groupAboutToMove); + connect(this, &Group::groupMoved, db, &Database::groupMoved); + connect(this, &Group::groupNonDataChange, db, &Database::markNonDataChange); + connect(this, &Group::modified, db, &Database::markAsModified); // clang-format on } @@ -1020,7 +1020,7 @@ void Group::cleanupParent() if (m_parent) { emit groupAboutToRemove(this); m_parent->m_children.removeAll(this); - emit groupModified(); + emitModified(); emit groupRemoved(); } } @@ -1189,7 +1189,7 @@ void Group::sortChildrenRecursively(bool reverse) child->sortChildrenRecursively(reverse); } - emit groupModified(); + emitModified(); } bool Group::GroupData::operator==(const Group::GroupData& other) const diff --git a/src/core/Group.h b/src/core/Group.h index 2997abe17..ce32cdc16 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -27,9 +27,10 @@ #include "core/Database.h" #include "core/Entry.h" #include "core/Global.h" +#include "core/ModifiableObject.h" #include "core/TimeInfo.h" -class Group : public QObject +class Group : public ModifiableObject { Q_OBJECT @@ -184,7 +185,6 @@ signals: void groupRemoved(); void aboutToMove(Group* group, Group* toGroup, int index); void groupMoved(); - void groupModified(); void groupNonDataChange(); void entryAboutToAdd(Entry* entry); void entryAdded(Entry* entry); diff --git a/src/core/Metadata.cpp b/src/core/Metadata.cpp index 859c5a491..65adba49a 100644 --- a/src/core/Metadata.cpp +++ b/src/core/Metadata.cpp @@ -29,12 +29,12 @@ const int Metadata::DefaultHistoryMaxItems = 10; const int Metadata::DefaultHistoryMaxSize = 6 * 1024 * 1024; Metadata::Metadata(QObject* parent) - : QObject(parent) + : ModifiableObject(parent) , m_customData(new CustomData(this)) , m_updateDatetime(true) { init(); - connect(m_customData, SIGNAL(customDataModified()), SIGNAL(metadataModified())); + connect(m_customData, &CustomData::modified, this, &Metadata::modified); } void Metadata::init() @@ -76,7 +76,7 @@ template bool Metadata::set(P& property, const V& value) { if (property != value) { property = value; - emit metadataModified(); + emitModified(); return true; } else { return false; @@ -90,7 +90,7 @@ template bool Metadata::set(P& property, const V& value, QDat if (m_updateDatetime) { dateTime = Clock::currentDateTimeUtc(); } - emit metadataModified(); + emitModified(); return true; } else { return false; @@ -386,7 +386,7 @@ void Metadata::addCustomIcon(const QUuid& uuid, const QImage& image) m_customIcons.insert(uuid, QIcon()); } - emit metadataModified(); + emitModified(); } void Metadata::removeCustomIcon(const QUuid& uuid) @@ -404,7 +404,7 @@ void Metadata::removeCustomIcon(const QUuid& uuid) m_customIconsRaw.remove(uuid); m_customIconsOrder.removeAll(uuid); Q_ASSERT(m_customIconsRaw.count() == m_customIconsOrder.count()); - emit metadataModified(); + emitModified(); } QUuid Metadata::findCustomIcon(const QImage& candidate) diff --git a/src/core/Metadata.h b/src/core/Metadata.h index dc09b3cca..d833a0ae8 100644 --- a/src/core/Metadata.h +++ b/src/core/Metadata.h @@ -30,11 +30,12 @@ #include "core/CustomData.h" #include "core/Global.h" +#include "core/ModifiableObject.h" class Database; class Group; -class Metadata : public QObject +class Metadata : public ModifiableObject { Q_OBJECT @@ -149,9 +150,6 @@ public: */ void copyAttributesFrom(const Metadata* other); -signals: - void metadataModified(); - private: template bool set(P& property, const V& value); template bool set(P& property, const V& value, QDateTime& dateTime); diff --git a/src/core/ModifiableObject.cpp b/src/core/ModifiableObject.cpp new file mode 100644 index 000000000..bb663adcc --- /dev/null +++ b/src/core/ModifiableObject.cpp @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2021 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "ModifiableObject.h" + +namespace +{ + template T findParent(const QObject* obj) + { + if (!obj) { + return {}; + } + + auto p = obj->parent(); + while (p) { + auto casted = qobject_cast(p); + if (casted) { + return casted; + } + p = p->parent(); + } + return {}; + } +} // namespace + +bool ModifiableObject::modifiedSignalEnabled() const +{ + auto p = this; + while (p) { + if (!p->m_emitModified) { + return false; + } + p = findParent(p); + } + return true; +} + +void ModifiableObject::setEmitModified(bool value) +{ + if (m_emitModified != value) { + m_emitModified = value; + emit emitModifiedChanged(m_emitModified); + } +} + +void ModifiableObject::emitModified() +{ + if (modifiedSignalEnabled()) { + emit modified(); + } +} diff --git a/src/core/ModifiableObject.h b/src/core/ModifiableObject.h new file mode 100644 index 000000000..f51d02563 --- /dev/null +++ b/src/core/ModifiableObject.h @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2021 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSXC_MODIFIABLEOBJECT_H +#define KEEPASSXC_MODIFIABLEOBJECT_H + +#include + +class ModifiableObject : public QObject +{ + Q_OBJECT +public: + using QObject::QObject; + +public: + /** + * @brief check if the modified signal is enabled. + * Note that this is NOT the same as m_emitModified. + * The signal is enabled if neither the current object nor any of its parents disabled the signal. + */ + bool modifiedSignalEnabled() const; + +public slots: + /** + * @brief set whether the modified signal should be emitted from this object and all its children. + * + * This means that even after calling `setEmitModified(true)` on this object, + * the signal may still be blocked in one of its parents. + * + * @param value + */ + void setEmitModified(bool value); + +protected: + void emitModified(); + +signals: + void modified(); + void emitModifiedChanged(bool value); + +private: + bool m_emitModified{true}; +}; + +#endif // KEEPASSXC_MODIFIABLEOBJECT_H diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp index f4341ef81..7b86e0d9f 100644 --- a/src/fdosecrets/objects/Collection.cpp +++ b/src/fdosecrets/objects/Collection.cpp @@ -475,7 +475,7 @@ namespace FdoSecrets } }); // Another possibility is the group being moved to recycle bin. - connect(m_exposedGroup.data(), &Group::groupModified, this, [this]() { + connect(m_exposedGroup.data(), &Group::modified, this, [this]() { if (inRecycleBin(m_exposedGroup->parentGroup())) { // reset the exposed group to none FdoSecrets::settings()->setExposedGroup(m_backend->database().data(), {}); @@ -483,7 +483,7 @@ namespace FdoSecrets }); // Monitor exposed group settings - connect(m_backend->database()->metadata()->customData(), &CustomData::customDataModified, this, [this]() { + connect(m_backend->database()->metadata()->customData(), &CustomData::modified, this, [this]() { if (!m_exposedGroup || backendLocked()) { return; } @@ -500,8 +500,8 @@ namespace FdoSecrets onEntryAdded(entry, false); } - // Do not connect to databaseModified signal because we only want signals for the subset under m_exposedGroup - connect(m_backend->database()->metadata(), &Metadata::metadataModified, this, &Collection::collectionChanged); + // Do not connect to Database::modified signal because we only want signals for the subset under m_exposedGroup + connect(m_backend->database()->metadata(), &Metadata::modified, this, &Collection::collectionChanged); connectGroupSignalRecursive(m_exposedGroup); } @@ -556,7 +556,7 @@ namespace FdoSecrets return; } - connect(group, &Group::groupModified, this, &Collection::collectionChanged); + connect(group, &Group::modified, this, &Collection::collectionChanged); connect(group, &Group::entryAdded, this, [this](Entry* entry) { onEntryAdded(entry, true); }); const auto children = group->children(); diff --git a/src/fdosecrets/objects/Item.cpp b/src/fdosecrets/objects/Item.cpp index b79649379..56132e721 100644 --- a/src/fdosecrets/objects/Item.cpp +++ b/src/fdosecrets/objects/Item.cpp @@ -63,7 +63,7 @@ namespace FdoSecrets : DBusObject(parent) , m_backend(backend) { - connect(m_backend.data(), &Entry::entryModified, this, &Item::itemChanged); + connect(m_backend, &Entry::modified, this, &Item::itemChanged); } DBusResult Item::locked(const DBusClientPtr& client, bool& locked) const diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp index 603aaba1e..1751efdb2 100644 --- a/src/fdosecrets/objects/Service.cpp +++ b/src/fdosecrets/objects/Service.cpp @@ -157,12 +157,11 @@ namespace FdoSecrets void Service::monitorDatabaseExposedGroup(DatabaseWidget* dbWidget) { Q_ASSERT(dbWidget); - connect( - dbWidget->database()->metadata()->customData(), &CustomData::customDataModified, this, [this, dbWidget]() { - if (!FdoSecrets::settings()->exposedGroup(dbWidget->database()).isNull() && !findCollection(dbWidget)) { - onDatabaseTabOpened(dbWidget, true); - } - }); + connect(dbWidget->database()->metadata()->customData(), &CustomData::modified, this, [this, dbWidget]() { + if (!FdoSecrets::settings()->exposedGroup(dbWidget->database()).isNull() && !findCollection(dbWidget)) { + onDatabaseTabOpened(dbWidget, true); + } + }); } void Service::ensureDefaultAlias() diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index ada611d37..16195fa74 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -99,6 +99,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) , m_opVaultOpenWidget(new OpVaultOpenWidget(this)) , m_groupView(new GroupView(m_db.data(), m_mainSplitter)) , m_saveAttempts(0) + , m_entrySearcher(new EntrySearcher(false)) { Q_ASSERT(m_db); @@ -211,7 +212,6 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) m_blockAutoSave = false; - m_EntrySearcher = new EntrySearcher(false); m_searchLimitGroup = config()->get(Config::SearchLimitGroup).toBool(); #ifdef WITH_XC_KEESHARE @@ -234,7 +234,13 @@ DatabaseWidget::DatabaseWidget(const QString& filePath, QWidget* parent) DatabaseWidget::~DatabaseWidget() { - delete m_EntrySearcher; + // Trigger any Database deletion related signals manually by + // explicitly clearing the Database pointer, instead of leaving it to ~QSharedPointer. + // QSharedPointer may behave differently depending on whether it is cleared by the `clear` method + // or by its destructor. In the latter case, the ref counter may not be correctly maintained + // if a copy of the QSharedPointer is created in any slots activated by the Database destructor. + // More details: https://github.com/keepassxreboot/keepassxc/issues/6393. + m_db.clear(); } QSharedPointer DatabaseWidget::database() const @@ -1061,10 +1067,10 @@ void DatabaseWidget::connectDatabaseSignals() SIGNAL(filePathChanged(QString, QString)), SIGNAL(databaseFilePathChanged(QString, QString))); - connect(m_db.data(), SIGNAL(databaseModified()), SIGNAL(databaseModified())); - connect(m_db.data(), SIGNAL(databaseModified()), SLOT(onDatabaseModified())); - connect(m_db.data(), SIGNAL(databaseSaved()), SIGNAL(databaseSaved())); - connect(m_db.data(), SIGNAL(databaseFileChanged()), this, SLOT(reloadDatabaseFile())); + connect(m_db.data(), &Database::modified, this, &DatabaseWidget::databaseModified); + connect(m_db.data(), &Database::modified, this, &DatabaseWidget::onDatabaseModified); + connect(m_db.data(), &Database::databaseSaved, this, &DatabaseWidget::databaseSaved); + connect(m_db.data(), &Database::databaseFileChanged, this, &DatabaseWidget::reloadDatabaseFile); } void DatabaseWidget::loadDatabase(bool accepted) @@ -1366,7 +1372,7 @@ void DatabaseWidget::search(const QString& searchtext) Group* searchGroup = m_searchLimitGroup ? currentGroup() : m_db->rootGroup(); - QList searchResult = m_EntrySearcher->search(searchtext, searchGroup); + QList searchResult = m_entrySearcher->search(searchtext, searchGroup); m_entryView->displaySearch(searchResult); m_lastSearchText = searchtext; @@ -1388,7 +1394,7 @@ void DatabaseWidget::search(const QString& searchtext) void DatabaseWidget::setSearchCaseSensitive(bool state) { - m_EntrySearcher->setCaseSensitive(state); + m_entrySearcher->setCaseSensitive(state); refreshSearch(); } diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index e56e65956..a8c08d6a2 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -290,7 +290,7 @@ private: int m_saveAttempts; // Search state - EntrySearcher* m_EntrySearcher; + QScopedPointer m_entrySearcher; QString m_lastSearchText; bool m_searchLimitGroup; diff --git a/src/gui/EditWidgetProperties.cpp b/src/gui/EditWidgetProperties.cpp index 84223c840..3e26352d3 100644 --- a/src/gui/EditWidgetProperties.cpp +++ b/src/gui/EditWidgetProperties.cpp @@ -63,7 +63,7 @@ void EditWidgetProperties::setCustomData(CustomData* customData) m_customData = customData; if (m_customData) { - connect(m_customData, SIGNAL(customDataModified()), SLOT(update())); + connect(m_customData, &CustomData::modified, this, &EditWidgetProperties::update); } update(); diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index df93c66f9..99bcb28f2 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -532,19 +532,23 @@ void EditEntryWidget::setupSSHAgent() m_sshAgentUi->commentTextLabel->setFont(fixedFont); m_sshAgentUi->publicKeyEdit->setFont(fixedFont); - connect(m_sshAgentUi->attachmentRadioButton, SIGNAL(clicked(bool)), SLOT(updateSSHAgentKeyInfo())); - connect(m_sshAgentUi->attachmentComboBox, SIGNAL(currentIndexChanged(int)), SLOT(updateSSHAgentAttachment())); - connect(m_sshAgentUi->externalFileRadioButton, SIGNAL(clicked(bool)), SLOT(updateSSHAgentKeyInfo())); - connect(m_sshAgentUi->externalFileEdit, SIGNAL(textChanged(QString)), SLOT(updateSSHAgentKeyInfo())); - connect(m_sshAgentUi->browseButton, SIGNAL(clicked()), SLOT(browsePrivateKey())); - connect(m_sshAgentUi->addToAgentButton, SIGNAL(clicked()), SLOT(addKeyToAgent())); - connect(m_sshAgentUi->removeFromAgentButton, SIGNAL(clicked()), SLOT(removeKeyFromAgent())); - connect(m_sshAgentUi->decryptButton, SIGNAL(clicked()), SLOT(decryptPrivateKey())); - connect(m_sshAgentUi->copyToClipboardButton, SIGNAL(clicked()), SLOT(copyPublicKey())); + // clang-format off + connect(m_sshAgentUi->attachmentRadioButton, &QRadioButton::clicked, + this, &EditEntryWidget::updateSSHAgentKeyInfo); + connect(m_sshAgentUi->attachmentComboBox, static_cast(&QComboBox::currentIndexChanged), + this, &EditEntryWidget::updateSSHAgentAttachment); + connect(m_sshAgentUi->externalFileRadioButton, &QRadioButton::clicked, + this, &EditEntryWidget::updateSSHAgentKeyInfo); + connect(m_sshAgentUi->externalFileEdit, &QLineEdit::textChanged, this, &EditEntryWidget::updateSSHAgentKeyInfo); + connect(m_sshAgentUi->browseButton, &QPushButton::clicked, this, &EditEntryWidget::browsePrivateKey); + connect(m_sshAgentUi->addToAgentButton, &QPushButton::clicked, this, &EditEntryWidget::addKeyToAgent); + connect(m_sshAgentUi->removeFromAgentButton, &QPushButton::clicked, this, &EditEntryWidget::removeKeyFromAgent); + connect(m_sshAgentUi->decryptButton, &QPushButton::clicked, this, &EditEntryWidget::decryptPrivateKey); + connect(m_sshAgentUi->copyToClipboardButton, &QPushButton::clicked, this, &EditEntryWidget::copyPublicKey); - connect(m_advancedUi->attachmentsWidget->entryAttachments(), - SIGNAL(entryAttachmentsModified()), - SLOT(updateSSHAgentAttachments())); + connect(m_advancedUi->attachmentsWidget->entryAttachments(), &EntryAttachments::modified, + this, &EditEntryWidget::updateSSHAgentAttachments); + // clang-format on addPage(tr("SSH Agent"), icons()->icon("utilities-terminal"), m_sshAgentWidget); } @@ -803,7 +807,7 @@ void EditEntryWidget::loadEntry(Entry* entry, m_create = create; m_history = history; - connect(m_entry, &Entry::entryModified, this, [this] { m_entryModifiedTimer.start(); }); + connect(m_entry, &Entry::modified, this, [this] { m_entryModifiedTimer.start(); }); if (history) { setHeadline(QString("%1 \u2022 %2").arg(parentName, tr("Entry history"))); diff --git a/src/gui/group/EditGroupWidget.cpp b/src/gui/group/EditGroupWidget.cpp index 7699171c9..953b55ca2 100644 --- a/src/gui/group/EditGroupWidget.cpp +++ b/src/gui/group/EditGroupWidget.cpp @@ -124,7 +124,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer< m_db = database; m_temporaryGroup.reset(group->clone(Entry::CloneNoFlags, Group::CloneNoFlags)); - connect(m_temporaryGroup->customData(), SIGNAL(customDataModified()), SLOT(setModified())); + connect(m_temporaryGroup->customData(), &CustomData::modified, this, [this]() { setModified(true); }); if (create) { setHeadline(tr("Add group")); diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 5d5bc7e22..d2547be42 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -43,14 +43,14 @@ ShareObserver::ShareObserver(QSharedPointer db, QObject* parent) : QObject(parent) , m_db(std::move(db)) { - connect(KeeShare::instance(), SIGNAL(activeChanged()), SLOT(handleDatabaseChanged())); + connect(KeeShare::instance(), &KeeShare::activeChanged, this, &ShareObserver::handleDatabaseChanged); - connect(m_db.data(), SIGNAL(groupDataChanged(Group*)), SLOT(handleDatabaseChanged())); - connect(m_db.data(), SIGNAL(groupAdded()), SLOT(handleDatabaseChanged())); - connect(m_db.data(), SIGNAL(groupRemoved()), SLOT(handleDatabaseChanged())); + connect(m_db.data(), &Database::groupDataChanged, this, &ShareObserver::handleDatabaseChanged); + connect(m_db.data(), &Database::groupAdded, this, &ShareObserver::handleDatabaseChanged); + connect(m_db.data(), &Database::groupRemoved, this, &ShareObserver::handleDatabaseChanged); - connect(m_db.data(), SIGNAL(databaseModified()), SLOT(handleDatabaseChanged())); - connect(m_db.data(), SIGNAL(databaseSaved()), SLOT(handleDatabaseSaved())); + connect(m_db.data(), &Database::modified, this, &ShareObserver::handleDatabaseChanged); + connect(m_db.data(), &Database::databaseSaved, this, &ShareObserver::handleDatabaseSaved); handleDatabaseChanged(); } diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp index 4e2e6eeb2..74b016198 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp +++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp @@ -87,7 +87,7 @@ void EditGroupWidgetKeeShare::setGroup(Group* temporaryGroup, QSharedPointermetadata()->setName("test1"); QTRY_COMPARE(spyModified.count(), 1); @@ -143,7 +143,7 @@ void TestDatabase::testEmptyRecycleBinOnDisabled() // Prevents assertion failures on CI systems when the data dir is not writable db->setReadOnly(false); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); db->emptyRecycleBin(); // The database must be unmodified in this test after emptying the recycle bin. @@ -159,7 +159,7 @@ void TestDatabase::testEmptyRecycleBinOnNotCreated() QVERIFY(db->open(filename, key, nullptr, false)); db->setReadOnly(false); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); db->emptyRecycleBin(); // The database must be unmodified in this test after emptying the recycle bin. @@ -175,7 +175,7 @@ void TestDatabase::testEmptyRecycleBinOnEmpty() QVERIFY(db->open(filename, key, nullptr, false)); db->setReadOnly(false); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); db->emptyRecycleBin(); // The database must be unmodified in this test after emptying the recycle bin. diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 28a9f7ec4..ad3c8ca8e 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -841,7 +841,7 @@ void TestGroup::testCopyDataFrom() group3->setName("TestGroup3"); group3->customData()->set("testKey", "value"); - QSignalSpy spyGroupModified(group.data(), SIGNAL(groupModified())); + QSignalSpy spyGroupModified(group.data(), SIGNAL(modified())); QSignalSpy spyGroupDataChanged(group.data(), SIGNAL(groupDataChanged(Group*))); group->copyDataFrom(group2.data()); diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index 7c9c3cc65..1d40c0c65 100644 --- a/tests/TestMerge.cpp +++ b/tests/TestMerge.cpp @@ -1479,7 +1479,7 @@ void TestMerge::testMergeNotModified() QScopedPointer dbSource( createTestDatabaseStructureClone(dbDestination.data(), Entry::CloneNoFlags, Group::CloneIncludeEntries)); - QSignalSpy modifiedSignalSpy(dbDestination.data(), SIGNAL(databaseModified())); + QSignalSpy modifiedSignalSpy(dbDestination.data(), SIGNAL(modified())); Merger merger(dbSource.data(), dbDestination.data()); merger.merge(); QTRY_VERIFY(modifiedSignalSpy.empty()); @@ -1491,7 +1491,7 @@ void TestMerge::testMergeModified() QScopedPointer dbSource( createTestDatabaseStructureClone(dbDestination.data(), Entry::CloneNoFlags, Group::CloneIncludeEntries)); - QSignalSpy modifiedSignalSpy(dbDestination.data(), SIGNAL(databaseModified())); + QSignalSpy modifiedSignalSpy(dbDestination.data(), SIGNAL(modified())); // Make sure the two changes have a different timestamp. QTest::qSleep(1); Entry* entry = dbSource->rootGroup()->findEntryByPath("entry1"); diff --git a/tests/TestModified.cpp b/tests/TestModified.cpp index dcaeca8ff..4c54c8b2f 100644 --- a/tests/TestModified.cpp +++ b/tests/TestModified.cpp @@ -61,7 +61,7 @@ void TestModified::testSignals() QScopedPointer db(new Database()); auto* root = db->rootGroup(); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); db->setKey(compositeKey); ++spyCount; @@ -88,7 +88,7 @@ void TestModified::testSignals() QScopedPointer db2(new Database()); auto* root2 = db2->rootGroup(); - QSignalSpy spyModified2(db2.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified2(db2.data(), SIGNAL(modified())); group1->setParent(root2); ++spyCount; @@ -149,7 +149,7 @@ void TestModified::testGroupSets() auto* group = new Group(); group->setParent(root); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); root->setUuid(QUuid::createUuid()); ++spyCount; @@ -223,7 +223,7 @@ void TestModified::testEntrySets() auto* entry = new Entry(); entry->setGroup(group); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); entry->setUuid(QUuid::createUuid()); ++spyCount; @@ -647,7 +647,7 @@ void TestModified::testCustomData() auto* entry = new Entry(); entry->setGroup(group); - QSignalSpy spyModified(db.data(), SIGNAL(databaseModified())); + QSignalSpy spyModified(db.data(), SIGNAL(modified())); db->metadata()->customData()->set("Key", "Value"); ++spyCount; @@ -667,3 +667,58 @@ void TestModified::testCustomData() group->customData()->set("Key", "Value"); QTRY_COMPARE(spyModified.count(), spyCount); } + +void TestModified::testBlockModifiedSignal() +{ + QScopedPointer db(new Database()); + auto entry = db->rootGroup()->addEntryWithPath("/abc"); + + QSignalSpy spyDbModified(db.data(), SIGNAL(modified())); + QSignalSpy spyMetadataModified(db->metadata(), SIGNAL(modified())); + QSignalSpy spyCustomDataModified(db->metadata()->customData(), SIGNAL(modified())); + QSignalSpy spyGroupModified(db->rootGroup(), SIGNAL(modified())); + QSignalSpy spyGroupCustomDataModified(db->rootGroup()->customData(), SIGNAL(modified())); + QSignalSpy spyEntryModified(entry, SIGNAL(modified())); + QSignalSpy spyEntryCustomDataModified(entry->customData(), SIGNAL(modified())); + QSignalSpy spyEntryAttributesModified(entry->attributes(), SIGNAL(modified())); + QSignalSpy spyEntryAttachmentModified(entry->attachments(), SIGNAL(modified())); + QSignalSpy spyEntryAutoTypeAssociationsModified(entry->autoTypeAssociations(), SIGNAL(modified())); + + QVERIFY(spyDbModified.isValid()); + QVERIFY(spyMetadataModified.isValid()); + QVERIFY(spyCustomDataModified.isValid()); + QVERIFY(spyGroupModified.isValid()); + QVERIFY(spyGroupCustomDataModified.isValid()); + QVERIFY(spyEntryModified.isValid()); + QVERIFY(spyEntryCustomDataModified.isValid()); + QVERIFY(spyEntryAttributesModified.isValid()); + QVERIFY(spyEntryAttachmentModified.isValid()); + QVERIFY(spyEntryAutoTypeAssociationsModified.isValid()); + + db->setEmitModified(false); + + auto* group1 = new Group(); + group1->setParent(db->rootGroup()); + db->metadata()->setName("Modified Database"); + db->metadata()->customData()->set("Key", "Value"); + + group1->customData()->set("abc", "dd"); + entry->setTitle("Another Title"); + entry->customData()->set("entryabc", "dd"); + entry->attributes()->set("aaa", "dd"); + entry->attachments()->set("aaa", {}); + entry->autoTypeAssociations()->add({"", ""}); + + db.reset(); + + QCOMPARE(spyDbModified.count(), 0); + QCOMPARE(spyMetadataModified.count(), 0); + QCOMPARE(spyCustomDataModified.count(), 0); + QCOMPARE(spyGroupModified.count(), 0); + QCOMPARE(spyGroupCustomDataModified.count(), 0); + QCOMPARE(spyEntryModified.count(), 0); + QCOMPARE(spyEntryCustomDataModified.count(), 0); + QCOMPARE(spyEntryAttributesModified.count(), 0); + QCOMPARE(spyEntryAttachmentModified.count(), 0); + QCOMPARE(spyEntryAutoTypeAssociationsModified.count(), 0); +} diff --git a/tests/TestModified.h b/tests/TestModified.h index 5f7efa4d4..f247d224e 100644 --- a/tests/TestModified.h +++ b/tests/TestModified.h @@ -34,6 +34,7 @@ private slots: void testHistoryItems(); void testHistoryMaxSize(); void testCustomData(); + void testBlockModifiedSignal(); }; #endif // KEEPASSX_TESTMODIFIED_H