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.

Sign up for GitHub

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

Move geometry/server version from nextcloud.cfg #5367

Closed
wants to merge 2 commits into from
Closed

Move geometry/server version from nextcloud.cfg #5367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8681ba
Move geometry/server version from nextcloud.cfg
tokun-mobica Jan 25, 2023
0d2b9d2
Move clientVersion from nextcloud.cfg
tokun-mobica Jan 26, 2023
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
19 changes: 10 additions & 9 deletions src/gui/accountmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ bool AccountManager::restore(bool alsoRestoreLegacySettings)
for (const auto &accountId : settingsChildGroups) {
settings->beginGroup(accountId);
if (!skipSettingsKeys.contains(settings->group())) {
if (const auto acc = loadAccountHelper(*settings)) {
if (const auto acc = loadAccountHelper(*settings, *ConfigFile::localSettings())) {
acc->_id = accountId;
if (auto accState = AccountState::loadFromSettings(acc, *settings)) {
auto jar = qobject_cast<CookieJar*>(acc->_am->cookieJar());
Expand Down Expand Up @@ -232,7 +232,7 @@ bool AccountManager::restoreFromLegacySettings()
const auto childGroups = settings->childGroups();
for (const auto &accountId : childGroups) {
settings->beginGroup(accountId);
if (const auto acc = loadAccountHelper(*settings)) {
if (const auto acc = loadAccountHelper(*settings, *ConfigFile::localSettings())) {
addAccount(acc);

return true;
Expand All @@ -248,7 +248,7 @@ void AccountManager::save(bool saveCredentials)
settings->setValue(QLatin1String(versionC), maxAccountsVersion);
for (const auto &acc : qAsConst(_accounts)) {
settings->beginGroup(acc->account()->id());
saveAccountHelper(acc->account().data(), *settings, saveCredentials);
saveAccountHelper(acc->account().data(), *settings, *ConfigFile::localSettings(), saveCredentials);
acc->writeToSettings(*settings);
settings->endGroup();
}
Expand All @@ -261,8 +261,9 @@ void AccountManager::saveAccount(Account *a)
{
qCDebug(lcAccountManager) << "Saving account" << a->url().toString();
const auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
const auto localData = ConfigFile::localSettings();
settings->beginGroup(a->id());
saveAccountHelper(a, *settings, false); // don't save credentials they might not have been loaded yet
saveAccountHelper(a, *settings, *localData, false); // don't save credentials they might not have been loaded yet
settings->endGroup();

settings->sync();
Expand All @@ -281,14 +282,13 @@ void AccountManager::saveAccountState(AccountState *a)
qCDebug(lcAccountManager) << "Saved account state settings, status:" << settings->status();
}

void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool saveCredentials)
void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, QSettings &localData, bool saveCredentials)
{
settings.setValue(QLatin1String(versionC), maxAccountVersion);
settings.setValue(QLatin1String(urlC), acc->_url.toString());
settings.setValue(QLatin1String(davUserC), acc->_davUser);
settings.setValue(QLatin1String(displayNameC), acc->_displayName);
settings.setValue(QLatin1String(serverVersionC), acc->_serverVersion);

localData.setValue(QLatin1String(serverVersionC), acc->_serverVersion);
if (acc->_credentials) {
if (saveCredentials) {
// Only persist the credentials if the parameter is set, on migration from 1.8.x
Expand Down Expand Up @@ -335,7 +335,7 @@ void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool s
}
}

AccountPtr AccountManager::loadAccountHelper(QSettings &settings)
AccountPtr AccountManager::loadAccountHelper(QSettings &settings, QSettings &localData)
{
const auto urlConfig = settings.value(QLatin1String(urlC));
if (!urlConfig.isValid()) {
Expand Down Expand Up @@ -386,7 +386,8 @@ AccountPtr AccountManager::loadAccountHelper(QSettings &settings)

qCInfo(lcAccountManager) << "Account for" << acc->url() << "using auth type" << authType;

acc->_serverVersion = settings.value(QLatin1String(serverVersionC)).toString();
acc->_serverVersion = localData.value(QLatin1String(serverVersionC)).toString();

acc->_davUser = settings.value(QLatin1String(davUserC), "").toString();

// We want to only restore settings for that auth type and the user value
Expand Down
4 changes: 2 additions & 2 deletions src/gui/accountmanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ class AccountManager : public QObject

private:
// saving and loading Account to settings
void saveAccountHelper(Account *account, QSettings &settings, bool saveCredentials = true);
AccountPtr loadAccountHelper(QSettings &settings);
void saveAccountHelper(Account *account, QSettings &settings, QSettings &localData, bool saveCredentials = true);
AccountPtr loadAccountHelper(QSettings &settings, QSettings &localData);

bool restoreFromLegacySettings();

Expand Down
30 changes: 24 additions & 6 deletions src/libsync/configfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ void ConfigFile::saveGeometry(QWidget *w)
{
#ifndef TOKEN_AUTH_ONLY
ASSERT(!w->objectName().isNull());
QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings(localConfigFile(), QSettings::IniFormat);
settings.beginGroup(w->objectName());
settings.setValue(QLatin1String(geometryC), w->saveGeometry());
settings.sync();
Expand All @@ -291,7 +291,7 @@ void ConfigFile::saveGeometryHeader(QHeaderView *header)
return;
ASSERT(!header->objectName().isEmpty());

QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings(localConfigFile(), QSettings::IniFormat);
settings.beginGroup(header->objectName());
settings.setValue(QLatin1String(geometryC), header->saveState());
settings.sync();
Expand All @@ -305,7 +305,7 @@ void ConfigFile::restoreGeometryHeader(QHeaderView *header)
return;
ASSERT(!header->objectName().isNull());

QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings(localConfigFile(), QSettings::IniFormat);
settings.beginGroup(header->objectName());
header->restoreState(settings.value(geometryC).toByteArray());
#endif
Expand Down Expand Up @@ -363,6 +363,17 @@ QString ConfigFile::configPath() const
return dir;
}

QString ConfigFile::localConfigFile() const
{
const QString settingspath = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppDataLocation is the wrong path here.
Use QStandardPaths::ConfigLocation.

Read the Qt docs for more:
https://doc.qt.io/qt-5/qstandardpaths.html#StandardLocation-enum

if(!QDir(settingspath).exists()) {
QDir().mkdir(settingspath);
}
QString filename = settingspath + QLatin1String("/") + QLatin1String("settings.cfg");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to the localConfigFile not being expressive of what this file is for, settings.cfg has the same issue


return filename;
}

static const QLatin1String exclFile("sync-exclude.lst");

QString ConfigFile::excludeFile(Scope scope) const
Expand Down Expand Up @@ -775,7 +786,7 @@ QVariant ConfigFile::getValue(const QString &param, const QString &group,
systemSetting = systemSettings.value(param, defaultValue);
}

QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings((param != QLatin1String(geometryC)) ? configFile() : localConfigFile(), QSettings::IniFormat);
if (!group.isEmpty())
settings.beginGroup(group);

Expand Down Expand Up @@ -1070,13 +1081,13 @@ void ConfigFile::setCertificatePasswd(const QString &cPasswd)

QString ConfigFile::clientVersionString() const
{
QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings(localConfigFile(), QSettings::IniFormat);
return settings.value(QLatin1String(clientVersionC), QString()).toString();
}

void ConfigFile::setClientVersionString(const QString &version)
{
QSettings settings(configFile(), QSettings::IniFormat);
QSettings settings(localConfigFile(), QSettings::IniFormat);
settings.setValue(QLatin1String(clientVersionC), version);
}

Expand All @@ -1094,6 +1105,13 @@ std::unique_ptr<QSettings> ConfigFile::settingsWithGroup(const QString &group, Q
return settings;
}

std::unique_ptr<QSettings> ConfigFile::localSettings(QObject *parent)
{
ConfigFile cfg;
std::unique_ptr<QSettings> settings(std::make_unique<QSettings>(cfg.localConfigFile(), QSettings::IniFormat, parent));
return settings;
}

void ConfigFile::setupDefaultExcludeFilePaths(ExcludedFiles &excludedFiles)
{
ConfigFile cfg;
Expand Down
4 changes: 4 additions & 0 deletions src/libsync/configfile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ class OWNCLOUDSYNC_EXPORT ConfigFile

[[nodiscard]] QString configPath() const;
[[nodiscard]] QString configFile() const;
[[nodiscard]] QString localConfigFile() const;
Comment on lines 47 to +48
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is impossible to understand what the difference between configFile and localConfigFile is from their names, and without really digging into the code. localConfigFile should have a name that is more indicative of the data it will hold

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. I hope for suggestions about more adequate name

[[nodiscard]] QString excludeFile(Scope scope) const;

static QString excludeFileFromSystem(); // doesn't access config dir

/**
Expand Down Expand Up @@ -209,6 +211,8 @@ class OWNCLOUDSYNC_EXPORT ConfigFile
with the given parent. If no parent is specified, the caller must destroy the settings */
static std::unique_ptr<QSettings> settingsWithGroup(const QString &group, QObject *parent = nullptr);

static std::unique_ptr<QSettings> localSettings(QObject *parent = nullptr);

/// Add the system and user exclude file path to the ExcludedFiles instance.
static void setupDefaultExcludeFilePaths(ExcludedFiles &excludedFiles);

Expand Down