Some more refactoring, splitting up methods

cura/PrinterOutput/NetworkedPrinterOutputDevice.py

@@ -147,6 +147,9 @@ class NetworkedPrinterOutputDevice(PrinterOutputDevice):
         request.setHeader(QNetworkRequest.UserAgentHeader, self._user_agent)
         return request
 
+    def createFormPart(self, content_header: str, data: bytes, content_type: Optional[str] = None) -> QHttpPart:
+        return self._createFormPart(content_header, data, content_type)
+
     def _createFormPart(self, content_header: str, data: bytes, content_type: Optional[str] = None) -> QHttpPart:
         part = QHttpPart()
 

plugins/UM3NetworkPrinting/src/SendMaterialJob.py

@@ -1,21 +1,20 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
+import json
+import os
+import urllib.parse
+from typing import Dict, TYPE_CHECKING, Set
 
-import json  # To decode the list of materials from the printer reply.
+from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest
-import os  # To walk over material files.
-import os.path  # To filter on material files.
-import urllib.parse  # For getting material IDs from their file names.
-from typing import Dict, TYPE_CHECKING
 
-from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest  # To listen to the reply from the printer.
+from UM.Job import Job
-
-from UM.Job import Job  # The interface we're implementing.
 from UM.Logger import Logger
-from UM.MimeTypeDatabase import MimeTypeDatabase  # To strip the extensions of the material profile files.
+from UM.MimeTypeDatabase import MimeTypeDatabase
 from UM.Resources import Resources
-from UM.Settings.ContainerRegistry import ContainerRegistry  # To find the GUIDs of materials.
+from cura.CuraApplication import CuraApplication
-from cura.CuraApplication import CuraApplication  # For the resource types.
+
-from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial, LocalMaterial
+# Absolute imports don't work in plugins
+from .Models import ClusterMaterial, LocalMaterial
 
 if TYPE_CHECKING:
     from .ClusterUM3OutputDevice import ClusterUM3OutputDevice
@@ -25,95 +24,136 @@ if TYPE_CHECKING:
 #
 #   This way it won't freeze up the interface while sending those materials.
 class SendMaterialJob(Job):
+
     def __init__(self, device: "ClusterUM3OutputDevice") -> None:
         super().__init__()
         self.device = device  # type: ClusterUM3OutputDevice
+        self._application = CuraApplication.getInstance()  # type: CuraApplication
 
     ##  Send the request to the printer and register a callback
     def run(self) -> None:
         self.device.get("materials/", on_finished = self.sendMissingMaterials)
 
-    ##  Process the reply from the printer and determine which materials should be updated and sent to the printer
+    ##  Process the materials reply from the printer.
     #
-    #   \param reply The reply from the printer, a json file
+    #   \param reply The reply from the printer, a json file.
-    def sendMissingMaterials(self, reply: QNetworkReply) -> None:
+    def _onGetRemoteMaterials(self, reply: QNetworkReply) -> None:
-        if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200:  # Got an error from the HTTP request.
+
-            Logger.log("e", "Couldn't request current material storage on printer. Not syncing materials.")
+        # Got an error from the HTTP request. If we did not receive a 200 something happened.
+        if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200:
+            Logger.log("e", "Error fetching materials from printer: %s", reply.errorString())
             return
 
-        # Collect materials from the printer's reply
+        # Collect materials from the printer's reply and send the missing ones if needed.
         try:
             remote_materials_by_guid = self._parseReply(reply)
-        except json.JSONDecodeError:
+            self._sendMissingMaterials(remote_materials_by_guid)
-            Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.")
+        except json.JSONDecodeError as e:
-            return
+            Logger.log("e", "Error parsing materials from printer: %s", e)
-        except KeyError:
+        except KeyError as e:
-            Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.")
+            Logger.log("e", "Error parsing materials from printer: %s", e)
-            return
+
+    ##  Determine which materials should be updated and send them to the printer.
+    #
+    #   \param remote_materials_by_guid The remote materials by GUID.
+    def _sendMissingMaterials(self, remote_materials_by_guid: Dict[str, ClusterMaterial]) -> None:
 
         # Collect local materials
         local_materials_by_guid = self._getLocalMaterials()
+        if len(local_materials_by_guid) == 0:
+            Logger.log("d", "There are no local materials to synchronize with the printer.")
+            return
 
         # Find out what materials are new or updated and must be sent to the printer
-        materials_to_send = {
+        material_ids_to_send = self._determineMaterialsToSend(local_materials_by_guid, remote_materials_by_guid)
-            material.id
+        if len(material_ids_to_send) == 0:
-            for guid, material in local_materials_by_guid.items()
+            Logger.log("d", "There are no remote materials to update.")
-            if guid not in remote_materials_by_guid or
+            return
-               material.version > remote_materials_by_guid[guid].version
-        }
 
         # Send materials to the printer
-        self.sendMaterialsToPrinter(materials_to_send)
+        self._sendMaterials(material_ids_to_send)
 
-    ##  Send the materials to the printer
+    ##  From the local and remote materials, determine which ones should be synchronized.
     #
-    #   The given materials will be loaded from disk en sent to to printer. The given id's will be mathed with
+    #   Makes a Set containing only the materials that are not on the printer yet or the ones that are newer in Cura.
-    #   filenames of the locally stored materials
     #
-    #   \param materials_to_send A set with id's of materials that must be sent
+    #   \param local_materials The local materials by GUID.
-    def sendMaterialsToPrinter(self, materials_to_send) -> None:
+    #   \param remote_materials The remote materials by GUID.
-        for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.MaterialInstanceContainer):
+    @staticmethod
+    def _determineMaterialsToSend(local_materials: Dict[str, LocalMaterial],
+                                  remote_materials: Dict[str, ClusterMaterial]) -> Set[str]:
+        return {
+            material.id for guid, material in local_materials.items()
+            if guid not in remote_materials or material.version > remote_materials[guid].version
+        }
+
+    ##  Send the materials to the printer.
+    #
+    #   The given materials will be loaded from disk en sent to to printer.
+    #   The given id's will be matched with filenames of the locally stored materials.
+    #
+    #   \param materials_to_send A set with id's of materials that must be sent.
+    def _sendMaterials(self, materials_to_send: Set[str]) -> None:
+        file_paths = Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.MaterialInstanceContainer)
+
+        # Find all local material files and send them if needed.
+        for file_path in file_paths:
             try:
                 mime_type = MimeTypeDatabase.getMimeTypeForFile(file_path)
             except MimeTypeDatabase.MimeTypeNotFoundError:
-                continue  # Not the sort of file we'd like to send then.
-
-            _, file_name = os.path.split(file_path)
-            material_id = urllib.parse.unquote_plus(mime_type.stripExtension(file_name))
-
-            if material_id not in materials_to_send:
                 continue
 
+            file_name = os.path.basename(file_path)
+            material_id = urllib.parse.unquote_plus(mime_type.stripExtension(file_name))
+            if material_id not in materials_to_send:
+                # If the material does not have to be sent we skip it.
+                continue
+
+            self._sendMaterialFile(file_path, file_name, material_id)
+
+    ##  Send a single material file to the printer.
+    #
+    #   Also add the material signature file if that is available.
+    #
+    #   \param file_path The path of the material file.
+    #   \param file_name The name of the material file.
+    #   \param material_id The ID of the material in the file.
+    def _sendMaterialFile(self, file_path: str, file_name: str, material_id: str) -> None:
+
             parts = []
+
+            # Add the material file.
             with open(file_path, "rb") as f:
-                parts.append(
+                parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\""
-                    self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name = file_name),
+                                                        .format(file_name = file_name), f.read()))
-                                                f.read()))
+
-            signature_file_path = file_path + ".sig"
+            # Add the material signature file if needed.
+            signature_file_path = "{}.sig".format(file_path)
             if os.path.exists(signature_file_path):
-                _, signature_file_name = os.path.split(signature_file_path)
+                signature_file_name = os.path.basename(signature_file_path)
                 with open(signature_file_path, "rb") as f:
-                    parts.append(self.device._createFormPart(
+                    parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\""
-                        "name=\"signature_file\"; filename=\"{file_name}\"".format(file_name = signature_file_name),
+                                                            .format(file_name = signature_file_name), f.read()))
-                        f.read()))
 
             Logger.log("d", "Syncing material {material_id} with cluster.".format(material_id = material_id))
             self.device.postFormWithParts(target = "materials/", parts = parts, on_finished = self.sendingFinished)
 
     ##  Check a reply from an upload to the printer and log an error when the call failed
-    def sendingFinished(self, reply: QNetworkReply) -> None:
+    @staticmethod
+    def sendingFinished(reply: QNetworkReply) -> None:
         if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200:
-            Logger.log("e", "Received error code from printer when syncing material: {code}".format(
+            Logger.log("e", "Received error code from printer when syncing material: {code}, {text}".format(
-                code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute)))
+                code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute),
-            Logger.log("e", reply.readAll().data().decode("utf-8"))
+                text = reply.errorString()
+            ))
 
     ##  Parse the reply from the printer
     #
     #   Parses the reply to a "/materials" request to the printer
     #
-    #   \return a dictionary of ClustMaterial objects by GUID
+    #   \return a dictionary of ClusterMaterial objects by GUID
     #   \throw json.JSONDecodeError Raised when the reply does not contain a valid json string
-    #   \throw KeyErrror Raised when on of the materials does not include a valid guid
+    #   \throw KeyError Raised when on of the materials does not include a valid guid
     @classmethod
     def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]:
         remote_materials_list = json.loads(reply.readAll().data().decode("utf-8"))
@@ -124,17 +164,21 @@ class SendMaterialJob(Job):
     #   Only the new newest version of the local materials is returned
     #
     #   \return a dictionary of LocalMaterial objects by GUID
-    @classmethod
+    def _getLocalMaterials(self) -> Dict[str, LocalMaterial]:
-    def _getLocalMaterials(cls):
+        result = {}  # type: Dict[str, LocalMaterial]
-        result = {}
+        container_registry = self._application.getContainerRegistry()
-        for material in ContainerRegistry.getInstance().findContainersMetadata(type = "material"):
+        material_containers = container_registry.findContainersMetadata(type = "material")
-            try:
-                localMaterial = LocalMaterial(**material)
 
-                if localMaterial.GUID not in result or localMaterial.version > result.get(localMaterial.GUID).version:
+        # Find the latest version of all material containers in the registry.
-                    result[localMaterial.GUID] = localMaterial
+        for m in material_containers:
-            except (ValueError):
+            try:
-                Logger.log("e", "Material {material_id} has invalid version number {number}.".format(
+                material = LocalMaterial(**m)
-                    material_id = material["id"], number = material["version"]))
+                if material.GUID not in result or material.version > result.get(material.GUID).version:
+                    result[material.GUID] = material
+            except ValueError as e:
+                Logger.log("w", "Local material {material_id} has invalid values: {e}".format(
+                    material_id = m["id"],
+                    e = e
+                ))
 
         return result