refactor!:deprecate QML upload from bus

never worked right, causes more issues than it helps

Author: JarbasAl

README.md

@@ -40,13 +40,6 @@ under mycroft.conf
     // "gui_file_server": true,
     // "file_server_port": 8000,
 
-    // Optional support for collecting GUI files for container support
-    // The ovos-gui container path for these files will be {XDG_CACHE_HOME}/ovos_gui_file_server.
-    // With the below configuration, the GUI client will have files prefixed with the configured host path,
-    // so the example below describes a situation where `{XDG_CACHE_HOME}/ovos_gui_file_server` maps
-    // to `/tmp/gui_files` on the filesystem where the GUI client is running.    
-    // "gui_file_host_path": "/tmp/gui_files",
-    
     // Optionally specify a default qt version for connected clients that don't report it
     "default_qt_version": 5
   },

ovos_gui/constants.py

@@ -0,0 +1,4 @@
+from ovos_config.locations import get_xdg_cache_save_path
+
+GUI_CACHE_PATH = get_xdg_cache_save_path('ovos_gui')
+

ovos_gui/gui_file_server.py

@@ -5,7 +5,6 @@
 from threading import Thread, Event
 
 from ovos_config import Configuration
-from ovos_utils.file_utils import get_temp_path
 from ovos_utils.log import LOG
 
 _HTTP_SERVER = None

ovos_gui/namespace.py

@@ -40,13 +40,12 @@
 over the GUI message bus.
 """
 import shutil
-from os import makedirs
 from os.path import join, dirname, isfile, exists
 from threading import Event, Lock, Timer
 from typing import List, Union, Optional, Dict
 
 from ovos_config.config import Configuration
-from ovos_utils.log import LOG, log_deprecation
+from ovos_utils.log import LOG
 
 from ovos_bus_client import Message, MessageBusClient
 from ovos_gui.bus import (
@@ -57,7 +56,7 @@
 )
 from ovos_gui.gui_file_server import start_gui_http_server
 from ovos_gui.page import GuiPage
-
+from ovos_gui.constants import GUI_CACHE_PATH
 namespace_lock = Lock()
 
 RESERVED_KEYS = ['__from', '__idle']
@@ -433,9 +432,6 @@ def __init__(self, core_bus: MessageBusClient):
         self._system_res_dir = join(dirname(__file__), "res", "gui")
         self._ready_event = Event()
         self.gui_file_server = None
-        self.gui_file_path = None  # HTTP Server local path
-        self.gui_file_host_path = None  # Docker host path
-        self._connected_frameworks: List[str] = list()
         self._init_gui_file_share()
         self._define_message_handlers()
 
@@ -450,20 +446,10 @@ def _init_gui_file_share(self):
         If `gui_file_server` is defined, resources will be served via HTTP
         """
         config = Configuration().get("gui", {})
-        self.gui_file_host_path = config.get("gui_file_host_path")
-
-        # Check for GUI file sharing via HTTP server or mounted host path
-        if config.get("gui_file_server") or self.gui_file_host_path:
-            from ovos_utils.xdg_utils import xdg_cache_home
-            if config.get("server_path"):
-                log_deprecation("`server_path` configuration is deprecated. "
-                                "Files will always be saved to "
-                                "XDG_CACHE_HOME/ovos_gui_file_server", "0.1.0")
-            self.gui_file_path = config.get("server_path") or \
-                                 join(xdg_cache_home(), "ovos_gui_file_server")
-            if config.get("gui_file_server"):
-                self.gui_file_server = start_gui_http_server(self.gui_file_path)
-            self._upload_system_resources()
+        # Check for GUI file sharing via HTTP server
+        if config.get("gui_file_server"):
+            self.gui_file_server = start_gui_http_server(GUI_CACHE_PATH)
+        self._cache_system_resources()
 
     def _define_message_handlers(self):
         """
@@ -474,7 +460,6 @@ def _define_message_handlers(self):
         self.core_bus.on("gui.page.delete", self.handle_delete_page)
         self.core_bus.on("gui.page.delete.all", self.handle_delete_all_pages)
         self.core_bus.on("gui.page.show", self.handle_show_page)
-        self.core_bus.on("gui.page.upload", self.handle_receive_gui_pages)
         self.core_bus.on("gui.status.request", self.handle_status_request)
         self.core_bus.on("gui.value.set", self.handle_set_value)
         self.core_bus.on("mycroft.gui.connected", self.handle_client_connected)
@@ -485,68 +470,6 @@ def _define_message_handlers(self):
 
     def handle_ready(self, message):
         self._ready_event.set()
-        self.core_bus.on("gui.volunteer_page_upload",
-                         self.handle_gui_pages_available)
-
-    def handle_gui_pages_available(self, message: Message):
-        """
-        Handle a skill or plugin advertising that it has GUI pages available to
-        upload. If there are connected clients, request pages for each connected
-        GUI framework.
-        @param message: `gui.volunteer_page_upload` message
-        """
-        if not any((self.gui_file_host_path, self.gui_file_server)):
-            LOG.debug("No GUI file server running or host path configured")
-            return
-
-        LOG.debug(f"Requesting resources for {self._connected_frameworks}")
-        for framework in self._connected_frameworks:
-            skill_id = message.data.get("skill_id")
-            self.core_bus.emit(message.reply("gui.request_page_upload",
-                                             {'skill_id': skill_id,
-                                              'framework': framework},
-                                             {"source": "gui",
-                                              "destination": ["skills",
-                                                              "PHAL"]}))
-
-    def handle_receive_gui_pages(self, message: Message):
-        """
-        Handle GUI resources from a skill or plugin. Pages are written to
-        `self.server_path` which is accessible via a lightweight HTTP server and
-        may additionally be mounted to a host path/volume in container setups.
-        @param message: Message containing UI resource file contents and meta
-            message.data:
-                pages: dict page_filename to encoded bytes content;
-                    paths are relative to the `framework` directory, so a page
-                    for framework `all` could be `qt5/subdir/file.qml` and the
-                    equivalent page for framework `qt5` would be
-                    `subdir/file.qml`
-                framework: `all` if all GUI resources are included, else the
-                    specific GUI framework (i.e. `qt5`, `qt6`)
-                __from: skill_id of module uploading GUI resources
-        """
-        for page, contents in message.data["pages"].items():
-            try:
-                if message.data.get("framework") == "all":
-                    # All GUI resources are uploaded
-                    resource_base_path = join(self.gui_file_path,
-                                              message.data['__from'])
-                else:
-                    resource_base_path = join(self.gui_file_path,
-                                              message.data['__from'],
-                                              message.data.get('framework') or
-                                              "qt5")
-                byte_contents = bytes.fromhex(contents)
-                file_path = join(resource_base_path, page)
-                LOG.debug(f"writing UI file: {file_path}")
-                makedirs(dirname(file_path), exist_ok=True)
-                with open(file_path, 'wb+') as f:
-                    f.write(byte_contents)
-            except Exception as e:
-                LOG.exception(f"Failed to write {page}: {e}")
-        if message.data["__from"] == self._active_homescreen:
-            # Configured home screen skill just uploaded pages, show it again
-            self.core_bus.emit(message.forward("homescreen.manager.show_active"))
 
     def handle_clear_namespace(self, message: Message):
         """
@@ -952,23 +875,9 @@ def handle_client_connected(self, message: Message):
         websocket_config = get_gui_websocket_config()
         port = websocket_config["base_port"]
         message = message.forward("mycroft.gui.port",
-                                  dict(port=port, gui_id=gui_id))
+                                  dict(port=port, gui_id=gui_id, framework=framework))
         self.core_bus.emit(message)
 
-        if self.gui_file_path or self.gui_file_host_path:
-            if not self._ready_event.wait(90):
-                LOG.warning("Not reported ready after 90s")
-            if framework not in self._connected_frameworks:
-                LOG.debug(f"Requesting page upload for {framework}")
-                self.core_bus.emit(Message("gui.request_page_upload",
-                                           {'framework': framework},
-                                           {"source": "gui",
-                                            "destination": ["skills", "PHAL"]}))
-
-        if framework not in self._connected_frameworks:
-            LOG.debug(f"Connecting framework: {framework}")
-            self._connected_frameworks.append(framework)
-
     def handle_page_interaction(self, message: Message):
         """
         Handles an event from the GUI indicating a page has been interacted with.
@@ -1037,13 +946,13 @@ def _del_namespace_in_remove_timers(self, namespace_name: str):
         if namespace_name in self.remove_namespace_timers:
             del self.remove_namespace_timers[namespace_name]
 
-    def _upload_system_resources(self):
+    def _cache_system_resources(self):
         """
         Copy system GUI resources to the served file path
         """
-        output_path = join(self.gui_file_path, "system")
+        output_path = f"{GUI_CACHE_PATH}/system"
         if exists(output_path):
             LOG.info(f"Removing existing system resources before updating")
             shutil.rmtree(output_path)
         shutil.copytree(self._system_res_dir, output_path)
-        LOG.debug(f"Copied system resources to {self.gui_file_path}")
+        LOG.debug(f"Copied system resources from {self._system_res_dir} to {output_path}")

ovos_gui/page.py

@@ -2,6 +2,7 @@
 from typing import Union, Optional
 from dataclasses import dataclass
 from ovos_utils.log import LOG
+from ovos_gui.constants import GUI_CACHE_PATH
 
 
 @dataclass
@@ -44,45 +45,32 @@ def get_file_extension(framework: str) -> str:
             return "qml"
         return ""
 
-    def get_uri(self, framework: str = "qt5", server_url: str = None) -> str:
+    @property
+    def _gui_cache(self):
+        return f"{GUI_CACHE_PATH}]/{self.res_namespace}"
+
+    @property
+    def res_namespace(self):
+        return "system" if self.page_id.startswith("SYSTEM") else self.namespace
+
+    def get_uri(self, framework: str = "qt5") -> str:
         """
         Get a valid URI for this Page.
-        @param framework: String GUI framework to get resources for
-        @param server_url: String server URL if available; this could be for a
-            web server (http://), or a container host path (file://)
+        @param framework: String GUI framework to get resources for (currently only 'qt5')
         @return: Absolute path to the requested resource
         """
         if self.url:
             LOG.warning(f"Static URI: {self.url}")
             return self.url
 
         res_filename = f"{self.page_id}.{self.get_file_extension(framework)}"
-        res_namespace = "system" if self.page_id.startswith("SYSTEM") else \
-            self.namespace
-        if server_url:
-            if "://" not in server_url:
-                if server_url.startswith("/"):
-                    LOG.debug(f"No schema in server_url, assuming 'file'")
-                    server_url = f"file://{server_url}"
-                else:
-                    LOG.debug(f"No schema in server_url, assuming 'http'")
-                    server_url = f"http://{server_url}"
-            path = f"{server_url}/{res_namespace}/{framework}/{res_filename}"
-            LOG.info(f"Resolved server URI: {path}")
-            return path
-        base_path = self.resource_dirs.get(framework)
-        if not base_path and self.resource_dirs.get("all"):
-            file_path = join(self.resource_dirs.get('all'), framework,
-                             res_filename)
+        if self.res_namespace == "system":
+            path = join(dirname(__file__), "res", "gui", framework,  res_filename)
         else:
-            file_path = join(base_path, res_filename)
-        if isfile(file_path):
-            return file_path
-        # Check system resources
-        file_path = join(dirname(__file__), "res", "gui", framework,
-                         res_filename)
-        if isfile(file_path):
-            return file_path
+            path = f"{self._gui_cache}/{framework}/{res_filename}"
+        LOG.debug(f"Resolved page URI: {path}")
+        if isfile(path):
+            return path
         raise FileNotFoundError(f"Unable to resolve resource file for "
                                 f"resource {res_filename} for framework "
                                 f"{framework}")

test/unittests/test_namespace.py

@@ -467,12 +467,12 @@ def test_upload_system_resources(self):
         test_dir = join(dirname(__file__), "upload_test")
         makedirs(test_dir, exist_ok=True)
         self.namespace_manager.gui_file_path = test_dir
-        self.namespace_manager._upload_system_resources()
+        self.namespace_manager._cache_system_resources()
         self.assertTrue(isdir(join(test_dir, "system", "qt5")))
         self.assertTrue(isfile(join(test_dir, "system", "qt5",
                                     "SYSTEM_TextFrame.qml")))
         # Test repeated copy doesn't raise any exception
-        self.namespace_manager._upload_system_resources()
+        self.namespace_manager._cache_system_resources()
         self.assertTrue(isdir(join(test_dir, "system", "qt5")))
         self.assertTrue(isfile(join(test_dir, "system", "qt5",
                                     "SYSTEM_TextFrame.qml")))