vfs: Fix plugin decision in wizards, sanitize loading

src/common/common.cmake

@@ -10,4 +10,5 @@ set(common_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/utility.cpp
     ${CMAKE_CURRENT_LIST_DIR}/remotepermissions.cpp
     ${CMAKE_CURRENT_LIST_DIR}/vfs.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/plugin.cpp
 )

src/common/plugin.cpp

@@ -19,10 +19,10 @@
 #include "plugin.h"
 
 #include "config.h"
-#include "logger.h"
 
 #include <QPluginLoader>
 #include <QDir>
+#include <QLoggingCategory>
 
 Q_LOGGING_CATEGORY(lcPluginLoader, "pluginLoader", QtInfoMsg)
 
@@ -30,16 +30,6 @@ namespace OCC {
 
 PluginFactory::~PluginFactory() = default;
 
-QObject *PluginLoader::createInternal(const QString& type, const QString &name, QObject* parent)
-{
-    auto factory = load<PluginFactory>(type, name);
-    if (!factory) {
-        return nullptr;
-    } else {
-        return factory->create(parent);
-    }
-}
-
 QString PluginLoader::pluginName(const QString &type, const QString &name)
 {
     return QString(QLatin1String("%1sync_%2_%3"))
@@ -48,12 +38,17 @@ QString PluginLoader::pluginName(const QString &type, const QString &name)
             .arg(name);
 }
 
-QObject *PluginLoader::loadPluginInternal(const QString& type, const QString &name)
+bool PluginLoader::isAvailable(const QString &type, const QString &name)
+{
+    QPluginLoader loader(pluginName(type, name));
+    return loader.load();
+}
+
+QObject *PluginLoader::load(const QString &type, const QString &name)
 {
     QString fileName = pluginName(type, name);
     QPluginLoader pluginLoader(fileName);
-    auto plugin = pluginLoader.load();
+    if (pluginLoader.load()) {
-    if(plugin) {
         qCInfo(lcPluginLoader) << "Loaded plugin" << fileName;
     } else {
         qCWarning(lcPluginLoader) << "Could not load plugin"
@@ -65,4 +60,14 @@ QObject *PluginLoader::loadPluginInternal(const QString& type, const QString &na
     return pluginLoader.instance();
 }
 
+QObject *PluginLoader::create(const QString &type, const QString &name, QObject *parent)
+{
+    auto factory = qobject_cast<PluginFactory *>(load(type, name));
+    if (!factory) {
+        return nullptr;
+    } else {
+        return factory->create(parent);
+    }
+}
+
 }

src/common/plugin.h

@@ -18,13 +18,13 @@
 
 #pragma once
 
-#include "owncloudlib.h"
+#include "ocsynclib.h"
 #include <QObject>
 #include <QPluginLoader>
 
 namespace OCC {
 
-class OWNCLOUDSYNC_EXPORT PluginFactory
+class OCSYNC_EXPORT PluginFactory
 {
 public:
     ~PluginFactory();
@@ -35,32 +35,20 @@ template<class PLUGIN_CLASS>
 class DefaultPluginFactory : public PluginFactory
 {
 public:
-    QObject* create(QObject* parent) override
+    QObject* create(QObject *parent) override
     {
         return new PLUGIN_CLASS(parent);
     }
 };
 
-class OWNCLOUDSYNC_EXPORT PluginLoader
+class OCSYNC_EXPORT PluginLoader
 {
 public:
     static QString pluginName(const QString &type, const QString &name);
 
-    template<class PLUGIN_CLASS, typename ... Args>
+    bool isAvailable(const QString &type, const QString &name);
-    PLUGIN_CLASS *create(Args&& ... args)
+    QObject *load(const QString &type, const QString &name);
-    {
+    QObject *create(const QString &type, const QString &name, QObject *parent = nullptr);
-        return qobject_cast<PLUGIN_CLASS*>(createInternal(std::forward<Args>(args)...));
-    }
-
-private:
-    template<class FACTORY_CLASS, typename ... Args>
-    FACTORY_CLASS *load(Args&& ... args)
-    {
-        return qobject_cast<FACTORY_CLASS*>(loadPluginInternal(std::forward<Args>(args)...));
-    }
-
-    QObject *loadPluginInternal(const QString& type, const QString &name);
-    QObject *createInternal(const QString& type, const QString &name, QObject* parent = nullptr);
 };
 
 }

src/common/vfs.cpp

@@ -17,6 +17,7 @@
  */
 
 #include "vfs.h"
+#include "plugin.h"
 
 using namespace OCC;
 
@@ -56,3 +57,38 @@ bool Vfs::modeFromString(const QString &str, Mode *mode)
     }
     return false;
 }
+
+static QString modeToPluginName(Vfs::Mode mode)
+{
+    if (mode == Vfs::WithSuffix)
+        return "suffix";
+    if (mode == Vfs::WindowsCfApi)
+        return "win";
+    return QString();
+}
+
+bool OCC::isVfsPluginAvailable(Vfs::Mode mode)
+{
+    auto name = modeToPluginName(mode);
+    if (name.isEmpty())
+        return false;
+    return PluginLoader().load("vfs", name);
+}
+
+Vfs::Mode OCC::bestAvailableVfsMode()
+{
+    if (isVfsPluginAvailable(Vfs::WindowsCfApi)) {
+        return Vfs::WindowsCfApi;
+    } else if (isVfsPluginAvailable(Vfs::WithSuffix)) {
+        return Vfs::WithSuffix;
+    }
+    return Vfs::Off;
+}
+
+Vfs *OCC::createVfsFromPlugin(Vfs::Mode mode, QObject *parent)
+{
+    auto name = modeToPluginName(mode);
+    if (name.isEmpty())
+        return nullptr;
+    return qobject_cast<Vfs *>(PluginLoader().create("vfs", name, parent));
+}

src/common/vfs.h

@@ -94,4 +94,8 @@ signals:
     void doneHydrating();
 };
 
+bool isVfsPluginAvailable(Vfs::Mode mode) OCSYNC_EXPORT;
+Vfs::Mode bestAvailableVfsMode() OCSYNC_EXPORT;
+Vfs *createVfsFromPlugin(Vfs::Mode mode, QObject *parent) OCSYNC_EXPORT;
+
 } // namespace OCC

src/gui/accountsettings.cpp

@@ -537,8 +537,7 @@ void AccountSettings::slotFolderWizardAccepted()
         folderWizard->property("targetPath").toString());
 
     if (folderWizard->property("useVirtualFiles").toBool()) {
-        // ### Determine which vfs is available?
+        definition.virtualFilesMode = bestAvailableVfsMode();
-        definition.virtualFilesMode = Vfs::WindowsCfApi;
     }
 
     {

src/gui/application.cpp

@@ -42,6 +42,7 @@
 #include "owncloudsetupwizard.h"
 #include "version.h"
 #include "csync_exclude.h"
+#include "common/vfs.h"
 
 #include "config.h"
 
@@ -268,6 +269,15 @@ Application::Application(int &argc, char **argv)
     if (!AbstractNetworkJob::httpTimeout)
         AbstractNetworkJob::httpTimeout = cfg.timeout();
 
+    // Check vfs plugins
+    if (Theme::instance()->showVirtualFilesOption() && bestAvailableVfsMode() == Vfs::Off) {
+        qCWarning(lcApplication) << "Theme wants to show vfs mode, but no vfs plugins are available";
+    }
+    if (isVfsPluginAvailable(Vfs::WindowsCfApi))
+        qCInfo(lcApplication) << "VFS windows plugin is available";
+    if (isVfsPluginAvailable(Vfs::WithSuffix))
+        qCInfo(lcApplication) << "VFS suffix plugin is available";
+
     _folderManager.reset(new FolderMan);
 
     connect(this, &SharedTools::QtSingleApplication::messageReceived, this, &Application::slotParseMessage);

src/gui/folder.cpp

@@ -33,7 +33,6 @@
 #include "localdiscoverytracker.h"
 #include "csync_exclude.h"
 #include "common/vfs.h"
-#include "plugin.h"
 #include "creds/abstractcredentials.h"
 
 #include <QTimer>
@@ -52,7 +51,7 @@ namespace OCC {
 Q_LOGGING_CATEGORY(lcFolder, "nextcloud.gui.folder", QtInfoMsg)
 
 Folder::Folder(const FolderDefinition &definition,
-    AccountState *accountState,
+    AccountState *accountState, Vfs *vfs,
     QObject *parent)
     : QObject(parent)
     , _accountState(accountState)
@@ -62,6 +61,7 @@ Folder::Folder(const FolderDefinition &definition,
     , _consecutiveFollowUpSyncs(0)
     , _journal(_definition.absoluteJournalPath())
     , _fileLog(new SyncRunFileLog)
+    , _vfs(vfs)
 {
     _timeSinceLastSyncStart.start();
     _timeSinceLastSyncDone.start();
@@ -118,32 +118,26 @@ Folder::Folder(const FolderDefinition &definition,
     connect(_engine.data(), &SyncEngine::itemCompleted,
         _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotItemCompleted);
 
-    // TODO cfapi: Move to function. Is this platform-universal?
+    // Potentially upgrade suffix vfs to windows vfs
-    PluginLoader pluginLoader;
+    if (_definition.virtualFilesMode == Vfs::WithSuffix && _definition.upgradeVfsMode) {
-    if (_definition.virtualFilesMode == Vfs::WindowsCfApi) {
+        if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi, this)) {
-        _vfs = pluginLoader.create<Vfs>("vfs", "win", this);
+            // Wipe the existing suffix files from fs and journal
-    }
+            SyncEngine::wipeVirtualFiles(path(), _journal, _vfs);
-    if (_definition.virtualFilesMode == Vfs::WithSuffix) {
-        _vfs = pluginLoader.create<Vfs>("vfs", "suffix", this);
 
-        // Attempt to switch to winvfs mode?
+            // Then switch to winvfs mode
-        if (_vfs && _definition.upgradeVfsMode) {
+            delete _vfs;
-            if (auto winvfs = pluginLoader.create<Vfs>("vfs", "win", this)) {
+            _vfs = winvfs;
-                // Set "suffix" vfs options and wipe the existing suffix files
+            _definition.virtualFilesMode = Vfs::WindowsCfApi;
-                SyncEngine::wipeVirtualFiles(path(), _journal, _vfs);
+            saveToSettings();
-
-                // Then switch to winvfs mode
-                _vfs = winvfs;
-                _definition.virtualFilesMode = Vfs::WindowsCfApi;
-                saveToSettings();
-            }
         }
     }
+
+    // Initialize the vfs plugin
     if (_definition.virtualFilesMode != Vfs::Off) {
-        if (!_vfs) {
+        ENFORCE(_vfs);
-            // ### error handling; possibly earlier than in the ctor
+        ENFORCE(_vfs->mode() == _definition.virtualFilesMode);
-            qFatal("Could not load any vfs plugin.");
+
-        }
+        _vfs->setParent(this);
 
         VfsSetupParams vfsParams;
         vfsParams.filesystemPath = path();

src/gui/folder.h

@@ -101,7 +101,11 @@ class Folder : public QObject
     Q_OBJECT
 
 public:
-    Folder(const FolderDefinition &definition, AccountState *accountState, QObject *parent = nullptr);
+    /** Create a new Folder
+     *
+     * The vfs instance will be parented to this.
+     */
+    Folder(const FolderDefinition &definition, AccountState *accountState, Vfs *vfs, QObject *parent = nullptr);
 
     ~Folder();
 

src/gui/folderman.cpp

@@ -259,7 +259,12 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
                     qCInfo(lcFolderMan) << "Successfully migrated syncjournal database.";
                 }
 
-                Folder *f = addFolderInternal(folderDefinition, account.data());
+                Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
+                if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
+                    qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode;
+                }
+
+                Folder *f = addFolderInternal(folderDefinition, account.data(), vfs);
                 f->saveToSettings();
 
                 continue;
@@ -279,7 +284,13 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
                 SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath());
             }
 
-            Folder *f = addFolderInternal(std::move(folderDefinition), account.data());
+            Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
+            if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
+                // TODO: Must do better error handling
+                qFatal("Could not load plugin");
+            }
+
+            Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), vfs);
             if (f) {
                 // Migration: Mark folders that shall be saved in a backwards-compatible way
                 if (backwardsCompatible)
@@ -494,7 +505,8 @@ Folder *FolderMan::setupFolderFromOldConfigFile(const QString &file, AccountStat
     folderDefinition.paused = paused;
     folderDefinition.ignoreHiddenFiles = ignoreHiddenFiles();
 
-    folder = addFolderInternal(folderDefinition, accountState);
+    Vfs *vfs = nullptr;
+    folder = addFolderInternal(folderDefinition, accountState, vfs);
     if (folder) {
         QStringList blackList = settings.value(QLatin1String("blackList")).toStringList();
         if (!blackList.empty()) {
@@ -982,7 +994,13 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
         return nullptr;
     }
 
-    auto folder = addFolderInternal(definition, accountState);
+    Vfs *vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode, nullptr);
+    if (!vfs && folderDefinition.virtualFilesMode != Vfs::Off) {
+        qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode;
+        return 0;
+    }
+
+    auto folder = addFolderInternal(definition, accountState, vfs);
 
     // Migration: The first account that's configured for a local folder shall
     // be saved in a backwards-compatible way.
@@ -994,6 +1012,7 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
     folder->setSaveBackwardsCompatible(oneAccountOnly);
 
     if (folder) {
+        folder->setSaveBackwardsCompatible(oneAccountOnly);
         folder->saveToSettings();
         emit folderSyncStateChange(folder);
         emit folderListChanged(_folderMap);
@@ -1003,8 +1022,10 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
     return folder;
 }
 
-Folder *FolderMan::addFolderInternal(FolderDefinition folderDefinition,
+Folder *FolderMan::addFolderInternal(
-    AccountState *accountState)
+    FolderDefinition folderDefinition,
+    AccountState *accountState,
+    Vfs *vfs)
 {
     auto alias = folderDefinition.alias;
     int count = 0;
@@ -1015,7 +1036,7 @@ Folder *FolderMan::addFolderInternal(FolderDefinition folderDefinition,
         folderDefinition.alias = alias + QString::number(++count);
     }
 
-    auto folder = new Folder(folderDefinition, accountState, this);
+    auto folder = new Folder(folderDefinition, accountState, vfs, this);
 
     if (_navigationPaneHelper.showInExplorerNavigationPane() && folderDefinition.navigationPaneClsid.isNull()) {
         folder->setNavigationPaneClsid(QUuid::createUuid());

src/gui/folderman.h

@@ -293,7 +293,7 @@ private:
      *  does not set an account on the new folder.
       */
     Folder *addFolderInternal(FolderDefinition folderDefinition,
-        AccountState *accountState);
+        AccountState *accountState, Vfs *vfs);
 
     /* unloads a folder object, does not delete it */
     void unloadFolder(Folder *);

src/gui/folderwizard.cpp

@@ -494,7 +494,7 @@ FolderWizardSelectiveSync::FolderWizardSelectiveSync(const AccountPtr &account)
     _selectiveSync = new SelectiveSyncWidget(account, this);
     layout->addWidget(_selectiveSync);
 
-    if (Theme::instance()->showVirtualFilesOption()) {
+    if (Theme::instance()->showVirtualFilesOption() && bestAvailableVfsMode() != Vfs::Off) {
         _virtualFilesCheckBox = new QCheckBox(tr("Use virtual files instead of downloading content immediately (experimental)"));
         connect(_virtualFilesCheckBox, &QCheckBox::clicked, this, &FolderWizardSelectiveSync::virtualFilesCheckboxClicked);
         connect(_virtualFilesCheckBox, &QCheckBox::stateChanged, this, [this](int state) {

src/gui/owncloudsetupwizard.cpp

@@ -636,8 +636,7 @@ void OwncloudSetupWizard::slotAssistantFinished(int result)
             folderDefinition.targetPath = FolderDefinition::prepareTargetPath(_remoteFolder);
             folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles();
             if (_ocWizard->useVirtualFileSync()) {
-                // ### determine best vfs mode!
+                folderDefinition.virtualFilesMode = bestAvailableVfsMode();
-                folderDefinition.virtualFilesMode = Vfs::WindowsCfApi;
             }
             if (folderMan->navigationPaneHelper().showInExplorerNavigationPane())
                 folderDefinition.navigationPaneClsid = QUuid::createUuid();

src/gui/wizard/owncloudadvancedsetuppage.cpp

@@ -102,7 +102,7 @@ void OwncloudAdvancedSetupPage::initializePage()
 {
     WizardCommon::initErrorLabel(_ui.errorLabel);
 
-    if (!Theme::instance()->showVirtualFilesOption()) {
+    if (!Theme::instance()->showVirtualFilesOption() || bestAvailableVfsMode() == Vfs::Off) {
         // If the layout were wrapped in a widget, the auto-grouping of the
         // radio buttons no longer works and there are surprising margins.
         // Just manually hide the button and remove the layout.

src/gui/wizard/owncloudwizard.cpp

@@ -31,6 +31,8 @@
 #include "wizard/webviewpage.h"
 #include "wizard/flow2authcredspage.h"
 
+#include "common/vfs.h"
+
 #include "QProgressIndicator.h"
 
 #include <QtCore>
@@ -328,16 +330,32 @@ void OwncloudWizard::bringToTop()
 
 void OwncloudWizard::askExperimentalVirtualFilesFeature(const std::function<void(bool enable)> &callback)
 {
-    auto msgBox = new QMessageBox(
+    auto bestVfsMode = bestAvailableVfsMode();
-        QMessageBox::Warning,
+    QMessageBox *msgBox = nullptr;
-        tr("Enable experimental feature?"),
+    if (bestVfsMode == Vfs::WindowsCfApi) {
-        tr("When the \"virtual files\" mode is enabled no files will be downloaded initially. "
+        msgBox = new QMessageBox(
-           "Instead, a tiny \"%1\" file will be created for each file that exists on the server. "
+            QMessageBox::Warning,
-           "The contents can be downloaded by running these files or by using their context menu."
+            tr("Enable experimental feature?"),
-           "\n\n"
+            tr("When the \"virtual files\" mode is enabled no files will be downloaded initially. "
-           "This is a new, experimental mode. If you decide to use it, please report any "
+               "Instead a placeholder file will be created for each file that exists on the server. "
-           "issues that come up.")
+               "When a file is opened its contents will be downloaded automatically. "
-            .arg(APPLICATION_DOTVIRTUALFILE_SUFFIX));
+               "Alternatively files can be downloaded manually by using their context menu."
+               "\n\n"
+               "This is a new, experimental mode. If you decide to use it, please report any "
+               "issues that come up."));
+    } else {
+        ASSERT(bestVfsMode == Vfs::WithSuffix);
+        msgBox = new QMessageBox(
+            QMessageBox::Warning,
+            tr("Enable experimental feature?"),
+            tr("When the \"virtual files\" mode is enabled no files will be downloaded initially. "
+               "Instead, a tiny \"%1\" file will be created for each file that exists on the server. "
+               "The contents can be downloaded by running these files or by using their context menu."
+               "\n\n"
+               "This is a new, experimental mode. If you decide to use it, please report any "
+               "issues that come up.")
+                .arg(APPLICATION_DOTVIRTUALFILE_SUFFIX));
+    }
     msgBox->addButton(tr("Enable experimental mode"), QMessageBox::AcceptRole);
     msgBox->addButton(tr("Stay safe"), QMessageBox::RejectRole);
     connect(msgBox, &QMessageBox::finished, msgBox, [callback, msgBox](int result) {

src/libsync/CMakeLists.txt

@@ -33,7 +33,6 @@ set(libsync_SRCS
     networkjobs.cpp
     owncloudpropagator.cpp
     nextcloudtheme.cpp
-    plugin.cpp
     progressdispatcher.cpp
     propagatorjobs.cpp
     propagatedownload.cpp

src/libsync/logger.h

@@ -23,7 +23,6 @@
 #include <qmutex.h>
 
 #include "common/utility.h"
-#include "logger.h"
 #include "owncloudlib.h"
 
 namespace OCC {

src/libsync/vfs/suffix/vfs_suffix.h

@@ -17,7 +17,7 @@
 #include <QScopedPointer>
 
 #include "common/vfs.h"
-#include "plugin.h"
+#include "common/plugin.h"
 
 namespace OCC {
 

test/testsyncvirtualfiles.cpp

@@ -8,7 +8,6 @@
 #include <QtTest>
 #include "syncenginetestutils.h"
 #include "common/vfs.h"
-#include "plugin.h"
 #include <syncengine.h>
 
 using namespace OCC;
@@ -63,7 +62,7 @@ void markForDehydration(FakeFolder &folder, const QByteArray &path)
 SyncOptions vfsSyncOptions()
 {
     SyncOptions options;
-    options._vfs = PluginLoader().create<Vfs>("vfs", "suffix");
+    options._vfs = createVfsFromPlugin(Vfs::WithSuffix, nullptr);
     return options;
 }