From 8e76c30dd104ce8044ecf0c53176f0f716e55f29 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Thu, 12 Dec 2019 22:40:17 -0500 Subject: [PATCH] Prevent reloading database while editing an entry or group * Fix #3933 and fix #3857. Interaction with entries and groups is disabled while the database is being reloaded or saved to prevent changes from occurring. Prevent the database from being reloading if an entry or group is currently being edited. * Fix #3941 - Only notify components when the database file actually changes (determined by checksum). This prevents spurious merge requests when the file is merely touched by another service (e.g., DropBox). * Fix code format of ElidedLabel.cpp --- src/core/FileWatcher.cpp | 30 +++++++++++++----------------- src/core/FileWatcher.h | 7 +++---- src/gui/DatabaseWidget.cpp | 30 ++++++++++++++++++++++++++++-- src/gui/DatabaseWidget.h | 1 + src/gui/widgets/ElidedLabel.cpp | 2 +- 5 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/core/FileWatcher.cpp b/src/core/FileWatcher.cpp index 0d1def31..fb8e9512 100644 --- a/src/core/FileWatcher.cpp +++ b/src/core/FileWatcher.cpp @@ -35,11 +35,10 @@ namespace FileWatcher::FileWatcher(QObject* parent) : QObject(parent) - , m_ignoreFileChange(false) { - connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(onWatchedFileChanged())); + connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(checkFileChanged())); + connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChanged())); connect(&m_fileChangeDelayTimer, SIGNAL(timeout()), SIGNAL(fileChanged())); - connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChecksum())); m_fileChangeDelayTimer.setSingleShot(true); m_fileIgnoreDelayTimer.setSingleShot(true); } @@ -101,17 +100,6 @@ void FileWatcher::resume() } } -void FileWatcher::onWatchedFileChanged() -{ - // Don't notify if we are ignoring events or already started a notification chain - if (shouldIgnoreChanges()) { - return; - } - - m_fileChecksum = calculateChecksum(); - m_fileChangeDelayTimer.start(0); -} - bool FileWatcher::shouldIgnoreChanges() { return m_filePath.isEmpty() || m_ignoreFileChange || m_fileIgnoreDelayTimer.isActive() @@ -123,15 +111,23 @@ bool FileWatcher::hasSameFileChecksum() return calculateChecksum() == m_fileChecksum; } -void FileWatcher::checkFileChecksum() +void FileWatcher::checkFileChanged() { if (shouldIgnoreChanges()) { return; } - if (!hasSameFileChecksum()) { - onWatchedFileChanged(); + // Prevent reentrance + m_ignoreFileChange = true; + + // Only trigger the change notice if there is a checksum mismatch + auto checksum = calculateChecksum(); + if (checksum != m_fileChecksum) { + m_fileChecksum = checksum; + m_fileChangeDelayTimer.start(0); } + + m_ignoreFileChange = false; } QByteArray FileWatcher::calculateChecksum() diff --git a/src/core/FileWatcher.h b/src/core/FileWatcher.h index fea05fc8..9b55badc 100644 --- a/src/core/FileWatcher.h +++ b/src/core/FileWatcher.h @@ -43,8 +43,7 @@ public slots: void resume(); private slots: - void onWatchedFileChanged(); - void checkFileChecksum(); + void checkFileChanged(); private: QByteArray calculateChecksum(); @@ -56,8 +55,8 @@ private: QTimer m_fileChangeDelayTimer; QTimer m_fileIgnoreDelayTimer; QTimer m_fileChecksumTimer; - int m_fileChecksumSizeBytes; - bool m_ignoreFileChange; + int m_fileChecksumSizeBytes = -1; + bool m_ignoreFileChange = false; }; class BulkFileWatcher : public QObject diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 04fad9a0..5c4bf39a 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -275,6 +275,11 @@ bool DatabaseWidget::isEntryEditActive() const return currentWidget() == m_editEntryWidget; } +bool DatabaseWidget::isGroupEditActive() const +{ + return currentWidget() == m_editGroupWidget; +} + bool DatabaseWidget::isEditWidgetModified() const { if (currentWidget() == m_editEntryWidget) { @@ -387,6 +392,8 @@ void DatabaseWidget::createEntry() void DatabaseWidget::replaceDatabase(QSharedPointer db) { + Q_ASSERT(!isEntryEditActive() && !isGroupEditActive()); + // Save off new parent UUID which will be valid when creating a new entry QUuid newParentUuid; if (m_newParent) { @@ -1421,7 +1428,8 @@ bool DatabaseWidget::lock() void DatabaseWidget::reloadDatabaseFile() { - if (!m_db || isLocked()) { + // Ignore reload if we are locked or currently editing an entry or group + if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive()) { return; } @@ -1441,6 +1449,11 @@ void DatabaseWidget::reloadDatabaseFile() } } + // Lock out interactions + m_entryView->setDisabled(true); + m_groupView->setDisabled(true); + QApplication::processEvents(); + QString error; auto db = QSharedPointer::create(m_db->filePath()); if (db->open(database()->key(), &error)) { @@ -1480,6 +1493,10 @@ void DatabaseWidget::reloadDatabaseFile() // Mark db as modified since existing data may differ from file or file was deleted m_db->markAsModified(); } + + // Return control + m_entryView->setDisabled(false); + m_groupView->setDisabled(false); } int DatabaseWidget::numberOfSelectedEntries() const @@ -1620,11 +1637,20 @@ bool DatabaseWidget::save() m_blockAutoSave = true; ++m_saveAttempts; - // TODO: Make this async, but lock out the database widget to prevent re-entrance + // TODO: Make this async + // Lock out interactions + m_entryView->setDisabled(true); + m_groupView->setDisabled(true); + QApplication::processEvents(); + bool useAtomicSaves = config()->get("UseAtomicSaves", true).toBool(); QString errorMessage; bool ok = m_db->save(&errorMessage, useAtomicSaves, config()->get("BackupBeforeSave").toBool()); + // Return control + m_entryView->setDisabled(false); + m_groupView->setDisabled(false); + if (ok) { m_saveAttempts = 0; m_blockAutoSave = false; diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 6f40c65c..9f0c5c97 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -81,6 +81,7 @@ public: bool isLocked() const; bool isSearchActive() const; bool isEntryEditActive() const; + bool isGroupEditActive() const; QString getCurrentSearch(); void refreshSearch(); diff --git a/src/gui/widgets/ElidedLabel.cpp b/src/gui/widgets/ElidedLabel.cpp index 187c2fc4..749f075c 100644 --- a/src/gui/widgets/ElidedLabel.cpp +++ b/src/gui/widgets/ElidedLabel.cpp @@ -105,7 +105,7 @@ void ElidedLabel::updateElidedText() const QFontMetrics metrix(font()); displayText = metrix.elidedText(m_rawText, m_elideMode, width() - 2); } - + bool hasUrl = !m_url.isEmpty(); setText(hasUrl ? htmlLinkTemplate.arg(m_url.toHtmlEscaped(), displayText) : displayText); setOpenExternalLinks(!hasUrl);