From fd769adb050901353172928a10a8f37d1055813c Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 21 Oct 2024 14:52:14 +0800 Subject: [PATCH 1/7] Mark non-const getters in Folder definition as const Signed-off-by: Claudio Cambra --- src/gui/folder.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gui/folder.h b/src/gui/folder.h index c79153a86..f55f9c62b 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -223,11 +223,11 @@ public: void setIgnoreHiddenFiles(bool ignore); // Used by the Socket API - SyncJournalDb *journalDb() { return &_journal; } - SyncEngine &syncEngine() { return *_engine; } - Vfs &vfs() { return *_vfs; } + SyncJournalDb *journalDb() const { return &_journal; } + SyncEngine &syncEngine() const { return *_engine; } + Vfs &vfs() const { return *_vfs; } - RequestEtagJob *etagJob() { return _requestEtagJob; } + RequestEtagJob *etagJob() const { return _requestEtagJob; } std::chrono::milliseconds msecSinceLastSync() const { return std::chrono::milliseconds(_timeSinceLastSyncDone.elapsed()); } std::chrono::milliseconds msecLastSyncDuration() const { return _lastSyncDuration; } int consecutiveFollowUpSyncs() const { return _consecutiveFollowUpSyncs; } From 20a4915e7ce329194e19c563215f6b9eda75c39f Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 21 Oct 2024 14:54:34 +0800 Subject: [PATCH 2/7] Store fetched folder as private member in filedetails Signed-off-by: Claudio Cambra --- src/gui/filedetails/filedetails.cpp | 12 ++++++------ src/gui/filedetails/filedetails.h | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/gui/filedetails/filedetails.cpp b/src/gui/filedetails/filedetails.cpp index 67ebb554c..e7897aaf5 100644 --- a/src/gui/filedetails/filedetails.cpp +++ b/src/gui/filedetails/filedetails.cpp @@ -61,15 +61,15 @@ void FileDetails::setLocalPath(const QString &localPath) _fileWatcher.addPath(localPath); connect(&_fileWatcher, &QFileSystemWatcher::fileChanged, this, &FileDetails::refreshFileDetails); - const auto folder = FolderMan::instance()->folderForPath(_localPath); - if (!folder) { + _folder = FolderMan::instance()->folderForPath(_localPath); + if (!_folder) { qCWarning(lcFileDetails) << "No folder found for path:" << _localPath << "will not load file details."; return; } - const auto file = _localPath.mid(folder->cleanPath().length() + 1); + const auto file = _localPath.mid(_folder->cleanPath().length() + 1); - if (!folder->journalDb()->getFileRecord(file, &_fileRecord)) { + if (!_folder->journalDb()->getFileRecord(file, &_fileRecord)) { qCWarning(lcFileDetails) << "Invalid file record for path:" << _localPath << "will not load file details."; @@ -77,9 +77,9 @@ void FileDetails::setLocalPath(const QString &localPath) _filelockState = _fileRecord._lockstate; updateLockExpireString(); - updateFileTagModel(folder); - _sharingAvailable = folder->accountState()->account()->capabilities().shareAPI(); + const auto account = _folder->accountState()->account(); + _sharingAvailable = account->capabilities().shareAPI(); Q_EMIT fileChanged(); } diff --git a/src/gui/filedetails/filedetails.h b/src/gui/filedetails/filedetails.h index 2834981af..a4dc7e593 100644 --- a/src/gui/filedetails/filedetails.h +++ b/src/gui/filedetails/filedetails.h @@ -73,6 +73,7 @@ private: QFileInfo _fileInfo; QFileSystemWatcher _fileWatcher; + Folder *_folder = nullptr; SyncJournalFileRecord _fileRecord; SyncJournalFileLockInfo _filelockState; QByteArray _numericFileId; From b8b8b480aafd4866d82643ae314766c8ab3dd018 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 21 Oct 2024 14:56:14 +0800 Subject: [PATCH 3/7] Simply pass AccountPtr to updateFileTagModel Signed-off-by: Claudio Cambra --- src/gui/filedetails/filedetails.cpp | 5 ++--- src/gui/filedetails/filedetails.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gui/filedetails/filedetails.cpp b/src/gui/filedetails/filedetails.cpp index e7897aaf5..2b76c70ad 100644 --- a/src/gui/filedetails/filedetails.cpp +++ b/src/gui/filedetails/filedetails.cpp @@ -80,6 +80,7 @@ void FileDetails::setLocalPath(const QString &localPath) const auto account = _folder->accountState()->account(); _sharingAvailable = account->capabilities().shareAPI(); + updateFileTagModel(account); Q_EMIT fileChanged(); } @@ -167,10 +168,8 @@ FileTagModel *FileDetails::fileTagModel() const return _fileTagModel.get(); } -void FileDetails::updateFileTagModel(const Folder * const folder) +void FileDetails::updateFileTagModel(const AccountPtr &account) { - Q_ASSERT(folder); - const auto account = folder->accountState()->account(); Q_ASSERT(account); const auto serverRelPath = QString(folder->remotePathTrailingSlash() + name()); diff --git a/src/gui/filedetails/filedetails.h b/src/gui/filedetails/filedetails.h index a4dc7e593..fef2b010a 100644 --- a/src/gui/filedetails/filedetails.h +++ b/src/gui/filedetails/filedetails.h @@ -66,7 +66,7 @@ signals: private slots: void refreshFileDetails(); void updateLockExpireString(); - void updateFileTagModel(const OCC::Folder * const folder); + void updateFileTagModel(const OCC::AccountPtr &account); private: QString _localPath; From bf2303cd6c8a4f4808c87370450630c917763769 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 21 Oct 2024 14:57:22 +0800 Subject: [PATCH 4/7] Ensure we are checking for tags in correct server path regardless of the particular local path Fixes issues with users who are using remote non-root sync folders Signed-off-by: Claudio Cambra --- src/gui/filedetails/filedetails.cpp | 6 +----- src/gui/filetagmodel.cpp | 12 ++++++++++-- src/gui/filetagmodel.h | 7 ++++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/gui/filedetails/filedetails.cpp b/src/gui/filedetails/filedetails.cpp index 2b76c70ad..9ebe8fac2 100644 --- a/src/gui/filedetails/filedetails.cpp +++ b/src/gui/filedetails/filedetails.cpp @@ -170,11 +170,7 @@ FileTagModel *FileDetails::fileTagModel() const void FileDetails::updateFileTagModel(const AccountPtr &account) { - Q_ASSERT(account); - - const auto serverRelPath = QString(folder->remotePathTrailingSlash() + name()); - - _fileTagModel = std::make_unique(serverRelPath, account); + _fileTagModel = std::make_unique(_fileRecord, _folder, account); Q_EMIT fileTagModelChanged(); } diff --git a/src/gui/filetagmodel.cpp b/src/gui/filetagmodel.cpp index 64cab4486..282ff84c1 100644 --- a/src/gui/filetagmodel.cpp +++ b/src/gui/filetagmodel.cpp @@ -20,13 +20,21 @@ Q_LOGGING_CATEGORY(lcFileTagModel, "nextcloud.gui.filetagmodel", QtInfoMsg) namespace OCC { -FileTagModel::FileTagModel(const QString &serverRelativePath, +FileTagModel::FileTagModel(const SyncJournalFileRecord &fileRecord, + const Folder * const syncFolder, const AccountPtr &account, QObject * const parent) : QAbstractListModel(parent) - , _serverRelativePath(serverRelativePath) , _account(account) { + _serverRelativePath = syncFolder->remotePathTrailingSlash() + fileRecord.path(); + + if (const auto vfsMode = syncFolder->vfs().mode(); fileRecord.isVirtualFile() && vfsMode == Vfs::WithSuffix) { + if (const auto suffix = syncFolder->vfs().fileSuffix(); !suffix.isEmpty() && _serverRelativePath.endsWith(suffix)) { + _serverRelativePath.chop(suffix.length()); + } + } + fetchFileTags(); } diff --git a/src/gui/filetagmodel.h b/src/gui/filetagmodel.h index 618702e8b..77933eccd 100644 --- a/src/gui/filetagmodel.h +++ b/src/gui/filetagmodel.h @@ -16,6 +16,8 @@ #include +#include "common/syncjournalfilerecord.h" +#include "gui/folder.h" #include "libsync/account.h" namespace OCC { @@ -31,7 +33,10 @@ class FileTagModel : public QAbstractListModel Q_PROPERTY(QString overflowTagsString READ overflowTagsString NOTIFY overflowTagsStringChanged) public: - explicit FileTagModel(const QString &serverRelativePath, const AccountPtr &account, QObject * const parent = nullptr); + explicit FileTagModel(const SyncJournalFileRecord &fileRecord, + const Folder *const syncFolder, + const AccountPtr &account, + QObject *const parent = nullptr); [[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override; [[nodiscard]] QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; From 8ef972211bf49aedc6c0dac2d83795e4a9fdf339 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 23 Oct 2024 14:53:37 +0800 Subject: [PATCH 5/7] Move setup of server relative path used in FileTagModel into FileDetails This fixes the build breakages in the file tag model tests Signed-off-by: Claudio Cambra --- src/gui/filedetails/filedetails.cpp | 33 +++++++++++++++++++++++++---- src/gui/filedetails/filedetails.h | 2 +- src/gui/filetagmodel.cpp | 12 ++--------- src/gui/filetagmodel.h | 3 +-- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/gui/filedetails/filedetails.cpp b/src/gui/filedetails/filedetails.cpp index 9ebe8fac2..bc8f2d489 100644 --- a/src/gui/filedetails/filedetails.cpp +++ b/src/gui/filedetails/filedetails.cpp @@ -62,6 +62,7 @@ void FileDetails::setLocalPath(const QString &localPath) connect(&_fileWatcher, &QFileSystemWatcher::fileChanged, this, &FileDetails::refreshFileDetails); _folder = FolderMan::instance()->folderForPath(_localPath); + Q_ASSERT(_folder); if (!_folder) { qCWarning(lcFileDetails) << "No folder found for path:" << _localPath << "will not load file details."; return; @@ -78,9 +79,23 @@ void FileDetails::setLocalPath(const QString &localPath) _filelockState = _fileRecord._lockstate; updateLockExpireString(); - const auto account = _folder->accountState()->account(); + const auto accountState = _folder->accountState(); + Q_ASSERT(accountState); + if (!accountState) { + qCWarning(lcFileDetails) << "No account state found for path:" << _localPath << "will not correctly load file details."; + return; + } + + const auto account = accountState->account(); + Q_ASSERT(account); + if (!account) { + qCWarning(lcFileDetails) << "No account found for path:" << _localPath << "will not correctly load file details."; + return; + } + _sharingAvailable = account->capabilities().shareAPI(); - updateFileTagModel(account); + + updateFileTagModel(); Q_EMIT fileChanged(); } @@ -168,9 +183,19 @@ FileTagModel *FileDetails::fileTagModel() const return _fileTagModel.get(); } -void FileDetails::updateFileTagModel(const AccountPtr &account) +void FileDetails::updateFileTagModel() { - _fileTagModel = std::make_unique(_fileRecord, _folder, account); + const auto localPath = _fileRecord.path(); + const auto relPath = localPath.mid(_folder->cleanPath().length() + 1); + QString serverPath = _folder->remotePathTrailingSlash() + _fileRecord.path(); + + if (const auto vfsMode = _folder->vfs().mode(); _fileRecord.isVirtualFile() && vfsMode == Vfs::WithSuffix) { + if (const auto suffix = _folder->vfs().fileSuffix(); !suffix.isEmpty() && serverPath.endsWith(suffix)) { + serverPath.chop(suffix.length()); + } + } + + _fileTagModel = std::make_unique(relPath, _folder->accountState()->account()); Q_EMIT fileTagModelChanged(); } diff --git a/src/gui/filedetails/filedetails.h b/src/gui/filedetails/filedetails.h index fef2b010a..8f0ff7165 100644 --- a/src/gui/filedetails/filedetails.h +++ b/src/gui/filedetails/filedetails.h @@ -66,7 +66,7 @@ signals: private slots: void refreshFileDetails(); void updateLockExpireString(); - void updateFileTagModel(const OCC::AccountPtr &account); + void updateFileTagModel(); private: QString _localPath; diff --git a/src/gui/filetagmodel.cpp b/src/gui/filetagmodel.cpp index 282ff84c1..64cab4486 100644 --- a/src/gui/filetagmodel.cpp +++ b/src/gui/filetagmodel.cpp @@ -20,21 +20,13 @@ Q_LOGGING_CATEGORY(lcFileTagModel, "nextcloud.gui.filetagmodel", QtInfoMsg) namespace OCC { -FileTagModel::FileTagModel(const SyncJournalFileRecord &fileRecord, - const Folder * const syncFolder, +FileTagModel::FileTagModel(const QString &serverRelativePath, const AccountPtr &account, QObject * const parent) : QAbstractListModel(parent) + , _serverRelativePath(serverRelativePath) , _account(account) { - _serverRelativePath = syncFolder->remotePathTrailingSlash() + fileRecord.path(); - - if (const auto vfsMode = syncFolder->vfs().mode(); fileRecord.isVirtualFile() && vfsMode == Vfs::WithSuffix) { - if (const auto suffix = syncFolder->vfs().fileSuffix(); !suffix.isEmpty() && _serverRelativePath.endsWith(suffix)) { - _serverRelativePath.chop(suffix.length()); - } - } - fetchFileTags(); } diff --git a/src/gui/filetagmodel.h b/src/gui/filetagmodel.h index 77933eccd..9f0a6b6d0 100644 --- a/src/gui/filetagmodel.h +++ b/src/gui/filetagmodel.h @@ -33,8 +33,7 @@ class FileTagModel : public QAbstractListModel Q_PROPERTY(QString overflowTagsString READ overflowTagsString NOTIFY overflowTagsStringChanged) public: - explicit FileTagModel(const SyncJournalFileRecord &fileRecord, - const Folder *const syncFolder, + explicit FileTagModel(const QString &serverRelativePath, const AccountPtr &account, QObject *const parent = nullptr); From f159f40ec97e74cc000efc931b04fec546442cb5 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 21 Nov 2024 15:32:07 +0800 Subject: [PATCH 6/7] Add missing nodiscards in folder methods Signed-off-by: Claudio Cambra --- src/gui/folder.h | 64 ++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/gui/folder.h b/src/gui/folder.h index f55f9c62b..8b59fc6a3 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -128,23 +128,23 @@ public: /** * The account the folder is configured on. */ - AccountState *accountState() const { return _accountState.data(); } + [[nodiscard]] AccountState *accountState() const { return _accountState.data(); } /** * alias or nickname */ - QString alias() const; - QString shortGuiRemotePathOrAppName() const; // since 2.0 we don't want to show aliases anymore, show the path instead + [[nodiscard]] QString alias() const; + [[nodiscard]] QString shortGuiRemotePathOrAppName() const; // since 2.0 we don't want to show aliases anymore, show the path instead /** * short local path to display on the GUI (native separators) */ - QString shortGuiLocalPath() const; + [[nodiscard]] QString shortGuiLocalPath() const; /** * canonical local folder path, always ends with / */ - QString path() const; + [[nodiscard]] QString path() const; /** * cleaned canonical folder path, like path() but never ends with a / @@ -152,39 +152,39 @@ public: * Wrapper for QDir::cleanPath(path()) except for "Z:/", * where it returns "Z:" instead of "Z:/". */ - QString cleanPath() const; + [[nodiscard]] QString cleanPath() const; /** * remote folder path, usually without trailing /, exception "/" */ - QString remotePath() const; + [[nodiscard]] QString remotePath() const; /** * remote folder path, always with a trailing / */ - QString remotePathTrailingSlash() const; + [[nodiscard]] QString remotePathTrailingSlash() const; [[nodiscard]] QString fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const; void setNavigationPaneClsid(const QUuid &clsid) { _definition.navigationPaneClsid = clsid; } - QUuid navigationPaneClsid() const { return _definition.navigationPaneClsid; } + [[nodiscard]] QUuid navigationPaneClsid() const { return _definition.navigationPaneClsid; } /** * remote folder path with server url */ - QUrl remoteUrl() const; + [[nodiscard]] QUrl remoteUrl() const; /** * switch sync on or off */ void setSyncPaused(bool); - bool syncPaused() const; + [[nodiscard]] bool syncPaused() const; /** * Returns true when the folder may sync. */ - bool canSync() const; + [[nodiscard]] bool canSync() const; void prepareToSync(); @@ -192,15 +192,15 @@ public: * True if the folder is busy and can't initiate * a synchronization */ - virtual bool isBusy() const; + [[nodiscard]] virtual bool isBusy() const; /** True if the folder is currently synchronizing */ - bool isSyncRunning() const; + [[nodiscard]] bool isSyncRunning() const; /** * return the last sync result with error message and status */ - SyncResult syncResult() const; + [[nodiscard]] SyncResult syncResult() const; /** * This is called when the sync folder definition is removed. Do cleanups here. @@ -219,19 +219,19 @@ public: * Ignore syncing of hidden files or not. This is defined in the * folder definition */ - bool ignoreHiddenFiles(); + [[nodiscard]] bool ignoreHiddenFiles(); void setIgnoreHiddenFiles(bool ignore); // Used by the Socket API - SyncJournalDb *journalDb() const { return &_journal; } - SyncEngine &syncEngine() const { return *_engine; } - Vfs &vfs() const { return *_vfs; } + [[nodiscard]] SyncJournalDb *journalDb() const { return &_journal; } + [[nodiscard]] SyncEngine &syncEngine() const { return *_engine; } + [[nodiscard]] Vfs &vfs() const { return *_vfs; } - RequestEtagJob *etagJob() const { return _requestEtagJob; } - std::chrono::milliseconds msecSinceLastSync() const { return std::chrono::milliseconds(_timeSinceLastSyncDone.elapsed()); } - std::chrono::milliseconds msecLastSyncDuration() const { return _lastSyncDuration; } - int consecutiveFollowUpSyncs() const { return _consecutiveFollowUpSyncs; } - int consecutiveFailingSyncs() const { return _consecutiveFailingSyncs; } + [[nodiscard]] RequestEtagJob *etagJob() const { return _requestEtagJob; } + [[nodiscard]] std::chrono::milliseconds msecSinceLastSync() const { return std::chrono::milliseconds(_timeSinceLastSyncDone.elapsed()); } + [[nodiscard]] std::chrono::milliseconds msecLastSyncDuration() const { return _lastSyncDuration; } + [[nodiscard]] int consecutiveFollowUpSyncs() const { return _consecutiveFollowUpSyncs; } + [[nodiscard]] int consecutiveFailingSyncs() const { return _consecutiveFailingSyncs; } /// Saves the folder data in the account's settings. void saveToSettings() const; @@ -244,12 +244,12 @@ public: /** * Returns whether a file inside this folder should be excluded. */ - bool isFileExcludedAbsolute(const QString &fullPath) const; + [[nodiscard]] bool isFileExcludedAbsolute(const QString &fullPath) const; /** * Returns whether a file inside this folder should be excluded. */ - bool isFileExcludedRelative(const QString &relativePath) const; + [[nodiscard]] bool isFileExcludedRelative(const QString &relativePath) const; /** Calls schedules this folder on the FolderMan after a short delay. * @@ -288,13 +288,13 @@ public: * and never have an automatic virtual file. But when it's on, the shell context menu will allow * users to make existing files virtual. */ - bool virtualFilesEnabled() const; + [[nodiscard]] bool virtualFilesEnabled() const; void setVirtualFilesEnabled(bool enabled); void setRootPinState(PinState state); /** Whether user desires a switch that couldn't be executed yet, see member */ - bool isVfsOnOffSwitchPending() const { return _vfsOnOffPending; } + [[nodiscard]] bool isVfsOnOffSwitchPending() const { return _vfsOnOffPending; } void setVfsOnOffSwitchPending(bool pending) { _vfsOnOffPending = pending; } void switchToVirtualFiles(); @@ -302,9 +302,9 @@ public: void processSwitchedToVirtualFiles(); /** Whether this folder should show selective sync ui */ - bool supportsSelectiveSync() const; + [[nodiscard]] bool supportsSelectiveSync() const; - QString fileFromLocalPath(const QString &localPath) const; + [[nodiscard]] QString fileFromLocalPath(const QString &localPath) const; void whitelistPath(const QString &path); void blacklistPath(const QString &path); @@ -347,9 +347,9 @@ public slots: void startSync(const QStringList &pathList = QStringList()); int slotDiscardDownloadProgress(); - int downloadInfoCount(); + [[nodiscard]] int downloadInfoCount(); int slotWipeErrorBlacklist(); - int errorBlackListEntryCount(); + [[nodiscard]] int errorBlackListEntryCount(); /** * Triggered by the folder watcher when a file/dir in this folder From 2769ae8a53030253911eb9f3da525011359d34a5 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 21 Nov 2024 15:39:27 +0800 Subject: [PATCH 7/7] Remove unused includes in filetagmodel Signed-off-by: Claudio Cambra --- src/gui/filetagmodel.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/gui/filetagmodel.h b/src/gui/filetagmodel.h index 9f0a6b6d0..37bb635c0 100644 --- a/src/gui/filetagmodel.h +++ b/src/gui/filetagmodel.h @@ -16,8 +16,6 @@ #include -#include "common/syncjournalfilerecord.h" -#include "gui/folder.h" #include "libsync/account.h" namespace OCC {