From fa6f3cd8474db46278441e9a069fe9e199d7568c Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 13 Nov 2018 11:46:26 +0100 Subject: [PATCH] vfs: Fix plugin decision in wizards, sanitize loading --- src/common/common.cmake | 1 + src/{libsync => common}/plugin.cpp | 33 ++++++++------- src/{libsync => common}/plugin.h | 26 ++++-------- src/common/vfs.cpp | 36 ++++++++++++++++ src/common/vfs.h | 4 ++ src/gui/accountsettings.cpp | 3 +- src/gui/application.cpp | 10 +++++ src/gui/folder.cpp | 44 +++++++++----------- src/gui/folder.h | 6 ++- src/gui/folderman.cpp | 35 ++++++++++++---- src/gui/folderman.h | 2 +- src/gui/folderwizard.cpp | 2 +- src/gui/owncloudsetupwizard.cpp | 3 +- src/gui/wizard/owncloudadvancedsetuppage.cpp | 2 +- src/gui/wizard/owncloudwizard.cpp | 38 ++++++++++++----- src/libsync/CMakeLists.txt | 1 - src/libsync/logger.h | 1 - src/libsync/vfs/suffix/vfs_suffix.h | 2 +- test/testsyncvirtualfiles.cpp | 3 +- 19 files changed, 164 insertions(+), 88 deletions(-) rename src/{libsync => common}/plugin.cpp (80%) rename src/{libsync => common}/plugin.h (63%) diff --git a/src/common/common.cmake b/src/common/common.cmake index 3caf829f7..88c870933 100644 --- a/src/common/common.cmake +++ b/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 ) diff --git a/src/libsync/plugin.cpp b/src/common/plugin.cpp similarity index 80% rename from src/libsync/plugin.cpp rename to src/common/plugin.cpp index f3c4c857d..59e570543 100644 --- a/src/libsync/plugin.cpp +++ b/src/common/plugin.cpp @@ -19,10 +19,10 @@ #include "plugin.h" #include "config.h" -#include "logger.h" #include #include +#include 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(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(plugin) { + if (pluginLoader.load()) { 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(load(type, name)); + if (!factory) { + return nullptr; + } else { + return factory->create(parent); + } +} + } diff --git a/src/libsync/plugin.h b/src/common/plugin.h similarity index 63% rename from src/libsync/plugin.h rename to src/common/plugin.h index e8f8aec44..b6471f88f 100644 --- a/src/libsync/plugin.h +++ b/src/common/plugin.h @@ -18,13 +18,13 @@ #pragma once -#include "owncloudlib.h" +#include "ocsynclib.h" #include #include namespace OCC { -class OWNCLOUDSYNC_EXPORT PluginFactory +class OCSYNC_EXPORT PluginFactory { public: ~PluginFactory(); @@ -35,32 +35,20 @@ template 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 - PLUGIN_CLASS *create(Args&& ... args) - { - return qobject_cast(createInternal(std::forward(args)...)); - } - -private: - template - FACTORY_CLASS *load(Args&& ... args) - { - return qobject_cast(loadPluginInternal(std::forward(args)...)); - } - - QObject *loadPluginInternal(const QString& type, const QString &name); - QObject *createInternal(const QString& type, const QString &name, QObject* parent = nullptr); + bool isAvailable(const QString &type, const QString &name); + QObject *load(const QString &type, const QString &name); + QObject *create(const QString &type, const QString &name, QObject *parent = nullptr); }; } diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index b6f4646eb..7cfb895e1 100644 --- a/src/common/vfs.cpp +++ b/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(PluginLoader().create("vfs", name, parent)); +} diff --git a/src/common/vfs.h b/src/common/vfs.h index 511551c87..23e11a0c5 100644 --- a/src/common/vfs.h +++ b/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 diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 630a47f59..e4dbe7256 100644 --- a/src/gui/accountsettings.cpp +++ b/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 = Vfs::WindowsCfApi; + definition.virtualFilesMode = bestAvailableVfsMode(); } { diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 310f055c1..e25f1c280 100644 --- a/src/gui/application.cpp +++ b/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); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 3f284938c..1d9bbc0f3 100644 --- a/src/gui/folder.cpp +++ b/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 @@ -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? - PluginLoader pluginLoader; - if (_definition.virtualFilesMode == Vfs::WindowsCfApi) { - _vfs = pluginLoader.create("vfs", "win", this); - } - if (_definition.virtualFilesMode == Vfs::WithSuffix) { - _vfs = pluginLoader.create("vfs", "suffix", this); - - // Attempt to switch to winvfs mode? - if (_vfs && _definition.upgradeVfsMode) { - if (auto winvfs = pluginLoader.create("vfs", "win", this)) { - // Set "suffix" vfs options and wipe the existing suffix files - SyncEngine::wipeVirtualFiles(path(), _journal, _vfs); - - // Then switch to winvfs mode - _vfs = winvfs; - _definition.virtualFilesMode = Vfs::WindowsCfApi; - saveToSettings(); - } + // Potentially upgrade suffix vfs to windows vfs + if (_definition.virtualFilesMode == Vfs::WithSuffix && _definition.upgradeVfsMode) { + if (auto winvfs = createVfsFromPlugin(Vfs::WindowsCfApi, this)) { + // Wipe the existing suffix files from fs and journal + SyncEngine::wipeVirtualFiles(path(), _journal, _vfs); + + // Then switch to winvfs mode + delete _vfs; + _vfs = winvfs; + _definition.virtualFilesMode = Vfs::WindowsCfApi; + saveToSettings(); } } + + // Initialize the vfs plugin if (_definition.virtualFilesMode != Vfs::Off) { - if (!_vfs) { - // ### error handling; possibly earlier than in the ctor - qFatal("Could not load any vfs plugin."); - } + ENFORCE(_vfs); + ENFORCE(_vfs->mode() == _definition.virtualFilesMode); + + _vfs->setParent(this); VfsSetupParams vfsParams; vfsParams.filesystemPath = path(); diff --git a/src/gui/folder.h b/src/gui/folder.h index b266ed402..ed6e1a314 100644 --- a/src/gui/folder.h +++ b/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(); diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 43ceae787..501d5b7ec 100644 --- a/src/gui/folderman.cpp +++ b/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, - AccountState *accountState) +Folder *FolderMan::addFolderInternal( + 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()); diff --git a/src/gui/folderman.h b/src/gui/folderman.h index e083d158f..bc49167b0 100644 --- a/src/gui/folderman.h +++ b/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 *); diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 47603ed50..32cfc7b61 100644 --- a/src/gui/folderwizard.cpp +++ b/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) { diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index d57c7e1a1..eb6656c76 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/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 = Vfs::WindowsCfApi; + folderDefinition.virtualFilesMode = bestAvailableVfsMode(); } if (folderMan->navigationPaneHelper().showInExplorerNavigationPane()) folderDefinition.navigationPaneClsid = QUuid::createUuid(); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index 09e392883..dadbd0735 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/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. diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index 4c9f4e6c1..7079f2fc1 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/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 @@ -328,16 +330,32 @@ void OwncloudWizard::bringToTop() void OwncloudWizard::askExperimentalVirtualFilesFeature(const std::function &callback) { - auto 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)); + auto bestVfsMode = bestAvailableVfsMode(); + QMessageBox *msgBox = nullptr; + if (bestVfsMode == Vfs::WindowsCfApi) { + 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 placeholder file will be created for each file that exists on the server. " + "When a file is opened its contents will be downloaded automatically. " + "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) { diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index bea212d10..353acf3d7 100644 --- a/src/libsync/CMakeLists.txt +++ b/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 diff --git a/src/libsync/logger.h b/src/libsync/logger.h index 12973c71c..c8ff3bce8 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -23,7 +23,6 @@ #include #include "common/utility.h" -#include "logger.h" #include "owncloudlib.h" namespace OCC { diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h index d66f95a7f..0387832c9 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.h +++ b/src/libsync/vfs/suffix/vfs_suffix.h @@ -17,7 +17,7 @@ #include #include "common/vfs.h" -#include "plugin.h" +#include "common/plugin.h" namespace OCC { diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp index 5a271fd0f..a90f006c9 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -8,7 +8,6 @@ #include #include "syncenginetestutils.h" #include "common/vfs.h" -#include "plugin.h" #include using namespace OCC; @@ -63,7 +62,7 @@ void markForDehydration(FakeFolder &folder, const QByteArray &path) SyncOptions vfsSyncOptions() { SyncOptions options; - options._vfs = PluginLoader().create("vfs", "suffix"); + options._vfs = createVfsFromPlugin(Vfs::WithSuffix, nullptr); return options; } -- 2.30.2