From 0f2109cd18cc8827bc30c592db14dd6afe2ed702 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Thu, 19 Feb 2026 10:06:54 +0100 Subject: [PATCH] Introduce Path object to siplify use of std::filesystem::path with QString --- src/gui/folder.cpp | 1 - src/libsync/CMakeLists.txt | 2 + src/libsync/common/filesystembase.cpp | 11 +++-- src/libsync/common/filesystembase.h | 4 +- src/libsync/path.cpp | 31 ++++++++++++ src/libsync/path.h | 68 ++++++++++++++++++++++++++ src/libsync/vfs/vfs.cpp | 15 +++++- src/libsync/vfs/vfs.h | 22 ++++----- src/plugins/vfs/cfapi/cfapiwrapper.cpp | 10 ++-- src/plugins/vfs/cfapi/vfs_cfapi.cpp | 15 +++--- test/testutils/syncenginetestutils.cpp | 1 - 11 files changed, 146 insertions(+), 34 deletions(-) create mode 100644 src/libsync/path.cpp create mode 100644 src/libsync/path.h diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index fb69337f6..a7d4ed84b 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -520,7 +520,6 @@ void Folder::startVfs() OC_ENFORCE(_vfs->mode() == _definition.virtualFilesMode); VfsSetupParams vfsParams(_accountState->account(), webDavUrl(), _definition.spaceId(), displayName(), _engine.get()); - vfsParams.filesystemPath = path(); vfsParams.journal = &_journal; vfsParams.providerDisplayName = Theme::instance()->appNameGUI(); vfsParams.providerName = Theme::instance()->appName(); diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index 4d3c6b02e..4c4dbe402 100644 --- a/src/libsync/CMakeLists.txt +++ b/src/libsync/CMakeLists.txt @@ -64,6 +64,8 @@ add_library(libsync SHARED abstractcorejob.cpp appprovider.cpp + + path.cpp ) if(WIN32) diff --git a/src/libsync/common/filesystembase.cpp b/src/libsync/common/filesystembase.cpp index 3e6a78da8..2e21b9c0c 100644 --- a/src/libsync/common/filesystembase.cpp +++ b/src/libsync/common/filesystembase.cpp @@ -51,12 +51,13 @@ namespace OCC { Q_LOGGING_CATEGORY(lcFileSystem, "sync.filesystem", QtInfoMsg) -std::filesystem::path FileSystem::toFilesystemPath(QString path) +std::filesystem::path FileSystem::toFilesystemPath(const QString &path) { #ifdef Q_OS_WIN - path = FileSystem::longWinPath(path); + return QtPrivate::toFilesystemPath(FileSystem::longWinPath(path)); +#else + return QtPrivate::toFilesystemPath(path); #endif - return std::filesystem::path(reinterpret_cast(path.cbegin()), reinterpret_cast(path.cend())); } QString FileSystem::fromFilesystemPath(const std::filesystem::path &path) @@ -71,9 +72,9 @@ QString FileSystem::fromFilesystemPath(const std::filesystem::path &path) return QDir::fromNativeSeparators(QString::fromWCharArray(view.data(), view.length())); #elif defined(Q_OS_MACOS) // based on QFile::decodeName - return QString::fromStdString(path.native()).normalized(QString::NormalizationForm_C); + return QtPrivate::fromFilesystemPath(path).normalized(QString::NormalizationForm_C); #else - return QString::fromStdString(path.native()); + return QtPrivate::fromFilesystemPath(path); #endif } diff --git a/src/libsync/common/filesystembase.h b/src/libsync/common/filesystembase.h index 5c48bae8c..bb61a220c 100644 --- a/src/libsync/common/filesystembase.h +++ b/src/libsync/common/filesystembase.h @@ -27,7 +27,6 @@ #include #include -#include class QFile; @@ -48,9 +47,10 @@ OPENCLOUD_SYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem) { OPENCLOUD_SYNC_EXPORT Q_NAMESPACE; - OPENCLOUD_SYNC_EXPORT std::filesystem::path toFilesystemPath(QString path); + OPENCLOUD_SYNC_EXPORT std::filesystem::path toFilesystemPath(const QString &path); OPENCLOUD_SYNC_EXPORT QString fromFilesystemPath(const std::filesystem::path &path); + /** * List of characters not allowd in filenames on Windows */ diff --git a/src/libsync/path.cpp b/src/libsync/path.cpp new file mode 100644 index 000000000..1b6741e2f --- /dev/null +++ b/src/libsync/path.cpp @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// SPDX-FileCopyrightText: 2025 Hannah von Reth + +#include "path.h" + +#include "libsync/common/filesystembase.h" + +OCC::FileSystem::Path::Path(QAnyStringView path) + : _path(toFilesystemPath(path.toString())) +{ +} + +OCC::FileSystem::Path::Path(const std::filesystem::path &path) + : _path(path) +{ +} + +OCC::FileSystem::Path::Path(std::filesystem::path &&path) + : _path(std::move(path)) +{ +} + +OCC::FileSystem::Path OCC::FileSystem::Path::relative(QAnyStringView path) +{ + return QtPrivate::toFilesystemPath(path.toString()); +} + +QString OCC::FileSystem::Path::toString() const +{ + return fromFilesystemPath(_path); +} diff --git a/src/libsync/path.h b/src/libsync/path.h new file mode 100644 index 000000000..f072e5510 --- /dev/null +++ b/src/libsync/path.h @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// SPDX-FileCopyrightText: 2025 Hannah von Reth + +#pragma once + +#include "libsync/opencloudsynclib.h" + +#include + +#include + +namespace OCC { +namespace FileSystem { + class OPENCLOUD_SYNC_EXPORT Path + { + public: + /** + * Will create an absolute path from the given QString. + * On Windows it will create a UNC path starting with "\\?\". + * For path segments use Path::relative. + * @param path + */ + explicit Path(QAnyStringView path); + + Path(const std::filesystem::path &path); + + Path(std::filesystem::path &&path); + + /** + * Creates a relative path segment + * @param path + * @return A relative path + */ + static Path relative(QAnyStringView path); + + const std::filesystem::path &get() const { return _path; } + + [[nodiscard]] operator std::filesystem::path() const { return _path; } + + explicit operator QString() const { return toString(); } + + Path operator/(const Path &other) const { return _path / other._path; } + Path operator/(const std::filesystem::path &other) const { return _path / other; } + Path operator/(QAnyStringView other) const { return _path / relative(other); } + + Path &operator/=(const Path &other) + { + _path /= other._path; + return *this; + } + Path &operator/=(const std::filesystem::path &other) + { + _path /= other; + return *this; + } + Path &operator/=(QAnyStringView other) + { + _path /= relative(other); + return *this; + } + + QString toString() const; + + private: + std::filesystem::path _path; + }; +} +} diff --git a/src/libsync/vfs/vfs.cpp b/src/libsync/vfs/vfs.cpp index 5da1b8992..885f5a0de 100644 --- a/src/libsync/vfs/vfs.cpp +++ b/src/libsync/vfs/vfs.cpp @@ -23,6 +23,7 @@ #include "libsync/common/syncjournaldb.h" #include "libsync/common/version.h" #include "libsync/filesystem.h" +#include "libsync/syncengine.h" #include #include @@ -100,7 +101,7 @@ void Vfs::wipeDehydratedVirtualFiles() // If the local file is a dehydrated placeholder, wipe it too. // Otherwise leave it to allow the next sync to have a new-new conflict. - const QString absolutePath = _setupParams->filesystemPath + relativePath; + const auto absolutePath = QString(_setupParams->root() / relativePath); if (QFile::exists(absolutePath)) { // according to our db this is a dehydrated file, check it to be sure if (isDehydratedPlaceholder(absolutePath)) { @@ -261,7 +262,9 @@ VfsSetupParams::VfsSetupParams(const AccountPtr &account, const QUrl &baseUrl, c , _syncEngine(syncEngine) , _spaceId(spaceId) , _folderDisplayName(folderDisplayName) + , _root(syncEngine->localPath()) { + Q_ASSERT(filesystemPath().endsWith('/'_L1)); } QString VfsSetupParams::folderDisplayName() const @@ -273,3 +276,13 @@ SyncEngine *VfsSetupParams::syncEngine() const { return _syncEngine; } + +QString VfsSetupParams::filesystemPath() const +{ + return _root.toString(); +} + +const FileSystem::Path &VfsSetupParams::root() const +{ + return _root; +} diff --git a/src/libsync/vfs/vfs.h b/src/libsync/vfs/vfs.h index 851475d62..4bb40247e 100644 --- a/src/libsync/vfs/vfs.h +++ b/src/libsync/vfs/vfs.h @@ -13,21 +13,21 @@ */ #pragma once -#include "../common/pinstate.h" -#include "../common/result.h" -#include "../common/syncfilestatus.h" -#include "../common/utility.h" -#include "assert.h" +#include "filesystem.h" #include "libsync/accountfwd.h" +#include "libsync/common/pinstate.h" +#include "libsync/common/result.h" +#include "libsync/common/syncfilestatus.h" +#include "libsync/common/utility.h" #include "libsync/discoveryinfo.h" #include "libsync/opencloudsynclib.h" +#include "libsync/path.h" #include #include #include #include -#include #include #include @@ -43,11 +43,7 @@ class HydrationJob; struct OPENCLOUD_SYNC_EXPORT VfsSetupParams { explicit VfsSetupParams(const AccountPtr &account, const QUrl &baseUrl, const QString &spaceId, const QString &folderDisplayName, SyncEngine *syncEngine); - /** The full path to the folder on the local filesystem - * - * Always ends with /. - */ - QString filesystemPath; + QString folderDisplayName() const; /// Account url, credentials etc for network calls @@ -69,11 +65,15 @@ struct OPENCLOUD_SYNC_EXPORT VfsSetupParams SyncEngine *syncEngine() const; + QString filesystemPath() const; + const FileSystem::Path &root() const; + private: QUrl _baseUrl; SyncEngine *_syncEngine; QString _spaceId; QString _folderDisplayName; + FileSystem::Path _root; }; /** Interface describing how to deal with virtual/placeholder files. diff --git a/src/plugins/vfs/cfapi/cfapiwrapper.cpp b/src/plugins/vfs/cfapi/cfapiwrapper.cpp index 9cfba493f..5a89ad0c9 100644 --- a/src/plugins/vfs/cfapi/cfapiwrapper.cpp +++ b/src/plugins/vfs/cfapi/cfapiwrapper.cpp @@ -223,7 +223,7 @@ void CALLBACK cfApiRename(const CF_CALLBACK_INFO *callbackInfo, const CF_CALLBAC if (callbackParameters->Rename.Flags & (CF_CALLBACK_RENAME_FLAG_TARGET_IN_SCOPE | CF_CALLBACK_RENAME_FLAG_SOURCE_IN_SCOPE) && // CF_CALLBACK_RENAME_FLAG_TARGET_IN_SCOPE is also set for any other sync root we manage // but in that case windows will hydrate the file before its moved - OCC::FileSystem::isChildPathOf(target, context.vfs->params().filesystemPath) && context.vfs->isDehydratedPlaceholder(context.path) + OCC::FileSystem::isChildPathOf(target, context.vfs->params().filesystemPath()) && context.vfs->isDehydratedPlaceholder(context.path) && context.vfs->params().syncEngine()->isExcluded(qtPath)) { reject = true; } @@ -356,7 +356,7 @@ QString createSyncRootID(const QString &providerName, const QUuid &accountUUID, void OCC::CfApiWrapper::registerSyncRoot(const VfsSetupParams ¶ms, const std::function &callback) { - const auto nativePath = QDir::toNativeSeparators(params.filesystemPath); + const auto nativePath = QDir::toNativeSeparators(params.filesystemPath()); winrt::StorageFolder::GetFolderFromPathAsync(reinterpret_cast(nativePath.utf16())) .Completed([params, callback](const winrt::IAsyncOperation &result, winrt::AsyncStatus status) { if (status != winrt::AsyncStatus::Completed) { @@ -365,7 +365,7 @@ void OCC::CfApiWrapper::registerSyncRoot(const VfsSetupParams ¶ms, const std } try { const auto iconPath = QCoreApplication::applicationFilePath(); - const auto id = createSyncRootID(params.providerName, params.account->uuid(), params.filesystemPath); + const auto id = createSyncRootID(params.providerName, params.account->uuid(), params.filesystemPath()); const auto version = params.providerVersion.toString(); winrt::StorageProviderSyncRootInfo info; @@ -400,7 +400,7 @@ void OCC::CfApiWrapper::registerSyncRoot(const VfsSetupParams ¶ms, const std } callback({}); } catch (const winrt::hresult_error &ex) { - callback(u"Failed to register sync root %1: %2"_s.arg(params.filesystemPath, Utility::formatWinError(ex.code()))); + callback(u"Failed to register sync root %1: %2"_s.arg(params.filesystemPath(), Utility::formatWinError(ex.code()))); } }); } @@ -431,7 +431,7 @@ OCC::Result OCC::CfApiWrapper::unregisterSyncRoot(const VfsSetupP try { std::lock_guard lock(sRegister_mutex); winrt::StorageProviderSyncRootManager::Unregister( - reinterpret_cast(createSyncRootID(params.providerName, params.account->uuid(), params.filesystemPath).utf16())); + reinterpret_cast(createSyncRootID(params.providerName, params.account->uuid(), params.filesystemPath()).utf16())); } catch (winrt::hresult_error const &ex) { return u"unregisterSyncRoot failed: %1"_s.arg(Utility::formatWinError(ex.code())); } diff --git a/src/plugins/vfs/cfapi/vfs_cfapi.cpp b/src/plugins/vfs/cfapi/vfs_cfapi.cpp index 15b1384f4..666fc410e 100644 --- a/src/plugins/vfs/cfapi/vfs_cfapi.cpp +++ b/src/plugins/vfs/cfapi/vfs_cfapi.cpp @@ -118,7 +118,7 @@ void VfsCfApi::startImpl(const VfsSetupParams ¶ms) cfapi::registerSyncRoot(params, [this](const QString &errorMessage) { if (errorMessage.isEmpty()) { - auto connectResult = cfapi::connectSyncRoot(this->params().filesystemPath, this); + auto connectResult = cfapi::connectSyncRoot(this->params().filesystemPath(), this); if (!connectResult) { qCCritical(lcCfApi) << u"Initialization failed, couldn't connect sync root:" << connectResult.error(); return; @@ -155,7 +155,7 @@ void VfsCfApi::stop() if (_connectionKey.Internal != 0) { const auto result = cfapi::disconnectSyncRoot(std::move(_connectionKey)); if (!result) { - qCCritical(lcCfApi) << u"Disconnect failed for" << params().filesystemPath << u":" << result.error(); + qCCritical(lcCfApi) << u"Disconnect failed for" << params().filesystemPath() << u":" << result.error(); } } } @@ -164,7 +164,7 @@ void VfsCfApi::unregisterFolder() { const auto result = cfapi::unregisterSyncRoot(params()); if (!result) { - qCCritical(lcCfApi) << u"Unregistration failed for" << params().filesystemPath << u":" << result.error(); + qCCritical(lcCfApi) << u"Unregistration failed for" << params().filesystemPath() << u":" << result.error(); } #if 0 @@ -197,15 +197,14 @@ Result VfsCfApi::updateMetadata(const Result VfsCfApi::createPlaceholder(const SyncFileItem &item) { - Q_ASSERT(params().filesystemPath.endsWith('/'_L1)); - const auto localPath = QDir::toNativeSeparators(params().filesystemPath + item.localName()); + const auto localPath = QDir::toNativeSeparators(params().filesystemPath() + item.localName()); const auto result = cfapi::createPlaceholderInfo(localPath, item._modtime, item._size, item._fileId); return result; } bool VfsCfApi::needsMetadataUpdate(const SyncFileItem &item) { - const QString path = params().filesystemPath + item.localName(); + const QString path = params().filesystemPath() + item.localName(); if (!QFileInfo::exists(path)) { return false; } @@ -258,13 +257,13 @@ bool VfsCfApi::setPinState(const QString &folderPath, PinState state) { qCDebug(lcCfApi) << u"setPinState" << folderPath << state; - const auto localPath = QDir::toNativeSeparators(params().filesystemPath + folderPath); + const auto localPath = QDir::toNativeSeparators(params().filesystemPath() + folderPath); return static_cast(cfapi::setPinState(localPath, state, cfapi::Recurse)); } Optional VfsCfApi::pinState(const QString &folderPath) { - const auto localPath = QDir::toNativeSeparators(params().filesystemPath + folderPath); + const auto localPath = QDir::toNativeSeparators(params().filesystemPath() + folderPath); const auto info = cfapi::findPlaceholderInfo(localPath); if (!info) { qCDebug(lcCfApi) << u"Couldn't find pin state for regular non-placeholder file" << localPath; diff --git a/test/testutils/syncenginetestutils.cpp b/test/testutils/syncenginetestutils.cpp index 90a7b24ee..751ce24db 100644 --- a/test/testutils/syncenginetestutils.cpp +++ b/test/testutils/syncenginetestutils.cpp @@ -892,7 +892,6 @@ void FakeFolder::switchToVfs(QSharedPointer vfs) _syncEngine->setSyncOptions(opts); OCC::VfsSetupParams vfsParams(account(), OCC::TestUtils::dummyDavUrl(), QString(), u"DisplayName"_s, &syncEngine()); - vfsParams.filesystemPath = localPath(); vfsParams.journal = _journalDb.get(); vfsParams.providerName = QStringLiteral("OC-TEST"); vfsParams.providerDisplayName = QStringLiteral("OC-TEST");