Merge branch 'CURA-6035_send_materials_fix' of github.com:Ultimaker/Cura into 4.0

cura/Machines/MaterialManager.py

@@ -302,6 +302,10 @@ class MaterialManager(QObject):
     def getMaterialGroupListByGUID(self, guid: str) -> Optional[List[MaterialGroup]]:
         return self._guid_material_groups_map.get(guid)
 
+    # Returns a dict of all material groups organized by root_material_id.
+    def getAllMaterialGroups(self) -> Dict[str, "MaterialGroup"]:
+        return self._material_group_map
+
     #
     # Return a dict with all root material IDs (k) and ContainerNodes (v) that's suitable for the given setup.
     #

plugins/UM3NetworkPrinting/src/SendMaterialJob.py

@@ -2,7 +2,6 @@
 # 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
 
 from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest
@@ -10,9 +9,7 @@ from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest
 from UM.Application import Application
 from UM.Job import Job
 from UM.Logger import Logger
-from UM.MimeTypeDatabase import MimeTypeDatabase
-from UM.Resources import Resources
-from cura.CuraApplication import CuraApplication
+
 # Absolute imports don't work in plugins
 from .Models import ClusterMaterial, LocalMaterial
 
@@ -37,7 +34,6 @@ class SendMaterialJob(Job):
     #
     #   \param reply The reply from the printer, a json file.
     def _onGetRemoteMaterials(self, reply: QNetworkReply) -> None:
-
         # 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())
@@ -52,7 +48,6 @@ class SendMaterialJob(Job):
     #
     #   \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:
@@ -91,22 +86,22 @@ class SendMaterialJob(Job):
     #
     #   \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)
+        container_registry = Application.getInstance().getContainerRegistry()
+        material_manager = Application.getInstance().getMaterialManager()
+        material_group_dict = material_manager.getAllMaterialGroups()
 
-        # 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:
+        for root_material_id in material_group_dict:
+            if root_material_id not in materials_to_send:
+                # If the material does not have to be sent we skip it.
                 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.
+            file_path = container_registry.getContainerFilePathById(root_material_id)
+            if not file_path:
+                Logger.log("w", "Cannot get file path for material container [%s]", root_material_id)
                 continue
 
-            self._sendMaterialFile(file_path, file_name, material_id)
+            file_name = os.path.basename(file_path)
+            self._sendMaterialFile(file_path, file_name, root_material_id)
 
     ##  Send a single material file to the printer.
     #
@@ -116,7 +111,6 @@ class SendMaterialJob(Job):
     #   \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.
@@ -171,27 +165,31 @@ class SendMaterialJob(Job):
     #   \return a dictionary of LocalMaterial objects by GUID
     def _getLocalMaterials(self) -> Dict[str, LocalMaterial]:
         result = {}  # type: Dict[str, LocalMaterial]
-        container_registry = Application.getInstance().getContainerRegistry()
-        material_containers = container_registry.findContainersMetadata(type = "material")
+        material_manager = Application.getInstance().getMaterialManager()
+
+        material_group_dict = material_manager.getAllMaterialGroups()
 
         # Find the latest version of all material containers in the registry.
-        for material in material_containers:
+        for root_material_id, material_group in material_group_dict.items():
+            material_metadata = material_group.root_material_node.getMetadata()
+
             try:
                 # material version must be an int
-                material["version"] = int(material["version"])
+                material_metadata["version"] = int(material_metadata["version"])
 
                 # Create a new local material
-                local_material = LocalMaterial(**material)
+                local_material = LocalMaterial(**material_metadata)
+                local_material.id = root_material_id
 
                 if local_material.GUID not in result or \
                         local_material.version > result.get(local_material.GUID).version:
                     result[local_material.GUID] = local_material
 
             except KeyError:
-                Logger.logException("w", "Local material {} has missing values.".format(material["id"]))
+                Logger.logException("w", "Local material {} has missing values.".format(material_metadata["id"]))
             except ValueError:
-                Logger.logException("w", "Local material {} has invalid values.".format(material["id"]))
+                Logger.logException("w", "Local material {} has invalid values.".format(material_metadata["id"]))
             except TypeError:
-                Logger.logException("w", "Local material {} has invalid values.".format(material["id"]))
+                Logger.logException("w", "Local material {} has invalid values.".format(material_metadata["id"]))
 
         return result

plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py

@@ -1,26 +1,29 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
+import copy
 import io
 import json
 from unittest import TestCase, mock
-from unittest.mock import patch, call
+from unittest.mock import patch, call, MagicMock
 
 from PyQt5.QtCore import QByteArray
 
-from UM.MimeTypeDatabase import MimeType
 from UM.Application import Application
+
+from cura.Machines.MaterialGroup import MaterialGroup
+from cura.Machines.MaterialNode import MaterialNode
+
 from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob
 
+_FILES_MAP = {"generic_pla_white": "/materials/generic_pla_white.xml.fdm_material",
+              "generic_pla_black": "/materials/generic_pla_black.xml.fdm_material",
+              }
+
 
 @patch("builtins.open", lambda _, __: io.StringIO("<xml></xml>"))
-@patch("UM.MimeTypeDatabase.MimeTypeDatabase.getMimeTypeForFile",
-       lambda _: MimeType(name = "application/x-ultimaker-material-profile", comment = "Ultimaker Material Profile",
-                          suffixes = ["xml.fdm_material"]))
-@patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"])
-@patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice")
-@patch("PyQt5.QtNetwork.QNetworkReply")
 class TestSendMaterialJob(TestCase):
+    # version 1
     _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white",
                              "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA",
                              "brand": "Generic", "material": "PLA", "color_name": "White",
@@ -29,6 +32,37 @@ class TestSendMaterialJob(TestCase):
                              "properties": {"density": "1.00", "diameter": "2.85", "weight": "750"},
                              "definition": "fdmprinter", "compatible": True}
 
+    # version 2
+    _LOCAL_MATERIAL_WHITE_NEWER = {"type": "material", "status": "unknown", "id": "generic_pla_white",
+                                   "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA",
+                                   "brand": "Generic", "material": "PLA", "color_name": "White",
+                                   "GUID": "badb0ee7-87c8-4f3f-9398-938587b67dce", "version": "2",
+                                   "color_code": "#ffffff",
+                                   "description": "Test PLA White", "adhesion_info": "Use glue.",
+                                   "approximate_diameter": "3",
+                                   "properties": {"density": "1.00", "diameter": "2.85", "weight": "750"},
+                                   "definition": "fdmprinter", "compatible": True}
+
+    # invalid version: "one"
+    _LOCAL_MATERIAL_WHITE_INVALID_VERSION = {"type": "material", "status": "unknown", "id": "generic_pla_white",
+                                             "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA",
+                                             "brand": "Generic", "material": "PLA", "color_name": "White",
+                                             "GUID": "badb0ee7-87c8-4f3f-9398-938587b67dce", "version": "one",
+                                             "color_code": "#ffffff",
+                                             "description": "Test PLA White", "adhesion_info": "Use glue.",
+                                             "approximate_diameter": "3",
+                                             "properties": {"density": "1.00", "diameter": "2.85", "weight": "750"},
+                                             "definition": "fdmprinter", "compatible": True}
+
+    _LOCAL_MATERIAL_WHITE_ALL_RESULT = {"generic_pla_white": MaterialGroup("generic_pla_white",
+                                                                           MaterialNode(_LOCAL_MATERIAL_WHITE))}
+
+    _LOCAL_MATERIAL_WHITE_NEWER_ALL_RESULT = {"generic_pla_white": MaterialGroup("generic_pla_white",
+                                                                                 MaterialNode(_LOCAL_MATERIAL_WHITE_NEWER))}
+
+    _LOCAL_MATERIAL_WHITE_INVALID_VERSION_ALL_RESULT = {"generic_pla_white": MaterialGroup("generic_pla_white",
+                                                                                           MaterialNode(_LOCAL_MATERIAL_WHITE_INVALID_VERSION))}
+
     _LOCAL_MATERIAL_BLACK = {"type": "material", "status": "unknown", "id": "generic_pla_black",
                              "base_file": "generic_pla_black", "setting_version": "5", "name": "Yellow CPE",
                              "brand": "Ultimaker", "material": "CPE", "color_name": "Black",
@@ -37,6 +71,9 @@ class TestSendMaterialJob(TestCase):
                              "properties": {"density": "1.01", "diameter": "2.85", "weight": "750"},
                              "definition": "fdmprinter", "compatible": True}
 
+    _LOCAL_MATERIAL_BLACK_ALL_RESULT = {"generic_pla_black": MaterialGroup("generic_pla_black",
+                                                                           MaterialNode(_LOCAL_MATERIAL_BLACK))}
+
     _REMOTE_MATERIAL_WHITE = {
         "guid": "badb0ee7-87c8-4f3f-9398-938587b67dce",
         "material": "PLA",
@@ -55,14 +92,17 @@ class TestSendMaterialJob(TestCase):
         "density": 1.00
     }
 
-    def test_run(self, device_mock, reply_mock):
+    def test_run(self):
+        device_mock = MagicMock()
         job = SendMaterialJob(device_mock)
         job.run()
 
         # We expect the materials endpoint to be called when the job runs.
         device_mock.get.assert_called_with("materials/", on_finished = job._onGetRemoteMaterials)
 
-    def test__onGetRemoteMaterials_withFailedRequest(self, reply_mock, device_mock):
+    def test__onGetRemoteMaterials_withFailedRequest(self):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
         reply_mock.attribute.return_value = 404
         job = SendMaterialJob(device_mock)
         job._onGetRemoteMaterials(reply_mock)
@@ -70,7 +110,9 @@ class TestSendMaterialJob(TestCase):
         # We expect the device not to be called for any follow up.
         self.assertEqual(0, device_mock.createFormPart.call_count)
 
-    def test__onGetRemoteMaterials_withWrongEncoding(self, reply_mock, device_mock):
+    def test__onGetRemoteMaterials_withWrongEncoding(self):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("cp500"))
         job = SendMaterialJob(device_mock)
@@ -79,7 +121,9 @@ class TestSendMaterialJob(TestCase):
         # Given that the parsing fails we do no expect the device to be called for any follow up.
         self.assertEqual(0, device_mock.createFormPart.call_count)
 
-    def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock):
+    def test__onGetRemoteMaterials_withBadJsonAnswer(self):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.")
         job = SendMaterialJob(device_mock)
@@ -88,7 +132,9 @@ class TestSendMaterialJob(TestCase):
         # Given that the parsing fails we do no expect the device to be called for any follow up.
         self.assertEqual(0, device_mock.createFormPart.call_count)
 
-    def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock):
+    def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
         reply_mock.attribute.return_value = 200
         remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy()
         del remote_material_without_guid["guid"]
@@ -99,18 +145,20 @@ class TestSendMaterialJob(TestCase):
         # Given that parsing fails we do not expect the device to be called for any follow up.
         self.assertEqual(0, device_mock.createFormPart.call_count)
 
+    @patch("cura.Machines.MaterialManager.MaterialManager")
     @patch("cura.Settings.CuraContainerRegistry")
     @patch("UM.Application")
     def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, application_mock, container_registry_mock,
-                                                                     reply_mock, device_mock):
+                                                                     material_manager_mock):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
+        application_mock.getContainerRegistry.return_value = container_registry_mock
+        application_mock.getMaterialManager.return_value = material_manager_mock
+
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii"))
 
-        localMaterialWhiteWithInvalidVersion = self._LOCAL_MATERIAL_WHITE.copy()
-        localMaterialWhiteWithInvalidVersion["version"] = "one"
-        container_registry_mock.findContainersMetadata.return_value = [localMaterialWhiteWithInvalidVersion]
-
-        application_mock.getContainerRegistry.return_value = container_registry_mock
+        material_manager_mock.getAllMaterialGroups.return_value = self._LOCAL_MATERIAL_WHITE_INVALID_VERSION_ALL_RESULT.copy()
 
         with mock.patch.object(Application, "getInstance", new = lambda: application_mock):
             job = SendMaterialJob(device_mock)
@@ -118,15 +166,16 @@ class TestSendMaterialJob(TestCase):
 
         self.assertEqual(0, device_mock.createFormPart.call_count)
 
-    @patch("cura.Settings.CuraContainerRegistry")
-    @patch("UM.Application")
-    def test__onGetRemoteMaterials_withNoUpdate(self, application_mock, container_registry_mock, reply_mock,
-                                                device_mock):
-        application_mock.getContainerRegistry.return_value = container_registry_mock
+    @patch("UM.Application.Application.getInstance")
+    def test__onGetRemoteMaterials_withNoUpdate(self, application_mock):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
+        container_registry_mock = application_mock.getContainerRegistry.return_value
+        material_manager_mock = application_mock.getMaterialManager.return_value
 
         device_mock.createFormPart.return_value = "_xXx_"
 
-        container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE]
+        material_manager_mock.getAllMaterialGroups.return_value = self._LOCAL_MATERIAL_WHITE_ALL_RESULT.copy()
 
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii"))
@@ -138,24 +187,25 @@ class TestSendMaterialJob(TestCase):
         self.assertEqual(0, device_mock.createFormPart.call_count)
         self.assertEqual(0, device_mock.postFormWithParts.call_count)
 
-    @patch("cura.Settings.CuraContainerRegistry")
-    @patch("UM.Application")
-    def test__onGetRemoteMaterials_withUpdatedMaterial(self, application_mock, container_registry_mock, reply_mock,
-                                                       device_mock):
-        application_mock.getContainerRegistry.return_value = container_registry_mock
+    @patch("UM.Application.Application.getInstance")
+    def test__onGetRemoteMaterials_withUpdatedMaterial(self, get_instance_mock):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
+        application_mock = get_instance_mock.return_value
+        container_registry_mock = application_mock.getContainerRegistry.return_value
+        material_manager_mock = application_mock.getMaterialManager.return_value
+
+        container_registry_mock.getContainerFilePathById = lambda x: _FILES_MAP.get(x)
 
         device_mock.createFormPart.return_value = "_xXx_"
 
-        localMaterialWhiteWithHigherVersion = self._LOCAL_MATERIAL_WHITE.copy()
-        localMaterialWhiteWithHigherVersion["version"] = "2"
-        container_registry_mock.findContainersMetadata.return_value = [localMaterialWhiteWithHigherVersion]
+        material_manager_mock.getAllMaterialGroups.return_value = self._LOCAL_MATERIAL_WHITE_NEWER_ALL_RESULT.copy()
 
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii"))
 
-        with mock.patch.object(Application, "getInstance", new = lambda: application_mock):
-            job = SendMaterialJob(device_mock)
-            job._onGetRemoteMaterials(reply_mock)
+        job = SendMaterialJob(device_mock)
+        job._onGetRemoteMaterials(reply_mock)
 
         self.assertEqual(1, device_mock.createFormPart.call_count)
         self.assertEqual(1, device_mock.postFormWithParts.call_count)
@@ -164,16 +214,21 @@ class TestSendMaterialJob(TestCase):
              call.postFormWithParts(target = "materials/", parts = ["_xXx_"], on_finished = job.sendingFinished)],
             device_mock.method_calls)
 
-    @patch("cura.Settings.CuraContainerRegistry")
-    @patch("UM.Application")
-    def test__onGetRemoteMaterials_withNewMaterial(self, application_mock, container_registry_mock, reply_mock,
-                                                   device_mock):
-        application_mock.getContainerRegistry.return_value = container_registry_mock
+    @patch("UM.Application.Application.getInstance")
+    def test__onGetRemoteMaterials_withNewMaterial(self, application_mock):
+        reply_mock = MagicMock()
+        device_mock = MagicMock()
+        container_registry_mock = application_mock.getContainerRegistry.return_value
+        material_manager_mock = application_mock.getMaterialManager.return_value
+
+        container_registry_mock.getContainerFilePathById = lambda x: _FILES_MAP.get(x)
 
         device_mock.createFormPart.return_value = "_xXx_"
 
-        container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE,
-                                                                       self._LOCAL_MATERIAL_BLACK]
+        all_results = self._LOCAL_MATERIAL_WHITE_ALL_RESULT.copy()
+        for key, value in self._LOCAL_MATERIAL_BLACK_ALL_RESULT.items():
+            all_results[key] = value
+        material_manager_mock.getAllMaterialGroups.return_value = all_results
 
         reply_mock.attribute.return_value = 200
         reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_BLACK]).encode("ascii"))