Merge branch 'mr/refactor-more-logging' into 'master'

Refactor all D-Bus methods to output more debug statements.

Closes #14

See merge request remotewebapp/rwa.support.sessionservice!11

1 files changed, 81 insertions, 33 deletions

diff --git a/rwa/support/sessionservice/service.py b/rwa/support/sessionservice/service.py
index 52bf627..23fd660 100755
--- a/rwa/support/sessionservice/service.py
+++ b/rwa/support/sessionservice/service.py
@@ -3,7 +3,7 @@
 # This file is part of Remote Support Desktop
 # https://gitlab.das-netzwerkteam.de/RemoteWebApp/rwa.support.sessionservice
 # Copyright 2020, 2021 Jonathan Weth <dev@jonathanweth.de>
-# Copyright 2020 Daniel Teichmann <daniel.teichmann@das-netzwerkteam.de>
+# Copyright 2020, 2021 Daniel Teichmann <daniel.teichmann@das-netzwerkteam.de>
 # Copyright 2020 Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
@@ -26,7 +26,6 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 import json
-import logging
 import signal
 import time
 from threading import Thread
@@ -36,10 +35,12 @@ import click
 import dbus
 import dbus.mainloop.glib
 import dbus.service
+import validators
 from gi.repository import GLib
 
 from .config import user_settings
 from .lock import is_locked, lock, unlock
+from .log import logging
 from .session import Session
 from .trigger import TriggerServerThread
 
@@ -77,6 +78,16 @@ class RWASupportSessionService(dbus.service.Object):
 
         logging.info("D-Bus service has been started.")
 
+    def _is_url(self, url: str) -> bool:
+        """Test if the given string is an url.
+
+        :param url: The string which should be an URL.
+        :return: Whether the string is an URL.
+        """
+        valid = validators.url(url)
+        logging.debug(f"Is {url} an URL: {valid}")
+        return valid
+
     def _get_web_app_hosts(self) -> str:
         """Get all registered RWA.Support.WebApp hosts.
 
@@ -97,6 +108,9 @@ class RWASupportSessionService(dbus.service.Object):
 
             ["https://example.org", "http://127.0.0.1:8000"]
         """
+        logging.info("D-Bus method call: %s()", "get_web_app_hosts")
+
+        logging.debug('Return to D-Bus caller: "%s"', self._get_web_app_hosts())
         return self._get_web_app_hosts()
 
     @dbus.service.method(
@@ -114,14 +128,23 @@ class RWASupportSessionService(dbus.service.Object):
 
             ["https://example.org", "http://127.0.0.1:8000"]
         """
-        user_settings.web_app_hosts.append(host)
-        user_settings.save_settings()
+        host = str(host)
+
+        logging.info('D-Bus method call: %s("%s")', "add_web_app_host", host)
+
+        if self._is_url(host):
+            user_settings.save_settings()
+            user_settings.web_app_hosts.append(host)
+            logging.debug('Added "%s" to "web_app_hosts" in user_settings', host)
+        else:
+            logging.warning("Given URL is not valid!")
+            logging.debug('Did not add "%s" to "web_app_hosts" in user_settings', host)
         return self._get_web_app_hosts()
 
     @dbus.service.method(
         "org.ArcticaProject.RWASupportSessionService", in_signature="i", out_signature="s"
     )
-    def remove_web_app_host(self, idx: int) -> str:
+    def remove_web_app_host(self, host_idx: int) -> str:
         """Remove a RWA.Support.WebApp host.
 
         :param idx: Index of web app host (D-Bus integer)
@@ -133,8 +156,19 @@ class RWASupportSessionService(dbus.service.Object):
 
             ["https://example.org", "http://127.0.0.1:8000"]
         """
-        del user_settings.web_app_hosts[idx]
-        user_settings.save_settings()
+        logging.info("D-Bus method call: %s(%d)", "remove_web_app_host", host_idx)
+
+        if host_idx >= 0 and host_idx < len(user_settings.web_app_hosts):
+            host = user_settings.web_app_hosts[host_idx]
+            del user_settings.web_app_hosts[host_idx]
+            user_settings.save_settings()
+            logging.debug('Removed web_app_hosts[%d]="%s" in user_settings', host_idx, host)
+        else:
+            logging.warning("Given host index is not valid!")
+            logging.debug(
+                "Did not remove web_app_hosts[%d] (not existent!) in " "user_settings", host_idx
+            )
+
         return self._get_web_app_hosts()
 
     @dbus.service.method(
@@ -158,21 +192,30 @@ class RWASupportSessionService(dbus.service.Object):
 
             {"status": "error", "type": "<type>"}
 
-        **Possible choices for error types:** ``multiple``, ``connection``
+        **Possible choices for error types:** ``multiple``,
+                                              ``connection``,
+                                              ``host_not_found``
         """
+        logging.info("D-Bus method call: %s(%d)", "start", host_idx)
+
         if ALLOW_ONLY_ONE_SESSION and len(self.sessions.values()) > 0:
             logging.warning(
                 "There is already one session running and the service "
                 "is configured to allow only one "
                 "session, so this session won't be started."
             )
-            return json.dumps({"status": "error", "type": "multiple"})
-        print(host_idx, user_settings.web_app_hosts)
+            response = json.dumps({"status": "error", "type": "multiple"})
+            logging.debug("The response to the D-Bus caller: '%s'", response)
+            return response
 
         try:
             host = user_settings.web_app_hosts[host_idx]
+            logging.debug('web_app_hosts[%d] is the following host: "%s"', host_idx, host)
         except IndexError:
-            return json.dumps({"status": "error", "type": "host_not_found"})
+            logging.error("web_app_hosts[%d] does not exist!", host_idx)
+            response = json.dumps({"status": "error", "type": "host_not_found"})
+            logging.debug("The response to the D-Bus caller: '%s'", response)
+            return response
 
         # Start session
         try:
@@ -189,11 +232,18 @@ class RWASupportSessionService(dbus.service.Object):
 
             logging.info(f"New session #{session.pid} was started with meta {return_json}.")
 
-            return json.dumps(return_json)
+            response = json.dumps(return_json)
+            logging.debug("The response to the D-Bus caller: '%s'", response)
+            return response
         except ConnectionError:
-            logging.error("There was a connection error while trying to reach the server.")
+            logging.error(
+                "There was a connection error while trying to reach "
+                "the RWA.Support.WebApp server."
+            )
 
-        return json.dumps({"status": "error", "type": "connection"})
+        response = json.dumps({"status": "error", "type": "connection"})
+        logging.debug("The response to the D-Bus caller: '%s'", response)
+        return response
 
     @dbus.service.method(
         "org.ArcticaProject.RWASupportSessionService", in_signature="i", out_signature="s"
@@ -223,15 +273,22 @@ class RWASupportSessionService(dbus.service.Object):
         ``dead``     There was a problem, so that the session is dead.
         ============ ======================
         """
-        return self._get_status(pid)
+        logging.info("D-Bus method call: %s(%d)", "status", pid)
+        response = self._get_status(pid)
+        logging.debug("The response to the D-Bus caller: '%s'", response)
+        return response
 
     @dbus.service.method(
         "org.ArcticaProject.RWASupportSessionService", in_signature="i", out_signature="s"
     )
     def refresh_status(self, pid: int) -> str:
         """Update status from WebApp before returning it here like :meth:`status`."""
+        logging.info("D-Bus method call: %s(%d)", "refresh_status", pid)
+
         self._update_session(pid)
-        return self._get_status(pid)
+        response = self._get_status(pid)
+        logging.debug("The response to the D-Bus caller: '%s'", response)
+        return response
 
     @dbus.service.method(
         "org.ArcticaProject.RWASupportSessionService", in_signature="i", out_signature="s"
@@ -248,12 +305,18 @@ class RWASupportSessionService(dbus.service.Object):
 
             {"id": <pid>, "status": "stopped"}
         """
+        logging.info("D-Bus method call: %s(%d)", "stop", pid)
+
         try:
             session = self.sessions[pid]
         except KeyError:
-            return json.dumps({"pid": pid, "status": "stopped"}, sort_keys=True)
+            response = json.dumps({"pid": pid, "status": "stopped"}, sort_keys=True)
+            logging.debug("The response to the D-Bus caller: '%s'", response)
+            return response
         session.stop()
-        return json.dumps({"id": pid, "status": "stopped"}, sort_keys=True)
+        response = json.dumps({"id": pid, "status": "stopped"}, sort_keys=True)
+        logging.debug("The response to the D-Bus caller: '%s'", response)
+        return response
 
     def _get_status(self, pid: int) -> str:
         try:
@@ -344,21 +407,6 @@ class RWASupportSessionService(dbus.service.Object):
             time.sleep(1)
 
 
-def str2bool(v: Union[str, bool, int]) -> bool:
-    """Return true or false if the given string can be interpreted as a boolean.
-
-    If it fails, raise an exception.
-    """
-    if isinstance(v, bool):
-        return v
-    if v.lower() in ("yes", "true", "t", "y", "1", 1):
-        return True
-    elif v.lower() in ("no", "false", "f", "n", "0", 0):
-        return False
-    else:
-        raise argparse.ArgumentTypeError("Boolean value expected.")
-
-
 @click.command()
 @click.option(
     "-m",