From 0fa84e960f4fd8c837d031764cb7932b7ee20ae1 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 20 May 2021 16:45:52 +0200 Subject: [PATCH 1/7] Fix network drive detection We can't rely on the file system detection because Windows is reporting the underlying file system. Fixes: #8272 --- src/common/vfs.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index e887937f066dc..84df80c76ec41 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -70,10 +70,18 @@ Result Vfs::checkAvailability(const QString &path) const auto mode = bestAvailableVfsMode(); #ifdef Q_OS_WIN if (mode == Mode::WindowsCfApi) { - const auto fs = FileSystem::fileSystemForPath(path); + const auto info = QFileInfo(path); + if (QDir(info.canonicalPath()).isRoot()) { + return tr("The Virtual filesystem feature does not support a drive as sync root"); + } + const auto fs = FileSystem::fileSystemForPath(info.absoluteFilePath()); if (fs != QLatin1String("NTFS")) { return tr("The Virtual filesystem feature requires a NTFS file system, %1 is using %2").arg(path, fs); } + const auto type = GetDriveTypeW(reinterpret_cast(QDir::toNativeSeparators(info.absoluteFilePath().mid(0, 3)).utf16())); + if (type == DRIVE_REMOTE) { + return tr("The Virtual filesystem feature is not supported on network drives"); + } } #else Q_UNUSED(mode) From 47132d3bebd1781c7e76efe7c9cebc6a905ae6f2 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Mon, 13 Sep 2021 15:01:34 +0200 Subject: [PATCH 2/7] Fix crash on missing sync root Fixes: #9016 --- src/common/vfs.cpp | 6 ++-- src/common/vfs.h | 2 +- src/gui/accountsettings.cpp | 5 +-- src/gui/folder.cpp | 37 +++++++++++++------- src/gui/folder.h | 2 +- src/gui/folderman.cpp | 5 +++ src/gui/folderman.h | 3 ++ src/gui/folderwizard.cpp | 5 +-- src/gui/wizard/owncloudadvancedsetuppage.cpp | 2 +- src/libsync/syncengine.cpp | 2 +- 10 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index 84df80c76ec41..69e51be50944c 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -24,6 +24,7 @@ #include "common/filesystembase.h" +#include #include #include @@ -65,9 +66,8 @@ Optional Vfs::modeFromString(const QString &str) return {}; } -Result Vfs::checkAvailability(const QString &path) +Result Vfs::checkAvailability(const QString &path, Vfs::Mode mode) { - const auto mode = bestAvailableVfsMode(); #ifdef Q_OS_WIN if (mode == Mode::WindowsCfApi) { const auto info = QFileInfo(path); @@ -87,7 +87,7 @@ Result Vfs::checkAvailability(const QString &path) Q_UNUSED(mode) Q_UNUSED(path) #endif - return true; + return {}; } void Vfs::start(const VfsSetupParams ¶ms) diff --git a/src/common/vfs.h b/src/common/vfs.h index b3c5df9d08262..1e925ccdca10b 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -125,7 +125,7 @@ class OCSYNC_EXPORT Vfs : public QObject static QString modeToString(Mode mode); static Optional modeFromString(const QString &str); - static Result checkAvailability(const QString &path); + static Result checkAvailability(const QString &path, OCC::Vfs::Mode mode); enum class AvailabilityError { diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 986ca7f8786f0..bc5bda41d1dec 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -710,8 +710,9 @@ void AccountSettings::slotCustomContextMenuRequested(const QPoint &pos) ac->setDisabled(Theme::instance()->enforceVirtualFilesSyncFolder()); } - if (Theme::instance()->showVirtualFilesOption() && !folder->virtualFilesEnabled() && Vfs::checkAvailability(folder->path())) { - const auto mode = bestAvailableVfsMode(); + if (const auto mode = bestAvailableVfsMode(); + Theme::instance()->showVirtualFilesOption() + && !folder->virtualFilesEnabled() && Vfs::checkAvailability(folder->path(), mode)) { if (mode == Vfs::WindowsCfApi || ConfigFile().showExperimentalOptions()) { ac = menu->addAction(tr("Enable virtual file support %1 …").arg(mode == Vfs::WindowsCfApi ? QString() : tr("(experimental)"))); // TODO: remove when UX decision is made diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 8b8bb0c16f828..6b7f2ad45fc34 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -81,7 +81,7 @@ Folder::Folder(const FolderDefinition &definition, _syncResult.setStatus(status); // check if the local path exists - checkLocalPath(); + const auto folderOk = checkLocalPath(); _syncResult.setFolder(_definition.alias); @@ -155,8 +155,10 @@ Folder::Folder(const FolderDefinition &definition, saveToSettings(); } - // Initialize the vfs plugin - startVfs(); + if (folderOk) { + // Initialize the vfs plugin + startVfs(); + } } Folder::~Folder() @@ -169,7 +171,7 @@ Folder::~Folder() _engine.reset(); } -void Folder::checkLocalPath() +bool Folder::checkLocalPath() { const QFileInfo fi(_definition.localPath); _canonicalLocalPath = fi.canonicalFilePath(); @@ -187,18 +189,22 @@ void Folder::checkLocalPath() if (FileSystem::isDir(_definition.localPath) && FileSystem::isReadable(_definition.localPath)) { qCDebug(lcFolder) << "Checked local path ok"; } else { + QString error; // Check directory again if (!FileSystem::fileExists(_definition.localPath, fi)) { - _syncResult.appendErrorString(tr("Local folder %1 does not exist.").arg(_definition.localPath)); - _syncResult.setStatus(SyncResult::SetupError); - } else if (!FileSystem::isDir(_definition.localPath)) { - _syncResult.appendErrorString(tr("%1 should be a folder but is not.").arg(_definition.localPath)); - _syncResult.setStatus(SyncResult::SetupError); - } else if (!FileSystem::isReadable(_definition.localPath)) { - _syncResult.appendErrorString(tr("%1 is not readable.").arg(_definition.localPath)); + 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); + } + if (!error.isEmpty()) { + _syncResult.appendErrorString(error); _syncResult.setStatus(SyncResult::SetupError); + return false; } } + return true; } QString Folder::shortGuiRemotePathOrAppName() const @@ -297,7 +303,7 @@ bool Folder::syncPaused() const bool Folder::canSync() const { - return !syncPaused() && accountState()->isConnected(); + return !syncPaused() && accountState()->isConnected() && _syncResult.status() != SyncResult::SetupError; } void Folder::setSyncPaused(bool paused) @@ -507,6 +513,13 @@ void Folder::startVfs() ENFORCE(_vfs); ENFORCE(_vfs->mode() == _definition.virtualFilesMode); + const auto result = Vfs::checkAvailability(path(), _vfs->mode()); + if (!result) { + _syncResult.appendErrorString(result.error()); + _syncResult.setStatus(SyncResult::SetupError); + return; + } + VfsSetupParams vfsParams; vfsParams.filesystemPath = path(); vfsParams.displayName = shortGuiRemotePathOrAppName(); diff --git a/src/gui/folder.h b/src/gui/folder.h index 8b59fc6a30978..7b1b53929223e 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -474,7 +474,7 @@ private slots: void showSyncResultPopup(); - void checkLocalPath(); + bool checkLocalPath(); SyncOptions initializeSyncOptions() const; diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 2f08c40a2f29c..93e55952474d4 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -2027,4 +2027,9 @@ void FolderMan::slotConnectToPushNotifications(Account *account) } } +bool FolderMan::checkVfsAvailability(const QString &path, Vfs::Mode mode) const +{ + return unsupportedConfiguration(path) && Vfs::checkAvailability(path, mode); +} + } // namespace OCC diff --git a/src/gui/folderman.h b/src/gui/folderman.h index ef892a57473e5..f0dafc4f211f7 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -231,6 +231,9 @@ class FolderMan : public QObject /** removes current user from the share **/ void leaveShare(const QString &localFile); + /** Whether or not vfs is supported in the location. */ + bool checkVfsAvailability(const QString &path, Vfs::Mode mode = bestAvailableVfsMode()) const; + signals: /** * signal to indicate a folder has changed its sync state. diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 3b8ac7aac9a9d..d5a2225276e82 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -620,9 +620,10 @@ void FolderWizardSelectiveSync::initializePage() bool FolderWizardSelectiveSync::validatePage() { - const bool useVirtualFiles = _virtualFilesCheckBox && _virtualFilesCheckBox->isChecked(); + const auto mode = bestAvailableVfsMode(); + const bool useVirtualFiles = (Theme::instance()->forceVirtualFilesOption() && mode == Vfs::WindowsCfApi) || (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked()); if (useVirtualFiles) { - const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString()); + const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode); if (!availability) { auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this); msg->setAttribute(Qt::WA_DeleteOnClose); diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index 08017098100c1..f515e1c716a65 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -393,7 +393,7 @@ bool OwncloudAdvancedSetupPage::isConfirmBigFolderChecked() const bool OwncloudAdvancedSetupPage::validatePage() { if (useVirtualFileSync()) { - const auto availability = Vfs::checkAvailability(localFolder()); + const auto availability = Vfs::checkAvailability(localFolder(), bestAvailableVfsMode()); if (!availability) { auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this); msg->setAttribute(Qt::WA_DeleteOnClose); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 0f8691cd05960..c64b9219ca3cd 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -541,7 +541,7 @@ void SyncEngine::startSync() _progressInfo->reset(); - if (!QDir(_localPath).exists()) { + if (!QFileInfo::exists(_localPath)) { _anotherSyncNeeded = DelayedFollowUp; // No _tr, it should only occur in non-mirall Q_EMIT syncError(QStringLiteral("Unable to find local sync folder."), ErrorCategory::GenericError); From cb05cedd100269b821af7534bd52ced833f2b94e Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Mon, 6 Sep 2021 14:13:01 +0200 Subject: [PATCH 3/7] Warn if we encounter an unsupported configuration --- src/gui/folderman.cpp | 21 +++++++++++++++++++++ src/gui/folderman.h | 2 ++ src/gui/folderstatusmodel.cpp | 11 +++++++++-- src/gui/folderwizard.cpp | 2 +- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 93e55952474d4..fe44f591babc1 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -44,6 +44,13 @@ constexpr auto settingsAccountsC = "Accounts"; constexpr auto settingsFoldersC = "Folders"; constexpr auto settingsVersionC = "version"; constexpr auto maxFoldersVersion = 1; +const char versionC[] = "version"; + +int numberOfSyncJournals(const QString &path) +{ + return QDir(path).entryList({ QStringLiteral(".sync_*.db"), QStringLiteral("._sync_*.db") }, QDir::Hidden | QDir::Files).size(); +} + } namespace OCC { @@ -1800,6 +1807,9 @@ static QString checkPathValidityRecursive(const QString &path) Utility::NtfsPermissionLookupRAII ntfs_perm; #endif const QFileInfo selFile(path); + if (numberOfSyncJournals(selFile.filePath()) != 0) { + return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selFile.filePath())); + } if (!FileSystem::fileExists(path)) { QString parentPath = selFile.dir().path(); @@ -2032,4 +2042,15 @@ bool FolderMan::checkVfsAvailability(const QString &path, Vfs::Mode mode) const return unsupportedConfiguration(path) && Vfs::checkAvailability(path, mode); } +Result FolderMan::unsupportedConfiguration(const QString &path) const +{ + if (numberOfSyncJournals(path) > 1) { + return tr("Multiple accounts are sharing the folder %1.\n" + "This configuration is know to lead to dataloss and is no longer supported.\n" + "Please consider removing this folder from the account and adding it again.") + .arg(path); + } + return {}; +} + } // namespace OCC diff --git a/src/gui/folderman.h b/src/gui/folderman.h index f0dafc4f211f7..ebfebb0b44359 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -234,6 +234,8 @@ class FolderMan : public QObject /** Whether or not vfs is supported in the location. */ bool checkVfsAvailability(const QString &path, Vfs::Mode mode = bestAvailableVfsMode()) const; + /** If the folder configuration is no longer supported this will return an error string */ + Result unsupportedConfiguration(const QString &path) const; signals: /** * signal to indicate a folder has changed its sync state. diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp index f712cf92cffdc..44b2402c43c4f 100644 --- a/src/gui/folderstatusmodel.cpp +++ b/src/gui/folderstatusmodel.cpp @@ -250,8 +250,15 @@ QVariant FolderStatusModel::data(const QModelIndex &index, int role) const return (folder->syncResult().hasUnresolvedConflicts()) ? QStringList(tr("There are unresolved conflicts. Click for details.")) : QStringList(); - case FolderStatusDelegate::FolderErrorMsg: - return folder->syncResult().errorStrings(); + case FolderStatusDelegate::FolderErrorMsg: { + auto errors = folder->syncResult().errorStrings(); + const auto legacyError = FolderMan::instance()->unsupportedConfiguration(folder->path()); + if (!legacyError) { + // the error message might contain new lines, the delegate only expect multiple single line values + errors.append(legacyError.error().split(QLatin1Char('\n'))); + } + return errors; + } case FolderStatusDelegate::FolderInfoMsg: return folder->virtualFilesEnabled() && folder->vfs().mode() != Vfs::Mode::WindowsCfApi ? QStringList(tr("Virtual file support is enabled.")) diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index d5a2225276e82..fe710af3dccdd 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -621,7 +621,7 @@ void FolderWizardSelectiveSync::initializePage() bool FolderWizardSelectiveSync::validatePage() { const auto mode = bestAvailableVfsMode(); - const bool useVirtualFiles = (Theme::instance()->forceVirtualFilesOption() && mode == Vfs::WindowsCfApi) || (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked()); + const bool useVirtualFiles = (mode == Vfs::WindowsCfApi) || (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked()); if (useVirtualFiles) { const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode); if (!availability) { From 638f3a86479553543ac9ed9566d06eb1c430acfa Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Wed, 11 Dec 2024 13:14:49 +0100 Subject: [PATCH 4/7] Update tests for folder check to match fix. Signed-off-by: Camila Ayres --- test/testfolderman.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index 7a15c17e03e92..f40c3c7e4e1c6 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -288,7 +288,6 @@ private slots: QVERIFY(!folder->isSyncRunning()); } - // those should be allowed // QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory).second @@ -311,15 +310,15 @@ private slots: QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url2).second.isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url2).second.isNull()); - // Now it will work because the account is different + // The following both fail because they are already sync folders even if for another account QUrl url3("http://anotherexample.org"); url3.setUserName("dummy"); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString()); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url3).second, QString()); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/sub/ownCloud1"))); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/ownCloud2"))); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).second.isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").second.isNull()); - QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/f").second.isNull()); + QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/f").second.isNull()); #ifndef Q_OS_WIN // no links on windows, no permissions // make a bunch of links @@ -338,7 +337,7 @@ private slots: // link 3 points to an existing sync folder. To make it fail, the account must be the same QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).second.isNull()); // while with a different account, this is fine - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString()); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(dirPath + "/link3")); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").second.isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").second.isNull()); From f9ec7380b07e1de8fc7df4f9b458b9632bd78f35 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Wed, 11 Dec 2024 15:32:59 +0100 Subject: [PATCH 5/7] Remove unused variable. Signed-off-by: Camila Ayres --- src/gui/folderman.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index fe44f591babc1..bb3aa4fbd9d92 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -44,7 +44,6 @@ constexpr auto settingsAccountsC = "Accounts"; constexpr auto settingsFoldersC = "Folders"; constexpr auto settingsVersionC = "version"; constexpr auto maxFoldersVersion = 1; -const char versionC[] = "version"; int numberOfSyncJournals(const QString &path) { From 7581f29201c420d02d5b0144b52c5f30e78d35ba Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 12 Dec 2024 09:44:49 +0100 Subject: [PATCH 6/7] Add [[nodiscard]] to functions. Signed-off-by: Camila Ayres --- src/gui/folderman.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/folderman.h b/src/gui/folderman.h index ebfebb0b44359..27a231adc9bed 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -232,10 +232,10 @@ class FolderMan : public QObject void leaveShare(const QString &localFile); /** Whether or not vfs is supported in the location. */ - bool checkVfsAvailability(const QString &path, Vfs::Mode mode = bestAvailableVfsMode()) const; + [[nodiscard]] bool checkVfsAvailability(const QString &path, Vfs::Mode mode = bestAvailableVfsMode()) const; /** If the folder configuration is no longer supported this will return an error string */ - Result unsupportedConfiguration(const QString &path) const; + [[nodiscard]] Result unsupportedConfiguration(const QString &path) const; signals: /** * signal to indicate a folder has changed its sync state. From d3d7963dc0597cfd105adeaf3b0313d9b1a69010 Mon Sep 17 00:00:00 2001 From: Camila Ayres Date: Thu, 12 Dec 2024 10:47:08 +0100 Subject: [PATCH 7/7] All conditions must be true to use virtual files in the selected location. Signed-off-by: Camila Ayres --- src/gui/folderwizard.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index fe710af3dccdd..8cf29f35e43c9 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -621,7 +621,7 @@ void FolderWizardSelectiveSync::initializePage() bool FolderWizardSelectiveSync::validatePage() { const auto mode = bestAvailableVfsMode(); - const bool useVirtualFiles = (mode == Vfs::WindowsCfApi) || (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked()); + const bool useVirtualFiles = (mode == Vfs::WindowsCfApi) && (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked()); if (useVirtualFiles) { const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode); if (!availability) {