refactor: move screenName to Settings

newkey: core/screenName <= General/screenName
remove screenName from AppConfig

src/apps/deskflow-gui/MainWindow.cpp

@@ -293,8 +293,6 @@ void MainWindow::connectSlots()
 
   connect(Settings::instance(), &Settings::settingsChanged, this, &MainWindow::settingsChanged);
 
-  connect(&m_appConfig, &AppConfig::screenNameChanged, this, &MainWindow::updateScreenName);
-
   connect(&m_coreProcess, &CoreProcess::starting, this, &MainWindow::coreProcessStarting, Qt::DirectConnection);
   connect(&m_coreProcess, &CoreProcess::error, this, &MainWindow::coreProcessError);
   connect(&m_coreProcess, &CoreProcess::logLine, this, &MainWindow::handleLogLine);
@@ -388,6 +386,9 @@ void MainWindow::settingsChanged(const QString &key)
     return;
   }
 
+  if (key == Settings::Core::ScreenName)
+    updateScreenName();
+
   if ((key == Settings::Security::Certificate) || (key == Settings::Security::KeySize) ||
       (key == Settings::Security::TlsEnabled) || (key == Settings::Security::CheckPeers)) {
     const auto certificate = Settings::value(Settings::Security::Certificate).toString();
@@ -1039,13 +1040,15 @@ void MainWindow::autoAddScreen(const QString name)
   if (r != kAutoAddScreenOk) {
     switch (r) {
     case kAutoAddScreenManualServer:
-      showConfigureServer(tr("Please add the server (%1) to the grid.").arg(m_appConfig.screenName()));
+      showConfigureServer(
+          tr("Please add the server (%1) to the grid.").arg(Settings::value(Settings::Core::ScreenName).toString())
+      );
       break;
 
     case kAutoAddScreenManualClient:
-      showConfigureServer(tr("Please drag the new client screen (%1) "
+      showConfigureServer(
-                             "to the desired position on the grid.")
+          tr("Please add the server (%1) to the grid.").arg(Settings::value(Settings::Core::ScreenName).toString())
-                              .arg(name));
+      );
       break;
     }
   }
@@ -1079,8 +1082,9 @@ void MainWindow::secureSocket(bool secureSocket)
 
 void MainWindow::updateScreenName()
 {
-  ui->lblComputerName->setText(m_appConfig.screenName());
+  const auto screenName = Settings::value(Settings::Core::ScreenName).toString();
-  ui->lineEditName->setText(m_appConfig.screenName());
+  ui->lblComputerName->setText(screenName);
+  ui->lineEditName->setText(screenName);
   m_serverConfig.updateServerName();
 }
 
@@ -1111,11 +1115,12 @@ void MainWindow::setHostName()
   ui->btnEditName->show();
 
   QString text = ui->lineEditName->text();
-  bool existingScreen = serverConfig().screenExists(text) && text != m_appConfig.screenName();
+  const auto screenName = Settings::value(Settings::Core::ScreenName).toString();
+  bool existingScreen = serverConfig().screenExists(text) && (text != screenName);
 
   if (!ui->lineEditName->hasAcceptableInput() || text.isEmpty() || existingScreen) {
     blockSignals(true);
-    ui->lineEditName->setText(m_appConfig.screenName());
+    ui->lineEditName->setText(screenName);
     blockSignals(false);
 
     const auto title = tr("Invalid Screen Name");
@@ -1135,7 +1140,7 @@ void MainWindow::setHostName()
   }
 
   ui->lblComputerName->setText(ui->lineEditName->text());
-  m_appConfig.setScreenName(ui->lineEditName->text());
+  Settings::setValue(Settings::Core::ScreenName, ui->lineEditName->text());
   applyConfig();
 }
 

src/apps/deskflow-gui/ServerConfig.cpp

@@ -139,8 +139,9 @@ void ServerConfig::commit()
     settings().setArrayIndex(i);
     auto &screen = screens()[i];
     screen.saveSettings(settings());
-    if (screen.isServer() && m_pAppConfig->screenName() != screen.name()) {
+    auto screenName = Settings::value(Settings::Core::ScreenName).toString();
-      m_pAppConfig->setScreenName(screen.name());
+    if (screen.isServer() && screenName != screen.name()) {
+      Settings::setValue(Settings::Core::ScreenName, screen.name());
     }
   }
   settings().endArray();
@@ -332,8 +333,8 @@ int ServerConfig::autoAddScreen(const QString name)
 {
   int serverIndex = -1;
   int targetIndex = -1;
-  if (!findScreenName(m_pAppConfig->screenName(), serverIndex) &&
+  const auto screenName = Settings::value(Settings::Core::ScreenName).toString();
-      !fixNoServer(m_pAppConfig->screenName(), serverIndex)) {
+  if (!findScreenName(screenName, serverIndex) && !fixNoServer(screenName, serverIndex)) {
     return kAutoAddScreenManualServer;
   }
 
@@ -389,16 +390,16 @@ int ServerConfig::autoAddScreen(const QString name)
   return kAutoAddScreenOk;
 }
 
-const QString &ServerConfig::getServerName() const
+const QString ServerConfig::getServerName() const
 {
-  return m_pAppConfig->screenName();
+  return Settings::value(Settings::Core::ScreenName).toString();
 }
 
 void ServerConfig::updateServerName()
 {
   for (auto &screen : screens()) {
     if (screen.isServer()) {
-      screen.setName(m_pAppConfig->screenName());
+      screen.setName(Settings::value(Settings::Core::ScreenName).toString());
       break;
     }
   }
@@ -445,11 +446,12 @@ bool ServerConfig::screenExists(const QString &screenName) const
 void ServerConfig::addClient(const QString &clientName)
 {
   int serverIndex = -1;
+  const auto screenName = Settings::value(Settings::Core::ScreenName).toString();
 
-  if (findScreenName(m_pAppConfig->screenName(), serverIndex)) {
+  if (findScreenName(screenName, serverIndex)) {
     m_Screens[serverIndex].markAsServer();
   } else {
-    fixNoServer(m_pAppConfig->screenName(), serverIndex);
+    fixNoServer(screenName, serverIndex);
   }
 
   m_Screens.addScreenByPriority(Screen(clientName));

src/apps/deskflow-gui/ServerConfig.h

@@ -155,7 +155,7 @@ public:
   void commit();
   int numScreens() const;
   int autoAddScreen(const QString name);
-  const QString &getServerName() const;
+  const QString getServerName() const;
   void updateServerName();
   const QString configFile() const;
   bool useExternalConfig() const;

src/lib/common/Settings.cpp

@@ -71,9 +71,8 @@ void Settings::cleanSettings()
 
 QVariant Settings::defaultValue(const QString &key)
 {
-  if ((key == Core::Scope) || (key == Settings::Gui::Autohide) || (key == Core::StartedBefore) ||
+  if ((key == Core::Scope) || (key == Gui::Autohide) || (key == Core::StartedBefore) || (key == Core::PreventSleep) ||
-      (key == Core::PreventSleep) || (key == Server::ExternalConfig) || (key == Client::InvertScrollDirection) ||
+      (key == Server::ExternalConfig) || (key == Client::InvertScrollDirection) || (key == Log::ToFile)) {
-      (key == Log::ToFile)) {
     return false;
   }
 
@@ -83,6 +82,9 @@ QVariant Settings::defaultValue(const QString &key)
     return true;
   }
 
+  if (key == Core::ScreenName)
+    return QSysInfo::machineHostName();
+
   if (key == Gui::WindowGeometry)
     return QRect();
 

src/lib/common/Settings.h

@@ -43,6 +43,7 @@ public:
     inline static const auto Port = QStringLiteral("core/port");
     inline static const auto PreventSleep = QStringLiteral("core/preventSleep");
     inline static const auto Scope = QStringLiteral("core/loadFromSystemScope");
+    inline static const auto ScreenName = QStringLiteral("core/screenName");
     inline static const auto StartedBefore = QStringLiteral("core/startedBefore");
   };
   struct Gui
@@ -122,6 +123,7 @@ private:
     , Core::Port
     , Core::PreventSleep
     , Core::Scope
+    , Core::ScreenName
     , Core::StartedBefore
     , Log::File
     , Log::Level

src/lib/gui/config/AppConfig.cpp

@@ -31,7 +31,7 @@ const char AppConfig::m_LogDir[] = "/var/log/";
 
 // TODO: instead, use key value pair table, which would be less fragile.
 const char *const AppConfig::m_SettingsName[] = {
-    "screenName",
+    "", // screenName moved to deskflow settings
     "", // port moved to deskflow settings
     "", // interface moved to deskflow settings
     "", // log level moved to deskflow settings
@@ -80,10 +80,7 @@ const char *const AppConfig::m_SettingsName[] = {
     "", // 46 require peer certs, Moved to deskflow settings
 };
 
-AppConfig::AppConfig(deskflow::gui::IConfigScopes &scopes, std::shared_ptr<Deps> deps)
+AppConfig::AppConfig(deskflow::gui::IConfigScopes &scopes, std::shared_ptr<Deps> deps) : m_Scopes(scopes), m_pDeps(deps)
-    : m_Scopes(scopes),
-      m_pDeps(deps),
-      m_ScreenName(deps->hostname())
 {
   determineScope();
   recall();
@@ -109,7 +106,6 @@ void AppConfig::recallFromCurrentScope()
 {
   using enum Setting;
 
-  recallScreenName();
   recallElevateMode();
 
   m_ServerGroupChecked = getFromCurrentScope(kServerGroupChecked, m_ServerGroupChecked).toBool();
@@ -118,23 +114,6 @@ void AppConfig::recallFromCurrentScope()
   m_EnableService = getFromCurrentScope(kEnableService, m_EnableService).toBool();
 }
 
-void AppConfig::recallScreenName()
-{
-  using enum Setting;
-
-  const auto &screenName = getFromCurrentScope(kScreenName, m_ScreenName).toString().trimmed();
-
-  // for some reason, the screen name can be saved as an empty string
-  // in the config file. this is probably a bug. if this happens, then default
-  // back to the hostname.
-  if (screenName.isEmpty()) {
-    qWarning("screen name was empty in config, setting to hostname");
-    m_ScreenName = m_pDeps->hostname();
-  } else {
-    m_ScreenName = screenName;
-  }
-}
-
 void AppConfig::commit()
 {
   using enum Setting;
@@ -146,7 +125,6 @@ void AppConfig::commit()
   saveToAllScopes(kServerGroupChecked, m_ServerGroupChecked);
 
   if (isActiveScopeWritable()) {
-    setInCurrentScope(kScreenName, m_ScreenName);
     setInCurrentScope(kElevateMode, static_cast<int>(m_ElevateMode));
     setInCurrentScope(kElevateModeLegacy, m_ElevateMode == ElevateMode::kAlways);
     setInCurrentScope(kUseInternalConfig, m_UseInternalConfig);
@@ -177,7 +155,7 @@ void AppConfig::determineScope()
   // ...failing that, check the system scope instead to see if an arbitrary
   // required setting is present. if it is, then we can assume that the system
   // scope should be used.
-  else if (m_Scopes.scopeContains(settingName(Setting::kScreenName), ConfigScopes::Scope::System)) {
+  else if (m_Scopes.scopeContains(settingName(Setting::kUseInternalConfig), ConfigScopes::Scope::System)) {
     qDebug("system settings scope contains screen name, using system scope");
     setLoadFromSystemScope(true);
   }
@@ -288,7 +266,7 @@ void AppConfig::loadScope(ConfigScopes::Scope scope)
 
   // only signal ready if there is at least one setting in the required scope.
   // this prevents the current settings from being set back to default.
-  if (m_Scopes.scopeContains(settingName(Setting::kScreenName), m_Scopes.activeScope())) {
+  if (m_Scopes.scopeContains(settingName(Setting::kUseInternalConfig), m_Scopes.activeScope())) {
     m_Scopes.signalReady();
   } else {
     qDebug("no screen name in scope, skipping");
@@ -328,11 +306,6 @@ IConfigScopes &AppConfig::scopes() const
   return m_Scopes;
 }
 
-const QString &AppConfig::screenName() const
-{
-  return m_ScreenName;
-}
-
 ProcessMode AppConfig::processMode() const
 {
   return m_EnableService ? ProcessMode::kService : ProcessMode::kDesktop;
@@ -386,12 +359,6 @@ void AppConfig::setClientGroupChecked(bool newValue)
   m_ClientGroupChecked = newValue;
 }
 
-void AppConfig::setScreenName(const QString &s)
-{
-  m_ScreenName = s;
-  Q_EMIT screenNameChanged();
-}
-
 void AppConfig::setElevateMode(ElevateMode em)
 {
   m_ElevateMode = em;

src/lib/gui/config/AppConfig.h

@@ -15,7 +15,6 @@
 #include "gui/paths.h"
 
 #include <QDir>
-#include <QHostInfo>
 #include <QObject>
 #include <QPoint>
 #include <QSize>
@@ -53,7 +52,7 @@ class AppConfig : public QObject, public deskflow::gui::IAppConfig
 private:
   enum class Setting
   {
-    kScreenName = 0,
+    // kScreenName = 0, Moved to deskflow settings
     // kPort = 1, moved to deskflow settings
     // kInterface = 2, moved to deskflow settings
     // kLogLevel = 3, moved to deskflow settings
@@ -106,10 +105,6 @@ public:
   struct Deps
   {
     virtual ~Deps() = default;
-    virtual QString hostname() const
-    {
-      return QHostInfo::localHostName();
-    }
   };
 
   explicit AppConfig(IConfigScopes &scopes, std::shared_ptr<Deps> deps = std::make_shared<Deps>());
@@ -129,7 +124,6 @@ public:
   IConfigScopes &scopes() const override;
   ProcessMode processMode() const override;
   ElevateMode elevateMode() const override;
-  const QString &screenName() const override;
   bool isActiveScopeWritable() const override;
   bool isActiveScopeSystem() const override;
   bool enableService() const override;
@@ -146,7 +140,6 @@ public:
   // Setters (overrides)
   //
 
-  void setScreenName(const QString &s) override;
   void setElevateMode(ElevateMode em) override;
   void setEnableService(bool enabled) override;
 
@@ -224,8 +217,4 @@ private:
 
   deskflow::gui::IConfigScopes &m_Scopes;
   std::shared_ptr<Deps> m_pDeps;
-  QString m_ScreenName;
-
-signals:
-  void screenNameChanged();
 };

src/lib/gui/config/IAppConfig.h

@@ -34,7 +34,6 @@ public:
   virtual IConfigScopes &scopes() const = 0;
   virtual ProcessMode processMode() const = 0;
   virtual ElevateMode elevateMode() const = 0;
-  virtual const QString &screenName() const = 0;
   virtual bool enableService() const = 0;
   virtual bool isActiveScopeSystem() const = 0;
   virtual bool isActiveScopeWritable() const = 0;
@@ -45,7 +44,6 @@ public:
   //
 
   virtual void setLoadFromSystemScope(bool loadFromSystemScope) = 0;
-  virtual void setScreenName(const QString &screenName) = 0;
   virtual void setElevateMode(ElevateMode elevateMode) = 0;
   virtual void setEnableService(bool enableService) = 0;
 };

src/lib/gui/core/CoreProcess.cpp

@@ -473,7 +473,7 @@ bool CoreProcess::addGenericArgs(QStringList &args, const ProcessMode processMod
   args << "-f"
        << "--debug" << Settings::logLevelText();
 
-  args << "--name" << m_appConfig.screenName();
+  args << "--name" << Settings::value(Settings::Core::ScreenName).toString();
 
   if (processMode != ProcessMode::kDesktop) {
 #if defined(Q_OS_WIN)

src/test/shared/gui/mocks/AppConfigMock.h

@@ -19,7 +19,6 @@ class AppConfigMock : public deskflow::gui::IAppConfig
 public:
   AppConfigMock()
   {
-    ON_CALL(*this, screenName()).WillByDefault(testing::ReturnRef(m_stub));
   }
 
   //
@@ -29,7 +28,6 @@ public:
   MOCK_METHOD(deskflow::gui::IConfigScopes &, scopes, (), (const, override));
   MOCK_METHOD(ProcessMode, processMode, (), (const, override));
   MOCK_METHOD(ElevateMode, elevateMode, (), (const, override));
-  MOCK_METHOD(const QString &, screenName, (), (const, override));
   MOCK_METHOD(bool, enableService, (), (const, override));
   MOCK_METHOD(bool, isActiveScopeSystem, (), (const, override));
   MOCK_METHOD(bool, isActiveScopeWritable, (), (const, override));
@@ -40,7 +38,6 @@ public:
   //
 
   MOCK_METHOD(void, setLoadFromSystemScope, (bool loadFromSystemScope), (override));
-  MOCK_METHOD(void, setScreenName, (const QString &screenName), (override));
   MOCK_METHOD(void, setElevateMode, (ElevateMode elevateMode), (override));
   MOCK_METHOD(void, setEnableService, (bool enableService), (override));
 

src/test/unittests/gui/config/AppConfigTests.cpp

@@ -41,15 +41,12 @@ struct DepsMock : public AppConfig::Deps
 {
   DepsMock()
   {
-    ON_CALL(*this, hostname()).WillByDefault(Return("stub"));
   }
 
   static std::shared_ptr<NiceMock<DepsMock>> makeNice()
   {
     return std::make_shared<NiceMock<DepsMock>>();
   }
-
-  MOCK_METHOD(QString, hostname, (), (const, override));
 };
 
 } // namespace
@@ -58,17 +55,6 @@ class AppConfigTests : public Test
 {
 };
 
-TEST_F(AppConfigTests, ctor_byDefault_screenNameIsHostname)
-{
-  NiceMock<ConfigScopesMock> scopes;
-  auto deps = DepsMock::makeNice();
-  ON_CALL(*deps, hostname()).WillByDefault(Return("test hostname"));
-
-  AppConfig appConfig(scopes, deps);
-
-  ASSERT_EQ(appConfig.screenName().toStdString(), "test hostname");
-}
-
 TEST_F(AppConfigTests, ctor_byDefault_getsFromScope)
 {
   NiceMock<ConfigScopesMock> scopes;
@@ -79,8 +65,6 @@ TEST_F(AppConfigTests, ctor_byDefault_getsFromScope)
   EXPECT_CALL(scopes, getFromScope(_, _, _)).Times(AnyNumber());
 
   AppConfig appConfig(scopes, deps);
-
-  ASSERT_EQ(appConfig.screenName().toStdString(), "test screen");
 }
 
 TEST_F(AppConfigTests, commit_byDefault_setsToScope)