Synergy 1267 endless notifications during connection (#7104)

* SYNERGY-1267 Endless notification during connection

* SYNERGY-1267 Endless notification. Server part

* SYNERGY-1267 Change notification trace level

* Update ChangeLog

* SYNERGY-1267 Fix Linux compilation

* SYNERGY-1267 Fix code smell

* SYNERGY-1267 Add requested changes

ChangeLog

@@ -2,7 +2,7 @@ v1.14.2-snapshot
 ===========
 Bug fixes:
 - #7070 Fix radio button shifted focus on macOS
-- #7038 Fix client and server different keyboard layout errors
+- #7038 | #7104 Fix client and server different keyboard layout errors
 - #7078 Fix clipboard re-enables automatically
 - #7077 Fix Ubuntu, CentOS and Debian build after SYNERGY-1161
 - #7080 Add trace if the system can't open file with trusted fingerprints

src/lib/client/Client.cpp

@@ -206,6 +206,7 @@ Client::handshakeComplete()
 {
     m_ready = true;
     m_screen->enable();
+    checkMissedLanguages();
     sendEvent(m_events->forClient().connected(), NULL);
 }
 
@@ -723,13 +724,14 @@ void
 Client::handleHello(const Event&, void*)
 {
     SInt16 major, minor;
-    String keyboardLayoutList("<unknown>");
-    if (!ProtocolUtil::readf(m_stream, kMsgHello, &major, &minor, &keyboardLayoutList)) {
+    String remoteLanguages;
+    if (!ProtocolUtil::readf(m_stream, kMsgHello, &major, &minor, &remoteLanguages)) {
         sendConnectionFailedEvent("Protocol error from server, check encryption settings");
         cleanupTimer();
         cleanupConnection();
         return;
     }
+    m_languageManager.setRemoteLanguages(remoteLanguages);
 
     // check versions
     LOG((CLOG_DEBUG1 "got hello version %d.%d", major, minor));
@@ -749,37 +751,11 @@ Client::handleHello(const Event&, void*)
     }
 
     // say hello back
-    LOG((CLOG_DEBUG1 "say hello version %d.%d", kProtocolMajorVersion, kProtocolMinorVersion));
-    String allKeyboardLayoutsStr;
-    for (const auto& layout : AppUtil::instance().getKeyboardLayoutList()) {
-        allKeyboardLayoutsStr += layout;
-    }
-
     LOG((CLOG_DEBUG1 "say hello version %d.%d", helloBackMajor, helloBackMinor));
+    auto localLanguages = m_languageManager.getSerializedLocalLanguages();
     ProtocolUtil::writef(m_stream, kMsgHelloBack,
                             helloBackMajor,
-                            helloBackMinor, & m_name, &allKeyboardLayoutsStr);
-
-    if(m_args.m_enableLangSync) {
-        std::vector<String> missed;
-        std::vector<String> supported;
-        AppUtil::getKeyboardLayoutsDiff(keyboardLayoutList,
-                                        AppUtil::instance().getKeyboardLayoutList(),
-                                        missed, supported);
-
-        if(!supported.empty()) {
-            LOG((CLOG_DEBUG "Supported server languages: %s",  AppUtil::joinStrVector(supported, ", ").c_str()));
-        }
-
-        if(!missed.empty()) {
-            auto result = AppUtil::joinStrVector(missed, ", ");
-            AppUtil::instance().showNotification("Language synchronization error",
-                                                 "These languages are required for client proper work: " + result);
-        }
-    }
-    else {
-        LOG((CLOG_DEBUG "Language sync logic is disabled."));
-    }
+                            helloBackMinor, & m_name, &localLanguages);
 
     // now connected but waiting to complete handshake
     setupScreen();
@@ -841,6 +817,21 @@ Client::onFileRecieveCompleted()
     }
 }
 
+void
+Client::checkMissedLanguages() const
+{
+    if (m_args.m_enableLangSync) {
+        auto missedLanguages = m_languageManager.getMissedLanguages();
+        if (!missedLanguages.empty()) {
+            AppUtil::instance().showNotification("Language synchronization error",
+                                                 "These languages are required for the client to work: " + missedLanguages);
+        }
+    }
+    else {
+        LOG((CLOG_DEBUG "Language sync logic is disabled."));
+    }
+}
+
 void
 Client::handleStopRetry(const Event&, void*)
 {

src/lib/client/Client.h

@@ -24,6 +24,7 @@
 #include "synergy/DragInformation.h"
 #include "synergy/INode.h"
 #include "synergy/ClientArgs.h"
+#include "synergy/languages/LanguageManager.h"
 #include "net/NetworkAddress.h"
 #include "base/EventTypes.h"
 #include "mt/CondVar.h"
@@ -208,6 +209,7 @@ private:
     void                handleStopRetry(const Event&, void*);
     void                onFileRecieveCompleted();
     void                sendClipboardThread(void*);
+    void                checkMissedLanguages() const;
 
 public:
     bool                m_mock;
@@ -241,4 +243,5 @@ private:
     size_t              m_maximumClipboardSize;
     lib::synergy::ClientArgs          m_args;
     size_t              m_resolvedAddressesCount = 0;
+    synergy::languages::LanguageManager m_languageManager;
 };

src/lib/server/ClientProxyUnknown.cpp

@@ -60,14 +60,11 @@ ClientProxyUnknown::ClientProxyUnknown(synergy::IStream* stream, double timeout,
     addStreamHandlers();
 
     LOG((CLOG_DEBUG1 "saying hello"));
-    String allKeyboardLayoutsStr;
-    for (const auto& layout : AppUtil::instance().getKeyboardLayoutList()) {
-        allKeyboardLayoutsStr += layout;
-    }
+    auto localLanguages = m_languageManager.getSerializedLocalLanguages();
     ProtocolUtil::writef(m_stream, kMsgHello,
                             kProtocolMajorVersion,
                             kProtocolMinorVersion,
-                            &allKeyboardLayoutsStr);
+                            &localLanguages);
 }
 
 ClientProxyUnknown::~ClientProxyUnknown()
@@ -186,7 +183,7 @@ ClientProxyUnknown::handleData(const Event&, void*)
     LOG((CLOG_DEBUG1 "parsing hello reply"));
 
     String name("<unknown>");
-    String keyboardLayoutList("<unknown>");
+
     try {
         // limit the maximum length of the hello
         UInt32 n = m_stream->getSize();
@@ -197,10 +194,13 @@ ClientProxyUnknown::handleData(const Event&, void*)
 
         // parse the reply to hello
         SInt16 major, minor;
+        String remoteLanguages;
         if (!ProtocolUtil::readf(m_stream, kMsgHelloBack,
-                                    &major, &minor, &name, &keyboardLayoutList)) {
+                                    &major, &minor, &name, &remoteLanguages)) {
             throw XBadClient();
         }
+        m_languageManager.setRemoteLanguages(remoteLanguages);
+        m_server->setLanguageManager(m_languageManager);
 
         // disallow invalid version numbers
         if (major <= 0 || minor < 0) {
@@ -254,22 +254,6 @@ ClientProxyUnknown::handleData(const Event&, void*)
             throw XIncompatibleClient(major, minor);
         }
 
-        std::vector<String> missed;
-        std::vector<String> supported;
-        AppUtil::getKeyboardLayoutsDiff(keyboardLayoutList,
-                                        AppUtil::instance().getKeyboardLayoutList(),
-                                        missed, supported);
-
-        if(!supported.empty()) {
-            LOG((CLOG_DEBUG "Supported client languages: %s",  AppUtil::joinStrVector(supported, ", ").c_str()));
-        }
-
-        if(!missed.empty()) {
-            auto result = AppUtil::joinStrVector(missed, ", ");
-            AppUtil::instance().showNotification("Language synchronization error",
-                                                 "These languages are required for server proper work: " + result);
-        }
-
         // the proxy is created and now proxy now owns the stream
         LOG((CLOG_DEBUG1 "created proxy for client \"%s\" version %d.%d", name.c_str(), major, minor));
         m_stream = NULL;

src/lib/server/ClientProxyUnknown.h

@@ -20,6 +20,7 @@
 
 #include "base/Event.h"
 #include "base/EventTypes.h"
+#include "synergy/languages/LanguageManager.h"
 
 class ClientProxy;
 class EventQueueTimer;
@@ -73,4 +74,5 @@ private:
     bool                m_ready;
     Server*                m_server;
     IEventQueue*        m_events;
+    synergy::languages::LanguageManager m_languageManager;
 };

src/lib/server/Server.cpp

@@ -348,6 +348,8 @@ Server::adoptClient(BaseClientProxy* client)
 		client->screensaver(true);
 	}
 
+	checkMissedLanguages();
+
 	// send notification
 	Server::ScreenConnectedInfo* info =
 		new Server::ScreenConnectedInfo(getName(client));
@@ -1931,6 +1933,16 @@ Server::sendDragInfo(BaseClientProxy* newScreen)
 	}
 }
 
+void
+Server::checkMissedLanguages() const
+{
+	auto missedLanguages = m_languageManager.getMissedLanguages();
+	if (!missedLanguages.empty()) {
+		AppUtil::instance().showNotification("Language synchronization error",
+											"These languages are required for the server to work: " + missedLanguages);
+	}
+}
+
 void
 Server::onMouseMoveSecondary(SInt32 dx, SInt32 dy)
 {

src/lib/server/Server.h

@@ -26,6 +26,7 @@
 #include "synergy/INode.h"
 #include "synergy/DragInformation.h"
 #include "synergy/ServerArgs.h"
+#include "synergy/languages/LanguageManager.h"
 #include "base/Event.h"
 #include "base/Stopwatch.h"
 #include "base/EventTypes.h"
@@ -155,7 +156,10 @@ public:
 
     //! Store ClientListener pointer
     void                setListener(ClientListener* p) { m_clientListener = p; }
-    
+
+    //! Set language manager
+    void                setLanguageManager(const synergy::languages::LanguageManager& manager) { m_languageManager = manager; }
+
     //@}
     //! @name accessors
     //@{
@@ -373,6 +377,9 @@ private:
     // send drag info to new client screen
     void                sendDragInfo(BaseClientProxy* newScreen);
 
+    //Shows notification if there are missed languages
+    void                checkMissedLanguages() const;
+
 public:
     bool                m_mock;
 
@@ -487,4 +494,5 @@ private:
 
     ClientListener*        m_clientListener;
     lib::synergy::ServerArgs            m_args;
+    synergy::languages::LanguageManager m_languageManager;
 };

src/lib/synergy/AppUtil.cpp

@@ -52,22 +52,3 @@ AppUtil::instance()
     assert(s_instance != nullptr);
     return *s_instance;
 }
-
-void
-AppUtil::getKeyboardLayoutsDiff(const String& remoteCompressedLanguages,
-                                const std::vector<String>& localLanguages,
-                                std::vector<String>& missedLang,
-                                std::vector<String>& supportedLang)
-{
-    missedLang.clear();
-    supportedLang.clear();
-    for(int i = 0; i <= (int)remoteCompressedLanguages.size() - 2; i +=2) {
-        auto layout = remoteCompressedLanguages.substr(i, 2);
-        if (std::find(localLanguages.begin(), localLanguages.end(), layout) == localLanguages.end()) {
-            missedLang.push_back(layout);
-        }
-        else {
-            supportedLang.push_back(layout);
-        }
-    }
-}

src/lib/synergy/AppUtil.h

@@ -33,11 +33,7 @@ public:
     static AppUtil& instance();
     static void exitAppStatic(int code) { instance().exitApp(code); }
     virtual void beforeAppExit() {}
-    static void getKeyboardLayoutsDiff(const String& remoteCompressedLanguages,
-                                       const std::vector<String>& localLanguages,
-                                       std::vector<String>& missedLang,
-                                       std::vector<String>& supportedLang);
-    
+
 private:
     IApp* m_app;
     static AppUtil* s_instance;

src/lib/synergy/CMakeLists.txt

@@ -14,8 +14,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-file(GLOB headers "*.h")
-file(GLOB sources "*.cpp")
+file(GLOB headers "*.h" "languages/*.h")
+file(GLOB sources "*.cpp" "languages/*.cpp")
 
 # arch
 if (WIN32)

src/lib/synergy/IAppUtil.h

@@ -32,18 +32,4 @@ public:
     virtual std::vector<String> getKeyboardLayoutList() = 0;
     virtual String getCurrentLanguageCode() = 0;
     virtual void showNotification(const String& title, const String& text) const = 0;
-
-    static String joinStrVector(const std::vector<String>& source, const String& delim)
-    {
-        String result;
-        for(size_t i = 0; i < source.size(); i++)
-        {
-          result += source[i];
-
-          if(i < source.size() - 1) {
-              result += delim;
-          }
-        }
-        return std::move(result);
-    }
 };

src/lib/synergy/languages/LanguageManager.cpp

@@ -0,0 +1,94 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2014-2021 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 <http://www.gnu.org/licenses/>.
+ */
+#include "LanguageManager.h"
+#include "base/Log.h"
+
+#include <algorithm>
+
+namespace  {
+
+String vectorToString(const std::vector<String>& vector, const String& delimiter = "")
+{
+    String string;
+    for (const auto& item : vector) {
+        if (&item != &vector[0]) {
+            string += delimiter;
+        }
+        string += item;
+    }
+    return string;
+}
+
+} //anonymous namespace
+
+namespace synergy {
+
+namespace languages {
+
+LanguageManager::LanguageManager(const std::vector<String>& localLanguages) :
+    m_localLanguages(localLanguages)
+{
+    LOG((CLOG_INFO "Local languages: %s", vectorToString(m_localLanguages, ", ").c_str()));
+}
+
+void LanguageManager::setRemoteLanguages(const String& remoteLanguages)
+{
+    m_remoteLanguages.clear();
+    if (!remoteLanguages.empty()) {
+        for (size_t i = 0; i <= remoteLanguages.size() - 2; i +=2) {
+            m_remoteLanguages.push_back(remoteLanguages.substr(i, 2));
+        }
+    }
+    LOG((CLOG_INFO "Remote languages: %s", vectorToString(m_remoteLanguages, ", ").c_str()));
+}
+
+const std::vector<String>& LanguageManager::getRemoteLanguages() const
+{
+    return m_remoteLanguages;
+}
+
+const std::vector<String>& LanguageManager::getLocalLanguages() const
+{
+    return m_localLanguages;
+}
+
+String LanguageManager::getMissedLanguages() const
+{
+    String missedLanguages;
+
+    for (const auto& language : m_remoteLanguages) {
+        if (std::find(m_localLanguages.begin(), m_localLanguages.end(), language) == m_localLanguages.end()) {
+            if (!missedLanguages.empty()) {
+                missedLanguages += ", ";
+            }
+            missedLanguages += language;
+        }
+    }
+
+    return missedLanguages;
+}
+
+String LanguageManager::getSerializedLocalLanguages() const
+{
+    return  vectorToString(m_localLanguages);
+}
+
+} //namespace languages
+
+} //namespace synergy
+
+

src/lib/synergy/languages/LanguageManager.h

@@ -0,0 +1,71 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2014-2021 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 <http://www.gnu.org/licenses/>.
+ */
+#ifndef SYNERGY_LANGUAGESLANGUAGEMANAGER_H
+#define SYNERGY_LANGUAGESLANGUAGEMANAGER_H
+
+#include "synergy/AppUtil.h"
+#include "base/String.h"
+#include <vector>
+
+namespace synergy {
+
+namespace languages {
+
+class LanguageManager
+{
+    std::vector<String> m_remoteLanguages;
+    std::vector<String> m_localLanguages;
+
+public:
+    explicit LanguageManager(const std::vector<String>& localLanguages = AppUtil::instance().getKeyboardLayoutList());
+
+    /**
+     * @brief setRemoteLanguages sets remote languages
+     * @param remoteLanguages is a string with sericalized languages
+     */
+    void setRemoteLanguages(const String& remoteLanguages);
+
+    /**
+     * @brief getRemoteLanguages getter for remote languages
+     * @return vector of remote languages
+     */
+    const std::vector<String>& getRemoteLanguages() const;
+
+    /**
+     * @brief getLocalLanguages getter for local languages
+     * @return vector of local languages
+     */
+    const std::vector<String>& getLocalLanguages() const;
+
+    /**
+     * @brief getMissedLanguages getter for missed languages on local machine
+     * @return difference between remote and local languages as a coma separated string
+     */
+    String getMissedLanguages() const;
+
+    /**
+     * @brief getSerializedLocalLanguages getter for local serialized languages
+     * @return serialized local languages as a string
+     */
+    String getSerializedLocalLanguages() const;
+};
+
+} //namespace languages
+
+} //namespace synergy
+
+#endif // SYNERGY_LANGUAGESLANGUAGEMANAGER_H

src/lib/synergy/unix/AppUtilUnix.cpp

@@ -172,8 +172,8 @@ AppUtilUnix::getCurrentLanguageCode()
 void
 AppUtilUnix::showNotification(const String & title, const String & text) const
 {
-    LOG((CLOG_DEBUG "Showing notification. Title: \"%s\". Text: \"%s\"", title.c_str(), text.c_str()));
 #if WINAPI_XWINDOWS
+    LOG((CLOG_INFO "Showing notification. Title: \"%s\". Text: \"%s\"", title.c_str(), text.c_str()));
     if (!notify_init("Synergy"))
     {
         LOG((CLOG_INFO "Failed to initialize libnotify"));

src/lib/synergy/win32/AppUtilWindows.cpp

@@ -249,7 +249,7 @@ public:
 void
 AppUtilWindows::showNotification(const String & title, const String & text) const
 {
-    LOG((CLOG_DEBUG "Showing notification. Title: \"%s\". Text: \"%s\"", title.c_str(), text.c_str()));
+    LOG((CLOG_INFO "Showing notification. Title: \"%s\". Text: \"%s\"", title.c_str(), text.c_str()));
     if (!WinToastLib::WinToast::isCompatible()) {
         LOG((CLOG_INFO "This system does not support toast notifications"));
         return;

src/test/unittests/CMakeLists.txt

@@ -14,8 +14,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-file(GLOB_RECURSE headers "*.h")
-file(GLOB_RECURSE sources "*.cpp")
+file(GLOB_RECURSE headers "*.h" "languages/*.h")
+file(GLOB_RECURSE sources "*.cpp" "languages/*.cpp")
 
 file(GLOB_RECURSE remove_platform "platform/*")
 list(REMOVE_ITEM headers ${remove_platform})

src/test/unittests/synergy/AppUtilTests.cpp

@@ -1,39 +0,0 @@
-/*
- * synergy -- mouse and keyboard sharing utility
- * Copyright (C) 2014-2016 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 <http://www.gnu.org/licenses/>.
- */
-
-#include "test/global/gtest.h"
-#include "synergy/AppUtil.h"
-
-TEST(AppUtilTests, keyboardLayoutTest)
-{
-    String layoutList1 = "ruenby";
-    std::vector<String> layoutList2 = { "ru", "en" };
-    std::vector<String> missed;
-    std::vector<String> supported;
-    AppUtil::getKeyboardLayoutsDiff(layoutList1, layoutList2, missed, supported);
-
-    EXPECT_EQ(missed, std::vector<String>{ "by" });
-    EXPECT_EQ(supported, layoutList2);
-}
-
-TEST(AppUtilTests, joinVectorTest)
-{
-    std::vector<String> layouts = { "ru", "en" };
-    auto result = AppUtil::joinStrVector(layouts, ", ");
-
-    EXPECT_EQ(result, "ru, en");
-}

src/test/unittests/synergy/languages/LanguageManagerTests.cpp

@@ -0,0 +1,58 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2014-2021 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "synergy/languages/LanguageManager.h"
+#include "test/global/gtest.h"
+
+
+TEST(LanguageManager, RemoteLanguagesTest)
+{
+    std::string remoteLanguages = "ruenuk";
+    synergy::languages::LanguageManager manager({"ru", "en", "uk"});
+
+    manager.setRemoteLanguages(remoteLanguages);
+    EXPECT_EQ((std::vector<std::string> {"ru", "en", "uk"}), manager.getRemoteLanguages());
+
+    manager.setRemoteLanguages(String());
+    EXPECT_TRUE(manager.getRemoteLanguages().empty());
+}
+
+TEST(LanguageManager, LocalLanguagesTest)
+{
+    std::vector<String> localLanguages = {"ru", "en", "uk"};
+    synergy::languages::LanguageManager manager(localLanguages);
+
+    EXPECT_EQ((std::vector<std::string> {"ru", "en", "uk"}), manager.getLocalLanguages());
+}
+
+TEST(LanguageManager, MissedLanguagesTest)
+{
+    String remoteLanguages = "ruenuk";
+    std::vector<String> localLanguages = {"en"};
+    synergy::languages::LanguageManager manager(localLanguages);
+
+    manager.setRemoteLanguages(remoteLanguages);
+    EXPECT_EQ("ru, uk", manager.getMissedLanguages());
+}
+
+TEST(LanguageManager, SerializeLocalLanguagesTest)
+{
+    std::vector<String> localLanguages = {"ru", "en", "uk"};
+    synergy::languages::LanguageManager manager(localLanguages);
+
+    EXPECT_EQ("ruenuk", manager.getSerializedLocalLanguages());
+}