Fix various issues with KeeShare

* Fix #3790, shares now use the standard FileWatcher class to detect remote file changes using checksums and file system triggers.

* Fix #3985, macOS file selection no longer hangs the app.

* Restore saving of KeeShare settings accidentally removed by 596d2cf

src/core/Database.cpp

@@ -58,7 +58,7 @@ Database::Database()
     connect(&m_modifiedTimer, SIGNAL(timeout()), SIGNAL(databaseModified()));
     connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames()));
     connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames()));
-    connect(m_fileWatcher, SIGNAL(fileChanged()), SIGNAL(databaseFileChanged()));
+    connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged);
 
     m_modified = false;
     m_emitModified = true;

src/core/FileWatcher.cpp

@@ -1,6 +1,5 @@
 /*
- *  Copyright (C) 2011 Felix Geyer <[email protected]>
- *  Copyright (C) 2017 KeePassXC Team <[email protected]>
+ *  Copyright (C) 2020 KeePassXC Team <[email protected]>
  *
  *  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
@@ -38,7 +37,7 @@ FileWatcher::FileWatcher(QObject* parent)
 {
     connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(checkFileChanged()));
     connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChanged()));
-    connect(&m_fileChangeDelayTimer, SIGNAL(timeout()), SIGNAL(fileChanged()));
+    connect(&m_fileChangeDelayTimer, &QTimer::timeout, this, [this] { emit fileChanged(m_filePath); });
     m_fileChangeDelayTimer.setSingleShot(true);
     m_fileIgnoreDelayTimer.setSingleShot(true);
 }
@@ -151,189 +150,3 @@ QByteArray FileWatcher::calculateChecksum()
     // prevents unnecessary merge requests on intermittent network shares
     return m_fileChecksum;
 }
-
-BulkFileWatcher::BulkFileWatcher(QObject* parent)
-    : QObject(parent)
-{
-    connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(handleFileChanged(QString)));
-    connect(&m_fileWatcher, SIGNAL(directoryChanged(QString)), SLOT(handleDirectoryChanged(QString)));
-    connect(&m_watchedFilesIgnoreTimer, SIGNAL(timeout()), this, SLOT(observeFileChanges()));
-    connect(&m_pendingSignalsTimer, SIGNAL(timeout()), this, SLOT(emitSignals()));
-    m_watchedFilesIgnoreTimer.setSingleShot(true);
-    m_pendingSignalsTimer.setSingleShot(true);
-}
-
-void BulkFileWatcher::clear()
-{
-    for (const QString& path : m_fileWatcher.files() + m_fileWatcher.directories()) {
-        const QFileInfo info(path);
-        m_fileWatcher.removePath(info.absoluteFilePath());
-        m_fileWatcher.removePath(info.absolutePath());
-    }
-    m_watchedPaths.clear();
-    m_watchedFilesInDirectory.clear();
-    m_watchedFilesIgnored.clear();
-}
-
-void BulkFileWatcher::removePath(const QString& path)
-{
-    const QFileInfo info(path);
-    const QString filePath = info.absoluteFilePath();
-    const QString directoryPath = info.absolutePath();
-    m_watchedFilesInDirectory[directoryPath].remove(filePath);
-    m_fileWatcher.removePath(filePath);
-    m_watchedPaths.remove(filePath);
-    if (m_watchedFilesInDirectory[directoryPath].isEmpty()) {
-        m_fileWatcher.removePath(directoryPath);
-        m_watchedPaths.remove(directoryPath);
-        m_watchedFilesInDirectory.remove(directoryPath);
-    }
-}
-
-void BulkFileWatcher::addPath(const QString& path)
-{
-    const QFileInfo info(path);
-    const QString filePath = info.absoluteFilePath();
-    const QString directoryPath = info.absolutePath();
-    if (!m_watchedPaths.value(filePath)) {
-        const bool fileSuccess = m_fileWatcher.addPath(filePath);
-        m_watchedPaths[filePath] = fileSuccess;
-    }
-    if (!m_watchedPaths.value(directoryPath)) {
-        const bool directorySuccess = m_fileWatcher.addPath(directoryPath);
-        m_watchedPaths[directoryPath] = directorySuccess;
-    }
-    m_watchedFilesInDirectory[directoryPath][filePath] = info.exists() ? info.lastModified().toMSecsSinceEpoch() : 0;
-}
-
-void BulkFileWatcher::handleFileChanged(const QString& path)
-{
-    const QFileInfo info(path);
-    const QString filePath = info.absoluteFilePath();
-    const QString directoryPath = info.absolutePath();
-    const QMap<QString, qint64>& watchedFiles = m_watchedFilesInDirectory[directoryPath];
-    const qint64 lastModificationTime = info.lastModified().toMSecsSinceEpoch();
-    const bool created = watchedFiles[filePath] == 0 && info.exists();
-    const bool deleted = watchedFiles[filePath] != 0 && !info.exists();
-    const bool changed = !created && !deleted && lastModificationTime != watchedFiles[filePath];
-
-    addPath(path);
-
-    if (m_watchedFilesIgnored[info.canonicalFilePath()] > Clock::currentDateTimeUtc()) {
-        // changes are blocked
-        return;
-    }
-    if (created) {
-        qDebug("File created %s", qPrintable(path));
-        scheduleSignal(Created, filePath);
-    }
-    if (changed) {
-        qDebug("File changed %s", qPrintable(path));
-        scheduleSignal(Updated, filePath);
-    }
-    if (deleted) {
-        qDebug("File removed %s", qPrintable(path));
-        scheduleSignal(Removed, filePath);
-    }
-}
-
-void BulkFileWatcher::handleDirectoryChanged(const QString& path)
-{
-    qDebug("Directory changed %s", qPrintable(path));
-    const QFileInfo directoryInfo(path);
-    const QString directoryPath = directoryInfo.absoluteFilePath();
-    QMap<QString, qint64>& watchedFiles = m_watchedFilesInDirectory[directoryPath];
-    for (const QString& filename : watchedFiles.keys()) {
-        const QFileInfo fileInfo(filename);
-        const QString filePath = fileInfo.absoluteFilePath();
-        const qint64 previousModificationTime = watchedFiles[filePath];
-        const qint64 lastModificationTime = fileInfo.lastModified().toMSecsSinceEpoch();
-        if (!fileInfo.exists() && previousModificationTime != 0) {
-            qDebug("Remove watch file %s", qPrintable(fileInfo.absoluteFilePath()));
-            m_fileWatcher.removePath(filePath);
-            m_watchedPaths.remove(filePath);
-            watchedFiles.remove(filePath);
-            scheduleSignal(Removed, filePath);
-        }
-        if (previousModificationTime == 0 && fileInfo.exists()) {
-            qDebug("Add watch file %s", qPrintable(fileInfo.absoluteFilePath()));
-            if (!m_watchedPaths.value(filePath)) {
-                const bool success = m_fileWatcher.addPath(filePath);
-                m_watchedPaths[filePath] = success;
-                watchedFiles[filePath] = lastModificationTime;
-            }
-            scheduleSignal(Created, filePath);
-        }
-        if (fileInfo.exists() && previousModificationTime != lastModificationTime) {
-            // this case is handled using
-            qDebug("Refresh watch file %s", qPrintable(fileInfo.absoluteFilePath()));
-            m_fileWatcher.removePath(fileInfo.absolutePath());
-            m_fileWatcher.addPath(fileInfo.absolutePath());
-            scheduleSignal(Updated, filePath);
-        }
-        m_watchedFilesInDirectory[directoryPath][filePath] = fileInfo.exists() ? lastModificationTime : 0;
-    }
-}
-
-void BulkFileWatcher::emitSignals()
-{
-    QMap<QString, QList<Signal>> queued;
-    m_pendingSignals.swap(queued);
-    for (const auto& path : queued.keys()) {
-        const auto& signal = queued[path];
-        if (signal.last() == Removed) {
-            qDebug("Emit %s removed", qPrintable(path));
-            emit fileRemoved(path);
-            continue;
-        }
-        if (signal.first() == Created) {
-            qDebug("Emit %s created", qPrintable(path));
-            emit fileCreated(path);
-            continue;
-        }
-        qDebug("Emit %s changed", qPrintable(path));
-        emit fileChanged(path);
-    }
-}
-
-void BulkFileWatcher::scheduleSignal(Signal signal, const QString& path)
-{
-    // we need to collect signals since the file watcher API may send multiple signals for a "single" change
-    // therefore we wait until the event loop finished before starting to import any changes
-    const QString filePath = QFileInfo(path).absoluteFilePath();
-    m_pendingSignals[filePath] << signal;
-
-    if (!m_pendingSignalsTimer.isActive()) {
-        m_pendingSignalsTimer.start();
-    }
-}
-
-void BulkFileWatcher::ignoreFileChanges(const QString& path)
-{
-    const QFileInfo info(path);
-    m_watchedFilesIgnored[info.canonicalFilePath()] = Clock::currentDateTimeUtc().addMSecs(FileChangeDelay);
-}
-
-void BulkFileWatcher::observeFileChanges(bool delayed)
-{
-    int timeout = 0;
-    if (delayed) {
-        timeout = FileChangeDelay;
-    } else {
-        const QDateTime current = Clock::currentDateTimeUtc();
-        for (const QString& key : m_watchedFilesIgnored.keys()) {
-            if (m_watchedFilesIgnored[key] < current) {
-                // We assume that there was no concurrent change of the database
-                // during our block - so no need to reimport
-                qDebug("Remove block from %s", qPrintable(key));
-                m_watchedFilesIgnored.remove(key);
-                continue;
-            }
-            qDebug("Keep block from %s", qPrintable(key));
-            timeout = qMin(timeout, static_cast<int>(current.msecsTo(m_watchedFilesIgnored[key])));
-        }
-    }
-    if (timeout > 0 && !m_watchedFilesIgnoreTimer.isActive()) {
-        m_watchedFilesIgnoreTimer.start(timeout);
-    }
-}

src/core/FileWatcher.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2018 KeePassXC Team <[email protected]>
+ *  Copyright (C) 2020 KeePassXC Team <[email protected]>
  *
  *  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
@@ -37,7 +37,7 @@ class FileWatcher : public QObject
     bool hasSameFileChecksum();
 
 signals:
-    void fileChanged();
+    void fileChanged(const QString& path);
 
 public slots:
     void pause();
@@ -60,53 +60,4 @@ private slots:
     bool m_ignoreFileChange = false;
 };
 
-class BulkFileWatcher : public QObject
-{
-    Q_OBJECT
-
-    enum Signal
-    {
-        Created,
-        Updated,
-        Removed
-    };
-
-public:
-    explicit BulkFileWatcher(QObject* parent = nullptr);
-
-    void clear();
-
-    void removePath(const QString& path);
-    void addPath(const QString& path);
-
-    void ignoreFileChanges(const QString& path);
-
-signals:
-    void fileCreated(QString);
-    void fileChanged(QString);
-    void fileRemoved(QString);
-
-public slots:
-    void observeFileChanges(bool delayed = false);
-
-private slots:
-    void handleFileChanged(const QString& path);
-    void handleDirectoryChanged(const QString& path);
-    void emitSignals();
-
-private:
-    void scheduleSignal(Signal event, const QString& path);
-
-private:
-    QMap<QString, bool> m_watchedPaths;
-    QMap<QString, QDateTime> m_watchedFilesIgnored;
-    QFileSystemWatcher m_fileWatcher;
-    QMap<QString, QMap<QString, qint64>> m_watchedFilesInDirectory;
-    // needed for Import/Export-References to prevent update after self-write
-    QTimer m_watchedFilesIgnoreTimer;
-    // needed to tolerate multiple signals for same event
-    QTimer m_pendingSignalsTimer;
-    QMap<QString, QList<Signal>> m_pendingSignals;
-};
-
 #endif // KEEPASSXC_FILEWATCHER_H

src/keeshare/KeeShare.cpp

@@ -48,7 +48,7 @@ KeeShare* KeeShare::instance()
 KeeShare::KeeShare(QObject* parent)
     : QObject(parent)
 {
-    connect(config(), SIGNAL(changed(Config::ConfigKey)), SLOT(handleSettingsChanged(Config::ConfigKey)));
+    connect(config(), &Config::changed, this, &KeeShare::handleSettingsChanged);
 }
 
 void KeeShare::init(QObject* parent)
@@ -117,7 +117,7 @@ void KeeShare::setReferenceTo(Group* group, const KeeShareSettings::Reference& r
         return;
     }
     const auto serialized = KeeShareSettings::Reference::serialize(reference);
-    const auto encoded = serialized.toUtf8().toBase64();
+    customData->set(KeeShare_Reference, serialized.toUtf8().toBase64());
 }
 
 bool KeeShare::isEnabled(const Group* group)