diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 29fb9d3d133..841388c94c7 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -141,16 +141,7 @@ void FileSystem::setFileReadOnly(const QString &filename, bool readonly) file.setPermissions(permissions); } -void FileSystem::setFolderMinimumPermissions(const QString &filename) -{ -#ifdef Q_OS_MAC - QFile::Permissions perm = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; - QFile file(filename); - file.setPermissions(perm); -#else - Q_UNUSED(filename); -#endif -} +void FileSystem::setFolderMinimumPermissions(const QString &filename) { } void FileSystem::setFileReadOnlyWeak(const QString &filename, bool readonly) diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index 76eaeb4b995..f5556f25e2c 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -54,8 +54,7 @@ namespace FileSystem { * List of characters not allowd in filenames on Windows */ constexpr_list auto IllegalFilenameCharsWindows = { - QLatin1Char('\\'), QLatin1Char(':'), QLatin1Char('?'), QLatin1Char('*'), QLatin1Char('"'), QLatin1Char('>'), QLatin1Char('<'), QLatin1Char('|') - }; + QLatin1Char('\\'), QLatin1Char(':'), QLatin1Char('?'), QLatin1Char('*'), QLatin1Char('"'), QLatin1Char('>'), QLatin1Char('<'), QLatin1Char('|')}; /** * @brief Mark the file as hidden (only has effects on windows) diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 858107c23bb..2a1f1bac33e 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -46,7 +46,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject { Q_OBJECT public: - explicit SyncJournalDb(const QString &dbFilePath, QObject *parent = nullptr); + explicit SyncJournalDb(const QString &dbFilePath, QObject *parent); ~SyncJournalDb() override; /// Create a journal path for a specific configuration diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index f185942e261..1d759309bda 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -236,7 +236,7 @@ Vfs::Mode OCC::VfsPluginManager::bestAvailableVfsMode() const return Vfs::Off; } -std::unique_ptr OCC::VfsPluginManager::createVfsFromPlugin(Vfs::Mode mode) const +Vfs *OCC::VfsPluginManager::createVfsFromPlugin(Vfs::Mode mode, QObject *parent) const { auto name = Utility::enumToString(mode); if (name.isEmpty()) @@ -261,7 +261,7 @@ std::unique_ptr OCC::VfsPluginManager::createVfsFromPlugin(Vfs::Mode mode) return nullptr; } - auto vfs = std::unique_ptr(qobject_cast(factory->create(nullptr))); + Vfs *vfs = qobject_cast(factory->create(parent)); if (!vfs) { qCCritical(lcPlugin) << "Plugin" << loader.fileName() << "does not create a Vfs instance"; return nullptr; diff --git a/src/common/vfs.h b/src/common/vfs.h index 597177555fd..1e9f19b2ffe 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -141,7 +141,7 @@ class OCSYNC_EXPORT Vfs : public QObject using AvailabilityResult = Result; public: - explicit Vfs(QObject* parent = nullptr); + explicit Vfs(QObject *parent); ~Vfs() override; virtual Mode mode() const = 0; @@ -302,7 +302,7 @@ class OCSYNC_EXPORT VfsPluginManager Vfs::Mode bestAvailableVfsMode() const; /// Create a VFS instance for the mode, returns nullptr on failure. - std::unique_ptr createVfsFromPlugin(Vfs::Mode mode) const; + Vfs *createVfsFromPlugin(Vfs::Mode mode, QObject *parent) const; static const VfsPluginManager &instance(); diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 77fb3c3129c..3a3a8742ddc 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -118,6 +118,7 @@ add_subdirectory(newaccountwizard) add_subdirectory(socketapi) add_subdirectory(spaces) add_subdirectory(FoldersGui) +add_subdirectory(foldermanagement) target_include_directories(owncloudGui PUBLIC ${CMAKE_SOURCE_DIR}/src/3rdparty/QProgressIndicator diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index d562fbd7f02..012c052afa7 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -28,6 +28,7 @@ #include "configfile.h" #include "filesystem.h" #include "folderman.h" +#include "foldermanagement/foldermanagementutils.h" #include "folderwatcher.h" #include "libsync/graphapi/spacesmanager.h" #include "localdiscoverytracker.h" @@ -91,14 +92,26 @@ using namespace FileSystem::SizeLiterals; Q_LOGGING_CATEGORY(lcFolder, "gui.folder", QtInfoMsg) -Folder::Folder(const FolderDefinition &definition, AccountState *accountState, std::unique_ptr &&vfs, bool ignoreHiddenFiles, QObject *parent) +Folder::Folder(const FolderDefinition &definition, AccountState *accountState, SyncJournalDb *journal, Vfs *vfs, bool ignoreHiddenFiles, QObject *parent) : QObject(parent) , _accountState(accountState) , _definition(definition) - , _journal(_definition.absoluteJournalPath()) , _fileLog(new SyncRunFileLog) - , _vfs(vfs.release()) { + journal->setParent(this); + _journal = journal; + + // this is temporary! we only clear the parent on the vfs pointer because for the time being, we are wrapping this in a shared pointer + // the shared pointer is marked for death, then we will be able to set the vfs parent to this, as it should be + vfs->setParent(this); + _vfs = vfs; + + // the FolderBuilder should fail if the account state or account are dead, so this should never trigger + Q_ASSERT(_accountState && _accountState->account()); + // FolderBuilder should also fail if it could not build the other dependencies so again, assert should never trigger + Q_ASSERT(_vfs); + Q_ASSERT(_journal); + _timeSinceLastSyncStart.start(); _timeSinceLastSyncDone.start(); @@ -107,60 +120,52 @@ Folder::Folder(const FolderDefinition &definition, AccountState *accountState, s status = SyncResult::Paused; } setSyncState(status); - // check if the starting conditions are legit - if (_accountState && _accountState->account() && checkLocalPath()) { - prepareFolder(path()); - // those errors should not persist over sessions - _journal.wipeErrorBlacklistCategory(SyncJournalErrorBlacklistRecord::Category::LocalSoftError); - // todo: the engine needs to be created externally, presumably by the folderman, and passed in by injection - // current impl can result in an invalid engine which is just a mess given the folder is useless without it - _engine.reset(new SyncEngine(_accountState->account(), webDavUrl(), path(), remotePath(), &_journal)); - // pass the setting if hidden files are to be ignored, will be read in csync_update - _engine->setIgnoreHiddenFiles(ignoreHiddenFiles); - - if (!_engine->loadDefaultExcludes()) { - qCWarning(lcFolder, "Could not read system exclude file"); - } - connect(_accountState, &AccountState::isConnectedChanged, this, &Folder::canSyncChanged); + // todo: the engine needs to be created externally, presumably by the folderman, and passed in by injection + // current impl can result in an invalid engine which is just a mess given the folder is useless without it + _engine.reset(new SyncEngine(_accountState->account(), webDavUrl(), path(), remotePath(), _journal)); + // pass the setting if hidden files are to be ignored, will be read in csync_update + _engine->setIgnoreHiddenFiles(ignoreHiddenFiles); + ConfigFile cfgFile; + _engine->setMoveToTrash(cfgFile.moveToTrash()); - connect(_engine.data(), &SyncEngine::started, this, &Folder::slotSyncStarted, Qt::QueuedConnection); - connect(_engine.data(), &SyncEngine::finished, this, &Folder::slotSyncFinished, Qt::QueuedConnection); + if (!_engine->loadDefaultExcludes()) { + qCWarning(lcFolder, "Could not read system exclude file"); + } - connect(_engine.data(), &SyncEngine::transmissionProgress, this, - [this](const ProgressInfo &pi) { Q_EMIT ProgressDispatcher::instance()->progressInfo(this, pi); }); + connect(_accountState, &AccountState::isConnectedChanged, this, &Folder::canSyncChanged); - connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::progressUpdate); + connect(_engine.data(), &SyncEngine::started, this, &Folder::slotSyncStarted, Qt::QueuedConnection); + connect(_engine.data(), &SyncEngine::finished, this, &Folder::slotSyncFinished, Qt::QueuedConnection); - connect(_engine.data(), &SyncEngine::itemCompleted, this, &Folder::slotItemCompleted); - connect(_engine.data(), &SyncEngine::seenLockedFile, FolderMan::instance(), &FolderMan::slotSyncOnceFileUnlocks); - connect(_engine.data(), &SyncEngine::aboutToPropagate, - this, &Folder::slotLogPropagationStart); - connect(_engine.data(), &SyncEngine::syncError, this, &Folder::slotSyncError); + connect(_engine.data(), &SyncEngine::transmissionProgress, this, + [this](const ProgressInfo &pi) { Q_EMIT ProgressDispatcher::instance()->progressInfo(this, pi); }); - connect(ProgressDispatcher::instance(), &ProgressDispatcher::folderConflicts, - this, &Folder::slotFolderConflicts); - connect(_engine.data(), &SyncEngine::excluded, this, [this](const QString &path) { Q_EMIT ProgressDispatcher::instance()->excluded(this, path); }); + connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::progressUpdate); - _localDiscoveryTracker.reset(new LocalDiscoveryTracker); - connect(_engine.data(), &SyncEngine::finished, - _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotSyncFinished); - connect(_engine.data(), &SyncEngine::itemCompleted, - _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotItemCompleted); + connect(_engine.data(), &SyncEngine::itemCompleted, this, &Folder::slotItemCompleted); + connect(_engine.data(), &SyncEngine::seenLockedFile, FolderMan::instance(), &FolderMan::slotSyncOnceFileUnlocks); + connect(_engine.data(), &SyncEngine::aboutToPropagate, this, &Folder::slotLogPropagationStart); + connect(_engine.data(), &SyncEngine::syncError, this, &Folder::slotSyncError); - connect(_accountState->account()->spacesManager(), &GraphApi::SpacesManager::spaceChanged, this, [this](GraphApi::Space *changedSpace) { - if (_definition.spaceId() == changedSpace->id()) { - emit spaceChanged(); - } - }); - if (space()) + connect(ProgressDispatcher::instance(), &ProgressDispatcher::folderConflicts, this, &Folder::slotFolderConflicts); + connect(_engine.data(), &SyncEngine::excluded, this, [this](const QString &path) { Q_EMIT ProgressDispatcher::instance()->excluded(this, path); }); + + _localDiscoveryTracker.reset(new LocalDiscoveryTracker); + connect(_engine.data(), &SyncEngine::finished, _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotSyncFinished); + connect(_engine.data(), &SyncEngine::itemCompleted, _localDiscoveryTracker.data(), &LocalDiscoveryTracker::slotItemCompleted); + + connect(_accountState->account()->spacesManager(), &GraphApi::SpacesManager::spaceChanged, this, [this](GraphApi::Space *changedSpace) { + if (_definition.spaceId() == changedSpace->id()) { emit spaceChanged(); + } + }); - // Potentially upgrade suffix vfs to windows vfs - OC_ENFORCE(_vfs); - // Initialize the vfs plugin. Do this after the UI is running, so we can show a dialog when something goes wrong. - QTimer::singleShot(0, this, &Folder::startVfs); - } + if (space()) + emit spaceChanged(); + + // Initialize the vfs plugin. Do this after the UI is running, so we can show a dialog when something goes wrong. + QTimer::singleShot(0, this, &Folder::startVfs); } Folder::~Folder() @@ -173,20 +178,6 @@ Folder::~Folder() _engine.reset(); } -Result Folder::checkPathLength(const QString &path) -{ -#ifdef Q_OS_WIN - if (path.size() > MAX_PATH) { - if (!FileSystem::longPathsEnabledOnWindows()) { - return tr("The path '%1' is too long. Please enable long paths in the Windows settings or choose a different folder.").arg(path); - } - } -#else - Q_UNUSED(path) -#endif - return {}; -} - GraphApi::Space *Folder::space() const { if (_accountState && _accountState->account() && _accountState->account()->spacesManager()) { @@ -195,106 +186,22 @@ GraphApi::Space *Folder::space() const return nullptr; } -bool Folder::checkLocalPath() -{ -#ifdef Q_OS_WIN - QNtfsPermissionCheckGuard ntfs_perm; -#endif - const QFileInfo fi(_definition.localPath()); - _canonicalLocalPath = fi.canonicalFilePath(); -#ifdef Q_OS_MAC - // Workaround QTBUG-55896 (Should be fixed in Qt 5.8) - _canonicalLocalPath = _canonicalLocalPath.normalized(QString::NormalizationForm_C); -#endif - if (_canonicalLocalPath.isEmpty()) { - qCWarning(lcFolder) << "Broken symlink:" << _definition.localPath(); - _canonicalLocalPath = _definition.localPath(); - } else if (!_canonicalLocalPath.endsWith(QLatin1Char('/'))) { - _canonicalLocalPath.append(QLatin1Char('/')); - } - - QString error; - if (fi.isDir() && fi.isReadable() && fi.isWritable()) { - auto pathLengthCheck = checkPathLength(_canonicalLocalPath); - if (!pathLengthCheck) { - error = pathLengthCheck.error(); - } - - if (error.isEmpty()) { - qCDebug(lcFolder) << "Checked local path ok"; - if (!_journal.open()) { - error = tr("%1 failed to open the database.").arg(_definition.localPath()); - } - } - } else { - // Check directory again - if (!FileSystem::fileExists(_definition.localPath(), fi)) { - error = tr("Local folder %1 does not exist.").arg(_definition.localPath()); - } else if (!fi.isDir()) { - error = tr("%1 should be a folder but is not.").arg(_definition.localPath()); - } else if (!fi.isReadable()) { - error = tr("%1 is not readable.").arg(_definition.localPath()); - } else if (!fi.isWritable()) { - error = tr("%1 is not writable.").arg(_definition.localPath()); - } - } - if (!error.isEmpty()) { - qCWarning(lcFolder) << error; - _syncResult.appendErrorString(error); - setSyncState(SyncResult::SetupError); - return false; - } - return true; -} - SyncOptions Folder::loadSyncOptions() { + if (!_accountState || !_accountState->account() || !_vfs) + return SyncOptions(); + SyncOptions opt(_vfs); - ConfigFile cfgFile; + opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6; - opt._moveFilesToTrash = cfgFile.moveToTrash(); - // got a nullptr hit here - this is so shady but best I can do for now - opt._parallelNetworkJobs = (_accountState && _accountState->account() && _accountState->account()->isHttp2Supported()) ? 20 : 6; + // apparently the env can override the default + int maxParallel = qEnvironmentVariableIntValue("OWNCLOUD_MAX_PARALLEL"); + if (maxParallel > 0) + opt._parallelNetworkJobs = maxParallel; - opt.fillFromEnvironmentVariables(); return opt; } -void Folder::prepareFolder(const QString &path) -{ -#ifdef Q_OS_WIN - // First create a Desktop.ini so that the folder and favorite link show our application's icon. - const QFileInfo desktopIniPath{QStringLiteral("%1/Desktop.ini").arg(path)}; - { - const QString updateIconKey = QStringLiteral("%1/UpdateIcon").arg(Theme::instance()->appName()); - QSettings desktopIni(desktopIniPath.absoluteFilePath(), QSettings::IniFormat); - if (desktopIni.value(updateIconKey, true).toBool()) { - qCInfo(lcFolder) << "Creating" << desktopIni.fileName() << "to set a folder icon in Explorer."; - desktopIni.setValue(QStringLiteral(".ShellClassInfo/IconResource"), QDir::toNativeSeparators(qApp->applicationFilePath())); - desktopIni.setValue(updateIconKey, true); - } else { - qCInfo(lcFolder) << "Skip icon update for" << desktopIni.fileName() << "," << updateIconKey << "is disabled"; - } - } - - const QString longFolderPath = FileSystem::longWinPath(path); - const QString longDesktopIniPath = FileSystem::longWinPath(desktopIniPath.absoluteFilePath()); - // Set the folder as system and Desktop.ini as hidden+system for explorer to pick it. - // https://msdn.microsoft.com/en-us/library/windows/desktop/cc144102 - const DWORD folderAttrs = GetFileAttributesW(reinterpret_cast(longFolderPath.utf16())); - if (!SetFileAttributesW(reinterpret_cast(longFolderPath.utf16()), folderAttrs | FILE_ATTRIBUTE_SYSTEM)) { - const auto error = GetLastError(); - qCWarning(lcFolder) << "SetFileAttributesW failed on" << longFolderPath << Utility::formatWinError(error); - } - if (!SetFileAttributesW(reinterpret_cast(longDesktopIniPath.utf16()), FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) { - const auto error = GetLastError(); - qCWarning(lcFolder) << "SetFileAttributesW failed on" << longDesktopIniPath << Utility::formatWinError(error); - } -#else - Q_UNUSED(path) -#endif -} - QString Folder::displayName() const { if (auto *s = space()) { @@ -323,7 +230,7 @@ QString Folder::shortGuiLocalPath() const QString Folder::cleanPath() const { - QString cleanedPath = QDir::cleanPath(_canonicalLocalPath); + QString cleanedPath = QDir::cleanPath(_definition.canonicalPath()); if (cleanedPath.length() == 3 && cleanedPath.endsWith(QLatin1String(":/"))) cleanedPath.remove(2, 1); @@ -538,25 +445,25 @@ void Folder::startVfs() VfsSetupParams vfsParams(_accountState->account(), webDavUrl(), _engine.get()); vfsParams.filesystemPath = path(); vfsParams.remotePath = remotePathTrailingSlash(); - vfsParams.journal = &_journal; + vfsParams.journal = _journal; vfsParams.providerDisplayName = Theme::instance()->appNameGUI(); vfsParams.providerName = Theme::instance()->appName(); vfsParams.providerVersion = Version::version(); vfsParams.multipleAccountsRegistered = AccountManager::instance()->accounts().size() > 1; - connect(&_engine->syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, _vfs.get(), &Vfs::fileStatusChanged); + connect(&_engine->syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged, _vfs, &Vfs::fileStatusChanged); - connect(_vfs.get(), &Vfs::started, this, [this] { + connect(_vfs, &Vfs::started, this, [this] { // Immediately mark the sqlite temporaries as excluded. They get recreated // on db-open and need to get marked again every time. - QString stateDbFile = _journal.databaseFilePath(); + QString stateDbFile = _journal->databaseFilePath(); _vfs->fileStatusChanged(stateDbFile + QStringLiteral("-wal"), SyncFileStatus::StatusExcluded); _vfs->fileStatusChanged(stateDbFile + QStringLiteral("-shm"), SyncFileStatus::StatusExcluded); _engine->setSyncOptions(loadSyncOptions()); registerFolderWatcher(); - connect(_vfs.get(), &Vfs::needSync, this, [this] { + connect(_vfs, &Vfs::needSync, this, [this] { if (canSync()) { // the vfs plugin detected that its metadata is out of sync and requests a new sync // the request has a high priority as it is probably issued after a user request @@ -573,7 +480,7 @@ void Folder::startVfs() FolderMan::instance()->scheduler()->enqueueFolder(this); } }); - connect(_vfs.get(), &Vfs::error, this, [this](const QString &error) { + connect(_vfs, &Vfs::error, this, [this](const QString &error) { _syncResult.appendErrorString(error); setSyncState(SyncResult::SetupError); _vfsIsReady = false; @@ -588,8 +495,7 @@ void Folder::slotDiscardDownloadProgress() // Delete from journal and from filesystem. QDir folderpath(_definition.localPath()); QSet keep_nothing; - const QVector deleted_infos = - _journal.getAndDeleteStaleDownloadInfos(keep_nothing); + const QVector deleted_infos = _journal->getAndDeleteStaleDownloadInfos(keep_nothing); for (const auto &deleted_info : deleted_infos) { const QString tmppath = folderpath.filePath(deleted_info._tmpfile); qCInfo(lcFolder) << "Deleting temporary file: " << tmppath; @@ -599,7 +505,7 @@ void Folder::slotDiscardDownloadProgress() int Folder::slotWipeErrorBlacklist() { - return _journal.wipeErrorBlacklist(); + return _journal->wipeErrorBlacklist(); } void Folder::slotWatchedPathsChanged(const QSet &paths, ChangeReason reason) @@ -640,7 +546,7 @@ void Folder::slotWatchedPathsChanged(const QSet &paths, ChangeReason re _localDiscoveryTracker->addTouchedPath(relativePath); SyncJournalFileRecord record; - _journal.getFileRecord(relativePath.toUtf8(), &record); + _journal->getFileRecord(relativePath.toUtf8(), &record); if (reason != ChangeReason::UnLock) { // Check that the mtime/size actually changed or there was // an attribute change (pin state) that caused the notification @@ -676,7 +582,7 @@ void Folder::implicitlyHydrateFile(const QString &relativepath) // Set in the database that we should download the file SyncJournalFileRecord record; - _journal.getFileRecord(relativepath.toUtf8(), &record); + _journal->getFileRecord(relativepath.toUtf8(), &record); if (!record.isValid()) { qCInfo(lcFolder) << "Did not find file in db"; return; @@ -686,7 +592,7 @@ void Folder::implicitlyHydrateFile(const QString &relativepath) return; } record._type = ItemTypeVirtualFileDownload; - _journal.setFileRecord(record); + _journal->setFileRecord(record); // Change the file's pin state if it's contradictory to being hydrated // (suffix-virtual file's pin state is stored at the hydrated path) @@ -730,8 +636,14 @@ void Folder::changeVfsMode(Vfs::Mode newMode) if (newMode == _definition.virtualFilesMode()) { return; } + // if we can't create the new mode, just ditch. + Vfs *newVfs = VfsPluginManager::instance().createVfsFromPlugin(newMode, this); + if (!newVfs) { + qCWarning(lcFolder) << "Unable to change vfs mode for Folder " << _definition.localPath() << " from " << _definition.virtualFilesMode() << " to " + << newMode << ". Leaving the original mode active."; + return; + } - // This is tested in TestSyncVirtualFiles::testWipeVirtualSuffixFiles, so for changes here, have them reflected in that test. const bool wasPaused = _definition.paused(); if (!wasPaused) { setSyncPaused(true); @@ -757,22 +669,26 @@ void Folder::changeVfsMode(Vfs::Mode newMode) _vfsIsReady = false; _vfs->stop(); _vfs->unregisterFolder(); - disconnect(_vfs.get(), nullptr, this, nullptr); - disconnect(&_engine->syncFileStatusTracker(), nullptr, _vfs.get(), nullptr); + disconnect(_vfs, nullptr, this, nullptr); + disconnect(&_engine->syncFileStatusTracker(), nullptr, _vfs, nullptr); // _vfs is a shared pointer... // Refactor todo: who is it shared with? It appears to be shared with the SyncOptions. SyncOptions instance is then // passed to the engine. It is not clear to me how/when the options vfs shared ptr gets updated to match this // new/reset instance but this should be high prio to work this out as wow this is dangerous. the todo is basically: eval the use of // this _vfs pointer and make it consistent and SAFE across uses - _vfs.reset(VfsPluginManager::instance().createVfsFromPlugin(newMode).release()); + // also todo: we have to cover the case that the createVfsFromPlugin returns nullptr! + Vfs *oldVfs = _vfs; + _vfs = newVfs; + oldVfs->deleteLater(); // Restart VFS. _definition.setVirtualFilesMode(newMode); + // note this is "on top of" started slot handling defined in startVfs if (newMode != Vfs::Off) { // schedule blacklisted folders for rediscovery - connect(_vfs.get(), &Vfs::started, this, [oldBlacklist, this] { + connect(_vfs, &Vfs::started, this, [oldBlacklist, this] { for (const auto &entry : oldBlacklist) { journalDb()->schedulePathForRemoteDiscovery(entry); // Refactoring todo: from what I can see, in 98% of cases the return val of setPinState is ignored @@ -837,7 +753,7 @@ void Folder::wipeForRemoval() // Unregister the socket API so it does not keep the .sync_journal file open FolderMan::instance()->socketApi()->slotUnregisterPath(this); - _journal.close(); // close the sync journal + _journal->close(); // close the sync journal // Remove db and temporaries const QString stateDbFile = _engine->journal()->databaseFilePath(); @@ -861,7 +777,8 @@ void Folder::wipeForRemoval() _vfs->stop(); _vfs->unregisterFolder(); - _vfs.reset(nullptr); // warning: folder now in an invalid state + + // don't kill vfs pointer, as it is a child of the Folder we should let it be auto-deleted by normal qobject destruction sequence. } bool Folder::reloadExcludes() @@ -936,9 +853,9 @@ void Folder::startSync() Q_EMIT syncStarted(); } -void Folder::reloadSyncOptions() +void Folder::setMoveToTrash(bool trashIt) { - _engine->setSyncOptions(loadSyncOptions()); + _engine->setMoveToTrash(trashIt); } void Folder::slotSyncError(const QString &message, ErrorCategory category) @@ -1085,12 +1002,12 @@ void Folder::warnOnNewExcludedItem(const SyncJournalFileRecord &record, QStringV // Note: This assumes we're getting file watcher notifications // for folders only on creation and deletion - if we got a notification // on content change that would create spurious warnings. - QFileInfo fi(_canonicalLocalPath + path); + QFileInfo fi(_definition.canonicalPath() + path); if (!fi.exists()) return; bool ok = false; - auto blacklist = _journal.getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, ok); + auto blacklist = _journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, ok); if (!ok) return; if (!blacklist.contains(path + QLatin1Char('/'))) @@ -1228,6 +1145,22 @@ void FolderDefinition::setLocalPath(const QString &path) if (!_localPath.endsWith(QLatin1Char('/'))) { _localPath.append(QLatin1Char('/')); } + + QFileInfo fi(_localPath); + _canonicalLocalPath = fi.canonicalFilePath(); + // commenting this out til I can verify it can really go away now. I checked the bug report and it was filed against QFileSystemWatcher, + // so I don't understand what it has to do with deriving the canonical path in the first place? + // #ifdef Q_OS_MAC + // Workaround QTBUG-55896 (Should be fixed in Qt 5.8) + // _canonicalLocalPath = _canonicalLocalPath.normalized(QString::NormalizationForm_C); + // #endif + if (_canonicalLocalPath.isEmpty()) { + qCWarning(lcFolder) << "Broken symlink:" << _localPath; + _canonicalLocalPath = _localPath; + } else if (!_canonicalLocalPath.endsWith(QLatin1Char('/'))) { + // the canonicalPath function strips off the trailing separator so we have to add it back, apparently + _canonicalLocalPath.append(QLatin1Char('/')); + } } void FolderDefinition::setTargetPath(const QString &path) diff --git a/src/gui/folder.h b/src/gui/folder.h index 380fadf39ac..060104f8536 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -45,10 +45,12 @@ class SyncRunFileLog; class FolderWatcher; class LocalDiscoveryTracker; + /** * @brief The FolderDefinition class * @ingroup gui */ + class OWNCLOUDGUI_EXPORT FolderDefinition { public: @@ -83,9 +85,12 @@ class OWNCLOUDGUI_EXPORT FolderDefinition void setVirtualFilesMode(Vfs::Mode mode) { _virtualFilesMode = mode; } /// Ensure / as separator and trailing /. + /// calling set here also ensures the canonical local path is properly created void setLocalPath(const QString &path); QString localPath() const { return _localPath; } + QString canonicalPath() const { return _canonicalLocalPath; } + /// Remove ending /, then ensure starting '/': so "/foo/bar" and "/". void setTargetPath(const QString &path); @@ -141,6 +146,8 @@ class OWNCLOUDGUI_EXPORT FolderDefinition QString _displayName; /// path on local machine (always trailing /) QString _localPath; + /// canonical local path + QString _canonicalLocalPath; /// path on remote (usually no trailing /, exception "/") QString _targetPath; bool _deployed = false; @@ -174,11 +181,10 @@ class OWNCLOUDGUI_EXPORT Folder : public QObject }; Q_ENUM(ChangeReason) - static void prepareFolder(const QString &path); /** Create a new Folder */ - Folder(const FolderDefinition &definition, AccountState *accountState, std::unique_ptr &&vfs, bool ignoreHiddenFiles, QObject *parent = nullptr); + Folder(const FolderDefinition &definition, AccountState *accountState, SyncJournalDb *journal, Vfs *vfs, bool ignoreHiddenFiles, QObject *parent); ~Folder() override; /** @@ -224,7 +230,7 @@ class OWNCLOUDGUI_EXPORT Folder : public QObject /** * canonical local folder path, always ends with / */ - QString path() const { return _canonicalLocalPath; } + QString path() const { return _definition.canonicalPath(); } /** * cleaned canonical folder path, like path() but never ends with a / @@ -300,14 +306,11 @@ class OWNCLOUDGUI_EXPORT Folder : public QObject void setSyncState(SyncResult::Status state); - void reloadSyncOptions(); + void setMoveToTrash(bool trashIt); // TODO: don't expose - SyncJournalDb *journalDb() - { - return &_journal; - } + SyncJournalDb *journalDb() { return _journal; } // TODO: don't expose SyncEngine &syncEngine() { @@ -353,8 +356,6 @@ class OWNCLOUDGUI_EXPORT Folder : public QObject uint32_t sortPriority() const { return _definition.priority(); } - static Result checkPathLength(const QString &path); - /** * * @return The corresponding space object or null @@ -462,8 +463,6 @@ private Q_SLOTS: private: void showSyncResultPopup(); - bool checkLocalPath(); - SyncOptions loadSyncOptions(); /** @@ -497,7 +496,7 @@ private Q_SLOTS: QPointer _accountState; FolderDefinition _definition; - QString _canonicalLocalPath; // As returned with QFileInfo:canonicalFilePath. Always ends with "/" + // QString _canonicalLocalPath; // As returned with QFileInfo:canonicalFilePath. Always ends with "/" SyncResult _syncResult; QScopedPointer _engine; @@ -514,7 +513,7 @@ private Q_SLOTS: /// Reset when no follow-up is requested. int _consecutiveFollowUpSyncs = 0; - mutable SyncJournalDb _journal; + SyncJournalDb *_journal = nullptr; QScopedPointer _fileLog; @@ -562,6 +561,8 @@ private Q_SLOTS: // it is also false that it is never null - it is reset in wipeForRemoval // extra fun is I have no idea what happens to the instance in the SyncOptions - is it still alive relative to the engine? // I don't see any handling of the engine or SyncOptions whatsoever in wipeForRemoval so we'll need to go spelunking. - QSharedPointer _vfs; + // final endpoint is to make this a raw pointer, pass it around and let the deps wrap it in a qpointer + // reconsider parenting before this step is taken + Vfs *_vfs; }; } diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 687191c6a1f..ebcfe0a4cdd 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -14,13 +14,15 @@ #include "folderman.h" +#include "accessmanager.h" #include "account.h" #include "accountmanager.h" #include "accountstate.h" -#include "accessmanager.h" #include "common/asserts.h" #include "configfile.h" #include "folder.h" +#include "foldermanagement/folderbuilder.h" +#include "foldermanagement/foldermanagementutils.h" #include "gui/networkinformation.h" #include "guiutility.h" #include "libsync/syncengine.h" @@ -45,13 +47,11 @@ using namespace std::chrono; using namespace std::chrono_literals; namespace { -qsizetype numberOfSyncJournals(const QString &path) -{ - return QDir(path).entryList({QStringLiteral(".sync_*.db"), QStringLiteral("._sync_*.db")}, QDir::Hidden | QDir::Files).size(); -} } namespace OCC { + + Q_LOGGING_CATEGORY(lcFolderMan, "gui.folder.manager", QtInfoMsg) void TrayOverallStatusResult::addResult(Folder *f) @@ -242,6 +242,7 @@ bool FolderMan::addFoldersFromConfigByAccount(QSettings &settings, AccountState Folder *folder = addFolder(account, folderDefinition); if (!folder) { + // todo: decide if we should actively remove the folder data from the config! I think we should but let's see continue; } @@ -306,7 +307,7 @@ void FolderMan::loadSpacesAndCreateFolders(AccountState *accountState, bool useV // prepare the root - reality check this as I think the user can change this from default? Yes they can but not for auto-loaded // folders eg on account creation, which is what is happening here. const QString localDir(accountState->account()->defaultSyncRoot()); - if (!prepareFolder(localDir)) { + if (!FolderManagementUtils::prepareFolder(localDir)) { return; } @@ -699,6 +700,15 @@ bool FolderMan::validateFolderDefinition(const FolderDefinition &folderDefinitio { if (folderDefinition.id().isEmpty() || folderDefinition.journalPath().isEmpty() || !ensureFilesystemSupported(folderDefinition)) return false; + + QString pathCheck = FolderManagementUtils::validateFolderPath(folderDefinition.localPath()); + if (!pathCheck.isEmpty()) { + // does this warrant popping an error dialog? + qCWarning(lcFolderMan) << "Folder definition path check failed with error: " << pathCheck; + return false; + } + + return true; } @@ -716,18 +726,18 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition QUuid accountId = accountState->account()->uuid(); if (Folder *f = folder(accountId, folderDefinition.spaceId())) { - qCWarning(lcFolderMan) << "Trying to add folder" << folderDefinition.localPath() << "but it already exists in folder list"; + qCWarning(lcFolderMan) << "Trying to add new folder for " << folderDefinition.localPath() << "but it already exists in folder list"; return f; } - auto vfs = VfsPluginManager::instance().createVfsFromPlugin(folderDefinition.virtualFilesMode()); - if (!vfs) { - qCWarning(lcFolderMan) << "Could not load plugin for mode" << folderDefinition.virtualFilesMode(); + FolderBuilder builder(folderDefinition); + auto folder = builder.buildFolder(accountState, _ignoreHiddenFiles, this); + + if (!folder) { + qCWarning(lcFolderMan) << "Unable to create Folder for " << folder->path() << " with spaceId " << folderDefinition.spaceId(); return nullptr; } - auto folder = new Folder(folderDefinition, accountState, std::move(vfs), _ignoreHiddenFiles, this); - qCInfo(lcFolderMan) << "Adding folder to Folder Map " << folder << folder->path(); QString spaceId = folder->spaceId(); // always add the folder even if it had a setup error - future add special handling for incomplete folders if possible @@ -965,7 +975,7 @@ static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderM return FolderMan::tr("The selected path is not a folder."); } - if (numberOfSyncJournals(selectedPathInfo.filePath()) != 0) { + if (FolderManagementUtils::numberOfSyncJournals(selectedPathInfo.filePath()) != 0) { return FolderMan::tr("The folder %1 is used in a folder sync connection.").arg(QDir::toNativeSeparators(selectedPathInfo.filePath())); } @@ -1048,9 +1058,9 @@ QString FolderMan::findExistingFolderAndCheckValidity(const QString &path, NewFo QNtfsPermissionCheckGuard ntfs_perm; #endif - auto pathLengthCheck = Folder::checkPathLength(path); - if (!pathLengthCheck) { - return pathLengthCheck.error(); + auto pathLengthCheck = FolderManagementUtils::checkPathLength(path); + if (!pathLengthCheck.isEmpty()) { + return pathLengthCheck; } // If this is a new folder, recurse up to the first parent that exists, to see if we can use that to create a new folder @@ -1136,19 +1146,18 @@ bool FolderMan::isSpaceSynced(GraphApi::Space *space) const return (space && folder(space->accountId(), space->id()) != nullptr); } -// this only seems to be triggered when changing the move to trash setting? -void FolderMan::slotReloadSyncOptions() +void FolderMan::slotUpdateMoveToTrash(bool trashIt) { for (auto *f : folders()) { if (f) { - f->reloadSyncOptions(); + f->setMoveToTrash(trashIt); } } } Folder *FolderMan::addFolderFromScratch(AccountState *accountState, FolderDefinition &&folderDefinition, bool useVfs) { - if (!FolderMan::prepareFolder(folderDefinition.localPath())) { + if (!FolderManagementUtils::prepareFolder(folderDefinition.localPath())) { return nullptr; } @@ -1170,9 +1179,9 @@ Folder *FolderMan::addFolderFromScratch(AccountState *accountState, FolderDefini if (newFolder->vfs().mode() != Vfs::WindowsCfApi) { Utility::setupFavLink(folderDefinition.localPath()); } - qCDebug(lcFolderMan) << "Local sync folder" << folderDefinition.localPath() << "successfully created!"; + qCDebug(lcFolderMan) << "Local sync folder" << folderDefinition.localPath() << "successfully created"; } else { - qCWarning(lcFolderMan) << "Failed to create local sync folder!"; + qCWarning(lcFolderMan) << "Failed to create local sync folder: " << folderDefinition.localPath(); } // we should not emit any folder list change from this function because it can be called in bulk as well as individual operations @@ -1218,19 +1227,7 @@ QString FolderMan::suggestSyncFolder(NewFolderType folderType, const QUuid &acco return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), Theme::instance()->appName(), folderType, accountUuid); } -bool FolderMan::prepareFolder(const QString &folder) -{ - if (!QFileInfo::exists(folder)) { - if (!OC_ENSURE(QDir().mkpath(folder))) { - return false; - } - // mac only - FileSystem::setFolderMinimumPermissions(folder); - // this is for windows - it sets up a desktop.ini file to handle the icon and deals with persmissions. - Folder::prepareFolder(folder); - } - return true; -} + std::unique_ptr FolderMan::createInstance() { diff --git a/src/gui/folderman.h b/src/gui/folderman.h index d9d266fc135..250b0b85980 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -52,6 +52,7 @@ class TrayOverallStatusResult SyncResult _overallStatus; }; + /** * @brief The FolderMan class * @ingroup gui @@ -168,13 +169,16 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject std::optional setupFoldersFromConfig(); /** + * + * Extremely important refactoring todo: addFolder should not be public! + * It is currently "required" for some tests which is not really cool, as it does not represent a complete/standalone impl. + * * core step in any add folder routine. it validates the definition, instantiates vfs, instantiates the folder and validates whether * it had setup errors. * - * it is up to the caller to connect the folder, save it to settings, etc. + * it is up to the caller to create the local sync folder (corresponding to the local path in the def) using the new FolderManagementUtils::prepareFolder, + * connect the folder, save it to settings, etc. * - * Refactoring todo: this should not be public! it is currently "required" for some tests which is not really cool, as it does not represent - * a complete/standalone impl. */ Folder *addFolder(AccountState *accountState, const FolderDefinition &folderDefinition); @@ -329,7 +333,7 @@ public Q_SLOTS: void slotSyncOnceFileUnlocks(const QString &path, FileSystem::LockMode mode); /// This slot will tell all sync engines to reload the sync options. - void slotReloadSyncOptions(); + void slotUpdateMoveToTrash(bool trashIt); // emits folderRemoved void removeFolderFromGui(Folder *f); @@ -356,13 +360,6 @@ private Q_SLOTS: private: explicit FolderMan(); - /** - * @brief prepareFolder sets up the folder with mac and windows specific operations - * @param folder path - * @return true if the folder path exists or can be successfully created - */ - [[nodiscard]] static bool prepareFolder(const QString &folder); - /** * Adds a folder "from scratch" as oppossd to from config, which requires less setup than when you create the folder * from some dynamic operation (eg folders from new account or via the gui add folder sync operations). diff --git a/src/gui/foldermanagement/CMakeLists.txt b/src/gui/foldermanagement/CMakeLists.txt new file mode 100644 index 00000000000..44e24991104 --- /dev/null +++ b/src/gui/foldermanagement/CMakeLists.txt @@ -0,0 +1,7 @@ +target_sources(owncloudGui PRIVATE + foldermanagementutils.h + foldermanagementutils.cpp + folderbuilder.h + folderbuilder.cpp +) + diff --git a/src/gui/foldermanagement/folderbuilder.cpp b/src/gui/foldermanagement/folderbuilder.cpp new file mode 100644 index 00000000000..efece920d1d --- /dev/null +++ b/src/gui/foldermanagement/folderbuilder.cpp @@ -0,0 +1,60 @@ +#include "folderbuilder.h" + +#include "common/syncjournaldb.h" +#include "common/vfs.h" +#include "folder.h" +#include "syncengine.h" + + +namespace OCC { + +Q_LOGGING_CATEGORY(lcFolderBuilder, "gui.folderbuilder", QtInfoMsg) + +FolderBuilder::FolderBuilder(const FolderDefinition &definition, QObject *parent) + : QObject(parent) + , _definition(definition) +{ +} + +Folder *FolderBuilder::buildFolder(AccountState *accountState, bool ignoreHiddenFiles, QObject *parent) +{ + if (!accountState || !accountState->account()) + return nullptr; + + SyncJournalDb *db = buildJournal(); + Vfs *vfs = buildVfs(); + if (db && vfs) + return new Folder(_definition, accountState, db, vfs, ignoreHiddenFiles, parent); + + return nullptr; +} + +SyncJournalDb *FolderBuilder::buildJournal() +{ + SyncJournalDb *journal = new SyncJournalDb(_definition.absoluteJournalPath(), this); + if (!journal->open()) { + qCWarning(lcFolderBuilder) << "Could not open database when creating new folder: " << _definition.absoluteJournalPath(); + return nullptr; + } + // those errors should not persist over sessions so kill them if there are any in an existing journal + journal->wipeErrorBlacklistCategory(SyncJournalErrorBlacklistRecord::Category::LocalSoftError); + return journal; +} + +Vfs *FolderBuilder::buildVfs() +{ + Vfs *vfs = VfsPluginManager::instance().createVfsFromPlugin(_definition.virtualFilesMode(), this); + if (vfs) + return vfs; + + qCWarning(lcFolderBuilder) << "Could not load plugin for mode" << _definition.virtualFilesMode(); + return nullptr; +} + +SyncEngine *FolderBuilder::buildEngine() +{ + return nullptr; +} + + +} diff --git a/src/gui/foldermanagement/folderbuilder.h b/src/gui/foldermanagement/folderbuilder.h new file mode 100644 index 00000000000..43ea2e4a76d --- /dev/null +++ b/src/gui/foldermanagement/folderbuilder.h @@ -0,0 +1,30 @@ +#pragma once + +#include + +#include "folder.h" + +namespace OCC { + +class SyncJournalDb; +class SyncEngine; +class Vfs; + +class FolderBuilder : public QObject +{ + Q_OBJECT + +public: + FolderBuilder(const FolderDefinition &definition, QObject *parent = nullptr); + + Folder *buildFolder(AccountState *accountState, bool ignoreHiddenFiles, QObject *parent); + + +private: + SyncJournalDb *buildJournal(); + SyncEngine *buildEngine(); + Vfs *buildVfs(); + + FolderDefinition _definition; +}; +} diff --git a/src/gui/foldermanagement/foldermanagementutils.cpp b/src/gui/foldermanagement/foldermanagementutils.cpp new file mode 100644 index 00000000000..18f716c39db --- /dev/null +++ b/src/gui/foldermanagement/foldermanagementutils.cpp @@ -0,0 +1,129 @@ +#include "foldermanagementutils.h" + +#include "common/asserts.h" +#include "common/filesystembase.h" +#include "theme.h" + +#include +#include +#include +#include + +#ifdef Q_OS_WIN +#include "common/utility_win.h" +#endif + + +namespace OCC { + +Q_LOGGING_CATEGORY(lcFolderManagementUtils, "gui.foldermanagementutils", QtInfoMsg) + +bool FolderManagementUtils::prepareFolder(const QString &path) +{ + if (!QFileInfo::exists(path)) { + if (!OC_ENSURE(QDir().mkpath(path))) { + return false; + } +#ifdef Q_OS_WIN + // First create a Desktop.ini so that the folder and favorite link show our application's icon. + const QFileInfo desktopIniPath{QStringLiteral("%1/Desktop.ini").arg(path)}; + { + const QString updateIconKey = QStringLiteral("%1/UpdateIcon").arg(Theme::instance()->appName()); + QSettings desktopIni(desktopIniPath.absoluteFilePath(), QSettings::IniFormat); + if (desktopIni.value(updateIconKey, true).toBool()) { + qCInfo(lcFolderManagementUtils) << "Creating" << desktopIni.fileName() << "to set a folder icon in Explorer."; + desktopIni.setValue(QStringLiteral(".ShellClassInfo/IconResource"), QDir::toNativeSeparators(qApp->applicationFilePath())); + desktopIni.setValue(updateIconKey, true); + } else { + qCInfo(lcFolderManagementUtils) << "Skip icon update for" << desktopIni.fileName() << "," << updateIconKey << "is disabled"; + } + } + + const QString longFolderPath = FileSystem::longWinPath(path); + const QString longDesktopIniPath = FileSystem::longWinPath(desktopIniPath.absoluteFilePath()); + // Set the folder as system and Desktop.ini as hidden+system for explorer to pick it. + // https://msdn.microsoft.com/en-us/library/windows/desktop/cc144102 + const DWORD folderAttrs = GetFileAttributesW(reinterpret_cast(longFolderPath.utf16())); + if (!SetFileAttributesW(reinterpret_cast(longFolderPath.utf16()), folderAttrs | FILE_ATTRIBUTE_SYSTEM)) { + const auto error = GetLastError(); + qCWarning(lcFolderManagementUtils) << "SetFileAttributesW failed on" << longFolderPath << Utility::formatWinError(error); + return false; + } + if (!SetFileAttributesW(reinterpret_cast(longDesktopIniPath.utf16()), FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) { + const auto error = GetLastError(); + qCWarning(lcFolderManagementUtils) << "SetFileAttributesW failed on" << longDesktopIniPath << Utility::formatWinError(error); + return false; + } +#else + QFile::Permissions perm = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; + QFile file(path); + if (!file.setPermissions(perm)) { + qCWarning(lcFolderManagementUtils) << "setPermissions failed on " << path; + return false; + } + +#endif + } + return true; +} + +qsizetype FolderManagementUtils::numberOfSyncJournals(const QString &path) +{ + return QDir(path).entryList({QStringLiteral(".sync_*.db"), QStringLiteral("._sync_*.db")}, QDir::Hidden | QDir::Files).size(); +} + +QString FolderManagementUtils::checkPathLength(const QString &path) +{ +#ifdef Q_OS_WIN + if (path.size() > MAX_PATH) { + if (!FileSystem::longPathsEnabledOnWindows()) { + return tr("The path '%1' is too long. Please enable long paths in the Windows settings or choose a different folder.").arg(path); + } + } +#else + Q_UNUSED(path) +#endif + return {}; +} + +QString FolderManagementUtils::validateFolderPath(const QString &path) +{ +#ifdef Q_OS_WIN + QNtfsPermissionCheckGuard ntfs_perm; +#endif + const QFileInfo fi(path); + QString error; + if (fi.isDir() && fi.isReadable() && fi.isWritable()) { + auto pathLengthCheck = FolderManagementUtils::checkPathLength(fi.canonicalFilePath()); + if (!pathLengthCheck.isEmpty()) { + error = pathLengthCheck; + } + + /* if (error.isEmpty()) { + qCDebug(lcFolderManagementUtils) << "Checked local path ok"; + if (!_journal.open()) { + error = tr("%1 failed to open the database.").arg(_definition.localPath()); + } + }*/ + } else { + // Check directory again + if (!FileSystem::fileExists(path, fi)) { + error = tr("Local folder %1 does not exist.").arg(path); + } else if (!fi.isDir()) { + error = tr("%1 should be a folder but is not.").arg(path); + } else if (!fi.isReadable()) { + error = tr("%1 is not readable.").arg(path); + } else if (!fi.isWritable()) { + error = tr("%1 is not writable.").arg(path); + } + } + if (!error.isEmpty()) { + qCWarning(lcFolderManagementUtils) << error; + // _syncResult.appendErrorString(error); + // setSyncState(SyncResult::SetupError); + // return error; + } + return error; +} + +} // OCC diff --git a/src/gui/foldermanagement/foldermanagementutils.h b/src/gui/foldermanagement/foldermanagementutils.h new file mode 100644 index 00000000000..cc94d69a5fc --- /dev/null +++ b/src/gui/foldermanagement/foldermanagementutils.h @@ -0,0 +1,52 @@ +#pragma once + +#include +#include + +namespace OCC { +/** + * @brief The FolderManagementUtils class is a "controlled" collection of general functions related to managing folders and their paths. + * + * indeed, this could be a namespace but I have seen too many abuses of "a namespace can be defined ANYWHERE" to be comfortable with it + * + * Declaring a final, non instantiable class prevents any creative "namespace" decls/defs + * + */ +class FolderManagementUtils final +{ + Q_DECLARE_TR_FUNCTIONS(FolderManagementUtils) + +public: + FolderManagementUtils() = delete; + ~FolderManagementUtils() = delete; + + /** + * @brief prepareFolder creates the folder from given path if it does not already exist, and configures the folder with mac and windows + * specific operations + * @param folder path + * @return true if the folder path exists and was successfully configured + */ + static bool prepareFolder(const QString &path); + + /** + * @brief numberOfSyncJournals counts sync journals by matching folder content with .sync_*.db or ._sync_*.db in the target folder + * @param path is the folder to check for number of sync journals + * @return the number of sync journals found in the given folder. + */ + static qsizetype numberOfSyncJournals(const QString &path); + + /** + * verifies that the length of the given path does not exceed system limits + * if the path fails the check, the return value will contain an error string + * if it passes, the return value will be empty + **/ + static QString checkPathLength(const QString &path); + + /** + * performs various checks on the folder path to ensure it exists and can be used as local sync destination. + * if the checks fail, the return value will contain the error + * if it passes, the return value will be empty. + **/ + static QString validateFolderPath(const QString &path); +}; +} diff --git a/src/gui/generalsettings.cpp b/src/gui/generalsettings.cpp index 9b490b4aa3c..d43c56605dc 100644 --- a/src/gui/generalsettings.cpp +++ b/src/gui/generalsettings.cpp @@ -66,7 +66,7 @@ GeneralSettings::GeneralSettings(QWidget *parent) _ui->moveToTrashCheckBox->setVisible(true); connect(_ui->moveToTrashCheckBox, &QCheckBox::toggled, this, [this](bool checked) { ConfigFile().setMoveToTrash(checked); - Q_EMIT syncOptionsChanged(); + Q_EMIT moveToTrashChanged(checked); }); // OEM themes are not obliged to ship mono icons, so there diff --git a/src/gui/generalsettings.h b/src/gui/generalsettings.h index e5c97593091..d3bec7ff33c 100644 --- a/src/gui/generalsettings.h +++ b/src/gui/generalsettings.h @@ -41,7 +41,7 @@ class GeneralSettings : public QWidget Q_SIGNALS: void showAbout(); - void syncOptionsChanged(); + void moveToTrashChanged(bool trashIt); private Q_SLOTS: void saveMiscSettings(); diff --git a/src/gui/settingsdialog.cpp b/src/gui/settingsdialog.cpp index 645e09ed3c5..7cd3e0bf0af 100644 --- a/src/gui/settingsdialog.cpp +++ b/src/gui/settingsdialog.cpp @@ -121,7 +121,7 @@ SettingsDialog::SettingsDialog(ownCloudGui *gui, QWidget *parent) _generalSettings = new GeneralSettings; _ui->stack->addWidget(_generalSettings); connect(_generalSettings, &GeneralSettings::showAbout, gui, &ownCloudGui::slotAbout); - connect(_generalSettings, &GeneralSettings::syncOptionsChanged, FolderMan::instance(), &FolderMan::slotReloadSyncOptions); + connect(_generalSettings, &GeneralSettings::moveToTrashChanged, FolderMan::instance(), &FolderMan::slotUpdateMoveToTrash); ConfigFile().restoreGeometry(this); #ifdef Q_OS_MAC diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 9a59d543a20..b1e753dae13 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -343,6 +343,11 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( const SyncFileItemPtr &item, PathTuple path, const LocalInfo &localEntry, const RemoteInfo &serverEntry, const SyncJournalFileRecord &dbEntry) { + if (!_discoveryData->_syncOptions.isValid()) { + qCWarning(lcDisco) << "vfs instance is null, unable to continue"; + return; + } + item->_checksumHeader = serverEntry.checksumHeader; item->_fileId = serverEntry.fileId; item->_remotePerm = serverEntry.remotePerm; @@ -387,7 +392,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( item->_size = serverEntry.size; if (dbEntry.isDirectory()) { // TODO: move the decision to the backend - if (_discoveryData->_syncOptions._vfs->mode() != Vfs::Off && _pinState != PinState::AlwaysLocal) { + if (_discoveryData->_syncOptions.vfs()->mode() != Vfs::Off && _pinState != PinState::AlwaysLocal) { item->_type = ItemTypeVirtualFile; } } @@ -435,10 +440,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( auto postProcessServerNew = [=]() mutable { // Turn new remote files into virtual files if the option is enabled. // TODO: move the decision to the backend - const auto &opts = _discoveryData->_syncOptions; - if (!localEntry.isValid() - && item->_type == ItemTypeFile - && opts._vfs->mode() != Vfs::Off + if (!localEntry.isValid() && item->_type == ItemTypeFile && _discoveryData->_syncOptions.vfs()->mode() != Vfs::Off && _pinState != PinState::AlwaysLocal) { item->_type = ItemTypeVirtualFile; } @@ -663,6 +665,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( const SyncFileItemPtr &item, const PathTuple &path, const LocalInfo &localEntry, const RemoteInfo &serverEntry, const SyncJournalFileRecord &dbEntry, QueryMode recurseQueryServer) { + if (!_discoveryData->_syncOptions.isValid()) { + qCWarning(lcDisco) << "vfs instance is null, unable to continue."; + return; + } const bool noServerEntry = (_queryServer != ParentNotChanged && !serverEntry.isValid()) || (_queryServer == ParentNotChanged && !dbEntry.isValid()); if (noServerEntry) { @@ -754,9 +760,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_direction = SyncFileItem::Down; item->setInstruction(CSYNC_INSTRUCTION_SYNC); item->_type = ItemTypeVirtualFileDehydration; - } else if (!serverModified - && (dbEntry._inode != localEntry.inode - || _discoveryData->_syncOptions._vfs->needsMetadataUpdate(*item))) { + } else if (!serverModified && (dbEntry._inode != localEntry.inode || _discoveryData->_syncOptions.vfs()->needsMetadataUpdate(*item))) { item->setInstruction(CSYNC_INSTRUCTION_UPDATE_METADATA); item->_direction = SyncFileItem::Down; } @@ -829,7 +833,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( auto postProcessLocalNew = [item, localEntry, this](const PathTuple &path) { if (localEntry.isVirtualFile) { - const bool isPlaceHolder = _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); + const bool isPlaceHolder = _discoveryData->_syncOptions.vfs()->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); if (isPlaceHolder) { qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local; item->setInstruction(CSYNC_INSTRUCTION_REMOVE); @@ -1371,8 +1375,13 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery() void ProcessDirectoryJob::startAsyncLocalQuery() { + if (!_discoveryData->_syncOptions.isValid()) { + qCInfo(lcDisco) << "vfs pointer is null, unable to continue"; + return; + } + QString localPath = _discoveryData->_localDir + _currentFolder._local; - auto localJob = new DiscoverySingleLocalDirectoryJob(localPath, _discoveryData->_syncOptions._vfs.data()); + auto localJob = new DiscoverySingleLocalDirectoryJob(localPath, _discoveryData->_syncOptions.vfs()); _discoveryData->_currentlyActiveJobs++; _pendingAsyncJobs++; @@ -1423,9 +1432,14 @@ void ProcessDirectoryJob::startAsyncLocalQuery() void ProcessDirectoryJob::computePinState(PinState parentState) { + if (!_discoveryData->_syncOptions.isValid()) { + qCInfo(lcDisco) << "vfs pointer is null, unable to continue"; + return; + } + _pinState = parentState; if (_queryLocal != ParentDontExist) { - if (auto state = _discoveryData->_syncOptions._vfs->pinState(_currentFolder._local)) // ouch! pin local or original? + if (auto state = _discoveryData->_syncOptions.vfs()->pinState(_currentFolder._local)) // ouch! pin local or original? _pinState = *state; } } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index fad08cee980..34b7a21fdbf 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -91,7 +91,7 @@ class DiscoverySingleLocalDirectoryJob : public QObject, public QRunnable void itemDiscovered(SyncFileItemPtr item); void childIgnored(bool b); -private Q_SLOTS: + private: QString _localPath; OCC::Vfs* _vfs; diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 3022357f982..c9492a8f3e7 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -805,18 +805,23 @@ QString OwncloudPropagator::adjustRenamedPath(const QString &original) const Result OwncloudPropagator::updatePlaceholder(const SyncFileItem &item, const QString &fileName, const QString &replacesFile) { + if (!_syncOptions.isValid()) { + QString error = "vfs instance is not available"; + return error; + } + Q_ASSERT([&] { if (item._type == ItemTypeVirtualFileDehydration) { // when dehydrating the file must not be pinned // don't use destination() with suffix placeholder - const auto pin = syncOptions()._vfs->pinState(item._file); + const auto pin = syncOptions().vfs()->pinState(item._file); if (pin && pin.get() == PinState::AlwaysLocal) { return false; } } return true; }()); - return syncOptions()._vfs->updateMetadata(item, fileName, replacesFile); + return syncOptions().vfs()->updateMetadata(item, fileName, replacesFile); } Result OwncloudPropagator::updateMetadata(const SyncFileItem &item) @@ -842,6 +847,7 @@ Result OwncloudPropagator::updateMetad PropagatorJob::PropagatorJob(OwncloudPropagator *propagator, const QString &path) : QObject(propagator) + , _propagator(propagator) , _path(path) , _jobState(NotYetStarted) { @@ -854,7 +860,7 @@ void PropagatorJob::setState(JobState state) OwncloudPropagator *PropagatorJob::propagator() const { - return qobject_cast(parent()); + return _propagator; } // ================================================================================ diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 827a1cd9e21..0df714a837a 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -135,6 +135,7 @@ public Q_SLOTS: OwncloudPropagator *propagator() const; private: + OwncloudPropagator *_propagator; QString _path; JobState _jobState; }; @@ -366,9 +367,10 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject bool _finishedEmitted; // used to ensure that finished is only emitted once public: - OwncloudPropagator( - Account *account, const SyncOptions &options, const QUrl &baseUrl, const QString &localDir, const QString &remoteFolder, SyncJournalDb *progressDb) - : _journal(progressDb) + OwncloudPropagator(Account *account, const SyncOptions &options, const QUrl &baseUrl, const QString &localDir, const QString &remoteFolder, + SyncJournalDb *progressDb, QObject *parent) + : QObject(parent) + , _journal(progressDb) , _finishedEmitted(false) , _anotherSyncNeeded(false) , _account(account) @@ -474,6 +476,9 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject */ DiskSpaceResult diskSpaceCheck() const; + bool moveToTrash() const { return _moveToTrash; } + void setMoveToTrash(bool trashIt) { _moveToTrash = trashIt; } + /** Handles a conflict by renaming the file 'item'. * * Sets up conflict records. @@ -539,10 +544,11 @@ private Q_SLOTS: QScopedPointer _rootJob; SyncOptions _syncOptions; bool _jobScheduled = false; + bool _moveToTrash = false; const QString _localDir; // absolute path to the local directory. ends with '/' const QString _remoteFolder; // remote folder, ends with '/' - const QUrl _webDavUrl; // full WebDAV URL, might be the same as in the account + const QUrl _webDavUrl; // full WebDAV URL, might be the same as in the }; /** diff --git a/src/libsync/owncloudpropagator_p.h b/src/libsync/owncloudpropagator_p.h index da2b0282b16..69c410335a4 100644 --- a/src/libsync/owncloudpropagator_p.h +++ b/src/libsync/owncloudpropagator_p.h @@ -15,9 +15,7 @@ #pragma once -#include "owncloudpropagator.h" #include "syncfileitem.h" -#include "networkjobs.h" #include #include diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 6fc6a68d922..69bdcf4ac33 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -354,13 +354,12 @@ QString GETFileJob::errorString() const void PropagateDownloadFile::start() { - if (propagator()->_abortRequested) + if (propagator()->_abortRequested || !propagator()->syncOptions().isValid()) return; _stopwatch.start(); - auto &syncOptions = propagator()->syncOptions(); - auto &vfs = syncOptions._vfs; + Vfs *vfs = propagator()->syncOptions().vfs(); const QString fsPath = propagator()->fullLocalPath(_item->_file); diff --git a/src/libsync/propagateremotedelete.cpp b/src/libsync/propagateremotedelete.cpp index 6e68f20d686..ae927a23752 100644 --- a/src/libsync/propagateremotedelete.cpp +++ b/src/libsync/propagateremotedelete.cpp @@ -13,9 +13,9 @@ */ #include "propagateremotedelete.h" -#include "owncloudpropagator_p.h" #include "account.h" #include "common/asserts.h" +#include "owncloudpropagator_p.h" #include diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index 1cc2fcba843..33561ec4731 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -13,11 +13,11 @@ */ #include "propagateremotemkdir.h" -#include "owncloudpropagator_p.h" #include "account.h" +#include "common/asserts.h" #include "common/syncjournalfilerecord.h" +#include "owncloudpropagator_p.h" #include "propagateremotedelete.h" -#include "common/asserts.h" #include #include diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 3695fbc10ec..8e72cf63d6e 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -121,6 +121,10 @@ void PropagateRemoteMove::slotMoveJobFinished() void PropagateRemoteMove::finalize() { + if (!propagator()->syncOptions().isValid()) { + qCWarning(lcPropagateRemoteMove) << "vfs instance is null, unable to finalize"; + return; + } // Retrieve old db data. // if reading from db failed still continue hoping that deleteFileRecord // reopens the db successfully. @@ -128,7 +132,7 @@ void PropagateRemoteMove::finalize() // to the new record. It is not a problem to skip it here. SyncJournalFileRecord oldRecord; propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord); - auto &vfs = propagator()->syncOptions()._vfs; + Vfs *vfs = propagator()->syncOptions().vfs(); auto pinState = vfs->pinState(_item->_originalFile); // Delete old db data. diff --git a/src/libsync/propagateuploadcommon.cpp b/src/libsync/propagateuploadcommon.cpp index 2c8c19d3a3c..3f4751e3265 100644 --- a/src/libsync/propagateuploadcommon.cpp +++ b/src/libsync/propagateuploadcommon.cpp @@ -12,19 +12,19 @@ * for more details. */ -#include "account.h" -#include "filesystem.h" -#include "networkjobs.h" -#include "owncloudpropagator_p.h" -#include "propagateremotedelete.h" -#include "propagateuploadfile.h" -#include "syncengine.h" +#include "propagateuploadcommon.h" +#include "account.h" #include "common/asserts.h" #include "common/checksums.h" #include "common/syncjournaldb.h" #include "common/syncjournalfilerecord.h" #include "common/utility.h" +#include "filesystem.h" +#include "networkjobs.h" +#include "owncloudpropagator_p.h" +#include "propagateremotedelete.h" +#include "syncengine.h" #include "libsync/theme.h" @@ -32,7 +32,7 @@ #include #include -#include +// #include using namespace std::chrono_literals; @@ -261,8 +261,7 @@ void PropagateUploadCommon::commonErrorHandling(AbstractNetworkJob *job) // Ensure errors that should eventually reset the chunked upload are tracked. checkResettingErrors(); - SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode, - &propagator()->_anotherSyncNeeded, replyContent); + SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode, &propagator()->_anotherSyncNeeded, replyContent); // Insufficient remote storage. if (_item->_httpErrorCode == 507) { @@ -359,7 +358,7 @@ QMap PropagateUploadCommon::headers() void PropagateUploadCommon::finalize() { - OC_ENFORCE(state() != Finished); + OC_ENFORCE(state() != Finished && propagator()->syncOptions().isValid()); // Update the quota, if known if (!_quotaUpdated) { @@ -397,7 +396,7 @@ void PropagateUploadCommon::finalize() // Files that were new on the remote shouldn't have online-only pin state // even if their parent folder is online-only. if (_item->instruction() & (CSYNC_INSTRUCTION_NEW | CSYNC_INSTRUCTION_TYPE_CHANGE)) { - auto &vfs = propagator()->syncOptions()._vfs; + Vfs *vfs = propagator()->syncOptions().vfs(); const auto pin = vfs->pinState(_item->_file); if (pin && *pin == PinState::OnlineOnly) { std::ignore = vfs->setPinState(_item->_file, PinState::Unspecified); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index cbdbc9b3987..0a6224d93c0 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -17,8 +17,6 @@ #include "common/syncjournaldb.h" #include "common/syncjournalfilerecord.h" #include "filesystem.h" -#include "owncloudpropagator.h" -#include "owncloudpropagator_p.h" #include "propagateremotemove.h" #include #include @@ -95,7 +93,7 @@ bool PropagateLocalRemove::removeRecursively(const QString &absolute) void PropagateLocalRemove::start() { - _moveToTrash = propagator()->syncOptions()._moveFilesToTrash; + _moveToTrash = propagator()->moveToTrash(); if (propagator()->_abortRequested) return; @@ -212,7 +210,7 @@ void PropagateLocalMkdir::setDeleteExistingFile(bool enabled) void PropagateLocalRename::start() { - if (propagator()->_abortRequested) + if (propagator()->_abortRequested || !propagator()->syncOptions().isValid()) return; QString existingFile = propagator()->fullLocalPath(propagator()->adjustRenamedPath(_item->_file)); @@ -253,7 +251,7 @@ void PropagateLocalRename::start() propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord); propagator()->_journal->deleteFileRecord(_item->_originalFile); - auto &vfs = propagator()->syncOptions()._vfs; + Vfs *vfs = propagator()->syncOptions().vfs(); auto pinState = vfs->pinState(_item->_originalFile); std::ignore = vfs->setPinState(_item->_originalFile, PinState::Inherited); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 320854c8f11..db60a5e03e6 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -499,37 +499,7 @@ void SyncEngine::slotDiscoveryFinished() _anotherSyncNeeded = true; } - Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end())); - - const auto regex = syncOptions().fileRegex(); - if (regex.isValid()) { - QSet names; - for (auto &i : _syncItems) { - if (regex.match(i->_file).hasMatch()) { - int index = -1; - QStringView ref; - do { - ref = QStringView(i->_file).mid(0, index); - names.insert(ref); - index = ref.lastIndexOf(QLatin1Char('/')); - } while (index > 0); - } - } - //std::erase_if c++20 - // https://en.cppreference.com/w/cpp/container/set/erase_if - const auto erase_if = [](auto &c, const auto &pred) { - auto old_size = c.size(); - for (auto i = c.begin(), last = c.end(); i != last;) { - if (pred(*i)) { - i = c.erase(i); - } else { - ++i; - } - } - return old_size - c.size(); - }; - erase_if(_syncItems, [&names](const SyncFileItemPtr &i) { return !names.contains(QStringView{i->_file}); }); - } + Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end())); qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) ####################################################" << _duration.duration(); @@ -548,18 +518,16 @@ void SyncEngine::slotDiscoveryFinished() // do a database commit _journal->commit(QStringLiteral("post treewalk")); - _propagator = QSharedPointer::create(_account, syncOptions(), _baseUrl, _localPath, _remotePath, _journal); - connect(_propagator.data(), &OwncloudPropagator::itemCompleted, - this, &SyncEngine::slotItemCompleted); - connect(_propagator.data(), &OwncloudPropagator::progress, - this, &SyncEngine::slotProgress); - connect(_propagator.data(), &OwncloudPropagator::updateFileTotal, - this, &SyncEngine::updateFileTotal); - connect(_propagator.data(), &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection); - connect(_propagator.data(), &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile); - connect(_propagator.data(), &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage); - connect(_propagator.data(), &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage); - connect(_propagator.data(), &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem); + _propagator = new OwncloudPropagator(_account, syncOptions(), _baseUrl, _localPath, _remotePath, _journal, this); + _propagator->setMoveToTrash(_moveToTrash); + connect(_propagator, &OwncloudPropagator::itemCompleted, this, &SyncEngine::slotItemCompleted); + connect(_propagator, &OwncloudPropagator::progress, this, &SyncEngine::slotProgress); + connect(_propagator, &OwncloudPropagator::updateFileTotal, this, &SyncEngine::updateFileTotal); + connect(_propagator, &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection); + connect(_propagator, &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile); + connect(_propagator, &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage); + connect(_propagator, &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage); + connect(_propagator, &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem); deleteStaleDownloadInfos(_syncItems); deleteStaleUploadInfos(_syncItems); @@ -637,7 +605,7 @@ void SyncEngine::finalize(bool success) Q_EMIT finished(success); // Delete the propagator only after emitting the signal. - _propagator.clear(); + // _propagator.clear(); _seenConflictFiles.clear(); _uniqueErrors.clear(); _localDiscoveryPaths.clear(); diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index fcfda13ae30..0add2ac3141 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -61,18 +61,17 @@ class OWNCLOUDSYNC_EXPORT SyncEngine : public QObject bool isSyncRunning() const { return _syncRunning; } - const SyncOptions &syncOptions() const - { - Q_ASSERT(_syncOptions); - return *_syncOptions; - } + const SyncOptions &syncOptions() const { return _syncOptions; } void setSyncOptions(const SyncOptions &options) { + Q_ASSERT(options.isValid()); _syncOptions = options; } bool ignoreHiddenFiles() const { return _ignore_hidden_files; } void setIgnoreHiddenFiles(bool ignore) { _ignore_hidden_files = ignore; } + void setMoveToTrash(bool trashIt) { _moveToTrash = trashIt; } + bool isExcluded(QStringView filePath) const; void addManualExclude(const QString &filePath); void addExcludeList(const QString &filePath); @@ -125,7 +124,7 @@ class OWNCLOUDSYNC_EXPORT SyncEngine : public QObject /** Access the last sync run's local discovery style */ LocalDiscoveryStyle lastLocalDiscoveryStyle() const { return _lastLocalDiscoveryStyle; } - auto getPropagator() { return _propagator; } // for the test + // auto getPropagator() { return _propagator; } // for the test Q_SIGNALS: @@ -211,7 +210,7 @@ private Q_SLOTS: QString _remoteRootEtag; SyncJournalDb *_journal; std::unique_ptr _discoveryPhase; - QSharedPointer _propagator; + OwncloudPropagator *_propagator; // List of all files with conflicts QSet _seenConflictFiles; @@ -229,8 +228,10 @@ private Q_SLOTS: // If ignored files should be ignored bool _ignore_hidden_files = false; + // should deleted files be moved to trash + bool _moveToTrash = false; - std::optional _syncOptions; + SyncOptions _syncOptions; bool _anotherSyncNeeded = false; diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp index 4e57a4d8fad..39ed5a8047a 100644 --- a/src/libsync/syncoptions.cpp +++ b/src/libsync/syncoptions.cpp @@ -19,7 +19,7 @@ using namespace OCC; -SyncOptions::SyncOptions(QSharedPointer vfs) +SyncOptions::SyncOptions(Vfs *vfs) : _vfs(vfs) { } @@ -28,27 +28,7 @@ SyncOptions::~SyncOptions() { } -void SyncOptions::fillFromEnvironmentVariables() +bool SyncOptions::isValid() const { - int maxParallel = qEnvironmentVariableIntValue("OWNCLOUD_MAX_PARALLEL"); - if (maxParallel > 0) - _parallelNetworkJobs = maxParallel; -} - -QRegularExpression SyncOptions::fileRegex() const -{ - return _fileRegex; -} - -void SyncOptions::setFilePattern(const QString &pattern) -{ - // full match or a path ending with this pattern - setPathPattern(QStringLiteral("(^|/|\\\\)") + pattern + QLatin1Char('$')); -} - -void SyncOptions::setPathPattern(const QString &pattern) -{ - _fileRegex.setPatternOptions( - Utility::fsCasePreservingButCaseInsensitive() ? QRegularExpression::CaseInsensitiveOption : QRegularExpression::NoPatternOption); - _fileRegex.setPattern(pattern); + return !_vfs.isNull(); } diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 4d60efc12c7..33eff99d699 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -33,43 +33,33 @@ namespace OCC { class OWNCLOUDSYNC_EXPORT SyncOptions { public: - explicit SyncOptions(QSharedPointer vfs); + // the VFS pointer must be stored in a QPointer as it may go out of scope from "above" + // it's owned by the Folder so if the folder dies, so does the vfs pointer. + // use isValid to see if the options should still be used, or test the vfs pointer directly to see if it's null or not. + explicit SyncOptions(Vfs *vfs = nullptr); ~SyncOptions(); + Vfs *vfs() const { return _vfs; } + /** If remotely deleted files are needed to move to trash */ - bool _moveFilesToTrash = false; + // bool _moveFilesToTrash = false; - /** Create a virtual file for new files instead of downloading. May not be null */ - QSharedPointer _vfs; /** The maximum number of active jobs in parallel */ int _parallelNetworkJobs = 6; - /** Reads settings from env vars where available. */ - void fillFromEnvironmentVariables(); - - /** A regular expression to match file names - * If no pattern is provided the default is an invalid regular expression. - */ - QRegularExpression fileRegex() const; - /** - * A pattern like *.txt, matching only file names + * @brief isValid indicates if the options are complete + * @return true if vfs is non-null, else false */ - void setFilePattern(const QString &pattern); - - /** - * A pattern like /own.*\/.*txt matching the full path - */ - void setPathPattern(const QString &pattern); - + bool isValid() const; private: - /** - * Only sync files that mathc the expression - * Invalid pattern by default. + /** Create a virtual file for new files instead of downloading. If vfs is null, isValid will return false indicating you should not use it + * implementation note: of course you can pass a nullptr to the ctr or it can be deleted from above so this is the "sanest" way to + * handle it. */ - QRegularExpression _fileRegex = QRegularExpression(QStringLiteral("(")); + QPointer _vfs; }; } diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index 2a0f234cafa..0576f49cd65 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -174,6 +174,7 @@ private Q_SLOTS: QDir dir2(dir.path()); // Folder in config and on disk: + // todo: where is this added "to the config" and what is the "config" we are even talking about here? QVERIFY(dir2.mkpath(QStringLiteral("sub/ownCloud1/folder/f"))); // Folders only on disk, not in configuration: QVERIFY(dir2.mkpath(QStringLiteral("ownCloud"))); @@ -186,13 +187,24 @@ private Q_SLOTS: auto newAccountState = createDummyAccount(); FolderMan *folderman = TestUtils::folderMan(); - // Add folder that is in the configuration, AND on disk: + + // todo: addFolder needs to be made private to FolderMan because there are other important impls "around" it depending on how the folder + // is being created. + // The only reason it's not currently private is because it's used in the tests, which is dubious. + // AT THE VERY LEAST, the folder needs to exist on disk *before* calling addFolder and with new refactoring, addFolder will fail if that is not the case + // historically, calling addFolder without an existing local dir resulted in a hideously broken Folder instance that is just waiting to crash ;) + QVERIFY(dir2.mkpath(QStringLiteral("sub/ownCloud/"))); QVERIFY(folderman->addFolder( newAccountState.get(), TestUtils::createDummyFolderDefinition(newAccountState->account(), dirPath + QStringLiteral("/sub/ownCloud/")))); - // Add folder that is in the configuration, not on disk: + + QVERIFY(dir2.mkpath(QStringLiteral("ownCloud (2)/"))); QVERIFY(folderman->addFolder( newAccountState.get(), TestUtils::createDummyFolderDefinition(newAccountState->account(), dirPath + QStringLiteral("/ownCloud (2)/")))); + // Test todo: verify that addFolder with path that has no existing local folder fails, and do some findGoodPathForNewSyncFolder tests around + // that too? With new updates, the folder will not be created, hence will not exist in the FolderMan folder container(s) which does change + // the results of the function under test, afaik + // TEST const auto folderType = FolderMan::NewFolderType::SpacesFolder; const auto uuid = newAccountState->account()->uuid(); @@ -210,6 +222,7 @@ private Q_SLOTS: // REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it. // We should still not suggest this folder as a new folder. + // todo: add a verify here to check if the folder exists in the first place before removing it QDir(dirPath + QStringLiteral("/ownCloud (2)/")).removeRecursively(); QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (3)")); QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType, uuid), diff --git a/test/testsyncconflict.cpp b/test/testsyncconflict.cpp index 12b3e47db25..1009526ec7c 100644 --- a/test/testsyncconflict.cpp +++ b/test/testsyncconflict.cpp @@ -373,7 +373,7 @@ private Q_SLOTS: if (filesAreDehydrated) { // the dehydrating the placeholder failed as the metadata is out of sync - QSignalSpy spy(fakeFolder.vfs().get(), &Vfs::needSync); + QSignalSpy spy(fakeFolder.vfs(), &Vfs::needSync); QVERIFY(!fakeFolder.applyLocalModificationsAndSync()); QVERIFY(spy.count() == 1); QVERIFY(fakeFolder.syncOnce()); diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp index a61c6831a6f..c098df6a8b5 100644 --- a/test/testsyncjournaldb.cpp +++ b/test/testsyncjournaldb.cpp @@ -21,7 +21,7 @@ class TestSyncJournalDB : public QObject public: TestSyncJournalDB() - : _db((_tempDir.path() + QStringLiteral("/sync.db"))) + : _db((_tempDir.path() + QStringLiteral("/sync.db")), nullptr) { QVERIFY(_tempDir.isValid()); } diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index 9d38221164b..88f5205ed11 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -705,7 +705,7 @@ private Q_SLOTS: ItemCompletedSpy completeSpy(fakeFolder); if (filesAreDehydrated) { // the dehydrating the placeholder failed as the metadata is out of sync - QSignalSpy spy(fakeFolder.vfs().get(), &Vfs::needSync); + QSignalSpy spy(fakeFolder.vfs(), &Vfs::needSync); QVERIFY(!fakeFolder.applyLocalModificationsAndSync()); QVERIFY(spy.count() == 1); QVERIFY(fakeFolder.syncOnce()); @@ -1013,8 +1013,9 @@ private Q_SLOTS: if (vfsMode != Vfs::Off) { - auto vfs = QSharedPointer(VfsPluginManager::instance().createVfsFromPlugin(vfsMode).release()); + auto vfs = VfsPluginManager::instance().createVfsFromPlugin(vfsMode, &fakeFolder); QVERIFY(vfs); + // todo: this is going to kill the parallel jobs count set above - I don't know if it matters so need to check fakeFolder.switchToVfs(vfs); fakeFolder.syncJournal().internalPinStates().setForPath("", PinState::OnlineOnly); diff --git a/test/testutils/syncenginetestutils.cpp b/test/testutils/syncenginetestutils.cpp index 79e6b521996..1fa1469fbaa 100644 --- a/test/testutils/syncenginetestutils.cpp +++ b/test/testutils/syncenginetestutils.cpp @@ -884,22 +884,18 @@ FakeFolder::FakeFolder(const FileInfo &fileTemplate, OCC::Vfs::Mode vfsMode, boo _fakeAm = dynamic_cast(_accountState->account()->accessManager()); Q_ASSERT(_fakeAm); - _journalDb.reset(new OCC::SyncJournalDb(localPath() + QStringLiteral(".sync_test.db"))); + _journalDb.reset(new OCC::SyncJournalDb(localPath() + QStringLiteral(".sync_test.db"), nullptr)); // TODO: davUrl _syncEngine.reset(new OCC::SyncEngine(account(), account()->davUrl(), localPath(), QString(), _journalDb.get())); - _syncEngine->setSyncOptions(OCC::SyncOptions { QSharedPointer(OCC::VfsPluginManager::instance().createVfsFromPlugin(vfsMode).release()) }); + OCC::Vfs *vfs = OCC::VfsPluginManager::instance().createVfsFromPlugin(vfsMode, this); + Q_ASSERT(vfs->mode() == vfsMode); + Q_ASSERT(!_syncEngine->syncOptions().isValid()); // Ignore temporary files from the download. (This is in the default exclude list, but we don't load it) _syncEngine->addManualExclude(QStringLiteral("]*.~*")); - auto vfs = _syncEngine->syncOptions()._vfs; - if (vfsMode != vfs->mode()) { - vfs.reset(OCC::VfsPluginManager::instance().createVfsFromPlugin(vfsMode).release()); - Q_ASSERT(vfs); - } - - // Ensure we have a valid Vfs instance "running" + // start the vfs instance switchToVfs(vfs); if (vfsMode != OCC::Vfs::Off) { @@ -914,18 +910,38 @@ FakeFolder::FakeFolder(const FileInfo &fileTemplate, OCC::Vfs::Mode vfsMode, boo OC_ENFORCE(syncOnce()) } -FakeFolder::~FakeFolder() { } +FakeFolder::~FakeFolder() +{ + if (_syncEngine && _syncEngine->syncOptions().isValid()) { + _syncEngine->syncOptions().vfs()->stop(); + _syncEngine->syncOptions().vfs()->unregisterFolder(); + } +} -void FakeFolder::switchToVfs(QSharedPointer vfs) +void FakeFolder::switchToVfs(OCC::Vfs *vfs) { + Q_ASSERT(vfs); + auto opts = _syncEngine->syncOptions(); - opts._vfs->stop(); - opts._vfs->unregisterFolder(); - QObject::disconnect(_syncEngine.get(), nullptr, opts._vfs.data(), nullptr); + if (opts.isValid()) { + // we might get strange problems with "dead" vfs instances that have not been stopped because the FakeFolder switchToVfs is missing + // *a lot* of extra steps that normal Folder implements when switching the mode + if (_syncEngine->isSyncRunning()) + execUntilFinished(); + OCC::Vfs *vfsToDie = opts.vfs(); + vfsToDie->stop(); + vfsToDie->unregisterFolder(); + QObject::disconnect(_syncEngine.get(), nullptr, vfsToDie, nullptr); + QObject::disconnect(&_syncEngine->syncFileStatusTracker(), nullptr, vfsToDie, nullptr); + QObject::disconnect(vfsToDie); + // this is "nice" to avoid waiting for the parent folder to die + vfsToDie->deleteLater(); + } - opts._vfs = vfs; - _syncEngine->setSyncOptions(opts); + // todo: nooooooo - the opts should be treated as immutable. that change is coming so this will have to end sometime, starting now. + // opts._vfs = vfs; + _syncEngine->setSyncOptions(OCC::SyncOptions{vfs}); OCC::VfsSetupParams vfsParams(account(), account()->davUrl(), &syncEngine()); vfsParams.filesystemPath = localPath(); @@ -935,17 +951,11 @@ void FakeFolder::switchToVfs(QSharedPointer vfs) vfsParams.providerDisplayName = QStringLiteral("OC-TEST"); vfsParams.providerVersion = QVersionNumber(0, 1, 0); vfsParams.multipleAccountsRegistered = false; - QObject::connect(_syncEngine.get(), &QObject::destroyed, this, [vfs]() { - vfs->stop(); - vfs->unregisterFolder(); - }); - QObject::connect(&_syncEngine->syncFileStatusTracker(), &OCC::SyncFileStatusTracker::fileStatusChanged, - vfs.data(), &OCC::Vfs::fileStatusChanged); - QObject::connect(vfs.get(), &OCC::Vfs::error, vfs.get(), [](const QString &error) { - QFAIL(qUtf8Printable(error)); - }); - QSignalSpy spy(vfs.get(), &OCC::Vfs::started); + QObject::connect(&_syncEngine->syncFileStatusTracker(), &OCC::SyncFileStatusTracker::fileStatusChanged, vfs, &OCC::Vfs::fileStatusChanged); + + QObject::connect(vfs, &OCC::Vfs::error, vfs, [](const QString &error) { QFAIL(qUtf8Printable(error)); }); + QSignalSpy spy(vfs, &OCC::Vfs::started); vfs->start(vfsParams); // don't use QVERIFY outside of the test slot @@ -1005,9 +1015,9 @@ bool FakeFolder::isDehydratedPlaceholder(const QString &filePath) return vfs()->isDehydratedPlaceholder(filePath); } -QSharedPointer FakeFolder::vfs() const +OCC::Vfs *FakeFolder::vfs() const { - return _syncEngine->syncOptions()._vfs; + return _syncEngine->syncOptions().vfs(); } void FakeFolder::toDisk(QDir &dir, const FileInfo &templateFi) diff --git a/test/testutils/syncenginetestutils.h b/test/testutils/syncenginetestutils.h index 7f144f1deed..ed3dd549a7b 100644 --- a/test/testutils/syncenginetestutils.h +++ b/test/testutils/syncenginetestutils.h @@ -574,7 +574,7 @@ class FakeFolder : public QObject FakeFolder(const FileInfo &fileTemplate, OCC::Vfs::Mode vfsMode = OCC::Vfs::Off, bool filesAreDehydrated = false); ~FakeFolder(); - void switchToVfs(QSharedPointer vfs); + void switchToVfs(OCC::Vfs *vfs); OCC::Account *account() const { return _accountState->account(); } OCC::SyncEngine &syncEngine() const { return *_syncEngine; } @@ -626,7 +626,7 @@ class FakeFolder : public QObject } bool isDehydratedPlaceholder(const QString &filePath); - QSharedPointer vfs() const; + OCC::Vfs *vfs() const; private: static void toDisk(QDir &dir, const FileInfo &templateFi); diff --git a/test/testwinvfs.cpp b/test/testwinvfs.cpp index bff7917c015..f24319eff48 100755 --- a/test/testwinvfs.cpp +++ b/test/testwinvfs.cpp @@ -681,6 +681,7 @@ private Q_SLOTS: fakeFolder.localModifier().insert(QStringLiteral("A/a3"), 100_B); QVERIFY(fakeFolder.applyLocalModificationsWithoutSync()); + // todo: review this. AFAIK it's already covered in FakeFolder::swtichToVfs or *should* be fakeFolder.vfs()->wipeDehydratedVirtualFiles(); fakeFolder.vfs()->stop(); fakeFolder.vfs()->unregisterFolder(); @@ -691,7 +692,7 @@ private Q_SLOTS: QVERIFY(!QFile::exists(l(QStringLiteral("A/B/b1")))); // Check that syncing with vfs disabled is fine - auto vfsOff = QSharedPointer(VfsPluginManager::instance().createVfsFromPlugin(Vfs::Off).release()); + auto vfsOff = VfsPluginManager::instance().createVfsFromPlugin(Vfs::Off, &fakeFolder); QVERIFY(vfsOff); fakeFolder.switchToVfs(vfsOff); QVERIFY(fakeFolder.applyLocalModificationsAndSync()); @@ -757,7 +758,7 @@ private Q_SLOTS: fakeFolder.remoteModifier().insert(QStringLiteral("A/a1"), 64_B); QVERIFY(fakeFolder.applyLocalModificationsAndSync()); - fakeFolder.switchToVfs(QSharedPointer(new VfsWin)); + fakeFolder.switchToVfs(new VfsWin(&fakeFolder)); QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); QVERIFY(fakeFolder.applyLocalModificationsAndSync());