Correct multiple issues with database saving

* Mark the database as clean after fully completing the file save operation INSTEAD of when merely writing the database to a file.

* Stop the modified timer when marking the database as clean, this prevents latent erroneous modified signals from being emitted.

* Do not restart the modified timer after a new change is detected while it is still running.
This commit is contained in:
Jonathan White 2019-12-08 09:52:01 -05:00
parent 6dd9702b79
commit f9cb2bd5df
3 changed files with 11 additions and 26 deletions

View File

@ -42,7 +42,6 @@ Database::Database()
: m_metadata(new Metadata(this)) : m_metadata(new Metadata(this))
, m_data() , m_data()
, m_rootGroup(nullptr) , m_rootGroup(nullptr)
, m_timer(new QTimer(this))
, m_fileWatcher(new FileWatcher(this)) , m_fileWatcher(new FileWatcher(this))
, m_emitModified(false) , m_emitModified(false)
, m_uuid(QUuid::createUuid()) , m_uuid(QUuid::createUuid())
@ -50,12 +49,12 @@ Database::Database()
setRootGroup(new Group()); setRootGroup(new Group());
rootGroup()->setUuid(QUuid::createUuid()); rootGroup()->setUuid(QUuid::createUuid());
rootGroup()->setName(tr("Root", "Root group name")); rootGroup()->setName(tr("Root", "Root group name"));
m_timer->setSingleShot(true); m_modifiedTimer.setSingleShot(true);
s_uuidMap.insert(m_uuid, this); s_uuidMap.insert(m_uuid, this);
connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified())); connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified()));
connect(m_timer, SIGNAL(timeout()), SIGNAL(databaseModified())); connect(&m_modifiedTimer, SIGNAL(timeout()), SIGNAL(databaseModified()));
connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames())); connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames()));
connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames())); connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames()));
connect(m_fileWatcher, SIGNAL(fileChanged()), SIGNAL(databaseFileChanged())); connect(m_fileWatcher, SIGNAL(fileChanged()), SIGNAL(databaseFileChanged()));
@ -229,6 +228,7 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath; auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath;
bool ok = performSave(canonicalFilePath, error, atomic, backup); bool ok = performSave(canonicalFilePath, error, atomic, backup);
if (ok) { if (ok) {
markAsClean();
setFilePath(filePath); setFilePath(filePath);
m_fileWatcher->start(canonicalFilePath, 30, 1); m_fileWatcher->start(canonicalFilePath, 30, 1);
} else { } else {
@ -343,7 +343,6 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
return false; return false;
} }
markAsClean();
return true; return true;
} }
@ -832,7 +831,7 @@ void Database::emptyRecycleBin()
void Database::setEmitModified(bool value) void Database::setEmitModified(bool value)
{ {
if (m_emitModified && !value) { if (m_emitModified && !value) {
m_timer->stop(); m_modifiedTimer.stop();
} }
m_emitModified = value; m_emitModified = value;
@ -846,8 +845,9 @@ bool Database::isModified() const
void Database::markAsModified() void Database::markAsModified()
{ {
m_modified = true; m_modified = true;
if (m_emitModified) { if (m_emitModified && !m_modifiedTimer.isActive()) {
startModifiedTimer(); // Small time delay prevents numerous consecutive saves due to repeated signals
m_modifiedTimer.start(150);
} }
} }
@ -855,6 +855,7 @@ void Database::markAsClean()
{ {
bool emitSignal = m_modified; bool emitSignal = m_modified;
m_modified = false; m_modified = false;
m_modifiedTimer.stop();
if (emitSignal) { if (emitSignal) {
emit databaseSaved(); emit databaseSaved();
} }
@ -869,18 +870,6 @@ Database* Database::databaseByUuid(const QUuid& uuid)
return s_uuidMap.value(uuid, nullptr); return s_uuidMap.value(uuid, nullptr);
} }
void Database::startModifiedTimer()
{
if (!m_emitModified) {
return;
}
if (m_timer->isActive()) {
m_timer->stop();
}
m_timer->start(150);
}
QSharedPointer<const CompositeKey> Database::key() const QSharedPointer<const CompositeKey> Database::key() const
{ {
return m_data.key; return m_data.key;

View File

@ -21,9 +21,9 @@
#include <QDateTime> #include <QDateTime>
#include <QHash> #include <QHash>
#include <QObject>
#include <QPointer> #include <QPointer>
#include <QScopedPointer> #include <QScopedPointer>
#include <QTimer>
#include "config-keepassx.h" #include "config-keepassx.h"
#include "crypto/kdf/AesKdf.h" #include "crypto/kdf/AesKdf.h"
@ -37,7 +37,6 @@ enum class EntryReferenceType;
class FileWatcher; class FileWatcher;
class Group; class Group;
class Metadata; class Metadata;
class QTimer;
class QIODevice; class QIODevice;
struct DeletedObject struct DeletedObject
@ -155,9 +154,6 @@ signals:
void databaseDiscarded(); void databaseDiscarded();
void databaseFileChanged(); void databaseFileChanged();
private slots:
void startModifiedTimer();
private: private:
struct DatabaseData struct DatabaseData
{ {
@ -211,7 +207,7 @@ private:
DatabaseData m_data; DatabaseData m_data;
QPointer<Group> m_rootGroup; QPointer<Group> m_rootGroup;
QList<DeletedObject> m_deletedObjects; QList<DeletedObject> m_deletedObjects;
QPointer<QTimer> m_timer; QTimer m_modifiedTimer;
QPointer<FileWatcher> m_fileWatcher; QPointer<FileWatcher> m_fileWatcher;
bool m_initialized = false; bool m_initialized = false;
bool m_modified = false; bool m_modified = false;

View File

@ -1370,7 +1370,7 @@ bool DatabaseWidget::lock()
if (m_db->isModified()) { if (m_db->isModified()) {
bool saved = false; bool saved = false;
// Attempt to save on exit, but don't block locking if it fails // Attempt to save on exit, but don't block locking if it fails
if (config()->get("AutoSaveOnExit").toBool()) { if (config()->get("AutoSaveOnExit").toBool() || config()->get("AutoSaveAfterEveryChange").toBool()) {
saved = save(); saved = save();
} }