From c212472e74687b5d839c1746e6bebe8d201e2707 Mon Sep 17 00:00:00 2001
From: markuspg <markuspg@users.noreply.github.com>
Date: Wed, 16 Sep 2020 09:57:18 +0200
Subject: [PATCH] Revise ClientHelpNotificationServer

---
 src/Lib/clienthelpnotificationserver.cpp | 49 +++++++++++++++---------
 src/Lib/clienthelpnotificationserver.h   | 19 +++++----
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/src/Lib/clienthelpnotificationserver.cpp b/src/Lib/clienthelpnotificationserver.cpp
index ef795b8..6e4d13d 100644
--- a/src/Lib/clienthelpnotificationserver.cpp
+++ b/src/Lib/clienthelpnotificationserver.cpp
@@ -17,13 +17,21 @@
  *  along with Labcontrol.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <memory>
-
 #include "clienthelpnotificationserver.h"
 #include "settings.h"
 
+#include <QNetworkConfigurationManager>
+#include <QNetworkSession>
+#include <QTcpServer>
+#include <QTcpSocket>
+
 extern std::unique_ptr<lc::Settings> settings;
 
+/*!
+ * \brief Create a new instance and start listening as soon as possible
+ *
+ * \param argParent The instance's parent QObject
+ */
 lc::ClientHelpNotificationServer::ClientHelpNotificationServer(
     QObject *argParent)
     : QObject{argParent}, hostAddress{settings->serverIP} {
@@ -31,11 +39,12 @@ lc::ClientHelpNotificationServer::ClientHelpNotificationServer(
   if (manager.capabilities() &
       QNetworkConfigurationManager::NetworkSessionRequired) {
     // Get saved network configuration
-    QSettings settings{QSettings::UserScope, QLatin1String{"QtProject"}};
-    settings.beginGroup(QLatin1String{"QtNetwork"});
+    QSettings qSettings{QSettings::UserScope, QLatin1String{"QtProject"}};
+    qSettings.beginGroup(QLatin1String{"QtNetwork"});
     const QString id =
-        settings.value(QLatin1String{"DefaultNetworkConfiguration"}).toString();
-    settings.endGroup();
+        qSettings.value(QLatin1String{"DefaultNetworkConfiguration"})
+            .toString();
+    qSettings.endGroup();
 
     // If the saved network configuration is not currently discovered use the
     // system default
@@ -56,6 +65,10 @@ lc::ClientHelpNotificationServer::ClientHelpNotificationServer(
           &ClientHelpNotificationServer::SendReply);
 }
 
+/*!
+ * \brief Open a new network session and start listening for incoming
+ * connections
+ */
 void lc::ClientHelpNotificationServer::OpenSession() {
   // Save the used configuration
   if (networkSession) {
@@ -69,14 +82,14 @@ void lc::ClientHelpNotificationServer::OpenSession() {
       id = config.identifier();
     }
 
-    QSettings settings(QSettings::UserScope, QLatin1String{"QtProject"});
-    settings.beginGroup(QLatin1String{"QtNetwork"});
-    settings.setValue(QLatin1String{"DefaultNetworkConfiguration"}, id);
-    settings.endGroup();
+    QSettings qSettings(QSettings::UserScope, QLatin1String{"QtProject"});
+    qSettings.beginGroup(QLatin1String{"QtNetwork"});
+    qSettings.setValue(QLatin1String{"DefaultNetworkConfiguration"}, id);
+    qSettings.endGroup();
   }
 
   helpMessageServer = new QTcpServer{this};
-  if (!helpMessageServer->listen(hostAddress,
+  if (!helpMessageServer->listen(QHostAddress{hostAddress},
                                  settings->clientHelpNotificationServerPort)) {
     QMessageBox messageBox{
         QMessageBox::Critical,
@@ -94,19 +107,17 @@ void lc::ClientHelpNotificationServer::SendReply() {
   QByteArray block;
   QDataStream out{&block, QIODevice::WriteOnly};
   out.setVersion(QDataStream::Qt_5_2);
-  out << (quint16)0;
+  out << static_cast<quint16>(0);
   out << QString{"Help demand retrieved."};
   out.device()->seek(0);
-  out << (quint16)(block.size() - sizeof(quint16));
+  out << static_cast<quint16>(static_cast<unsigned int>(block.size()) -
+                              sizeof(quint16));
 
-  QTcpSocket *clientConnection = helpMessageServer->nextPendingConnection();
-  QString peerAddress = clientConnection->peerAddress().toString();
+  const auto clientConnection = helpMessageServer->nextPendingConnection();
+  const auto peerAddress = clientConnection->peerAddress().toString();
   QString peerName;
-  bool unknownClient = false;
   if (settings->clIPsToClMap.contains(peerAddress)) {
     peerName = settings->clIPsToClMap[peerAddress]->name;
-  } else {
-    unknownClient = true;
   }
 
   connect(clientConnection, &QTcpSocket::disconnected, clientConnection,
@@ -114,7 +125,7 @@ void lc::ClientHelpNotificationServer::SendReply() {
   clientConnection->write(block);
   clientConnection->disconnectFromHost();
 
-  if (unknownClient) {
+  if (peerName.isEmpty()) {
     QMessageBox requestReceivedBox{
         QMessageBox::Information, tr("Unknown client asked for help."),
         tr("An unknown client with IP '%1' asked for help.").arg(peerAddress),
diff --git a/src/Lib/clienthelpnotificationserver.h b/src/Lib/clienthelpnotificationserver.h
index d5e5e8d..3a7ed3b 100644
--- a/src/Lib/clienthelpnotificationserver.h
+++ b/src/Lib/clienthelpnotificationserver.h
@@ -20,27 +20,26 @@
 #ifndef CLIENTHELPNOTIFICATIONSERVER_H
 #define CLIENTHELPNOTIFICATIONSERVER_H
 
-#include "src/Lib/client.h"
-
-#include <QHostAddress>
-#include <QMessageBox>
 #include <QObject>
-#include <QtNetwork>
+
+class QNetworkSession;
+class QTcpServer;
 
 namespace lc {
 
+/*!
+ * \brief A server listing for connections from clients which will be
+ * interpreted as help requests
+ */
 class ClientHelpNotificationServer : public QObject {
   Q_OBJECT
+
 public:
   explicit ClientHelpNotificationServer(QObject *argParent = nullptr);
 
-signals:
-
-public slots:
-
 private:
   QTcpServer *helpMessageServer = nullptr;
-  const QHostAddress hostAddress;
+  const QString hostAddress;
   QNetworkSession *networkSession = nullptr;
 
 private slots: