Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: correctly detects network drive. #7636

Merged
merged 7 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/common/vfs.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/common/vfs.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/common/vfs.cpp

File src/common/vfs.cpp does not conform to Custom style guidelines. (lines 27)
* Copyright (C) by Dominik Schmidt <[email protected]>
*
* This library is free software; you can redistribute it and/or
Expand All @@ -16,7 +16,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/

#include "config.h"

Check failure on line 19 in src/common/vfs.cpp

View workflow job for this annotation

GitHub Actions / build

src/common/vfs.cpp:19:10 [clang-diagnostic-error]

'config.h' file not found
#include "vfs.h"
#include "plugin.h"
#include "version.h"
Expand All @@ -24,6 +24,7 @@

#include "common/filesystembase.h"

#include <QDir>
#include <QPluginLoader>
#include <QLoggingCategory>

Expand Down Expand Up @@ -65,21 +66,28 @@
return {};
}

Result<bool, QString> Vfs::checkAvailability(const QString &path)
Result<void, QString> Vfs::checkAvailability(const QString &path, Vfs::Mode mode)
{
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<const wchar_t *>(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)
Q_UNUSED(path)
#endif
return true;
return {};
}

void Vfs::start(const VfsSetupParams &params)
Expand Down
2 changes: 1 addition & 1 deletion src/common/vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
#pragma once

#include "ocsynclib.h"

Check failure on line 16 in src/common/vfs.h

View workflow job for this annotation

GitHub Actions / build

src/common/vfs.h:16:10 [clang-diagnostic-error]

'ocsynclib.h' file not found
#include "result.h"
#include "syncfilestatus.h"
#include "pinstate.h"
Expand Down Expand Up @@ -125,7 +125,7 @@
static QString modeToString(Mode mode);
static Optional<Mode> modeFromString(const QString &str);

static Result<bool, QString> checkAvailability(const QString &path);
static Result<void, QString> checkAvailability(const QString &path, OCC::Vfs::Mode mode);

enum class AvailabilityError
{
Expand Down
5 changes: 3 additions & 2 deletions src/gui/accountsettings.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/accountsettings.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/accountsettings.cpp

File src/gui/accountsettings.cpp does not conform to Custom style guidelines. (lines 714)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -710,8 +710,9 @@
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
Expand Down
37 changes: 25 additions & 12 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*/
#include "common/syncjournaldb.h"

Check failure on line 16 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:16:10 [clang-diagnostic-error]

'common/syncjournaldb.h' file not found
#include "config.h"

#include "account.h"
Expand Down Expand Up @@ -81,7 +81,7 @@
_syncResult.setStatus(status);

// check if the local path exists
checkLocalPath();
const auto folderOk = checkLocalPath();

_syncResult.setFolder(_definition.alias);

Expand Down Expand Up @@ -155,8 +155,10 @@
saveToSettings();
}

// Initialize the vfs plugin
startVfs();
if (folderOk) {
// Initialize the vfs plugin
startVfs();
}
}

Folder::~Folder()
Expand All @@ -169,7 +171,7 @@
_engine.reset();
}

void Folder::checkLocalPath()
bool Folder::checkLocalPath()

Check warning on line 174 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:174:14 [modernize-use-trailing-return-type]

use a trailing return type for this function

Check warning on line 174 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:174:14 [readability-convert-member-functions-to-static]

method 'checkLocalPath' can be made static
{
const QFileInfo fi(_definition.localPath);
_canonicalLocalPath = fi.canonicalFilePath();
Expand All @@ -187,18 +189,22 @@
if (FileSystem::isDir(_definition.localPath) && FileSystem::isReadable(_definition.localPath)) {
qCDebug(lcFolder) << "Checked local path ok";
} else {
QString error;

Check warning on line 192 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:192:17 [cppcoreguidelines-init-variables]

variable 'error' is not initialized
// Check directory again
if (!FileSystem::fileExists(_definition.localPath, fi)) {

Check warning on line 194 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:194:65 [bugprone-branch-clone]

repeated branch in conditional chain
_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
Expand Down Expand Up @@ -297,7 +303,7 @@

bool Folder::canSync() const
{
return !syncPaused() && accountState()->isConnected();
return !syncPaused() && accountState()->isConnected() && _syncResult.status() != SyncResult::SetupError;
}

void Folder::setSyncPaused(bool paused)
Expand Down Expand Up @@ -507,7 +513,14 @@
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;

Check warning on line 523 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:523:20 [cppcoreguidelines-init-variables]

variable 'vfsParams' is not initialized
vfsParams.filesystemPath = path();
vfsParams.displayName = shortGuiRemotePathOrAppName();
vfsParams.alias = alias();
Expand Down
2 changes: 1 addition & 1 deletion src/gui/folder.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#ifndef MIRALL_FOLDER_H
#define MIRALL_FOLDER_H

#include "syncresult.h"

Check failure on line 20 in src/gui/folder.h

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.h:20:10 [clang-diagnostic-error]

'syncresult.h' file not found
#include "progressdispatcher.h"
#include "common/syncjournaldb.h"
#include "networkjobs.h"
Expand Down Expand Up @@ -474,7 +474,7 @@

void showSyncResultPopup();

void checkLocalPath();
bool checkLocalPath();

SyncOptions initializeSyncOptions() const;

Expand Down
25 changes: 25 additions & 0 deletions src/gui/folderman.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/folderman.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/folderman.cpp

File src/gui/folderman.cpp does not conform to Custom style guidelines. (lines 50)
* Copyright (C) by Klaas Freitag <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -44,6 +44,12 @@
constexpr auto settingsFoldersC = "Folders";
constexpr auto settingsVersionC = "version";
constexpr auto maxFoldersVersion = 1;

int numberOfSyncJournals(const QString &path)
{
return QDir(path).entryList({ QStringLiteral(".sync_*.db"), QStringLiteral("._sync_*.db") }, QDir::Hidden | QDir::Files).size();
}

}

namespace OCC {
Expand Down Expand Up @@ -1800,6 +1806,9 @@
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();
Expand Down Expand Up @@ -2027,4 +2036,20 @@
}
}

bool FolderMan::checkVfsAvailability(const QString &path, Vfs::Mode mode) const
{
return unsupportedConfiguration(path) && Vfs::checkAvailability(path, mode);
}

Result<void, QString> 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
5 changes: 5 additions & 0 deletions src/gui/folderman.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#ifndef FOLDERMAN_H
#define FOLDERMAN_H

#include <QByteArray>

Check failure on line 19 in src/gui/folderman.h

View workflow job for this annotation

GitHub Actions / build

src/gui/folderman.h:19:10 [clang-diagnostic-error]

'QByteArray' file not found
#include <QObject>
#include <QQueue>
#include <QList>
Expand Down Expand Up @@ -231,6 +231,11 @@
/** removes current user from the share **/
void leaveShare(const QString &localFile);

/** Whether or not vfs is supported in the location. */
[[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 */
[[nodiscard]] Result<void, QString> unsupportedConfiguration(const QString &path) const;
signals:
/**
* signal to indicate a folder has changed its sync state.
Expand Down
11 changes: 9 additions & 2 deletions src/gui/folderstatusmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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."))
Expand Down
5 changes: 3 additions & 2 deletions src/gui/folderwizard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,9 +620,10 @@ void FolderWizardSelectiveSync::initializePage()

bool FolderWizardSelectiveSync::validatePage()
{
const bool useVirtualFiles = _virtualFilesCheckBox && _virtualFilesCheckBox->isChecked();
const auto mode = bestAvailableVfsMode();
const bool useVirtualFiles = (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);
Expand Down
2 changes: 1 addition & 1 deletion src/gui/wizard/owncloudadvancedsetuppage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* for more details.
*/

#include <QDir>

Check failure on line 16 in src/gui/wizard/owncloudadvancedsetuppage.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/wizard/owncloudadvancedsetuppage.cpp:16:10 [clang-diagnostic-error]

'QDir' file not found
#include <QFileDialog>
#include <QUrl>
#include <QTimer>
Expand Down Expand Up @@ -393,7 +393,7 @@
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);
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/syncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 5 additions & 6 deletions test/testfolderman.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*

Check notice on line 1 in test/testfolderman.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/testfolderman.cpp

File test/testfolderman.cpp does not conform to Custom style guidelines. (lines 316, 317, 340)
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
*
*/

#include <qglobal.h>

Check failure on line 8 in test/testfolderman.cpp

View workflow job for this annotation

GitHub Actions / build

test/testfolderman.cpp:8:10 [clang-diagnostic-error]

'qglobal.h' file not found
#include <QTemporaryDir>
#include <QtTest>

Expand Down Expand Up @@ -288,7 +288,6 @@
QVERIFY(!folder->isSyncRunning());
}


// those should be allowed
// QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory).second

Expand All @@ -311,15 +310,15 @@
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
Expand All @@ -338,7 +337,7 @@
// 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());
Expand Down
Loading