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 #3895, macOS file selection no longer hangs the app.

* Restore saving of KeeShare settings accidentally removed by 596d2cf 

5 files changed, 81 insertions, 85 deletions

diff --git a/src/keeshare/KeeShare.cpp b/src/keeshare/KeeShare.cpp
index 567558bdc..beff3d950 100644
--- a/src/keeshare/KeeShare.cpp
+++ b/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)
diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp
index c4c37b916..6dc7a748d 100644
--- a/src/keeshare/ShareObserver.cpp
+++ b/src/keeshare/ShareObserver.cpp
@@ -32,12 +32,14 @@ namespace
         const QFileInfo info(database->filePath());
         return info.absoluteDir().absoluteFilePath(path);
     }
+
+    constexpr int FileWatchPeriod = 30;
+    constexpr int FileWatchSize = 5;
 } // End Namespace
 
 ShareObserver::ShareObserver(QSharedPointer<Database> db, QObject* parent)
     : QObject(parent)
     , m_db(std::move(db))
-    , m_fileWatcher(new BulkFileWatcher(this))
 {
     connect(KeeShare::instance(), SIGNAL(activeChanged()), SLOT(handleDatabaseChanged()));
 
@@ -48,10 +50,6 @@ ShareObserver::ShareObserver(QSharedPointer<Database> db, QObject* parent)
     connect(m_db.data(), SIGNAL(databaseModified()), SLOT(handleDatabaseChanged()));
     connect(m_db.data(), SIGNAL(databaseSaved()), SLOT(handleDatabaseSaved()));
 
-    connect(m_fileWatcher, SIGNAL(fileCreated(QString)), SLOT(handleFileCreated(QString)));
-    connect(m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(handleFileUpdated(QString)));
-    connect(m_fileWatcher, SIGNAL(fileRemoved(QString)), SLOT(handleFileDeleted(QString)));
-
     handleDatabaseChanged();
 }
 
@@ -61,39 +59,33 @@ ShareObserver::~ShareObserver()
 
 void ShareObserver::deinitialize()
 {
-    m_fileWatcher->clear();
     m_groupToReference.clear();
-    m_referenceToGroup.clear();
+    m_shareToGroup.clear();
+    m_fileWatchers.clear();
 }
 
 void ShareObserver::reinitialize()
 {
-    struct Update
-    {
-        Group* group;
-        KeeShareSettings::Reference oldReference;
-        KeeShareSettings::Reference newReference;
-    };
-
-    QList<Update> updated;
-    const QList<Group*> groups = m_db->rootGroup()->groupsRecursive(true);
-    for (Group* group : groups) {
-        const Update couple{group, m_groupToReference.value(group), KeeShare::referenceOf(group)};
-        if (couple.oldReference == couple.newReference) {
+    QList<QPair<Group*, KeeShareSettings::Reference>> shares;
+    for (Group* group : m_db->rootGroup()->groupsRecursive(true)) {
+        auto oldReference = m_groupToReference.value(group);
+        auto newReference = KeeShare::referenceOf(group);
+        if (oldReference == newReference) {
             continue;
         }
 
-        m_groupToReference.remove(couple.group);
-        m_referenceToGroup.remove(couple.oldReference);
-        const auto oldResolvedPath = resolvePath(couple.oldReference.path, m_db);
+        const auto oldResolvedPath = resolvePath(oldReference.path, m_db);
+        m_groupToReference.remove(group);
         m_shareToGroup.remove(oldResolvedPath);
-        if (couple.newReference.isValid()) {
-            m_groupToReference[couple.group] = couple.newReference;
-            m_referenceToGroup[couple.newReference] = couple.group;
-            const auto newResolvedPath = resolvePath(couple.newReference.path, m_db);
-            m_shareToGroup[newResolvedPath] = couple.group;
+        m_fileWatchers.remove(oldResolvedPath);
+
+        if (newReference.isValid()) {
+            m_groupToReference[group] = newReference;
+            const auto newResolvedPath = resolvePath(newReference.path, m_db);
+            m_shareToGroup[newResolvedPath] = group;
         }
-        updated << couple;
+
+        shares.append({group, newReference});
     }
 
     QStringList success;
@@ -101,25 +93,27 @@ void ShareObserver::reinitialize()
     QStringList error;
     QMap<QString, QStringList> imported;
     QMap<QString, QStringList> exported;
-    for (const auto& update : asConst(updated)) {
-        if (!update.oldReference.path.isEmpty()) {
-            const auto oldResolvedPath = resolvePath(update.oldReference.path, m_db);
-            m_fileWatcher->removePath(oldResolvedPath);
-        }
 
-        if (!update.newReference.path.isEmpty() && update.newReference.type != KeeShareSettings::Inactive) {
-            const auto newResolvedPath = resolvePath(update.newReference.path, m_db);
-            m_fileWatcher->addPath(newResolvedPath);
+    for (const auto& share : shares) {
+        auto group = share.first;
+        auto& reference = share.second;
+
+        if (!reference.path.isEmpty() && reference.type != KeeShareSettings::Inactive) {
+            const auto newResolvedPath = resolvePath(reference.path, m_db);
+            auto fileWatcher = QSharedPointer<FileWatcher>::create(this);
+            connect(fileWatcher.data(), &FileWatcher::fileChanged, this, &ShareObserver::handleFileUpdated);
+            fileWatcher->start(newResolvedPath, FileWatchPeriod, FileWatchSize);
+            m_fileWatchers.insert(newResolvedPath, fileWatcher);
         }
-        if (update.newReference.isExporting()) {
-            exported[update.newReference.path] << update.group->name();
+        if (reference.isExporting()) {
+            exported[reference.path] << group->name();
             // export is only on save
         }
 
-        if (update.newReference.isImporting()) {
-            imported[update.newReference.path] << update.group->name();
+        if (reference.isImporting()) {
+            imported[reference.path] << group->name();
             // import has to occur immediately
-            const auto result = this->importShare(update.newReference.path);
+            const auto result = this->importShare(reference.path);
             if (!result.isValid()) {
                 // tolerable result - blocked import or missing source
                 continue;
@@ -136,11 +130,13 @@ void ShareObserver::reinitialize()
             }
         }
     }
+
     for (auto it = imported.cbegin(); it != imported.cend(); ++it) {
         if (it.value().count() > 1) {
             warning << tr("Multiple import source path to %1 in %2").arg(it.key(), it.value().join(", "));
         }
     }
+
     for (auto it = exported.cbegin(); it != exported.cend(); ++it) {
         if (it.value().count() > 1) {
             error << tr("Conflicting export target path %1 in %2").arg(it.key(), it.value().join(", "));
@@ -184,21 +180,9 @@ void ShareObserver::handleDatabaseChanged()
     }
 }
 
-void ShareObserver::handleFileCreated(const QString& path)
-{
-    // there is currently no difference in handling an added share or updating from one
-    this->handleFileUpdated(path);
-}
-
-void ShareObserver::handleFileDeleted(const QString& path)
-{
-    Q_UNUSED(path);
-    // There is nothing we can or should do for now, ignore deletion
-}
-
 void ShareObserver::handleFileUpdated(const QString& path)
 {
-    const Result result = this->importShare(path);
+    const Result result = importShare(path);
     if (!result.isValid()) {
         return;
     }
@@ -287,9 +271,16 @@ QList<ShareObserver::Result> ShareObserver::exportShares()
     for (auto it = references.cbegin(); it != references.cend(); ++it) {
         const auto& reference = it.value().first();
         const QString resolvedPath = resolvePath(reference.config.path, m_db);
-        m_fileWatcher->ignoreFileChanges(resolvedPath);
+        auto watcher = m_fileWatchers.value(resolvedPath);
+        if (watcher) {
+            watcher->stop();
+        }
+
         results << ShareExport::intoContainer(resolvedPath, reference.config, reference.group);
-        m_fileWatcher->observeFileChanges(true);
+
+        if (watcher) {
+            watcher->start(resolvedPath, FileWatchPeriod, FileWatchSize);
+        }
     }
     return results;
 }
diff --git a/src/keeshare/ShareObserver.h b/src/keeshare/ShareObserver.h
index df81fb395..b98d58981 100644
--- a/src/keeshare/ShareObserver.h
+++ b/src/keeshare/ShareObserver.h
@@ -20,12 +20,13 @@
 
 #include <QMap>
 #include <QObject>
+#include <QSharedPointer>
 #include <QStringList>
 
 #include "gui/MessageWidget.h"
 #include "keeshare/KeeShareSettings.h"
 
-class BulkFileWatcher;
+class FileWatcher;
 class Group;
 class Database;
 
@@ -67,9 +68,7 @@ signals:
 private slots:
     void handleDatabaseChanged();
     void handleDatabaseSaved();
-    void handleFileCreated(const QString& path);
     void handleFileUpdated(const QString& path);
-    void handleFileDeleted(const QString& path);
 
 private:
     Result importShare(const QString& path);
@@ -81,11 +80,9 @@ private:
 
 private:
     QSharedPointer<Database> m_db;
-    QMap<KeeShareSettings::Reference, QPointer<Group>> m_referenceToGroup;
     QMap<QPointer<Group>, KeeShareSettings::Reference> m_groupToReference;
     QMap<QString, QPointer<Group>> m_shareToGroup;
-
-    BulkFileWatcher* m_fileWatcher;
+    QMap<QString, QSharedPointer<FileWatcher>> m_fileWatchers;
 };
 
 #endif // KEEPASSXC_SHAREOBSERVER_H
diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp
index 16fbce07f..a3b71220f 100644
--- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp
+++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp
@@ -49,7 +49,7 @@ EditGroupWidgetKeeShare::EditGroupWidgetKeeShare(QWidget* parent)
     connect(m_ui->typeComboBox, SIGNAL(currentIndexChanged(int)), SLOT(selectType()));
     connect(m_ui->clearButton, SIGNAL(clicked(bool)), SLOT(clearInputs()));
 
-    connect(KeeShare::instance(), SIGNAL(activeChanged()), SLOT(showSharingState()));
+    connect(KeeShare::instance(), SIGNAL(activeChanged()), SLOT(updateSharingState()));
 
     const auto types = QList<KeeShareSettings::Type>()
                        << KeeShareSettings::Inactive << KeeShareSettings::ImportFrom << KeeShareSettings::ExportTo
@@ -94,9 +94,16 @@ void EditGroupWidgetKeeShare::setGroup(Group* temporaryGroup, QSharedPointer<Dat
     update();
 }
 
-void EditGroupWidgetKeeShare::showSharingState()
+void EditGroupWidgetKeeShare::updateSharingState()
 {
-    if (!m_temporaryGroup) {
+    // Only enable controls if we are active
+    bool isEnabled = m_ui->typeComboBox->currentData().toInt() > KeeShareSettings::Inactive;
+    m_ui->pathEdit->setEnabled(isEnabled);
+    m_ui->pathSelectionButton->setEnabled(isEnabled);
+    m_ui->passwordEdit->setEnabled(isEnabled);
+
+    if (!m_temporaryGroup || !isEnabled) {
+        m_ui->messageWidget->hideMessage();
         return;
     }
 
@@ -107,6 +114,8 @@ void EditGroupWidgetKeeShare::showSharingState()
 #if defined(WITH_XC_KEESHARE_SECURE)
     supportedExtensions << KeeShare::signedContainerFileType();
 #endif
+
+    // Custom message for active KeeShare reference
     const auto reference = KeeShare::referenceOf(m_temporaryGroup);
     if (!reference.path.isEmpty()) {
         bool supported = false;
@@ -157,26 +166,23 @@ void EditGroupWidgetKeeShare::showSharingState()
                 MessageWidget::Warning);
             return;
         }
-
-        m_ui->messageWidget->hide();
     }
+
+    // Standard message for state of KeeShare service
     const auto active = KeeShare::active();
     if (!active.in && !active.out) {
         m_ui->messageWidget->showMessage(
             tr("KeeShare is currently disabled. You can enable import/export in the application settings.",
                "KeeShare is a proper noun"),
             MessageWidget::Information);
-        return;
-    }
-    if (active.in && !active.out) {
+    } else if (active.in && !active.out) {
         m_ui->messageWidget->showMessage(tr("Database export is currently disabled by application settings."),
                                          MessageWidget::Information);
-        return;
-    }
-    if (!active.in && active.out) {
+    } else if (!active.in && active.out) {
         m_ui->messageWidget->showMessage(tr("Database import is currently disabled by application settings."),
                                          MessageWidget::Information);
-        return;
+    } else {
+        m_ui->messageWidget->hideMessage();
     }
 }
 
@@ -191,9 +197,9 @@ void EditGroupWidgetKeeShare::update()
         m_ui->typeComboBox->setCurrentIndex(reference.type);
         m_ui->passwordEdit->setText(reference.password);
         m_ui->pathEdit->setText(reference.path);
-
-        showSharingState();
     }
+
+    updateSharingState();
 }
 
 void EditGroupWidgetKeeShare::clearInputs()
@@ -204,6 +210,7 @@ void EditGroupWidgetKeeShare::clearInputs()
     m_ui->passwordEdit->clear();
     m_ui->pathEdit->clear();
     m_ui->typeComboBox->setCurrentIndex(KeeShareSettings::Inactive);
+    updateSharingState();
 }
 
 void EditGroupWidgetKeeShare::selectPath()
@@ -255,17 +262,14 @@ void EditGroupWidgetKeeShare::launchPathSelectionDialog()
     }
     switch (reference.type) {
     case KeeShareSettings::ImportFrom:
-        filename = fileDialog()->getFileName(
-            this, tr("Select import source"), defaultDirPath, filters, nullptr, QFileDialog::DontConfirmOverwrite);
+        filename = fileDialog()->getOpenFileName(this, tr("Select import source"), defaultDirPath, filters);
         break;
     case KeeShareSettings::ExportTo:
-        filename = fileDialog()->getFileName(
-            this, tr("Select export target"), defaultDirPath, filters, nullptr, QFileDialog::Option(0));
+        filename = fileDialog()->getSaveFileName(this, tr("Select export target"), defaultDirPath, filters);
         break;
     case KeeShareSettings::SynchronizeWith:
     case KeeShareSettings::Inactive:
-        filename = fileDialog()->getFileName(
-            this, tr("Select import/export file"), defaultDirPath, filters, nullptr, QFileDialog::Option(0));
+        filename = fileDialog()->getSaveFileName(this, tr("Select import/export file"), defaultDirPath, filters);
         break;
     }
 
@@ -286,6 +290,8 @@ void EditGroupWidgetKeeShare::launchPathSelectionDialog()
     m_ui->pathEdit->setText(filename);
     selectPath();
     config()->set(Config::KeeShare_LastShareDir, QFileInfo(filename).absolutePath());
+
+    updateSharingState();
 }
 
 void EditGroupWidgetKeeShare::selectPassword()
@@ -306,4 +312,6 @@ void EditGroupWidgetKeeShare::selectType()
     auto reference = KeeShare::referenceOf(m_temporaryGroup);
     reference.type = static_cast<KeeShareSettings::Type>(m_ui->typeComboBox->currentData().toInt());
     KeeShare::setReferenceTo(m_temporaryGroup, reference);
+
+    updateSharingState();
 }
diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.h b/src/keeshare/group/EditGroupWidgetKeeShare.h
index 54eef2bb0..ae4ae193c 100644
--- a/src/keeshare/group/EditGroupWidgetKeeShare.h
+++ b/src/keeshare/group/EditGroupWidgetKeeShare.h
@@ -40,7 +40,7 @@ public:
     void setGroup(Group* temporaryGroup, QSharedPointer<Database> database);
 
 private slots:
-    void showSharingState();
+    void updateSharingState();
 
 private slots:
     void update();