diff --git a/ChangeLog b/ChangeLog index 5eda12676..9d65411a8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -75,6 +75,7 @@ Enhancements: - #7429 Parse date numbers as long instead of int - #7430 Improve setting enable logic and test coverage - #7431 Improve handling of Qt-related warnings and errors +- #7432 Only show close to tray reminder when not quitting the app # 1.14.6 diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 3abeb4fd2..96e469df0 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -259,7 +259,11 @@ void MainWindow::connectSlots() { m_pActionRestore, &QAction::triggered, // [this]() { showAndActivate(); }); - connect(m_pActionQuit, &QAction::triggered, qApp, &QCoreApplication::quit); + connect(m_pActionQuit, &QAction::triggered, qApp, [this] { + qDebug("quitting application"); + m_Quitting = true; + qApp->quit(); + }); connect( &m_VersionChecker, &VersionChecker::updateFound, this, @@ -316,6 +320,17 @@ void MainWindow::onShown() { showActivationDialog(); } } + + // if a critical error was shown just before the main window (i.e. on app + // load), it will be hidden behind the main window. therefore we need to raise + // it up in front of the main window. + // HACK: because the `onShown` event happens just as the window is shown, the + // message box has a chance of being raised under the main window. to solve + // this we delay the error dialog raise by a split second. this seems a bit + // hacky and fragile, so maybe there's a better approach. + const auto kCriticalDialogDelay = 100; + QTimer::singleShot( + kCriticalDialogDelay, [] { messages::raiseCriticalDialog(); }); } void MainWindow::onLicenseHandlerSerialKeyChanged(const QString &serialKey) { @@ -674,7 +689,9 @@ void MainWindow::handleLogLine(const QString &line) { const auto scrollAtBottom = qAbs(currentScroll - maxScroll) <= kScrollBottomThreshold; - m_pLogOutput->appendPlainText(line); + // only trim end instead of the whole line to prevent tab-indented debug + // filenames from losing their indentation. + m_pLogOutput->appendPlainText(trimEnd(line)); if (scrollAtBottom) { verticalScroll->setValue(verticalScroll->maximum()); @@ -730,14 +747,14 @@ void MainWindow::checkFingerprint(const QString &line) { QMessageBox::StandardButton fingerprintReply = QMessageBox::information( this, QString("Security question"), QString( - "You are connecting to a server. Here is it's fingerprint:\n\n" - "%1\n\n" - "Compare this fingerprint to the one on your server's screen." + "

You are connecting to a server.

" + "

Here is it's TLS fingerprint:

" + "

%1

" + "

Compare this fingerprint to the one on your server's screen. " "If the two don't match exactly, then it's probably not the server " - "you're expecting (it could be a malicious user).\n\n" - "To automatically trust this fingerprint for future " - "connections, click Yes. To reject this fingerprint and " - "disconnect from the server, click No.") + "you're expecting (it could be a malicious user).

" + "

Do you want to trust this fingerprint for future " + "connections? If you don't, a connection cannot be made.

") .arg(fingerprint), QMessageBox::Yes | QMessageBox::No); @@ -762,18 +779,23 @@ void MainWindow::showEvent(QShowEvent *event) { } void MainWindow::closeEvent(QCloseEvent *event) { + if (m_Quitting) { + qDebug("skipping close event handle on quit"); + return; + } + if (!m_AppConfig.closeToTray()) { + qDebug("window will not hide to tray"); return; } - qDebug("window will hide to tray"); - if (!m_AppConfig.showCloseReminder()) { - return; + if (m_AppConfig.showCloseReminder()) { + messages::showCloseReminder(this); + m_AppConfig.setShowCloseReminder(false); } - messages::showCloseReminder(this); - m_AppConfig.setShowCloseReminder(false); m_ConfigScopes.save(); + qDebug("window should hide to tray"); } void MainWindow::showFirstRunMessage() { @@ -791,7 +813,7 @@ void MainWindow::showFirstRunMessage() { void MainWindow::showDevThanksMessage() { if (!m_AppConfig.showDevThanks()) { - qDebug("skipping dev thanks message, disabled in settings"); + qDebug("skipping dev thanks message"); return; } @@ -1139,5 +1161,6 @@ void MainWindow::showAndActivate() { } showNormal(); + raise(); activateWindow(); } diff --git a/src/gui/src/MainWindow.h b/src/gui/src/MainWindow.h index 355e1b3d8..f6b492b27 100644 --- a/src/gui/src/MainWindow.h +++ b/src/gui/src/MainWindow.h @@ -92,7 +92,6 @@ private slots: // // Manual slots // - void onCreated(); void onShown(); void onConfigScopesSaving(); @@ -194,6 +193,7 @@ private: bool m_SecureSocket = false; bool m_SaveWindow = false; LicenseHandler m_LicenseHandler; + bool m_Quitting = false; synergy::gui::ConfigScopes &m_ConfigScopes; AppConfig &m_AppConfig; diff --git a/src/gui/src/main.cpp b/src/gui/src/main.cpp index 2d71e8a8b..02159d720 100644 --- a/src/gui/src/main.cpp +++ b/src/gui/src/main.cpp @@ -66,8 +66,8 @@ int main(int argc, char *argv[]) { // HACK: set org name to app name for backwards compatibility. QCoreApplication::setOrganizationName(kAppName); - // HACK: set org domain to url for backwards compatibility. - QCoreApplication::setOrganizationDomain(kUrlWebsite); + // used as a prefix for settings paths, and must not be a url. + QCoreApplication::setOrganizationDomain(kOrgDomain); QSynergyApplication app(argc, argv); diff --git a/src/lib/gui/Logger.cpp b/src/lib/gui/Logger.cpp index 87792de6c..76bb2547b 100644 --- a/src/lib/gui/Logger.cpp +++ b/src/lib/gui/Logger.cpp @@ -55,24 +55,23 @@ QString printLine( auto logLine = QString("[%1] %2: %3").arg(datetime).arg(type).arg(message); QTextStream stream(&logLine); + stream << Qt::endl; + if (!fileLine.isEmpty()) { - stream << Qt::endl << "\t" + fileLine; + stream << "\t" + fileLine << Qt::endl; } - QString logLineReturn = logLine; - QTextStream streamReturn(&logLineReturn); - streamReturn << Qt::endl; - - auto logLineReturn_c = qPrintable(logLineReturn); + auto logLineBytes = logLine.toUtf8(); + auto logLine_c = logLineBytes.constData(); #if defined(Q_OS_WIN) // Debug output is viewable using either VS Code, Visual Studio, DebugView, or // DbgView++ (only one can be used at once). It's important to send output to // the debug output API, because it's difficult to view stdout and stderr from // a Windows GUI app. - OutputDebugStringA(logLineReturn_c); + OutputDebugStringA(logLine_c); #else - fprintf(out, "%s", logLineReturn_c); + fprintf(out, "%s", logLine_c); fflush(out); #endif diff --git a/src/lib/gui/config/ConfigScopes.cpp b/src/lib/gui/config/ConfigScopes.cpp index 71a7e15c0..90b3cde23 100644 --- a/src/lib/gui/config/ConfigScopes.cpp +++ b/src/lib/gui/config/ConfigScopes.cpp @@ -17,99 +17,41 @@ #include "ConfigScopes.h" +#include "proxy/QSettingsProxy.h" + #include #include #include +#include +#include #include -const auto kSystemConfigFilename = "SystemConfig.ini"; - -#if defined(Q_OS_UNIX) -const auto kUnixSystemConfigPath = "/usr/local/etc/symless/"; -#endif - namespace synergy::gui { using namespace proxy; -QString getSystemSettingPath() { - const QString settingFilename(kSystemConfigFilename); -#if defined(Q_OS_WIN) - return QCoreApplication::applicationDirPath() + QDir::separator(); -#elif defined(Q_OS_MAC) - // it would be nice to use /Library dir, but qt has no elevate system. - return kUnixSystemConfigPath + settingFilename; -#elif defined(Q_OS_LINUX) - // qt already adds application and filename to the end of the path on linux. - return kUnixSystemConfigPath; -#else - qFatal("unsupported platform"); - return ""; -#endif +// +// ConfigScopes::Deps +// + +std::shared_ptr ConfigScopes::Deps::makeUserSettings() { + return std::make_shared(); } -#if defined(Q_OS_WIN) -void loadWindowsLegacy(QSettings &settings) { - if (QFile(settings.fileName()).exists()) { - qDebug("system settings already exist, skipping legacy load"); - return; - } - - QSettings::setPath( - QSettings::IniFormat, QSettings::SystemScope, kSystemConfigFilename); - QSettings oldSystemSettings( - QSettings::IniFormat, QSettings::SystemScope, - QCoreApplication::organizationName(), - QCoreApplication::applicationName()); - - if (QFile(oldSystemSettings.fileName()).exists()) { - for (const auto &key : oldSystemSettings.allKeys()) { - settings.setValue(key, oldSystemSettings.value(key)); - } - } - - QSettings::setPath( - QSettings::IniFormat, QSettings::SystemScope, getSystemSettingPath()); +std::shared_ptr ConfigScopes::Deps::makeSystemSettings() { + return std::make_shared(); } -#endif -ConfigScopes::ConfigScopes() { - auto orgName = QCoreApplication::organizationName(); - if (orgName.isEmpty()) { - qFatal("unable to load config, organization name is empty"); - return; - } else { - qDebug() << "org name for config:" << orgName; - } +// +// ConfigScopes +// - auto appName = QCoreApplication::applicationName(); - if (appName.isEmpty()) { - qFatal("unable to load config, application name is empty"); - return; - } else { - qDebug() << "app name for config:" << appName; - } +ConfigScopes::ConfigScopes(std::shared_ptr deps) + : m_pUserSettingsProxy(deps->makeUserSettings()), + m_pSystemSettingsProxy(deps->makeSystemSettings()) { - // default to user scope. - // if we set the scope specifically then we also have to set the application - // name and the organisation name which breaks backwards compatibility. - m_pUserSettings = std::make_unique(); - m_userSettingsProxy.set(*m_pUserSettings); - - QSettings::setPath( - QSettings::Format::IniFormat, QSettings::Scope::SystemScope, - getSystemSettingPath()); - - // Config will default to User settings if they exist, - // otherwise it will load System setting and save them to User settings - m_pSystemSettings = std::make_unique( - QSettings::Format::IniFormat, QSettings::Scope::SystemScope, orgName, - appName); - m_systemSettingsProxy.set(*m_pSystemSettings); - -#if defined(Q_OS_WIN) - loadWindowsLegacy(*m_pSystemSettings); -#endif + m_pUserSettingsProxy->loadUser(); + m_pSystemSettingsProxy->loadSystem(); } void ConfigScopes::signalReady() { emit ready(); } @@ -119,8 +61,8 @@ void ConfigScopes::save() { emit saving(); qDebug("writing config to filesystem"); - m_pUserSettings->sync(); - m_pSystemSettings->sync(); + m_pUserSettingsProxy->sync(); + m_pSystemSettingsProxy->sync(); } bool ConfigScopes::isActiveScopeWritable() const { @@ -136,9 +78,9 @@ ConfigScopes::Scope ConfigScopes::activeScope() const { return m_currentScope; } bool ConfigScopes::scopeContains(const QString &name, Scope scope) const { switch (scope) { case Scope::User: - return m_pUserSettings->contains(name); + return m_pUserSettingsProxy->contains(name); case Scope::System: - return m_pSystemSettings->contains(name); + return m_pSystemSettingsProxy->contains(name); default: return activeSettings().contains(name); } @@ -146,17 +88,17 @@ bool ConfigScopes::scopeContains(const QString &name, Scope scope) const { QSettingsProxy &ConfigScopes::activeSettings() { if (m_currentScope == Scope::User) { - return m_userSettingsProxy; + return *m_pUserSettingsProxy; } else { - return m_systemSettingsProxy; + return *m_pSystemSettingsProxy; } } const QSettingsProxy &ConfigScopes::activeSettings() const { if (m_currentScope == Scope::User) { - return m_userSettingsProxy; + return *m_pUserSettingsProxy; } else { - return m_systemSettingsProxy; + return *m_pSystemSettingsProxy; } } @@ -168,9 +110,9 @@ QVariant ConfigScopes::getFromScope( const QString &name, const QVariant &defaultValue, Scope scope) const { switch (scope) { case Scope::User: - return m_pUserSettings->value(name, defaultValue); + return m_pUserSettingsProxy->value(name, defaultValue); case Scope::System: - return m_pSystemSettings->value(name, defaultValue); + return m_pSystemSettingsProxy->value(name, defaultValue); default: return activeSettings().value(name, defaultValue); } @@ -180,10 +122,10 @@ void ConfigScopes::setInScope( const QString &name, const QVariant &value, Scope scope) { switch (scope) { case Scope::User: - m_pUserSettings->setValue(name, value); + m_pUserSettingsProxy->setValue(name, value); break; case Scope::System: - m_pSystemSettings->setValue(name, value); + m_pSystemSettingsProxy->setValue(name, value); break; default: activeSettings().setValue(name, value); diff --git a/src/lib/gui/config/ConfigScopes.h b/src/lib/gui/config/ConfigScopes.h index 47ea1544a..25307e1c8 100644 --- a/src/lib/gui/config/ConfigScopes.h +++ b/src/lib/gui/config/ConfigScopes.h @@ -33,7 +33,13 @@ class ConfigScopes : public QObject, public IConfigScopes { Q_OBJECT public: - explicit ConfigScopes(); + struct Deps { + virtual ~Deps() = default; + virtual std::shared_ptr makeUserSettings(); + virtual std::shared_ptr makeSystemSettings(); + }; + + explicit ConfigScopes(std::shared_ptr deps = std::make_shared()); ~ConfigScopes() override = default; void signalReady() override; @@ -59,10 +65,8 @@ signals: private: Scope m_currentScope = Scope::User; - std::unique_ptr m_pUserSettings; - std::unique_ptr m_pSystemSettings; - QSettingsProxy m_userSettingsProxy; - QSettingsProxy m_systemSettingsProxy; + std::shared_ptr m_pUserSettingsProxy; + std::shared_ptr m_pSystemSettingsProxy; }; } // namespace synergy::gui diff --git a/src/lib/gui/constants.h b/src/lib/gui/constants.h index ff7689a15..ad23c0f84 100644 --- a/src/lib/gui/constants.h +++ b/src/lib/gui/constants.h @@ -21,28 +21,10 @@ namespace synergy::gui { -const int kDebugLogLevel = 1; - -#ifdef SYNERGY_PRODUCT_NAME -const QString kProductName = SYNERGY_PRODUCT_NAME; -#else -const QString kProductName; -#endif - -#ifdef SYNERGY_LICENSED_PRODUCT -const bool kLicensedProduct = true; -#else -const bool kLicensedProduct = false; -#endif - -#ifdef SYNERGY_ENABLE_ACTIVATION -#ifndef SYNERGY_LICENSED_PRODUCT -#error "activation requires licensed product" -#endif -const bool kEnableActivation = true; -#else -const bool kEnableActivation = false; -#endif // SYNERGY_ENABLE_ACTIVATION +// important: this is used for settings paths on some platforms, +// and must not be a url. qt automatically converts this to reverse domain +// notation (rdn), e.g. com.symless +const auto kOrgDomain = "symless.com"; const auto kLinkBuy = R"(Buy now)"; const auto kLinkRenew = R"(Renew now)"; @@ -65,4 +47,25 @@ const auto kUrlDownload = QString("%1/download?%2").arg(kUrlProduct, kUrlSourceQuery); const auto kUrlBugReport = QString("%1/issues").arg(kUrlGitHub); +#ifdef SYNERGY_PRODUCT_NAME +const QString kProductName = SYNERGY_PRODUCT_NAME; +#else +const QString kProductName; +#endif + +#ifdef SYNERGY_LICENSED_PRODUCT +const bool kLicensedProduct = true; +#else +const bool kLicensedProduct = false; +#endif + +#ifdef SYNERGY_ENABLE_ACTIVATION +#ifndef SYNERGY_LICENSED_PRODUCT +#error "activation requires licensed product" +#endif +const bool kEnableActivation = true; +#else +const bool kEnableActivation = false; +#endif // SYNERGY_ENABLE_ACTIVATION + } // namespace synergy::gui diff --git a/src/lib/gui/core/ServerConnection.cpp b/src/lib/gui/core/ServerConnection.cpp index bc489f17a..6591c8b76 100644 --- a/src/lib/gui/core/ServerConnection.cpp +++ b/src/lib/gui/core/ServerConnection.cpp @@ -49,38 +49,62 @@ ServerConnection::ServerConnection( void ServerConnection::handleLogLine(const QString &logLine) { ServerMessage message(logLine); - if (!m_appConfig.useExternalConfig() && message.isNewClientMessage() && - !m_ignoredClients.contains(message.getClientName())) { - handleNewClient(message.getClientName()); + if (!message.isNewClientMessage()) { + return; } + + if (m_appConfig.useExternalConfig()) { + qDebug("external config enabled, skipping new client prompt"); + return; + } + + const auto client = message.getClientName(); + + if (m_receivedClients.contains(client)) { + qDebug( + "already got request, skipping new client prompt for: %s", + qPrintable(client)); + return; + } + + handleNewClient(message.getClientName()); } void ServerConnection::handleNewClient(const QString &clientName) { using enum messages::NewClientPromptResult; + m_receivedClients.append(clientName); + + if (m_messageShowing) { + qDebug("new client message already shown, skipping"); + return; + } + if (m_serverConfig.isFull()) { qDebug( - "server config full, skipping add client prompt for: %s", + "server config full, skipping new client prompt for: %s", qPrintable(clientName)); return; } if (m_serverConfig.screenExists(clientName)) { qDebug( - "client already added, skipping add client prompt for: %s", + "client already added, skipping new client prompt for: %s", qPrintable(clientName)); return; } emit messageShowing(); + m_messageShowing = true; const auto result = m_pDeps->showNewClientPrompt(m_pParent, clientName); + m_messageShowing = false; + if (result == Add) { qDebug("accepted dialog, adding client: %s", qPrintable(clientName)); emit configureClient(clientName); } else if (result == Ignore) { qDebug("declined dialog, ignoring client: %s", qPrintable(clientName)); - m_ignoredClients.append(clientName); } else { qFatal("unexpected add client result"); } diff --git a/src/lib/gui/core/ServerConnection.h b/src/lib/gui/core/ServerConnection.h index 5351b7e6d..67fafe06a 100644 --- a/src/lib/gui/core/ServerConnection.h +++ b/src/lib/gui/core/ServerConnection.h @@ -53,7 +53,8 @@ private: IAppConfig &m_appConfig; IServerConfig &m_serverConfig; std::shared_ptr m_pDeps; - QStringList m_ignoredClients; + QStringList m_receivedClients; + bool m_messageShowing = false; }; } // namespace synergy::gui diff --git a/src/lib/gui/messages.cpp b/src/lib/gui/messages.cpp index 037cadb19..36e413a45 100644 --- a/src/lib/gui/messages.cpp +++ b/src/lib/gui/messages.cpp @@ -40,6 +40,12 @@ struct Errors { std::unique_ptr Errors::s_criticalMessage; QStringList Errors::s_ignoredErrors; +void raiseCriticalDialog() { + if (Errors::s_criticalMessage) { + Errors::s_criticalMessage->raise(); + } +} + void showErrorDialog( const QString &message, const QMessageLogContext &context, QtMsgType type) { auto filename = QFileInfo(context.file).fileName(); @@ -72,7 +78,9 @@ void showErrorDialog( if (type == QtFatalMsg) { // create a blocking message box for fatal errors, as we want to wait // until the dialog is dismissed before aborting the app. - QMessageBox::critical(nullptr, title, text); + QMessageBox critical( + QMessageBox::Critical, title, text, QMessageBox::Abort); + critical.exec(); } else if (!Errors::s_ignoredErrors.contains(message)) { // prevent message boxes piling up by deleting the last one if it exists. // if none exists yet, then nothing will happen. @@ -85,6 +93,7 @@ void showErrorDialog( Errors::s_criticalMessage = std::make_unique( QMessageBox::Critical, title, text, QMessageBox::Ok | QMessageBox::Ignore); + Errors::s_criticalMessage->open(); QAction::connect( diff --git a/src/lib/gui/messages.h b/src/lib/gui/messages.h index 239f8c197..9c20f3f11 100644 --- a/src/lib/gui/messages.h +++ b/src/lib/gui/messages.h @@ -31,6 +31,8 @@ enum class NewClientPromptResult { Add, Ignore }; void messageHandler( QtMsgType type, const QMessageLogContext &context, const QString &msg); +void raiseCriticalDialog(); + void showFirstRunMessage( QWidget *parent, bool closeToTray, bool enableService, bool isServer); diff --git a/src/lib/gui/proxy/QSettingsProxy.cpp b/src/lib/gui/proxy/QSettingsProxy.cpp index 7e4fdeebd..b25df356b 100644 --- a/src/lib/gui/proxy/QSettingsProxy.cpp +++ b/src/lib/gui/proxy/QSettingsProxy.cpp @@ -17,8 +17,151 @@ #include "QSettingsProxy.h" +#include "common/constants.h" +#include "gui/Logger.h" + +#include +#include +#include +#include +#include + namespace synergy::gui::proxy { +const auto kLegacyOrgDomain = "http-symless-com"; + +const auto kSystemConfigFilename = "SystemConfig.ini"; + +#if defined(Q_OS_UNIX) +const auto kUnixSystemConfigPath = "/usr/local/etc/symless/"; +#endif + +// +// Free functions +// + +QString getSystemSettingPath() { + const QString settingFilename(kSystemConfigFilename); +#if defined(Q_OS_WIN) + return QCoreApplication::applicationDirPath() + QDir::separator(); +#elif defined(Q_OS_MAC) + // it would be nice to use /Library dir, but qt has no elevate system. + return kUnixSystemConfigPath + settingFilename; +#elif defined(Q_OS_LINUX) + // qt already adds application and filename to the end of the path on linux. + return kUnixSystemConfigPath; +#else +#error "unsupported platform" +#endif +} + +void migrateLegacySystemSettings(QSettings &settings) { + if (QFile(settings.fileName()).exists()) { + qDebug("system settings already exist, skipping migration"); + return; + } + + QSettings::setPath( + QSettings::IniFormat, QSettings::SystemScope, kSystemConfigFilename); + QSettings oldSystemSettings( + QSettings::IniFormat, QSettings::SystemScope, + QCoreApplication::organizationName(), + QCoreApplication::applicationName()); + + if (QFile(oldSystemSettings.fileName()).exists()) { + for (const auto &key : oldSystemSettings.allKeys()) { + settings.setValue(key, oldSystemSettings.value(key)); + } + } + + QSettings::setPath( + QSettings::IniFormat, QSettings::SystemScope, getSystemSettingPath()); +} + +void migrateLegacyUserSettings(QSettings &newSettings) { + QString newPath = newSettings.fileName(); + QFile newFile(newPath); + if (newFile.exists()) { + qInfo("user settings already exist, skipping migration"); + return; + } + + QSettings oldSettings(kLegacyOrgDomain, kAppName); + QString oldPath = oldSettings.fileName(); + + if (oldPath.isEmpty()) { + qInfo("no legacy settings to migrate, filename empty"); + return; + } + + if (!QFile(oldPath).exists()) { + qInfo("no legacy settings to migrate, file does not exist"); + return; + } + + QFileInfo oldFileInfo(oldPath); + QFileInfo newFileInfo(newPath); + + qDebug( + "migrating legacy settings: '%s' -> '%s'", // + qPrintable(oldFileInfo.fileName()), qPrintable(newFileInfo.fileName())); + + QStringList keys = oldSettings.allKeys(); + for (const QString &key : keys) { + QVariant oldValue = oldSettings.value(key); + newSettings.setValue(key, oldValue); + logVerbose( + QString("migrating setting '%1' = '%2'").arg(key, oldValue.toString())); + } + + newSettings.sync(); +} + +// +// QSettingsProxy +// + +void QSettingsProxy::loadUser() { + m_pSettings = std::make_unique(); + +#if defined(Q_OS_MAC) + // on mac, we used to save settings to "com.http-symless-com.Synergy.plist" + // because `setOrganizationName` was historically called using a url instead + // of an actual domain (e.g. symless.com). + migrateLegacyUserSettings(*m_pSettings); +#endif // Q_OS_MAC +} + +void QSettingsProxy::loadSystem() { + auto orgName = QCoreApplication::organizationName(); + if (orgName.isEmpty()) { + qFatal("unable to load config, organization name is empty"); + return; + } else { + qDebug() << "org name for config:" << orgName; + } + + auto appName = QCoreApplication::applicationName(); + if (appName.isEmpty()) { + qFatal("unable to load config, application name is empty"); + return; + } else { + qDebug() << "app name for config:" << appName; + } + + QSettings::setPath( + QSettings::Format::IniFormat, QSettings::Scope::SystemScope, + getSystemSettingPath()); + + m_pSettings = std::make_unique( + QSettings::Format::IniFormat, QSettings::Scope::SystemScope, orgName, + appName); + +#if defined(Q_OS_WIN) + migrateLegacySystemSettings(*m_pSettings); +#endif // Q_OS_WIN +} + int QSettingsProxy::beginReadArray(const QString &prefix) { return m_pSettings->beginReadArray(prefix); } diff --git a/src/lib/gui/proxy/QSettingsProxy.h b/src/lib/gui/proxy/QSettingsProxy.h index 82422dc11..51ff1ce9e 100644 --- a/src/lib/gui/proxy/QSettingsProxy.h +++ b/src/lib/gui/proxy/QSettingsProxy.h @@ -21,9 +21,16 @@ namespace synergy::gui::proxy { +QString getSystemSettingPath(); + class QSettingsProxy { public: virtual ~QSettingsProxy() = default; + + virtual void loadUser(); + virtual void loadSystem(); + + virtual void sync() { m_pSettings->sync(); } virtual int beginReadArray(const QString &prefix); virtual void beginWriteArray(const QString &prefix); virtual void setArrayIndex(int i); @@ -39,11 +46,10 @@ public: virtual bool contains(const QString &key) const; virtual QString fileName() const { return m_pSettings->fileName(); } - void set(QSettings &settings) { m_pSettings = &settings; } QSettings &get() const { return *m_pSettings; } private: - QSettings *m_pSettings; + std::unique_ptr m_pSettings; }; } // namespace synergy::gui::proxy diff --git a/src/lib/gui/string_utils.h b/src/lib/gui/string_utils.h index 1a9ca96f4..a75a36852 100644 --- a/src/lib/gui/string_utils.h +++ b/src/lib/gui/string_utils.h @@ -25,3 +25,11 @@ inline bool strToTrue(const QString &str) { return str.toLower() == "true" || str == "1"; } + +inline QString trimEnd(const QString &str) { + QString result = str; + while (!result.isEmpty() && result.at(result.size() - 1).isSpace()) { + result.chop(1); + } + return result; +} diff --git a/src/test/unittests/gui/config/ConfigScopesTests.cpp b/src/test/unittests/gui/config/ConfigScopesTests.cpp new file mode 100644 index 000000000..8662c4066 --- /dev/null +++ b/src/test/unittests/gui/config/ConfigScopesTests.cpp @@ -0,0 +1,206 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2024 Symless Ltd. + * + * This package is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * found in the file LICENSE that should have accompanied this file. + * + * This package is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "gui/config/ConfigScopes.h" + +#include "gmock/gmock.h" +#include +#include +#include + +using namespace testing; +using namespace synergy::gui; +using namespace synergy::gui::proxy; + +namespace { + +class QSettingsProxyMock : public QSettingsProxy { +public: + MOCK_METHOD(void, loadSystem, (), (override)); + MOCK_METHOD(void, loadUser, (), (override)); + MOCK_METHOD(QString, fileName, (), (const, override)); + MOCK_METHOD(void, sync, (), (override)); + MOCK_METHOD(bool, isWritable, (), (const, override)); + MOCK_METHOD(bool, contains, (const QString &), (const, override)); + MOCK_METHOD(QVariant, value, (const QString &), (const, override)); + MOCK_METHOD( + QVariant, value, (const QString &, const QVariant &), (const, override)); + MOCK_METHOD(void, setValue, (const QString &, const QVariant &), (override)); +}; + +struct DepsMock : public ConfigScopes::Deps { + DepsMock() { + ON_CALL(*this, makeUserSettings()).WillByDefault(Return(m_pUserSettings)); + ON_CALL(*this, makeSystemSettings()) + .WillByDefault(Return(m_pSystemSettings)); + } + + MOCK_METHOD( + std::shared_ptr, makeUserSettings, (), (override)); + MOCK_METHOD( + std::shared_ptr, makeSystemSettings, (), (override)); + + std::shared_ptr m_pUserSettings = + std::make_shared>(); + std::shared_ptr m_pSystemSettings = + std::make_shared>(); +}; + +} // namespace + +TEST(ConfigScopesTests, ctor_callsMakeUserSettings) { + std::shared_ptr> deps = + std::make_shared>(); + + EXPECT_CALL(*deps, makeUserSettings()).Times(1); + + ConfigScopes configScopes(deps); +} + +TEST(ConfigScopesTests, ctor_callsMakeSystemSettings) { + std::shared_ptr> deps = + std::make_shared>(); + + EXPECT_CALL(*deps, makeSystemSettings()).Times(1); + + ConfigScopes configScopes(deps); +} + +TEST(ConfigScopesTests, save_syncsBothScopes) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + + EXPECT_CALL(*deps->m_pUserSettings, sync()).Times(1); + EXPECT_CALL(*deps->m_pSystemSettings, sync()).Times(1); + + configScopes.save(); +} + +TEST(ConfigScopesTests, activeSettings_returnsUserSettingsByDefault) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + + EXPECT_EQ(&configScopes.activeSettings(), deps->m_pUserSettings.get()); +} + +TEST(ConfigScopesTests, activeSettings_returnsSystemSettingsWhenSystemScope) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + configScopes.setActiveScope(ConfigScopes::Scope::System); + + EXPECT_EQ(&configScopes.activeSettings(), deps->m_pSystemSettings.get()); +} + +TEST(ConfigScopesTests, setActiveScope_setsCurrentScope) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + + configScopes.setActiveScope(ConfigScopes::Scope::System); + + EXPECT_EQ(configScopes.activeScope(), ConfigScopes::Scope::System); +} + +TEST( + ConfigScopesTests, + isActiveScopeWritable_returnsTrueWhenUserSettingsWritable) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + + EXPECT_CALL(*deps->m_pUserSettings, isWritable()).WillOnce(Return(true)); + + EXPECT_TRUE(configScopes.isActiveScopeWritable()); +} + +TEST( + ConfigScopesTests, + scopeContains_byDefault_returnsTrueWhenUserSettingsContainsKey) { + std::shared_ptr> deps = + std::make_shared>(); + + ON_CALL(*deps->m_pUserSettings, contains(_)).WillByDefault(Return(true)); + + ConfigScopes configScopes(deps); + + EXPECT_TRUE(configScopes.scopeContains("stub")); +} + +TEST( + ConfigScopesTests, + scopeContains_userScope_returnsTrueWhenUserSettingsContainsKey) { + std::shared_ptr> deps = + std::make_shared>(); + + ON_CALL(*deps->m_pUserSettings, contains(_)).WillByDefault(Return(true)); + + ConfigScopes configScopes(deps); + + EXPECT_TRUE(configScopes.scopeContains("stub", ConfigScopes::Scope::User)); +} + +TEST( + ConfigScopesTests, + scopeContains_systemScope_returnsTrueWhenSystemSettingsContainsKey) { + std::shared_ptr> deps = + std::make_shared>(); + + ON_CALL(*deps->m_pSystemSettings, contains(_)).WillByDefault(Return(true)); + + ConfigScopes configScopes(deps); + + EXPECT_TRUE(configScopes.scopeContains("stub", ConfigScopes::Scope::System)); +} + +TEST(ConfigScopesTests, activeFilePath_returnsUserSettingsFileNameByDefault) { + std::shared_ptr> deps = + std::make_shared>(); + ON_CALL(*deps->m_pUserSettings, fileName()).WillByDefault(Return("test")); + + ConfigScopes configScopes(deps); + + EXPECT_EQ(configScopes.activeFilePath(), "test"); +} + +TEST(ConfigScopesTests, getFromScope_byDefault_returnsValueFromActiveSettings) { + std::shared_ptr> deps = + std::make_shared>(); + ON_CALL(*deps->m_pUserSettings, value(_, _)).WillByDefault(Return("test")); + + ConfigScopes configScopes(deps); + + EXPECT_EQ(configScopes.getFromScope("stub"), "test"); +} + +TEST(ConfigScopesTests, setInScope_byDefault_setsValueInActiveSettings) { + std::shared_ptr> deps = + std::make_shared>(); + + ConfigScopes configScopes(deps); + + EXPECT_CALL(*deps->m_pUserSettings, setValue(_, _)).Times(1); + + configScopes.setInScope("stub", "test"); +}