From d90d92bc45621613e120ab14ca42474bc3bd575d Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Thu, 2 Dec 2021 18:15:52 +0100 Subject: [PATCH] Fix SonarCloud issue - FolderDefinition::load can never fail, so just return the newly loaded FolderDefinition instead of passing it in by reference. This also removes an if statement, reducing the complexity. - FolderMan::addFolderInternal can never return a nullptr, so replace the if check in FolderMan::addFolder by an assert. --- src/gui/folder.cpp | 43 +++++++-------- src/gui/folder.h | 3 +- src/gui/folderman.cpp | 123 +++++++++++++++++++++--------------------- src/gui/folderman.h | 4 +- 4 files changed, 88 insertions(+), 85 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index f570f43c26d..3617ccaef50 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1337,54 +1337,55 @@ void FolderDefinition::save(QSettings &settings, const FolderDefinition &folder) } } -bool FolderDefinition::load(const QSettings &settings, const QString &alias, FolderDefinition *folder) +FolderDefinition FolderDefinition::load(const QSettings &settings, const QString &alias) { - folder->alias = FolderMan::unescapeAlias(alias); - folder->localPath = settings.value(localPathC).toString(); - folder->journalPath = settings.value(journalPathC).toString(); - folder->targetPath = settings.value(targetPathC).toString(); - folder->paused = settings.value(pausedC).toBool(); - folder->ignoreHiddenFiles = settings.value(ignoreHiddenFilesC, QVariant(true)).toBool(); - folder->isInNavigationPane = settings.value(isInNavigationPaneC, QVariant(true)).toBool(); + FolderDefinition folder; + folder.alias = FolderMan::unescapeAlias(alias); + folder.localPath = settings.value(localPathC).toString(); + folder.journalPath = settings.value(journalPathC).toString(); + folder.targetPath = settings.value(targetPathC).toString(); + folder.paused = settings.value(pausedC).toBool(); + folder.ignoreHiddenFiles = settings.value(ignoreHiddenFilesC, QVariant(true)).toBool(); + folder.isInNavigationPane = settings.value(isInNavigationPaneC, QVariant(true)).toBool(); if (settings.contains(uuidC)) { - folder->uuid = settings.value(uuidC).toUuid(); + folder.uuid = settings.value(uuidC).toUuid(); } else if (settings.contains(navigationPaneClsidC)) { // For backwards compatibility, will be changed when saved: - folder->uuid = settings.value(navigationPaneClsidC).toUuid(); - if (!folder->isInNavigationPane) { - folder->isInNavigationPane = true; + folder.uuid = settings.value(navigationPaneClsidC).toUuid(); + if (!folder.isInNavigationPane) { + folder.isInNavigationPane = true; } } // Sanity check: - if (folder->uuid.isNull()) { - folder->uuid = QUuid::createUuid(); + if (folder.uuid.isNull()) { + folder.uuid = QUuid::createUuid(); } - folder->virtualFilesMode = Vfs::Off; + folder.virtualFilesMode = Vfs::Off; QString vfsModeString = settings.value(virtualFilesModeC).toString(); if (!vfsModeString.isEmpty()) { if (auto mode = Vfs::modeFromString(vfsModeString)) { - folder->virtualFilesMode = *mode; + folder.virtualFilesMode = *mode; } else { qCWarning(lcFolder) << "Unknown virtualFilesMode:" << vfsModeString << "assuming 'off'"; } } else { if (settings.value(usePlaceholdersC).toBool()) { - folder->virtualFilesMode = Vfs::WithSuffix; - folder->upgradeVfsMode = true; // maybe winvfs is available? + folder.virtualFilesMode = Vfs::WithSuffix; + folder.upgradeVfsMode = true; // maybe winvfs is available? } } // Old settings can contain paths with native separators. In the rest of the // code we assume /, so clean it up now. - folder->localPath = prepareLocalPath(folder->localPath); + folder.localPath = prepareLocalPath(folder.localPath); // Target paths also have a convention - folder->targetPath = prepareTargetPath(folder->targetPath); + folder.targetPath = prepareTargetPath(folder.targetPath); - return true; + return folder; } QString FolderDefinition::prepareLocalPath(const QString &path) diff --git a/src/gui/folder.h b/src/gui/folder.h index afcb2362a75..4ec7534c620 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -85,8 +85,7 @@ class FolderDefinition static void save(QSettings &settings, const FolderDefinition &folder); /// Reads a folder definition from the current settings group. - static bool load(const QSettings &settings, const QString &alias, - FolderDefinition *folder); + static FolderDefinition load(const QSettings &settings, const QString &alias); /** The highest version in the settings that load() can read * diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index f42e646e2ad..363ac344ee7 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -306,78 +306,79 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, } settings.endGroup(); - FolderDefinition folderDefinition; settings.beginGroup(folderAlias); - if (FolderDefinition::load(settings, folderAlias, &folderDefinition)) { - const auto defaultJournalPath = [&account, folderDefinition] { - // if we would have booth the 2.9.0 file name and the lagacy file - // with the md5 infix we prefer the 2.9.0 version - const QDir info(folderDefinition.localPath); - const QString defaultPath = SyncJournalDb::makeDbName(folderDefinition.localPath); - if (info.exists(defaultPath)) { - return defaultPath; - } - // 2.6 - QString legacyPath = makeLegacyDbName(folderDefinition, account->account()); - if (info.exists(legacyPath)) { - return legacyPath; - } - // pre 2.6 - legacyPath.replace(QLatin1String(".sync_"), QLatin1String("._sync_")); - if (info.exists(legacyPath)) { - return legacyPath; - } + FolderDefinition folderDefinition = FolderDefinition::load(settings, folderAlias); + const auto defaultJournalPath = [&account, folderDefinition] { + // if we would have both the 2.9.0 file name and the lagacy file + // with the md5 infix we prefer the 2.9.0 version + const QDir info(folderDefinition.localPath); + const QString defaultPath = SyncJournalDb::makeDbName(folderDefinition.localPath); + if (info.exists(defaultPath)) { return defaultPath; - }(); - - // Migration: Old settings don't have journalPath - if (folderDefinition.journalPath.isEmpty()) { - folderDefinition.journalPath = defaultJournalPath; } - - // Migration: ._ files sometimes can't be created. - // So if the configured journalPath has a dot-underscore ("._sync_*.db") - // but the current default doesn't have the underscore, switch to the - // new default if no db exists yet. - if (folderDefinition.journalPath.startsWith("._sync_") - && defaultJournalPath.startsWith(".sync_") - && !QFile::exists(folderDefinition.absoluteJournalPath())) { - folderDefinition.journalPath = defaultJournalPath; + // 2.6 + QString legacyPath = makeLegacyDbName(folderDefinition, account->account()); + if (info.exists(legacyPath)) { + return legacyPath; } - - // Migration: If an old .csync_journal.db is found, move it to the new name. - if (backwardsCompatible) { - SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath()); + // pre 2.6 + legacyPath.replace(QLatin1String(".sync_"), QLatin1String("._sync_")); + if (info.exists(legacyPath)) { + return legacyPath; } + return defaultPath; + }(); - auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode); - if (!vfs) { - // TODO: Must do better error handling - qFatal("Could not load plugin"); - } + // Migration: Old settings don't have journalPath + if (folderDefinition.journalPath.isEmpty()) { + folderDefinition.journalPath = defaultJournalPath; + } - Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs)); - if (f) { - // Migrate the old "usePlaceholders" setting to the root folder pin state - if (settings.value(versionC, 1).toInt() == 1 - && settings.value(QLatin1String("usePlaceholders"), false).toBool()) { - qCInfo(lcFolderMan) << "Migrate: From usePlaceholders to PinState::OnlineOnly"; - f->setRootPinState(PinState::OnlineOnly); - } + // Migration: ._ files sometimes can't be created. + // So if the configured journalPath has a dot-underscore ("._sync_*.db") + // but the current default doesn't have the underscore, switch to the + // new default if no db exists yet. + if (folderDefinition.journalPath.startsWith("._sync_") + && defaultJournalPath.startsWith(".sync_") + && !QFile::exists(folderDefinition.absoluteJournalPath())) { + folderDefinition.journalPath = defaultJournalPath; + } - // Migration: Mark folders that shall be saved in a backwards-compatible way - if (backwardsCompatible) - f->setSaveBackwardsCompatible(true); - if (foldersWithPlaceholders) - f->setSaveInFoldersWithPlaceholders(); + // Migration: If an old .csync_journal.db is found, move it to the new name. + if (backwardsCompatible) { + SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath()); + } - // save possible changes from the migration - f->saveToSettings(); + auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode); + if (!vfs) { + // TODO: Must do better error handling + qFatal("Could not load plugin"); + } - scheduleFolder(f); - emit folderSyncStateChange(f); - } + Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs)); + Q_ASSERT(f); + + // Migrate the old "usePlaceholders" setting to the root folder pin state + if (settings.value(versionC, 1).toInt() == 1 + && settings.value(QLatin1String("usePlaceholders"), false).toBool()) { + qCInfo(lcFolderMan) << "Migrate: From usePlaceholders to PinState::OnlineOnly"; + f->setRootPinState(PinState::OnlineOnly); + } + + // Migration: Mark folders that shall be saved in a backwards-compatible way + if (backwardsCompatible) { + f->setSaveBackwardsCompatible(true); + } + if (foldersWithPlaceholders) { + f->setSaveInFoldersWithPlaceholders(); } + + // save possible changes from the migration + f->saveToSettings(); + + scheduleFolder(f); + emit folderSyncStateChange(f); + settings.endGroup(); } } diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 5c93110bc74..74e88f74050 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -329,7 +329,9 @@ private slots: private: /** Adds a new folder, does not add it to the account settings and * does not set an account on the new folder. - */ + * + * \returns the newly created Folder, which can never be a nullptr. + */ Folder *addFolderInternal(FolderDefinition folderDefinition, AccountState *accountState, std::unique_ptr vfs);