From e9e8c49b2d9d0c898c88dced34b8179b8bc1f0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 14:16:45 +0100 Subject: [PATCH 01/34] Added tests for SendMaterialJob and refactored SendMaterialJob for better testability. This is part of a larger project to create tests for the UM3NetworkPrinting plugin in preparation for printing from the cloud --- plugins/UM3NetworkPrinting/src/Models.py | 87 +++++ .../UM3NetworkPrinting/src/SendMaterialJob.py | 124 ++++--- .../UM3NetworkPrinting/TestSendMaterialJob.py | 327 ++++++++++++++++++ 3 files changed, 491 insertions(+), 47 deletions(-) create mode 100644 plugins/UM3NetworkPrinting/src/Models.py create mode 100644 tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py new file mode 100644 index 0000000000..6d42b39370 --- /dev/null +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -0,0 +1,87 @@ +# Copyright (c) 2018 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. +from typing import Optional + + +class BaseModel: + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + def __eq__(self, other): + return self.__dict__ == other.__dict__ if type(self) == type(other) else False + + +## Represents an item in the cluster API response for installed materials. +class ClusterMaterial(BaseModel): + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.version = int(self.version) + self.density = float(self.density) + + guid: None # type: Optional[str] + + material: None # type: Optional[str] + + brand: None # type: Optional[str] + + version = None # type: Optional[int] + + color: None # type: Optional[str] + + density: None # type: Optional[float] + + +class LocalMaterialProperties(BaseModel): + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.density = float(self.density) + self.diameter = float(self.diameter) + self.weight = float(self.weight) + + density: None # type: Optional[float] + + diameter: None # type: Optional[float] + + weight: None # type: Optional[int] + + +class LocalMaterial(BaseModel): + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.properties = LocalMaterialProperties(**self.properties) + self.approximate_diameter = float(self.approximate_diameter) + self.version = int(self.version) + + GUID: None # type: Optional[str] + + id: None # type: Optional[str] + + type: None # type: Optional[str] + + status: None # type: Optional[str] + + base_file: None # type: Optional[str] + + setting_version: None # type: Optional[str] + + version = None # type: Optional[int] + + name: None # type: Optional[str] + + brand: None # type: Optional[str] + + material: None # type: Optional[str] + + color_name: None # type: Optional[str] + + description: None # type: Optional[str] + + adhesion_info: None # type: Optional[str] + + approximate_diameter: None # type: Optional[float] + + properties: None # type: LocalMaterialProperties + + definition: None # type: Optional[str] + + compatible: None # type: Optional[bool] diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 8491e79c29..126ed07317 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -1,99 +1,129 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import json #To understand the list of materials from the printer reply. -import os #To walk over material files. -import os.path #To filter on material files. -from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest #To listen to the reply from the printer. -from typing import Any, Dict, Set, TYPE_CHECKING -import urllib.parse #For getting material IDs from their file names. +import json # To understand the list of materials from the printer reply. +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 UM.Job import Job #The interface we're implementing. +from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest # To listen to the reply from the printer. + +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 # To strip the extensions of the material profile files. from UM.Resources import Resources -from UM.Settings.ContainerRegistry import ContainerRegistry #To find the GUIDs of materials. - -from cura.CuraApplication import CuraApplication #For the resource types. +from UM.Settings.ContainerRegistry import ContainerRegistry # To find the GUIDs of materials. +from cura.CuraApplication import CuraApplication # For the resource types. +from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial, LocalMaterial if TYPE_CHECKING: from .ClusterUM3OutputDevice import ClusterUM3OutputDevice + ## Asynchronous job to send material profiles to the printer. # # 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.device = device # type: ClusterUM3OutputDevice def run(self) -> None: - self.device.get("materials/", on_finished = self.sendMissingMaterials) + self.device.get("materials/", on_finished=self.sendMissingMaterials) def sendMissingMaterials(self, reply: QNetworkReply) -> None: - if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200: #Got an error from the HTTP request. + 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.") return - remote_materials_list = reply.readAll().data().decode("utf-8") + # Collect materials from the printer's reply try: - remote_materials_list = json.loads(remote_materials_list) + remote_materials_by_guid = self._parseReply(reply) except json.JSONDecodeError: Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") return - try: - remote_materials_by_guid = {material["guid"]: material for material in remote_materials_list} #Index by GUID. except KeyError: Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.") return - container_registry = ContainerRegistry.getInstance() - local_materials_list = filter(lambda material: ("GUID" in material and "version" in material and "id" in material), container_registry.findContainersMetadata(type = "material")) - local_materials_by_guid = {material["GUID"]: material for material in local_materials_list if material["id"] == material["base_file"]} - for material in local_materials_list: #For each GUID get the material with the highest version number. - try: - if int(material["version"]) > local_materials_by_guid[material["GUID"]]["version"]: - local_materials_by_guid[material["GUID"]] = material - except ValueError: - Logger.log("e", "Material {material_id} has invalid version number {number}.".format(material_id = material["id"], number = material["version"])) - continue + # Collect local materials + local_materials_by_guid = self._getLocalMaterials() - materials_to_send = set() #type: Set[Dict[str, Any]] - for guid, material in local_materials_by_guid.items(): - if guid not in remote_materials_by_guid: - materials_to_send.add(material["id"]) - continue - try: - if int(material["version"]) > remote_materials_by_guid[guid]["version"]: - materials_to_send.add(material["id"]) - continue - except KeyError: - Logger.log("e", "Current material storage on printer was an invalid reply (missing version).") - return + # Find out what materials are new or updated annd must be sent to the printer + materials_to_send = { + material.id + for guid, material in local_materials_by_guid.items() + if guid not in remote_materials_by_guid or + material.version > remote_materials_by_guid[guid].version + } + # Send materials to the printer + self.sendMaterialsToPrinter(materials_to_send) + + def sendMaterialsToPrinter(self, materials_to_send): for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.MaterialInstanceContainer): try: mime_type = MimeTypeDatabase.getMimeTypeForFile(file_path) except MimeTypeDatabase.MimeTypeNotFoundError: - continue #Not the sort of file we'd like to send then. + 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 parts = [] with open(file_path, "rb") as f: - parts.append(self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name = file_name), f.read())) + parts.append( + self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name=file_name), + f.read())) signature_file_path = file_path + ".sig" if os.path.exists(signature_file_path): _, signature_file_name = os.path.split(signature_file_path) with open(signature_file_path, "rb") as f: - parts.append(self.device._createFormPart("name=\"signature_file\"; filename=\"{file_name}\"".format(file_name = signature_file_name), f.read())) + parts.append(self.device._createFormPart( + "name=\"signature_file\"; filename=\"{file_name}\"".format(file_name=signature_file_name), + 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) + 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) def sendingFinished(self, reply: QNetworkReply): if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200: - Logger.log("e", "Received error code from printer when syncing material: {code}".format(code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) - Logger.log("e", reply.readAll().data().decode("utf-8")) \ No newline at end of file + Logger.log("e", "Received error code from printer when syncing material: {code}".format( + code=reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) + Logger.log("e", reply.readAll().data().decode("utf-8")) + + ## Parse the reply from the printer + # + # Parses the reply to a "/materials" request to the printer + # + # \return a dictionary of ClustMaterial 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 + @classmethod + def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: + remote_materials_list = json.loads(reply.readAll().data().decode("utf-8")) + return {material["guid"]: ClusterMaterial(**material) for material in remote_materials_list} + + ## Retrieves a list of local materials + # + # Only the new newest version of the local materials is returned + # + # \return a dictionary of LocalMaterial objects by GUID + @classmethod + def _getLocalMaterials(cls): + result = {} + for material in ContainerRegistry.getInstance().findContainersMetadata(type="material"): + try: + localMaterial = LocalMaterial(**material) + + if localMaterial.GUID not in result or localMaterial.version > result.get(localMaterial.GUID).version: + result[localMaterial.GUID] = localMaterial + except (ValueError): + Logger.log("e", "Material {material_id} has invalid version number {number}.".format( + material_id=material["id"], number=material["version"])) + + return result diff --git a/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py b/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py new file mode 100644 index 0000000000..3cdb73af22 --- /dev/null +++ b/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py @@ -0,0 +1,327 @@ +# Copyright (c) 2018 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. + +import io +import json +from typing import Any, List +from unittest import TestCase, mock +from unittest.mock import patch, call + +from PyQt5.QtCore import QByteArray + +from UM.Logger import Logger +from UM.MimeTypeDatabase import MimeType +from UM.Settings.ContainerRegistry import ContainerInterface, ContainerRegistryInterface, \ + DefinitionContainerInterface, ContainerRegistry +from plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice import ClusterUM3OutputDevice +from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial +from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob + +# All log entries written to Log.log by the class-under-test are written to this list. It is cleared before each test +# run and check afterwards +_logentries = [] + + +def new_log(*args): + _logentries.append(args) + + +class TestContainerRegistry(ContainerRegistryInterface): + def __init__(self): + self.containersMetaData = None + + def findContainers(self, *, ignore_case: bool = False, **kwargs: Any) -> List[ContainerInterface]: + raise NotImplementedError() + + def findDefinitionContainers(self, **kwargs: Any) -> List[DefinitionContainerInterface]: + raise NotImplementedError() + + @classmethod + def getApplication(cls) -> "Application": + raise NotImplementedError() + + def getEmptyInstanceContainer(self) -> "InstanceContainer": + raise NotImplementedError() + + def isReadOnly(self, container_id: str) -> bool: + raise NotImplementedError() + + def setContainersMetadata(self, value): + self.containersMetaData = value + + def findContainersMetadata(self, type): + return self.containersMetaData + + +class FakeDevice(ClusterUM3OutputDevice): + def _createFormPart(self, content_header, data, content_type=None): + return "xxx" + + +class TestSendMaterialJob(TestCase): + _LOCALMATERIAL_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', + 'GUID': 'badb0ee7-87c8-4f3f-9398-938587b67dce', 'version': '1', '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} + _LOCALMATERIAL_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', + 'GUID': '5fbb362a-41f9-4818-bb43-15ea6df34aa4', 'version': '1', 'color_code': '#000000', + 'description': 'Test PLA Black', 'adhesion_info': 'Use glue.', 'approximate_diameter': '3', + 'properties': {'density': '1.01', 'diameter': '2.85', 'weight': '750'}, + 'definition': 'fdmprinter', 'compatible': True} + + _REMOTEMATERIAL_WHITE = { + "guid": "badb0ee7-87c8-4f3f-9398-938587b67dce", + "material": "PLA", + "brand": "Generic", + "version": 1, + "color": "White", + "density": 1.00 + } + _REMOTEMATERIAL_BLACK = { + "guid": "5fbb362a-41f9-4818-bb43-15ea6df34aa4", + "material": "PLA", + "brand": "Generic", + "version": 2, + "color": "Black", + "density": 1.00 + } + + def setUp(self): + # Make sure the we start with clean (log) slate + _logentries.clear() + + def tearDown(self): + # If there are still log entries that were not checked something is wrong or we must add checks for them + self.assertEqual(len(_logentries), 0) + + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + def test_run(self, device_mock): + with mock.patch.object(Logger, 'log', new=new_log): + job = SendMaterialJob(device_mock) + job.run() + + device_mock.get.assert_called_with("materials/", on_finished=job.sendMissingMaterials) + self.assertEqual(0, len(_logentries)) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_withFailedRequest(self, reply_mock): + reply_mock.attribute.return_value = 404 + + with mock.patch.object(Logger, 'log', new=new_log): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0)]) + self._assertLogEntries([('e', "Couldn't request current material storage on printer. Not syncing materials.")], + _logentries) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') + + with mock.patch.object(Logger, 'log', new=new_log): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries( + [('e', "Request material storage on printer: I didn't understand the printer's answer.")], + _logentries) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_withMissingGuid(self, reply_mock): + reply_mock.attribute.return_value = 200 + remoteMaterialWithoutGuid = self._REMOTEMATERIAL_WHITE.copy() + del remoteMaterialWithoutGuid["guid"] + reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithoutGuid]).encode("ascii")) + + with mock.patch.object(Logger, 'log', new=new_log): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries( + [('e', "Request material storage on printer: Printer's answer was missing GUIDs.")], + _logentries) + + @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + + containerRegistry = TestContainerRegistry() + localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() + localMaterialWhiteWithInvalidVersion["version"] = "one" + containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries([('e', "Material generic_pla_white has invalid version number one.")], _logentries) + + @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_WithMultipleLocalVersionsLowFirst(self, reply_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + + containerRegistry = TestContainerRegistry() + localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() + localMaterialWhiteWithHigherVersion["version"] = "2" + containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries([], _logentries) + + @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_MaterialMissingOnPrinter(self, reply_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray( + json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + + containerRegistry = TestContainerRegistry() + containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries([], _logentries) + + @patch("builtins.open", lambda a, b: io.StringIO("")) + @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") + def test_sendMaterialsToPrinter(self, device): + with mock.patch.object(Logger, "log", new=new_log): + SendMaterialJob(device).sendMaterialsToPrinter({'generic_pla_white'}) + + self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def xtest_sendMissingMaterials(self, reply_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray( + json.dumps([self._REMOTEMATERIAL_WHITE], self._REMOTEMATERIAL_BLACK).encode("ascii")) + + containerRegistry = TestContainerRegistry() + containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + SendMaterialJob(None).sendMissingMaterials(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self._assertLogEntries([], _logentries) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendingFinished_success(self, reply_mock) -> None: + reply_mock.attribute.return_value = 200 + with mock.patch.object(Logger, 'log', new=new_log): + SendMaterialJob(None).sendingFinished(reply_mock) + + reply_mock.attribute.assert_called_once_with(0) + self.assertEqual(0, len(_logentries)) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendingFinished_failed(self, reply_mock) -> None: + reply_mock.attribute.return_value = 404 + reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') + + with mock.patch.object(Logger, 'log', new=new_log): + SendMaterialJob(None).sendingFinished(reply_mock) + + reply_mock.attribute.assert_called_with(0) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.attribute(0), call.readAll()]) + + self._assertLogEntries([ + ("e", "Received error code from printer when syncing material: 404"), + ("e", "Six sick hicks nick six slick bricks with picks and sticks.") + ], _logentries) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_parseReply(self, reply_mock): + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + + response = SendMaterialJob._parseReply(reply_mock) + + self.assertTrue(len(response) == 1) + self.assertEqual(next(iter(response.values())), ClusterMaterial(**self._REMOTEMATERIAL_WHITE)) + + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_parseReplyWithInvalidMaterial(self, reply_mock): + remoteMaterialWithInvalidVersion = self._REMOTEMATERIAL_WHITE.copy() + remoteMaterialWithInvalidVersion["version"] = "one" + reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithInvalidVersion]).encode("ascii")) + + with self.assertRaises(ValueError): + SendMaterialJob._parseReply(reply_mock) + + def test__getLocalMaterials(self): + containerRegistry = TestContainerRegistry() + containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + local_materials = SendMaterialJob(None)._getLocalMaterials() + + self.assertTrue(len(local_materials) == 2) + + def test__getLocalMaterialsWithMultipleVersions(self): + containerRegistry = TestContainerRegistry() + localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() + localMaterialWithNewerVersion["version"] = 2 + containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + local_materials = SendMaterialJob(None)._getLocalMaterials() + + self.assertTrue(len(local_materials) == 1) + self.assertTrue(list(local_materials.values())[0].version == 2) + + containerRegistry.setContainersMetadata([localMaterialWithNewerVersion, self._LOCALMATERIAL_WHITE]) + + with mock.patch.object(Logger, "log", new=new_log): + with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + local_materials = SendMaterialJob(None)._getLocalMaterials() + + self.assertTrue(len(local_materials) == 1) + self.assertTrue(list(local_materials.values())[0].version == 2) + + def _assertLogEntries(self, first, second): + """ + Inspects the two sets of log entry tuples and fails when they are not the same + :param first: The first set of tuples + :param second: The second set of tuples + """ + self.assertEqual(len(first), len(second)) + + while len(first) > 0: + e1, m1 = first[0] + e2, m2 = second[0] + self.assertEqual(e1, e2) + self.assertEqual(m1, m2) + first.pop(0) + second.pop(0) From 421af26f87892484381d398a6cbdf05e96a26165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 14:54:49 +0100 Subject: [PATCH 02/34] Extra test on test_sendMaterialsToPrinter, removed unused code --- .../UM3NetworkPrinting/TestSendMaterialJob.py | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py b/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py index 3cdb73af22..172b42cb8b 100644 --- a/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py +++ b/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py @@ -212,28 +212,15 @@ class TestSendMaterialJob(TestCase): suffixes=["xml.fdm_material"])) @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - def test_sendMaterialsToPrinter(self, device): + def test_sendMaterialsToPrinter(self, device_mock): + device_mock._createFormPart.return_value = "_xXx_" with mock.patch.object(Logger, "log", new=new_log): - SendMaterialJob(device).sendMaterialsToPrinter({'generic_pla_white'}) + job = SendMaterialJob(device_mock) + job.sendMaterialsToPrinter({'generic_pla_white'}) self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) - - @patch("PyQt5.QtNetwork.QNetworkReply") - def xtest_sendMissingMaterials(self, reply_mock): - reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray( - json.dumps([self._REMOTEMATERIAL_WHITE], self._REMOTEMATERIAL_BLACK).encode("ascii")) - - containerRegistry = TestContainerRegistry() - containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries([], _logentries) + self.assertEqual([call._createFormPart('name="file"; filename="generic_pla_white.xml.fdm_material"', ''), + call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) @patch("PyQt5.QtNetwork.QNetworkReply") def test_sendingFinished_success(self, reply_mock) -> None: From 695d45ffbed22396035f0d32f616e610712a1923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 15:54:07 +0100 Subject: [PATCH 03/34] Initialize the models with None --- plugins/UM3NetworkPrinting/src/Models.py | 48 ++++++++++++------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index 6d42b39370..89bf665377 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -18,17 +18,17 @@ class ClusterMaterial(BaseModel): self.version = int(self.version) self.density = float(self.density) - guid: None # type: Optional[str] + guid = None # type: Optional[str] - material: None # type: Optional[str] + material = None # type: Optional[str] - brand: None # type: Optional[str] + brand = None # type: Optional[str] version = None # type: Optional[int] - color: None # type: Optional[str] + color = None # type: Optional[str] - density: None # type: Optional[float] + density = None # type: Optional[float] class LocalMaterialProperties(BaseModel): @@ -38,11 +38,11 @@ class LocalMaterialProperties(BaseModel): self.diameter = float(self.diameter) self.weight = float(self.weight) - density: None # type: Optional[float] + density = None # type: Optional[float] - diameter: None # type: Optional[float] + diameter = None # type: Optional[float] - weight: None # type: Optional[int] + weight = None # type: Optional[int] class LocalMaterial(BaseModel): @@ -52,36 +52,36 @@ class LocalMaterial(BaseModel): self.approximate_diameter = float(self.approximate_diameter) self.version = int(self.version) - GUID: None # type: Optional[str] + GUID = None # type: Optional[str] - id: None # type: Optional[str] + id = None # type: Optional[str] - type: None # type: Optional[str] + type = None # type: Optional[str] - status: None # type: Optional[str] + status = None # type: Optional[str] - base_file: None # type: Optional[str] + base_file = None # type: Optional[str] - setting_version: None # type: Optional[str] + setting_version = None # type: Optional[str] version = None # type: Optional[int] - name: None # type: Optional[str] + name = None # type: Optional[str] - brand: None # type: Optional[str] + brand = None # type: Optional[str] - material: None # type: Optional[str] + material = None # type: Optional[str] - color_name: None # type: Optional[str] + color_name = None # type: Optional[str] - description: None # type: Optional[str] + description = None # type: Optional[str] - adhesion_info: None # type: Optional[str] + adhesion_info = None # type: Optional[str] - approximate_diameter: None # type: Optional[float] + approximate_diameter = None # type: Optional[float] - properties: None # type: LocalMaterialProperties + properties = None # type: LocalMaterialProperties - definition: None # type: Optional[str] + definition = None # type: Optional[str] - compatible: None # type: Optional[bool] + compatible = None # type: Optional[bool] From 0062250238030932b15dd36511c96c4bc1e6af3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 16:06:36 +0100 Subject: [PATCH 04/34] Moved the test to the plugin directory --- .../UM3NetworkPrinting/tests}/TestSendMaterialJob.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {tests/plugins/UM3NetworkPrinting => plugins/UM3NetworkPrinting/tests}/TestSendMaterialJob.py (100%) diff --git a/tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py similarity index 100% rename from tests/plugins/UM3NetworkPrinting/TestSendMaterialJob.py rename to plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py From 1000625d6947fe701a25cdff5f13c2ff045e29f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 16:07:17 +0100 Subject: [PATCH 05/34] Added spaces around all = --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 126ed07317..baea3b9a78 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -30,7 +30,7 @@ class SendMaterialJob(Job): self.device = device # type: ClusterUM3OutputDevice def run(self) -> None: - self.device.get("materials/", on_finished=self.sendMissingMaterials) + self.device.get("materials/", on_finished = self.sendMissingMaterials) def sendMissingMaterials(self, reply: QNetworkReply) -> None: if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200: # Got an error from the HTTP request. @@ -77,23 +77,23 @@ class SendMaterialJob(Job): parts = [] with open(file_path, "rb") as f: parts.append( - self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name=file_name), + self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name = file_name), f.read())) signature_file_path = file_path + ".sig" if os.path.exists(signature_file_path): _, signature_file_name = os.path.split(signature_file_path) with open(signature_file_path, "rb") as f: parts.append(self.device._createFormPart( - "name=\"signature_file\"; filename=\"{file_name}\"".format(file_name=signature_file_name), + "name=\"signature_file\"; filename=\"{file_name}\"".format(file_name = signature_file_name), 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) + 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) def sendingFinished(self, reply: QNetworkReply): if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200: Logger.log("e", "Received error code from printer when syncing material: {code}".format( - code=reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) + code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) Logger.log("e", reply.readAll().data().decode("utf-8")) ## Parse the reply from the printer @@ -116,7 +116,7 @@ class SendMaterialJob(Job): @classmethod def _getLocalMaterials(cls): result = {} - for material in ContainerRegistry.getInstance().findContainersMetadata(type="material"): + for material in ContainerRegistry.getInstance().findContainersMetadata(type = "material"): try: localMaterial = LocalMaterial(**material) @@ -124,6 +124,6 @@ class SendMaterialJob(Job): result[localMaterial.GUID] = localMaterial except (ValueError): Logger.log("e", "Material {material_id} has invalid version number {number}.".format( - material_id=material["id"], number=material["version"])) + material_id = material["id"], number = material["version"])) return result From 565f009e9b6ca4b447145b38c2aa6cb3ef8d4169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 16:34:44 +0100 Subject: [PATCH 06/34] Added comments --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index baea3b9a78..62b98bcdbd 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -1,7 +1,7 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import json # To understand the list of materials from the printer reply. +import json # To decode the list of materials from the printer reply. 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. @@ -29,9 +29,13 @@ class SendMaterialJob(Job): super().__init__() self.device = device # type: ClusterUM3OutputDevice + ## 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 + # + # \param reply The reply from the printer, a json file def sendMissingMaterials(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.") @@ -50,7 +54,7 @@ class SendMaterialJob(Job): # Collect local materials local_materials_by_guid = self._getLocalMaterials() - # Find out what materials are new or updated annd must be sent to the printer + # Find out what materials are new or updated and must be sent to the printer materials_to_send = { material.id for guid, material in local_materials_by_guid.items() @@ -61,7 +65,13 @@ class SendMaterialJob(Job): # Send materials to the printer self.sendMaterialsToPrinter(materials_to_send) - def sendMaterialsToPrinter(self, materials_to_send): + ## 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 mathed with + # filenames of the locally stored materials + # + # \param materials_to_send A set with id's of materials that must be sent + def sendMaterialsToPrinter(self, materials_to_send) -> None: for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.MaterialInstanceContainer): try: mime_type = MimeTypeDatabase.getMimeTypeForFile(file_path) @@ -90,7 +100,8 @@ class SendMaterialJob(Job): 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) - def sendingFinished(self, reply: QNetworkReply): + ## Check a reply from an upload to the printer and log an error when the call failed + def sendingFinished(self, reply: QNetworkReply) -> None: if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) != 200: Logger.log("e", "Received error code from printer when syncing material: {code}".format( code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) From 22aac7b5665e6dbcda2107be8e0831b11e8d28b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 16 Nov 2018 16:35:08 +0100 Subject: [PATCH 07/34] Renamed TestContainerRegistry to ContainerRegistryMock --- .../UM3NetworkPrinting/tests/TestSendMaterialJob.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 172b42cb8b..03d2a81e89 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -26,7 +26,7 @@ def new_log(*args): _logentries.append(args) -class TestContainerRegistry(ContainerRegistryInterface): +class ContainerRegistryMock(ContainerRegistryInterface): def __init__(self): self.containersMetaData = None @@ -156,7 +156,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - containerRegistry = TestContainerRegistry() + containerRegistry = ContainerRegistryMock() localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() localMaterialWhiteWithInvalidVersion["version"] = "one" containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) @@ -175,7 +175,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - containerRegistry = TestContainerRegistry() + containerRegistry = ContainerRegistryMock() localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() localMaterialWhiteWithHigherVersion["version"] = "2" containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) @@ -195,7 +195,7 @@ class TestSendMaterialJob(TestCase): reply_mock.readAll.return_value = QByteArray( json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - containerRegistry = TestContainerRegistry() + containerRegistry = ContainerRegistryMock() containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) with mock.patch.object(Logger, "log", new=new_log): @@ -266,7 +266,7 @@ class TestSendMaterialJob(TestCase): SendMaterialJob._parseReply(reply_mock) def test__getLocalMaterials(self): - containerRegistry = TestContainerRegistry() + containerRegistry = ContainerRegistryMock() containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) with mock.patch.object(Logger, "log", new=new_log): @@ -276,7 +276,7 @@ class TestSendMaterialJob(TestCase): self.assertTrue(len(local_materials) == 2) def test__getLocalMaterialsWithMultipleVersions(self): - containerRegistry = TestContainerRegistry() + containerRegistry = ContainerRegistryMock() localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() localMaterialWithNewerVersion["version"] = 2 containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) From 23e957e1c5c8fdbb36368f319feb57218dd7ab92 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 10:44:24 +0100 Subject: [PATCH 08/34] Some more refactoring, splitting up methods --- .../NetworkedPrinterOutputDevice.py | 3 + .../UM3NetworkPrinting/src/SendMaterialJob.py | 180 +++++++++++------- 2 files changed, 115 insertions(+), 68 deletions(-) diff --git a/cura/PrinterOutput/NetworkedPrinterOutputDevice.py b/cura/PrinterOutput/NetworkedPrinterOutputDevice.py index 35d2ce014a..9a3be936a2 100644 --- a/cura/PrinterOutput/NetworkedPrinterOutputDevice.py +++ b/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() diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 62b98bcdbd..6763901151 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/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. -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 -from PyQt5.QtNetwork import QNetworkReply, QNetworkRequest # To listen to the reply from the printer. - -from UM.Job import Job # The interface we're implementing. +from UM.Job import Job 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 # For the resource types. -from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial, LocalMaterial +from cura.CuraApplication import CuraApplication + +# 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 - def sendMissingMaterials(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.") + # \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()) 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: - Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") - return - except KeyError: - Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.") - return + self._sendMissingMaterials(remote_materials_by_guid) + except json.JSONDecodeError as e: + Logger.log("e", "Error parsing materials from printer: %s", e) + except KeyError as e: + Logger.log("e", "Error parsing materials from printer: %s", e) + + ## 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.id - for guid, material in local_materials_by_guid.items() - if guid not in remote_materials_by_guid or - material.version > remote_materials_by_guid[guid].version - } + material_ids_to_send = self._determineMaterialsToSend(local_materials_by_guid, remote_materials_by_guid) + if len(material_ids_to_send) == 0: + Logger.log("d", "There are no remote materials to update.") + return # 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 - # filenames of the locally stored materials + # Makes a Set containing only the materials that are not on the printer yet or the ones that are newer in Cura. # - # \param materials_to_send A set with id's of materials that must be sent - def sendMaterialsToPrinter(self, materials_to_send) -> None: - for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.MaterialInstanceContainer): + # \param local_materials The local materials by GUID. + # \param remote_materials The remote materials by GUID. + @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( - self.device._createFormPart("name=\"file\"; filename=\"{file_name}\"".format(file_name = file_name), - f.read())) - signature_file_path = file_path + ".sig" + parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\"" + .format(file_name = file_name), f.read())) + + # 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( - "name=\"signature_file\"; filename=\"{file_name}\"".format(file_name = signature_file_name), - f.read())) + parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\"" + .format(file_name = signature_file_name), 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( - code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute))) - Logger.log("e", reply.readAll().data().decode("utf-8")) + Logger.log("e", "Received error code from printer when syncing material: {code}, {text}".format( + code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute), + 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(cls): - result = {} - for material in ContainerRegistry.getInstance().findContainersMetadata(type = "material"): - try: - localMaterial = LocalMaterial(**material) + def _getLocalMaterials(self) -> Dict[str, LocalMaterial]: + result = {} # type: Dict[str, LocalMaterial] + container_registry = self._application.getContainerRegistry() + material_containers = container_registry.findContainersMetadata(type = "material") - if localMaterial.GUID not in result or localMaterial.version > result.get(localMaterial.GUID).version: - result[localMaterial.GUID] = localMaterial - except (ValueError): - Logger.log("e", "Material {material_id} has invalid version number {number}.".format( - material_id = material["id"], number = material["version"])) + # Find the latest version of all material containers in the registry. + for m in material_containers: + try: + material = LocalMaterial(**m) + 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 From ee9210d8d1ce895d22636c74d8bf3218f39460f2 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 10:57:47 +0100 Subject: [PATCH 09/34] Rewrite tests --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 2 +- .../tests/TestSendMaterialJob.py | 417 +++++++++--------- 2 files changed, 198 insertions(+), 221 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 6763901151..349e6929ff 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -32,7 +32,7 @@ class SendMaterialJob(Job): ## Send the request to the printer and register a callback def run(self) -> None: - self.device.get("materials/", on_finished = self.sendMissingMaterials) + self.device.get("materials/", on_finished = self._onGetRemoteMaterials) ## Process the materials reply from the printer. # diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 03d2a81e89..bc6e1def14 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,6 +1,5 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. - import io import json from typing import Any, List @@ -17,16 +16,9 @@ from plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice import ClusterUM3Outp from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob -# All log entries written to Log.log by the class-under-test are written to this list. It is cleared before each test -# run and check afterwards -_logentries = [] - - -def new_log(*args): - _logentries.append(args) - class ContainerRegistryMock(ContainerRegistryInterface): + def __init__(self): self.containersMetaData = None @@ -59,14 +51,16 @@ class FakeDevice(ClusterUM3OutputDevice): class TestSendMaterialJob(TestCase): - _LOCALMATERIAL_WHITE = {'type': 'material', 'status': 'unknown', 'id': 'generic_pla_white', + + _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', 'GUID': 'badb0ee7-87c8-4f3f-9398-938587b67dce', 'version': '1', '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} - _LOCALMATERIAL_BLACK = {'type': 'material', 'status': 'unknown', 'id': 'generic_pla_black', + + _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', 'GUID': '5fbb362a-41f9-4818-bb43-15ea6df34aa4', 'version': '1', 'color_code': '#000000', @@ -74,7 +68,7 @@ class TestSendMaterialJob(TestCase): 'properties': {'density': '1.01', 'diameter': '2.85', 'weight': '750'}, 'definition': 'fdmprinter', 'compatible': True} - _REMOTEMATERIAL_WHITE = { + _REMOTE_MATERIAL_WHITE = { "guid": "badb0ee7-87c8-4f3f-9398-938587b67dce", "material": "PLA", "brand": "Generic", @@ -82,7 +76,8 @@ class TestSendMaterialJob(TestCase): "color": "White", "density": 1.00 } - _REMOTEMATERIAL_BLACK = { + + _REMOTE_MATERIAL_BLACK = { "guid": "5fbb362a-41f9-4818-bb43-15ea6df34aa4", "material": "PLA", "brand": "Generic", @@ -91,224 +86,206 @@ class TestSendMaterialJob(TestCase): "density": 1.00 } - def setUp(self): - # Make sure the we start with clean (log) slate - _logentries.clear() - - def tearDown(self): - # If there are still log entries that were not checked something is wrong or we must add checks for them - self.assertEqual(len(_logentries), 0) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") def test_run(self, device_mock): - with mock.patch.object(Logger, 'log', new=new_log): - job = SendMaterialJob(device_mock) - job.run() - - device_mock.get.assert_called_with("materials/", on_finished=job.sendMissingMaterials) - self.assertEqual(0, len(_logentries)) + job = SendMaterialJob(device_mock) + job.run() + device_mock.get.assert_called_with("materials/", on_finished=job._onGetRemoteMaterials) + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withFailedRequest(self, reply_mock): + def test_sendMissingMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 - - with mock.patch.object(Logger, 'log', new=new_log): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - + SendMaterialJob(device_mock).run() reply_mock.attribute.assert_called_with(0) self.assertEqual(reply_mock.method_calls, [call.attribute(0)]) - self._assertLogEntries([('e', "Couldn't request current material storage on printer. Not syncing materials.")], - _logentries) + self.assertEqual(device_mock._onGetRemoteMaterials.method_calls, []) - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock): - reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') - - with mock.patch.object(Logger, 'log', new=new_log): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries( - [('e', "Request material storage on printer: I didn't understand the printer's answer.")], - _logentries) - - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withMissingGuid(self, reply_mock): - reply_mock.attribute.return_value = 200 - remoteMaterialWithoutGuid = self._REMOTEMATERIAL_WHITE.copy() - del remoteMaterialWithoutGuid["guid"] - reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithoutGuid]).encode("ascii")) - - with mock.patch.object(Logger, 'log', new=new_log): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries( - [('e', "Request material storage on printer: Printer's answer was missing GUIDs.")], - _logentries) - - @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): - reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - - containerRegistry = ContainerRegistryMock() - localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() - localMaterialWhiteWithInvalidVersion["version"] = "one" - containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries([('e', "Material generic_pla_white has invalid version number one.")], _logentries) - - @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_WithMultipleLocalVersionsLowFirst(self, reply_mock): - reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - - containerRegistry = ContainerRegistryMock() - localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() - localMaterialWhiteWithHigherVersion["version"] = "2" - containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries([], _logentries) - - @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_MaterialMissingOnPrinter(self, reply_mock): - reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray( - json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - - containerRegistry = ContainerRegistryMock() - containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - SendMaterialJob(None).sendMissingMaterials(reply_mock) - - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self._assertLogEntries([], _logentries) - - @patch("builtins.open", lambda a, b: io.StringIO("")) - @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") - def test_sendMaterialsToPrinter(self, device_mock): - device_mock._createFormPart.return_value = "_xXx_" - with mock.patch.object(Logger, "log", new=new_log): - job = SendMaterialJob(device_mock) - job.sendMaterialsToPrinter({'generic_pla_white'}) - - self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) - self.assertEqual([call._createFormPart('name="file"; filename="generic_pla_white.xml.fdm_material"', ''), - call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendingFinished_success(self, reply_mock) -> None: + def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 - with mock.patch.object(Logger, 'log', new=new_log): - SendMaterialJob(None).sendingFinished(reply_mock) - - reply_mock.attribute.assert_called_once_with(0) - self.assertEqual(0, len(_logentries)) - - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendingFinished_failed(self, reply_mock) -> None: - reply_mock.attribute.return_value = 404 reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') - - with mock.patch.object(Logger, 'log', new=new_log): - SendMaterialJob(None).sendingFinished(reply_mock) - + SendMaterialJob(device_mock).run() reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.attribute(0), call.readAll()]) + self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + self.assertEqual(device_mock._onGetRemoteMaterials.method_calls, []) - self._assertLogEntries([ - ("e", "Received error code from printer when syncing material: 404"), - ("e", "Six sick hicks nick six slick bricks with picks and sticks.") - ], _logentries) - - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_parseReply(self, reply_mock): - reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - - response = SendMaterialJob._parseReply(reply_mock) - - self.assertTrue(len(response) == 1) - self.assertEqual(next(iter(response.values())), ClusterMaterial(**self._REMOTEMATERIAL_WHITE)) - - @patch("PyQt5.QtNetwork.QNetworkReply") - def test_parseReplyWithInvalidMaterial(self, reply_mock): - remoteMaterialWithInvalidVersion = self._REMOTEMATERIAL_WHITE.copy() - remoteMaterialWithInvalidVersion["version"] = "one" - reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithInvalidVersion]).encode("ascii")) - - with self.assertRaises(ValueError): - SendMaterialJob._parseReply(reply_mock) - - def test__getLocalMaterials(self): - containerRegistry = ContainerRegistryMock() - containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - local_materials = SendMaterialJob(None)._getLocalMaterials() - - self.assertTrue(len(local_materials) == 2) - - def test__getLocalMaterialsWithMultipleVersions(self): - containerRegistry = ContainerRegistryMock() - localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() - localMaterialWithNewerVersion["version"] = 2 - containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - local_materials = SendMaterialJob(None)._getLocalMaterials() - - self.assertTrue(len(local_materials) == 1) - self.assertTrue(list(local_materials.values())[0].version == 2) - - containerRegistry.setContainersMetadata([localMaterialWithNewerVersion, self._LOCALMATERIAL_WHITE]) - - with mock.patch.object(Logger, "log", new=new_log): - with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - local_materials = SendMaterialJob(None)._getLocalMaterials() - - self.assertTrue(len(local_materials) == 1) - self.assertTrue(list(local_materials.values())[0].version == 2) - - def _assertLogEntries(self, first, second): - """ - Inspects the two sets of log entry tuples and fails when they are not the same - :param first: The first set of tuples - :param second: The second set of tuples - """ - self.assertEqual(len(first), len(second)) - - while len(first) > 0: - e1, m1 = first[0] - e2, m2 = second[0] - self.assertEqual(e1, e2) - self.assertEqual(m1, m2) - first.pop(0) - second.pop(0) + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendMissingMaterials_withMissingGuid(self, reply_mock): + # reply_mock.attribute.return_value = 200 + # remoteMaterialWithoutGuid = self._REMOTEMATERIAL_WHITE.copy() + # del remoteMaterialWithoutGuid["guid"] + # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithoutGuid]).encode("ascii")) + # + # with mock.patch.object(Logger, 'log', new=new_log): + # SendMaterialJob(None).sendMissingMaterials(reply_mock) + # + # reply_mock.attribute.assert_called_with(0) + # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + # self._assertLogEntries( + # [('e', "Request material storage on printer: Printer's answer was missing GUIDs.")], + # _logentries) + # + # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): + # reply_mock.attribute.return_value = 200 + # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + # + # containerRegistry = ContainerRegistryMock() + # localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() + # localMaterialWhiteWithInvalidVersion["version"] = "one" + # containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # SendMaterialJob(None).sendMissingMaterials(reply_mock) + # + # reply_mock.attribute.assert_called_with(0) + # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + # self._assertLogEntries([('e', "Material generic_pla_white has invalid version number one.")], _logentries) + # + # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendMissingMaterials_WithMultipleLocalVersionsLowFirst(self, reply_mock): + # reply_mock.attribute.return_value = 200 + # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + # + # containerRegistry = ContainerRegistryMock() + # localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() + # localMaterialWhiteWithHigherVersion["version"] = "2" + # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # SendMaterialJob(None).sendMissingMaterials(reply_mock) + # + # reply_mock.attribute.assert_called_with(0) + # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + # self._assertLogEntries([], _logentries) + # + # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendMissingMaterials_MaterialMissingOnPrinter(self, reply_mock): + # reply_mock.attribute.return_value = 200 + # reply_mock.readAll.return_value = QByteArray( + # json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + # + # containerRegistry = ContainerRegistryMock() + # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # SendMaterialJob(None).sendMissingMaterials(reply_mock) + # + # reply_mock.attribute.assert_called_with(0) + # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) + # self._assertLogEntries([], _logentries) + # + # @patch("builtins.open", lambda a, b: io.StringIO("")) + # @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") + # def test_sendMaterialsToPrinter(self, device_mock): + # device_mock._createFormPart.return_value = "_xXx_" + # with mock.patch.object(Logger, "log", new=new_log): + # job = SendMaterialJob(device_mock) + # job.sendMaterialsToPrinter({'generic_pla_white'}) + # + # self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) + # self.assertEqual([call._createFormPart('name="file"; filename="generic_pla_white.xml.fdm_material"', ''), + # call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) + # + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendingFinished_success(self, reply_mock) -> None: + # reply_mock.attribute.return_value = 200 + # with mock.patch.object(Logger, 'log', new=new_log): + # SendMaterialJob(None).sendingFinished(reply_mock) + # + # reply_mock.attribute.assert_called_once_with(0) + # self.assertEqual(0, len(_logentries)) + # + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_sendingFinished_failed(self, reply_mock) -> None: + # reply_mock.attribute.return_value = 404 + # reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') + # + # with mock.patch.object(Logger, 'log', new=new_log): + # SendMaterialJob(None).sendingFinished(reply_mock) + # + # reply_mock.attribute.assert_called_with(0) + # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.attribute(0), call.readAll()]) + # + # self._assertLogEntries([ + # ("e", "Received error code from printer when syncing material: 404"), + # ("e", "Six sick hicks nick six slick bricks with picks and sticks.") + # ], _logentries) + # + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_parseReply(self, reply_mock): + # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) + # + # response = SendMaterialJob._parseReply(reply_mock) + # + # self.assertTrue(len(response) == 1) + # self.assertEqual(next(iter(response.values())), ClusterMaterial(**self._REMOTEMATERIAL_WHITE)) + # + # @patch("PyQt5.QtNetwork.QNetworkReply") + # def test_parseReplyWithInvalidMaterial(self, reply_mock): + # remoteMaterialWithInvalidVersion = self._REMOTEMATERIAL_WHITE.copy() + # remoteMaterialWithInvalidVersion["version"] = "one" + # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithInvalidVersion]).encode("ascii")) + # + # with self.assertRaises(ValueError): + # SendMaterialJob._parseReply(reply_mock) + # + # def test__getLocalMaterials(self): + # containerRegistry = ContainerRegistryMock() + # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # local_materials = SendMaterialJob(None)._getLocalMaterials() + # + # self.assertTrue(len(local_materials) == 2) + # + # def test__getLocalMaterialsWithMultipleVersions(self): + # containerRegistry = ContainerRegistryMock() + # localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() + # localMaterialWithNewerVersion["version"] = 2 + # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # local_materials = SendMaterialJob(None)._getLocalMaterials() + # + # self.assertTrue(len(local_materials) == 1) + # self.assertTrue(list(local_materials.values())[0].version == 2) + # + # containerRegistry.setContainersMetadata([localMaterialWithNewerVersion, self._LOCALMATERIAL_WHITE]) + # + # with mock.patch.object(Logger, "log", new=new_log): + # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): + # local_materials = SendMaterialJob(None)._getLocalMaterials() + # + # self.assertTrue(len(local_materials) == 1) + # self.assertTrue(list(local_materials.values())[0].version == 2) + # + # def _assertLogEntries(self, first, second): + # """ + # Inspects the two sets of log entry tuples and fails when they are not the same + # :param first: The first set of tuples + # :param second: The second set of tuples + # """ + # self.assertEqual(len(first), len(second)) + # + # while len(first) > 0: + # e1, m1 = first[0] + # e2, m2 = second[0] + # self.assertEqual(e1, e2) + # self.assertEqual(m1, m2) + # first.pop(0) + # second.pop(0) From f8f133d2ef92dfabb636eac70b09e681bd4b4d63 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 13:42:28 +0100 Subject: [PATCH 10/34] Fix running tests in plugin using pytest --- .../tests/TestLegacyProfileReader.py | 9 +++++---- .../tests/TestVersionUpgrade25to26.py | 11 ++++++++--- .../tests/TestVersionUpgrade26to27.py | 9 +++++---- .../tests/TestVersionUpgrade27to30.py | 8 ++++---- .../tests/TestVersionUpgrade34to35.py | 10 ++++++---- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py b/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py index 480a61f301..3c9e46b6d8 100644 --- a/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py +++ b/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py @@ -1,8 +1,7 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. - import configparser # An input for some functions we're testing. -import os.path # To find the integration test .ini files. +import os # To find the integration test .ini files. import pytest # To register tests with. import unittest.mock # To mock the application, plug-in and container registry out. @@ -11,13 +10,15 @@ import UM.PluginRegistry # To mock the plug-in registry out. import UM.Settings.ContainerRegistry # To mock the container registry out. import UM.Settings.InstanceContainer # To intercept the serialised data from the read() function. -import LegacyProfileReader as LegacyProfileReaderModule # To get the directory of the module. -from LegacyProfileReader import LegacyProfileReader # The module we're testing. +import plugins.LegacyProfileReader.LegacyProfileReader as LegacyProfileReaderModule +from plugins.LegacyProfileReader.LegacyProfileReader import LegacyProfileReader + @pytest.fixture def legacy_profile_reader(): return LegacyProfileReader() + test_prepareDefaultsData = [ { "defaults": diff --git a/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py b/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py index 9d7c7646cc..588c0cb3db 100644 --- a/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py +++ b/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py @@ -1,16 +1,17 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import configparser +import pytest -import configparser #To check whether the appropriate exceptions are raised. -import pytest #To register tests with. +from plugins.VersionUpgrade.VersionUpgrade25to26 import VersionUpgrade25to26 -import VersionUpgrade25to26 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): return VersionUpgrade25to26.VersionUpgrade25to26() + test_cfg_version_good_data = [ { "test_name": "Simple", @@ -60,6 +61,7 @@ setting_version = -3 } ] + ## Tests the technique that gets the version number from CFG files. # # \param data The parametrised data to test with. It contains a test name @@ -116,6 +118,7 @@ version = 1.2 } ] + ## Tests whether getting a version number from bad CFG files gives an # exception. # @@ -155,6 +158,7 @@ foo = bar } ] + ## Tests whether the settings that should be removed are removed for the 2.6 # version of preferences. @pytest.mark.parametrize("data", test_upgrade_preferences_removed_settings_data) @@ -200,6 +204,7 @@ type = instance_container } ] + ## Tests whether the settings that should be removed are removed for the 2.6 # version of instance containers. @pytest.mark.parametrize("data", test_upgrade_instance_container_removed_settings_data) diff --git a/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py b/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py index eebaca23c6..45d41e7a1b 100644 --- a/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py +++ b/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py @@ -1,15 +1,16 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import configparser +import pytest -import configparser #To check whether the appropriate exceptions are raised. -import pytest #To register tests with. +from plugins.VersionUpgrade.VersionUpgrade26to27.VersionUpgrade26to27 import VersionUpgrade26to27 -import VersionUpgrade26to27 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade26to27.VersionUpgrade26to27() + return VersionUpgrade26to27() + test_cfg_version_good_data = [ { diff --git a/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py b/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py index cae08ebcfd..7b77b85993 100644 --- a/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py +++ b/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py @@ -1,15 +1,15 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import configparser +import pytest -import configparser #To parse the resulting config files. -import pytest #To register tests with. +from plugins.VersionUpgrade.VersionUpgrade27to30.VersionUpgrade27to30 import VersionUpgrade27to30 -import VersionUpgrade27to30 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade27to30.VersionUpgrade27to30() + return VersionUpgrade27to30() test_cfg_version_good_data = [ { diff --git a/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py b/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py index b74e6f35ac..4f77fcd093 100644 --- a/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py +++ b/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py @@ -1,15 +1,16 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import configparser +import pytest -import configparser #To parse the resulting config files. -import pytest #To register tests with. +from plugins.VersionUpgrade.VersionUpgrade34to35.VersionUpgrade34to35 import VersionUpgrade34to35 -import VersionUpgrade34to35 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade34to35.VersionUpgrade34to35() + return VersionUpgrade34to35() + test_upgrade_version_nr_data = [ ("Empty config file", @@ -25,6 +26,7 @@ test_upgrade_version_nr_data = [ ) ] + ## Tests whether the version numbers are updated. @pytest.mark.parametrize("test_name, file_data", test_upgrade_version_nr_data) def test_upgradeVersionNr(test_name, file_data, upgrader): From dc17bd849968d180dc061aff6ca7eaed53bfcff1 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 13:54:45 +0100 Subject: [PATCH 11/34] Fix the first tests --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 1 + .../tests/TestSendMaterialJob.py | 23 ++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 349e6929ff..572558e352 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -157,6 +157,7 @@ class SendMaterialJob(Job): @classmethod def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: remote_materials_list = json.loads(reply.readAll().data().decode("utf-8")) + print("remote_materials_list", remote_materials_list) return {material["guid"]: ClusterMaterial(**material) for material in remote_materials_list} ## Retrieves a list of local materials diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index bc6e1def14..b51d978ed2 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -90,26 +90,33 @@ class TestSendMaterialJob(TestCase): def test_run(self, device_mock): 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) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") def test_sendMissingMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 - SendMaterialJob(device_mock).run() - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0)]) - self.assertEqual(device_mock._onGetRemoteMaterials.method_calls, []) + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + # We expect the error string to be retrieved and the device not to be called for any follow up. + self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) + self.assertEqual([], device_mock.method_calls) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') - SendMaterialJob(device_mock).run() - reply_mock.attribute.assert_called_with(0) - self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - self.assertEqual(device_mock._onGetRemoteMaterials.method_calls, []) + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + # We expect the reply to be called once to try to get the printers from the list (readAll()). + # Given that the parsing there fails we do no expect the device to be called for any follow up. + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([], device_mock.method_calls) # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendMissingMaterials_withMissingGuid(self, reply_mock): From 0b1ac87354d53457dcfba0f927e749639a7600a0 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:03:43 +0100 Subject: [PATCH 12/34] Fix some formatting, cleanup import --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 9 +++-- .../tests/TestSendMaterialJob.py | 36 ++++++++----------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 572558e352..62414763b6 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -48,10 +48,10 @@ class SendMaterialJob(Job): try: remote_materials_by_guid = self._parseReply(reply) self._sendMissingMaterials(remote_materials_by_guid) - except json.JSONDecodeError as e: - Logger.log("e", "Error parsing materials from printer: %s", e) - except KeyError as e: - Logger.log("e", "Error parsing materials from printer: %s", e) + except json.JSONDecodeError: + Logger.logException("w", "Error parsing materials from printer") + except KeyError: + Logger.logException("w", "Error parsing materials from printer") ## Determine which materials should be updated and send them to the printer. # @@ -157,7 +157,6 @@ class SendMaterialJob(Job): @classmethod def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: remote_materials_list = json.loads(reply.readAll().data().decode("utf-8")) - print("remote_materials_list", remote_materials_list) return {material["guid"]: ClusterMaterial(**material) for material in remote_materials_list} ## Retrieves a list of local materials diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index b51d978ed2..0e907f58eb 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,19 +1,13 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import io -import json from typing import Any, List -from unittest import TestCase, mock +from unittest import TestCase from unittest.mock import patch, call from PyQt5.QtCore import QByteArray -from UM.Logger import Logger -from UM.MimeTypeDatabase import MimeType -from UM.Settings.ContainerRegistry import ContainerInterface, ContainerRegistryInterface, \ - DefinitionContainerInterface, ContainerRegistry +from UM.Settings.ContainerRegistry import ContainerInterface, ContainerRegistryInterface, DefinitionContainerInterface from plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice import ClusterUM3OutputDevice -from plugins.UM3NetworkPrinting.src.Models import ClusterMaterial from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob @@ -45,7 +39,7 @@ class ContainerRegistryMock(ContainerRegistryInterface): return self.containersMetaData -class FakeDevice(ClusterUM3OutputDevice): +class MockOutputDevice(ClusterUM3OutputDevice): def _createFormPart(self, content_header, data, content_type=None): return "xxx" @@ -53,20 +47,20 @@ class FakeDevice(ClusterUM3OutputDevice): class TestSendMaterialJob(TestCase): _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', - 'GUID': 'badb0ee7-87c8-4f3f-9398-938587b67dce', 'version': '1', '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} + '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': '1', '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_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', - 'GUID': '5fbb362a-41f9-4818-bb43-15ea6df34aa4', 'version': '1', 'color_code': '#000000', - 'description': 'Test PLA Black', 'adhesion_info': 'Use glue.', 'approximate_diameter': '3', - 'properties': {'density': '1.01', 'diameter': '2.85', 'weight': '750'}, - 'definition': 'fdmprinter', 'compatible': True} + 'base_file': 'generic_pla_black', 'setting_version': 5, 'name': 'Yellow CPE', + 'brand': 'Ultimaker', 'material': 'CPE', 'color_name': 'Black', + 'GUID': '5fbb362a-41f9-4818-bb43-15ea6df34aa4', 'version': '1', 'color_code': '#000000', + 'description': 'Test PLA Black', 'adhesion_info': 'Use glue.', 'approximate_diameter': '3', + 'properties': {'density': '1.01', 'diameter': '2.85', 'weight': '750'}, + 'definition': 'fdmprinter', 'compatible': True} _REMOTE_MATERIAL_WHITE = { "guid": "badb0ee7-87c8-4f3f-9398-938587b67dce", From d65114bd5652398bf9b3fae91dc38b53fae0e35f Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:08:58 +0100 Subject: [PATCH 13/34] use call_count to assert device was not called --- .../tests/TestSendMaterialJob.py | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 0e907f58eb..9b1e1066ba 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -97,7 +97,7 @@ class TestSendMaterialJob(TestCase): # We expect the error string to be retrieved and the device not to be called for any follow up. self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) - self.assertEqual([], device_mock.method_calls) + self.assertEqual(0, device_mock.call_count) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") @@ -110,7 +110,7 @@ class TestSendMaterialJob(TestCase): # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that the parsing there fails we do no expect the device to be called for any follow up. self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual([], device_mock.method_calls) + self.assertEqual(0, device_mock.call_count) # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendMissingMaterials_withMissingGuid(self, reply_mock): @@ -274,19 +274,3 @@ class TestSendMaterialJob(TestCase): # # self.assertTrue(len(local_materials) == 1) # self.assertTrue(list(local_materials.values())[0].version == 2) - # - # def _assertLogEntries(self, first, second): - # """ - # Inspects the two sets of log entry tuples and fails when they are not the same - # :param first: The first set of tuples - # :param second: The second set of tuples - # """ - # self.assertEqual(len(first), len(second)) - # - # while len(first) > 0: - # e1, m1 = first[0] - # e2, m2 = second[0] - # self.assertEqual(e1, e2) - # self.assertEqual(m1, m2) - # first.pop(0) - # second.pop(0) From 9d8583a3b67682442d0ab4383bdf748770105af2 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:10:35 +0100 Subject: [PATCH 14/34] Revert "Fix running tests in plugin using pytest" This reverts commit f8f133d2ef92dfabb636eac70b09e681bd4b4d63. --- .../tests/TestLegacyProfileReader.py | 9 ++++----- .../tests/TestVersionUpgrade25to26.py | 11 +++-------- .../tests/TestVersionUpgrade26to27.py | 9 ++++----- .../tests/TestVersionUpgrade27to30.py | 8 ++++---- .../tests/TestVersionUpgrade34to35.py | 10 ++++------ 5 files changed, 19 insertions(+), 28 deletions(-) diff --git a/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py b/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py index 3c9e46b6d8..480a61f301 100644 --- a/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py +++ b/plugins/LegacyProfileReader/tests/TestLegacyProfileReader.py @@ -1,7 +1,8 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. + import configparser # An input for some functions we're testing. -import os # To find the integration test .ini files. +import os.path # To find the integration test .ini files. import pytest # To register tests with. import unittest.mock # To mock the application, plug-in and container registry out. @@ -10,15 +11,13 @@ import UM.PluginRegistry # To mock the plug-in registry out. import UM.Settings.ContainerRegistry # To mock the container registry out. import UM.Settings.InstanceContainer # To intercept the serialised data from the read() function. -import plugins.LegacyProfileReader.LegacyProfileReader as LegacyProfileReaderModule -from plugins.LegacyProfileReader.LegacyProfileReader import LegacyProfileReader - +import LegacyProfileReader as LegacyProfileReaderModule # To get the directory of the module. +from LegacyProfileReader import LegacyProfileReader # The module we're testing. @pytest.fixture def legacy_profile_reader(): return LegacyProfileReader() - test_prepareDefaultsData = [ { "defaults": diff --git a/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py b/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py index 588c0cb3db..9d7c7646cc 100644 --- a/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py +++ b/plugins/VersionUpgrade/VersionUpgrade25to26/tests/TestVersionUpgrade25to26.py @@ -1,17 +1,16 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import configparser -import pytest -from plugins.VersionUpgrade.VersionUpgrade25to26 import VersionUpgrade25to26 +import configparser #To check whether the appropriate exceptions are raised. +import pytest #To register tests with. +import VersionUpgrade25to26 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): return VersionUpgrade25to26.VersionUpgrade25to26() - test_cfg_version_good_data = [ { "test_name": "Simple", @@ -61,7 +60,6 @@ setting_version = -3 } ] - ## Tests the technique that gets the version number from CFG files. # # \param data The parametrised data to test with. It contains a test name @@ -118,7 +116,6 @@ version = 1.2 } ] - ## Tests whether getting a version number from bad CFG files gives an # exception. # @@ -158,7 +155,6 @@ foo = bar } ] - ## Tests whether the settings that should be removed are removed for the 2.6 # version of preferences. @pytest.mark.parametrize("data", test_upgrade_preferences_removed_settings_data) @@ -204,7 +200,6 @@ type = instance_container } ] - ## Tests whether the settings that should be removed are removed for the 2.6 # version of instance containers. @pytest.mark.parametrize("data", test_upgrade_instance_container_removed_settings_data) diff --git a/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py b/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py index 45d41e7a1b..eebaca23c6 100644 --- a/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py +++ b/plugins/VersionUpgrade/VersionUpgrade26to27/tests/TestVersionUpgrade26to27.py @@ -1,16 +1,15 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import configparser -import pytest -from plugins.VersionUpgrade.VersionUpgrade26to27.VersionUpgrade26to27 import VersionUpgrade26to27 +import configparser #To check whether the appropriate exceptions are raised. +import pytest #To register tests with. +import VersionUpgrade26to27 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade26to27() - + return VersionUpgrade26to27.VersionUpgrade26to27() test_cfg_version_good_data = [ { diff --git a/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py b/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py index 7b77b85993..cae08ebcfd 100644 --- a/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py +++ b/plugins/VersionUpgrade/VersionUpgrade27to30/tests/TestVersionUpgrade27to30.py @@ -1,15 +1,15 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import configparser -import pytest -from plugins.VersionUpgrade.VersionUpgrade27to30.VersionUpgrade27to30 import VersionUpgrade27to30 +import configparser #To parse the resulting config files. +import pytest #To register tests with. +import VersionUpgrade27to30 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade27to30() + return VersionUpgrade27to30.VersionUpgrade27to30() test_cfg_version_good_data = [ { diff --git a/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py b/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py index 4f77fcd093..b74e6f35ac 100644 --- a/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py +++ b/plugins/VersionUpgrade/VersionUpgrade34to35/tests/TestVersionUpgrade34to35.py @@ -1,16 +1,15 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -import configparser -import pytest -from plugins.VersionUpgrade.VersionUpgrade34to35.VersionUpgrade34to35 import VersionUpgrade34to35 +import configparser #To parse the resulting config files. +import pytest #To register tests with. +import VersionUpgrade34to35 #The module we're testing. ## Creates an instance of the upgrader to test with. @pytest.fixture def upgrader(): - return VersionUpgrade34to35() - + return VersionUpgrade34to35.VersionUpgrade34to35() test_upgrade_version_nr_data = [ ("Empty config file", @@ -26,7 +25,6 @@ test_upgrade_version_nr_data = [ ) ] - ## Tests whether the version numbers are updated. @pytest.mark.parametrize("test_name, file_data", test_upgrade_version_nr_data) def test_upgradeVersionNr(test_name, file_data, upgrader): From 66fbadf2dea2a734a4055b8f866b5397303b2069 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:37:56 +0100 Subject: [PATCH 15/34] Convert all single quotes to double quotes --- .../tests/TestSendMaterialJob.py | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 9b1e1066ba..22e96f5ed0 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -46,21 +46,21 @@ class MockOutputDevice(ClusterUM3OutputDevice): class TestSendMaterialJob(TestCase): - _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', - 'GUID': 'badb0ee7-87c8-4f3f-9398-938587b67dce', 'version': '1', '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 = {"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": "1", "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_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', - 'GUID': '5fbb362a-41f9-4818-bb43-15ea6df34aa4', 'version': '1', 'color_code': '#000000', - 'description': 'Test PLA Black', 'adhesion_info': 'Use glue.', 'approximate_diameter': '3', - 'properties': {'density': '1.01', 'diameter': '2.85', 'weight': '750'}, - 'definition': 'fdmprinter', 'compatible': True} + _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", + "GUID": "5fbb362a-41f9-4818-bb43-15ea6df34aa4", "version": "1", "color_code": "#000000", + "description": "Test PLA Black", "adhesion_info": "Use glue.", "approximate_diameter": "3", + "properties": {"density": "1.01", "diameter": "2.85", "weight": "750"}, + "definition": "fdmprinter", "compatible": True} _REMOTE_MATERIAL_WHITE = { "guid": "badb0ee7-87c8-4f3f-9398-938587b67dce", @@ -103,7 +103,7 @@ class TestSendMaterialJob(TestCase): @patch("PyQt5.QtNetwork.QNetworkReply") def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 - reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') + reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -119,13 +119,13 @@ class TestSendMaterialJob(TestCase): # del remoteMaterialWithoutGuid["guid"] # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithoutGuid]).encode("ascii")) # - # with mock.patch.object(Logger, 'log', new=new_log): + # with mock.patch.object(Logger, "log", new=new_log): # SendMaterialJob(None).sendMissingMaterials(reply_mock) # # reply_mock.attribute.assert_called_with(0) # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) # self._assertLogEntries( - # [('e', "Request material storage on printer: Printer's answer was missing GUIDs.")], + # [("e", "Request material storage on printer: Printer"s answer was missing GUIDs.")], # _logentries) # # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) @@ -145,7 +145,7 @@ class TestSendMaterialJob(TestCase): # # reply_mock.attribute.assert_called_with(0) # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([('e', "Material generic_pla_white has invalid version number one.")], _logentries) + # self._assertLogEntries([("e", "Material generic_pla_white has invalid version number one.")], _logentries) # # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) # @patch("PyQt5.QtNetwork.QNetworkReply") @@ -194,16 +194,16 @@ class TestSendMaterialJob(TestCase): # device_mock._createFormPart.return_value = "_xXx_" # with mock.patch.object(Logger, "log", new=new_log): # job = SendMaterialJob(device_mock) - # job.sendMaterialsToPrinter({'generic_pla_white'}) + # job.sendMaterialsToPrinter({"generic_pla_white"}) # # self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) - # self.assertEqual([call._createFormPart('name="file"; filename="generic_pla_white.xml.fdm_material"', ''), + # self.assertEqual([call._createFormPart("name="file"; filename="generic_pla_white.xml.fdm_material"", ""), # call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) # # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendingFinished_success(self, reply_mock) -> None: # reply_mock.attribute.return_value = 200 - # with mock.patch.object(Logger, 'log', new=new_log): + # with mock.patch.object(Logger, "log", new=new_log): # SendMaterialJob(None).sendingFinished(reply_mock) # # reply_mock.attribute.assert_called_once_with(0) @@ -212,9 +212,9 @@ class TestSendMaterialJob(TestCase): # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendingFinished_failed(self, reply_mock) -> None: # reply_mock.attribute.return_value = 404 - # reply_mock.readAll.return_value = QByteArray(b'Six sick hicks nick six slick bricks with picks and sticks.') + # reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") # - # with mock.patch.object(Logger, 'log', new=new_log): + # with mock.patch.object(Logger, "log", new=new_log): # SendMaterialJob(None).sendingFinished(reply_mock) # # reply_mock.attribute.assert_called_with(0) From 60dd1303936a09c83a562067c9a8193c2f3d5e43 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:39:12 +0100 Subject: [PATCH 16/34] Use logException where possible --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 62414763b6..6260752f3f 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -175,10 +175,7 @@ class SendMaterialJob(Job): material = LocalMaterial(**m) 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 - )) + except ValueError: + Logger.logException("w", "Local material {} has invalid values.".format(m["id"])) return result From c04ce7fce8df24298c1708a044ebfa4afee8a411 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 15:44:07 +0100 Subject: [PATCH 17/34] Use call_count on specific method to be more precise --- plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 22e96f5ed0..a71ded75b6 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -97,7 +97,7 @@ class TestSendMaterialJob(TestCase): # We expect the error string to be retrieved and the device not to be called for any follow up. self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) - self.assertEqual(0, device_mock.call_count) + self.assertEqual(0, device_mock.createFormPart.call_count) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") @@ -110,7 +110,7 @@ class TestSendMaterialJob(TestCase): # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that the parsing there fails we do no expect the device to be called for any follow up. self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual(0, device_mock.call_count) + self.assertEqual(0, device_mock.createFormPart.call_count) # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendMissingMaterials_withMissingGuid(self, reply_mock): From 2497325d606a9246bb383fa87e2dc1707f99e321 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 19 Nov 2018 16:35:19 +0100 Subject: [PATCH 18/34] Test with named tuples, not working yet --- plugins/UM3NetworkPrinting/src/Models.py | 111 +++++------------- .../UM3NetworkPrinting/src/SendMaterialJob.py | 14 +-- .../tests/TestSendMaterialJob.py | 35 +++--- 3 files changed, 53 insertions(+), 107 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index 89bf665377..e8efa577f6 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,87 +1,32 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import Optional +from collections import namedtuple +ClusterMaterial = namedtuple('ClusterMaterial', [ + 'guid', + 'material', + 'brand', + 'version', + 'color', + 'density' +]) -class BaseModel: - def __init__(self, **kwargs): - self.__dict__.update(kwargs) - - def __eq__(self, other): - return self.__dict__ == other.__dict__ if type(self) == type(other) else False - - -## Represents an item in the cluster API response for installed materials. -class ClusterMaterial(BaseModel): - def __init__(self, **kwargs): - super().__init__(**kwargs) - self.version = int(self.version) - self.density = float(self.density) - - guid = None # type: Optional[str] - - material = None # type: Optional[str] - - brand = None # type: Optional[str] - - version = None # type: Optional[int] - - color = None # type: Optional[str] - - density = None # type: Optional[float] - - -class LocalMaterialProperties(BaseModel): - def __init__(self, **kwargs): - super().__init__(**kwargs) - self.density = float(self.density) - self.diameter = float(self.diameter) - self.weight = float(self.weight) - - density = None # type: Optional[float] - - diameter = None # type: Optional[float] - - weight = None # type: Optional[int] - - -class LocalMaterial(BaseModel): - def __init__(self, **kwargs): - super().__init__(**kwargs) - self.properties = LocalMaterialProperties(**self.properties) - self.approximate_diameter = float(self.approximate_diameter) - self.version = int(self.version) - - GUID = None # type: Optional[str] - - id = None # type: Optional[str] - - type = None # type: Optional[str] - - status = None # type: Optional[str] - - base_file = None # type: Optional[str] - - setting_version = None # type: Optional[str] - - version = None # type: Optional[int] - - name = None # type: Optional[str] - - brand = None # type: Optional[str] - - material = None # type: Optional[str] - - color_name = None # type: Optional[str] - - description = None # type: Optional[str] - - adhesion_info = None # type: Optional[str] - - approximate_diameter = None # type: Optional[float] - - properties = None # type: LocalMaterialProperties - - definition = None # type: Optional[str] - - compatible = None # type: Optional[bool] +LocalMaterial = namedtuple('LocalMaterial', [ + 'GUID', + 'id', + 'type', + 'status', + 'base_file', + 'setting_version', + 'version', + 'name', + 'brand', + 'material', + 'color_name', + 'description', + 'adhesion_info', + 'approximate_diameter', + 'properties', + 'definition', + 'compatible' +]) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 6260752f3f..cbe79aef6a 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -156,8 +156,8 @@ class SendMaterialJob(Job): # \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")) - return {material["guid"]: ClusterMaterial(**material) for material in remote_materials_list} + remote_materials = json.loads(reply.readAll().data().decode("utf-8")) + return {material["id"]: ClusterMaterial(**material) for material in remote_materials} ## Retrieves a list of local materials # @@ -170,12 +170,12 @@ class SendMaterialJob(Job): material_containers = container_registry.findContainersMetadata(type = "material") # Find the latest version of all material containers in the registry. - for m in material_containers: + local_materials = {} # type: Dict[str, LocalMaterial] + for material in material_containers: try: - material = LocalMaterial(**m) + material = LocalMaterial(**material) if material.GUID not in result or material.version > result.get(material.GUID).version: - result[material.GUID] = material + local_materials[material.GUID] = material except ValueError: - Logger.logException("w", "Local material {} has invalid values.".format(m["id"])) - + Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) return result diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index a71ded75b6..73bca2b0ad 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,5 +1,7 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import json + from typing import Any, List from unittest import TestCase from unittest.mock import patch, call @@ -108,26 +110,25 @@ class TestSendMaterialJob(TestCase): job._onGetRemoteMaterials(reply_mock) # We expect the reply to be called once to try to get the printers from the list (readAll()). - # Given that the parsing there fails we do no expect the device to be called for any follow up. + # Given that the parsing fails we do no expect the device to be called for any follow up. self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_withMissingGuid(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # remoteMaterialWithoutGuid = self._REMOTEMATERIAL_WHITE.copy() - # del remoteMaterialWithoutGuid["guid"] - # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithoutGuid]).encode("ascii")) - # - # with mock.patch.object(Logger, "log", new=new_log): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries( - # [("e", "Request material storage on printer: Printer"s answer was missing GUIDs.")], - # _logentries) - # + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test_sendMissingMaterials_withMissingGuid(self, reply_mock, device_mock): + reply_mock.attribute.return_value = 200 + remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy() + del remote_material_without_guid["guid"] + reply_mock.readAll.return_value = QByteArray(json.dumps([remote_material_without_guid]).encode("ascii")) + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + # We expect the reply to be called once to try to get the printers from the list (readAll()). + # Given that parsing fails we do not expect the device to be called for any follow up. + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual(1, device_mock.createFormPart.call_count) + # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) # @patch("PyQt5.QtNetwork.QNetworkReply") # def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): From 481ca8cd2f2e61ba7591695c7b318346cad4364d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:33:52 +0100 Subject: [PATCH 19/34] Fixed some bugs and added the color_code field to the named tuple --- plugins/UM3NetworkPrinting/src/Models.py | 1 + .../UM3NetworkPrinting/src/SendMaterialJob.py | 20 +++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index e8efa577f6..a9210ac5b4 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -23,6 +23,7 @@ LocalMaterial = namedtuple('LocalMaterial', [ 'brand', 'material', 'color_name', + 'color_code', 'description', 'adhesion_info', 'approximate_diameter', diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index cbe79aef6a..0599101379 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -2,6 +2,7 @@ # Cura is released under the terms of the LGPLv3 or higher. import json import os +import re import urllib.parse from typing import Dict, TYPE_CHECKING, Set @@ -19,7 +20,6 @@ from .Models import ClusterMaterial, LocalMaterial if TYPE_CHECKING: from .ClusterUM3OutputDevice import ClusterUM3OutputDevice - ## Asynchronous job to send material profiles to the printer. # # This way it won't freeze up the interface while sending those materials. @@ -50,7 +50,7 @@ class SendMaterialJob(Job): self._sendMissingMaterials(remote_materials_by_guid) except json.JSONDecodeError: Logger.logException("w", "Error parsing materials from printer") - except KeyError: + except TypeError: Logger.logException("w", "Error parsing materials from printer") ## Determine which materials should be updated and send them to the printer. @@ -75,7 +75,8 @@ class SendMaterialJob(Job): ## From the local and remote materials, determine which ones should be synchronized. # - # Makes a Set containing only the materials that are not on the printer yet or the ones that are newer in Cura. + # Makes a Set of id's containing only the id's of the materials that are not on the printer yet or the ones that + # are newer in Cura. # # \param local_materials The local materials by GUID. # \param remote_materials The remote materials by GUID. @@ -157,7 +158,7 @@ class SendMaterialJob(Job): @classmethod def _parseReply(cls, reply: QNetworkReply) -> Dict[str, ClusterMaterial]: remote_materials = json.loads(reply.readAll().data().decode("utf-8")) - return {material["id"]: ClusterMaterial(**material) for material in remote_materials} + return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} ## Retrieves a list of local materials # @@ -170,12 +171,19 @@ class SendMaterialJob(Job): material_containers = container_registry.findContainersMetadata(type = "material") # Find the latest version of all material containers in the registry. - local_materials = {} # type: Dict[str, LocalMaterial] for material in material_containers: try: material = LocalMaterial(**material) + + # material version must be an int + if not re.match("\d+", material.version): + Logger.logException("w", "Local material {} has invalid version '{}'." + .format(material["id"], material.version)) + continue + if material.GUID not in result or material.version > result.get(material.GUID).version: - local_materials[material.GUID] = material + result[material.GUID] = material except ValueError: Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) + return result From ca6074429290b6ae5fa1f3ccfbfa74107ed1284a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:34:11 +0100 Subject: [PATCH 20/34] Made the tests work with the named tuples Tests only use the _onGetRemoteMaterial --- .../tests/TestSendMaterialJob.py | 313 +++++++----------- 1 file changed, 123 insertions(+), 190 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 73bca2b0ad..f4604580fe 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,53 +1,23 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +import io import json - -from typing import Any, List -from unittest import TestCase +from unittest import TestCase, mock from unittest.mock import patch, call from PyQt5.QtCore import QByteArray -from UM.Settings.ContainerRegistry import ContainerInterface, ContainerRegistryInterface, DefinitionContainerInterface -from plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice import ClusterUM3OutputDevice +from UM.MimeTypeDatabase import MimeType +from cura.CuraApplication import CuraApplication from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob -class ContainerRegistryMock(ContainerRegistryInterface): - - def __init__(self): - self.containersMetaData = None - - def findContainers(self, *, ignore_case: bool = False, **kwargs: Any) -> List[ContainerInterface]: - raise NotImplementedError() - - def findDefinitionContainers(self, **kwargs: Any) -> List[DefinitionContainerInterface]: - raise NotImplementedError() - - @classmethod - def getApplication(cls) -> "Application": - raise NotImplementedError() - - def getEmptyInstanceContainer(self) -> "InstanceContainer": - raise NotImplementedError() - - def isReadOnly(self, container_id: str) -> bool: - raise NotImplementedError() - - def setContainersMetadata(self, value): - self.containersMetaData = value - - def findContainersMetadata(self, type): - return self.containersMetaData - - -class MockOutputDevice(ClusterUM3OutputDevice): - def _createFormPart(self, content_header, data, content_type=None): - return "xxx" - - +@patch("builtins.open", lambda _, __: io.StringIO("")) +@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"]) class TestSendMaterialJob(TestCase): - _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", @@ -88,11 +58,11 @@ class TestSendMaterialJob(TestCase): 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) + device_mock.get.assert_called_with("materials/", on_finished = job._onGetRemoteMaterials) @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withFailedRequest(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -103,7 +73,7 @@ class TestSendMaterialJob(TestCase): @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withBadJsonAnswer(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): 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) @@ -116,7 +86,7 @@ class TestSendMaterialJob(TestCase): @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") @patch("PyQt5.QtNetwork.QNetworkReply") - def test_sendMissingMaterials_withMissingGuid(self, reply_mock, device_mock): + def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy() del remote_material_without_guid["guid"] @@ -127,151 +97,114 @@ class TestSendMaterialJob(TestCase): # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that parsing fails we do not expect the device to be called for any follow up. self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(0, device_mock.createFormPart.call_count) - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_WithInvalidVersionInLocalMaterial(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # localMaterialWhiteWithInvalidVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWhiteWithInvalidVersion["version"] = "one" - # containerRegistry.setContainersMetadata([localMaterialWhiteWithInvalidVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([("e", "Material generic_pla_white has invalid version number one.")], _logentries) - # - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_WithMultipleLocalVersionsLowFirst(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # localMaterialWhiteWithHigherVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWhiteWithHigherVersion["version"] = "2" - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWhiteWithHigherVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([], _logentries) - # - # @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: []) - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendMissingMaterials_MaterialMissingOnPrinter(self, reply_mock): - # reply_mock.attribute.return_value = 200 - # reply_mock.readAll.return_value = QByteArray( - # json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # containerRegistry = ContainerRegistryMock() - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # SendMaterialJob(None).sendMissingMaterials(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.readAll()]) - # self._assertLogEntries([], _logentries) - # - # @patch("builtins.open", lambda a, b: io.StringIO("")) - # @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") - # def test_sendMaterialsToPrinter(self, device_mock): - # device_mock._createFormPart.return_value = "_xXx_" - # with mock.patch.object(Logger, "log", new=new_log): - # job = SendMaterialJob(device_mock) - # job.sendMaterialsToPrinter({"generic_pla_white"}) - # - # self._assertLogEntries([("d", "Syncing material generic_pla_white with cluster.")], _logentries) - # self.assertEqual([call._createFormPart("name="file"; filename="generic_pla_white.xml.fdm_material"", ""), - # call.postFormWithParts(on_finished=job.sendingFinished, parts = ["_xXx_"], target = "materials/")], device_mock.method_calls) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendingFinished_success(self, reply_mock) -> None: - # reply_mock.attribute.return_value = 200 - # with mock.patch.object(Logger, "log", new=new_log): - # SendMaterialJob(None).sendingFinished(reply_mock) - # - # reply_mock.attribute.assert_called_once_with(0) - # self.assertEqual(0, len(_logentries)) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_sendingFinished_failed(self, reply_mock) -> None: - # reply_mock.attribute.return_value = 404 - # reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") - # - # with mock.patch.object(Logger, "log", new=new_log): - # SendMaterialJob(None).sendingFinished(reply_mock) - # - # reply_mock.attribute.assert_called_with(0) - # self.assertEqual(reply_mock.method_calls, [call.attribute(0), call.attribute(0), call.readAll()]) - # - # self._assertLogEntries([ - # ("e", "Received error code from printer when syncing material: 404"), - # ("e", "Six sick hicks nick six slick bricks with picks and sticks.") - # ], _logentries) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_parseReply(self, reply_mock): - # reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTEMATERIAL_WHITE]).encode("ascii")) - # - # response = SendMaterialJob._parseReply(reply_mock) - # - # self.assertTrue(len(response) == 1) - # self.assertEqual(next(iter(response.values())), ClusterMaterial(**self._REMOTEMATERIAL_WHITE)) - # - # @patch("PyQt5.QtNetwork.QNetworkReply") - # def test_parseReplyWithInvalidMaterial(self, reply_mock): - # remoteMaterialWithInvalidVersion = self._REMOTEMATERIAL_WHITE.copy() - # remoteMaterialWithInvalidVersion["version"] = "one" - # reply_mock.readAll.return_value = QByteArray(json.dumps([remoteMaterialWithInvalidVersion]).encode("ascii")) - # - # with self.assertRaises(ValueError): - # SendMaterialJob._parseReply(reply_mock) - # - # def test__getLocalMaterials(self): - # containerRegistry = ContainerRegistryMock() - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, self._LOCALMATERIAL_BLACK]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 2) - # - # def test__getLocalMaterialsWithMultipleVersions(self): - # containerRegistry = ContainerRegistryMock() - # localMaterialWithNewerVersion = self._LOCALMATERIAL_WHITE.copy() - # localMaterialWithNewerVersion["version"] = 2 - # containerRegistry.setContainersMetadata([self._LOCALMATERIAL_WHITE, localMaterialWithNewerVersion]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 1) - # self.assertTrue(list(local_materials.values())[0].version == 2) - # - # containerRegistry.setContainersMetadata([localMaterialWithNewerVersion, self._LOCALMATERIAL_WHITE]) - # - # with mock.patch.object(Logger, "log", new=new_log): - # with mock.patch.object(ContainerRegistry, "getInstance", lambda: containerRegistry): - # local_materials = SendMaterialJob(None)._getLocalMaterials() - # - # self.assertTrue(len(local_materials) == 1) - # self.assertTrue(list(local_materials.values())[0].version == 2) + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, reply_mock, device_mock, application_mock, + container_registry_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 + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withNoUpdate(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + self.assertEqual(0, device_mock.postFormWithParts.call_count) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withUpdatedMaterial(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + localMaterialWhiteWithHigherVersion = self._LOCAL_MATERIAL_WHITE.copy() + localMaterialWhiteWithHigherVersion["version"] = "2" + container_registry_mock.findContainersMetadata.return_value = [localMaterialWhiteWithHigherVersion] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(1, device_mock.postFormWithParts.call_count) + self.assertEquals( + [call.createFormPart("name=\"file\"; filename=\"generic_pla_white.xml.fdm_material\"", ""), + call.postFormWithParts(target = "materials/", parts = ["_xXx_"], on_finished = job.sendingFinished)], + device_mock.method_calls) + + @patch("cura.Settings.CuraContainerRegistry") + @patch("cura.CuraApplication") + @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") + @patch("PyQt5.QtNetwork.QNetworkReply") + def test__onGetRemoteMaterials_withNewMaterial(self, reply_mock, device_mock, application_mock, + container_registry_mock): + application_mock.getContainerRegistry.return_value = container_registry_mock + + device_mock.createFormPart.return_value = "_xXx_" + + container_registry_mock.findContainersMetadata.return_value = [self._LOCAL_MATERIAL_WHITE, + self._LOCAL_MATERIAL_BLACK] + + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_BLACK]).encode("ascii")) + + with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) + self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) + self.assertEqual(1, device_mock.createFormPart.call_count) + self.assertEqual(1, device_mock.postFormWithParts.call_count) + self.assertEquals( + [call.createFormPart("name=\"file\"; filename=\"generic_pla_white.xml.fdm_material\"", ""), + call.postFormWithParts(target = "materials/", parts = ["_xXx_"], on_finished = job.sendingFinished)], + device_mock.method_calls) From f3338aa187bfe607664d0d79d5848d7cdee5fc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Tue, 20 Nov 2018 16:53:01 +0100 Subject: [PATCH 21/34] Fixed the failing tests --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 0599101379..fe0cd98964 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -84,8 +84,9 @@ class SendMaterialJob(Job): 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 + material.id + for guid, material in local_materials.items() + if guid not in remote_materials or int(material.version) > remote_materials[guid].version } ## Send the materials to the printer. From 9e8be286af8736a97bf05db3b4bb5b0063be944b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Wed, 21 Nov 2018 10:12:53 +0100 Subject: [PATCH 22/34] Used NamedTuple from typing iso namedtuple from collections so we can at least give type hints --- plugins/UM3NetworkPrinting/src/Models.py | 54 +++++++++---------- .../UM3NetworkPrinting/src/SendMaterialJob.py | 51 +++++++++--------- .../tests/TestSendMaterialJob.py | 4 +- 3 files changed, 55 insertions(+), 54 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index a9210ac5b4..e84a39db5a 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,33 +1,33 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from collections import namedtuple +from typing import NamedTuple -ClusterMaterial = namedtuple('ClusterMaterial', [ - 'guid', - 'material', - 'brand', - 'version', - 'color', - 'density' +ClusterMaterial = NamedTuple("ClusterMaterial", [ + ("guid", str), + ("material", str), + ("brand", str), + ("version", int), + ("color", str), + ("density", str), ]) -LocalMaterial = namedtuple('LocalMaterial', [ - 'GUID', - 'id', - 'type', - 'status', - 'base_file', - 'setting_version', - 'version', - 'name', - 'brand', - 'material', - 'color_name', - 'color_code', - 'description', - 'adhesion_info', - 'approximate_diameter', - 'properties', - 'definition', - 'compatible' +LocalMaterial = NamedTuple("LocalMaterial", [ + ("GUID", str), + ("id", str), + ("type", str), + ("status", str), + ("base_file", str), + ("setting_version", int), + ("version", int), + ("name", str), + ("brand", str), + ("material", str), + ("color_name", str), + ("color_code", str), + ("description", str), + ("adhesion_info", str), + ("approximate_diameter", str), + ("properties", str), + ("definition", str), + ("compatible", str), ]) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index fe0cd98964..ee8dd8042d 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/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 re import urllib.parse from typing import Dict, TYPE_CHECKING, Set @@ -13,13 +12,13 @@ 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 if TYPE_CHECKING: from .ClusterUM3OutputDevice import ClusterUM3OutputDevice + ## Asynchronous job to send material profiles to the printer. # # This way it won't freeze up the interface while sending those materials. @@ -86,7 +85,7 @@ class SendMaterialJob(Job): return { material.id for guid, material in local_materials.items() - if guid not in remote_materials or int(material.version) > remote_materials[guid].version + if guid not in remote_materials or material.version > remote_materials[guid].version } ## Send the materials to the printer. @@ -122,23 +121,23 @@ class SendMaterialJob(Job): # \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 = [] + parts = [] - # Add the material file. - with open(file_path, "rb") as f: - parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\"" - .format(file_name = file_name), f.read())) + # Add the material file. + with open(file_path, "rb") as f: + parts.append(self.device.createFormPart("name=\"file\"; filename=\"{file_name}\"" + .format(file_name = file_name), f.read())) - # 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.basename(signature_file_path) - with open(signature_file_path, "rb") as f: - parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\"" - .format(file_name = signature_file_name), f.read())) + # 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.basename(signature_file_path) + with open(signature_file_path, "rb") as f: + parts.append(self.device.createFormPart("name=\"signature_file\"; filename=\"{file_name}\"" + .format(file_name = signature_file_name), 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) + 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 @staticmethod @@ -174,16 +173,18 @@ class SendMaterialJob(Job): # Find the latest version of all material containers in the registry. for material in material_containers: try: - material = LocalMaterial(**material) - # material version must be an int - if not re.match("\d+", material.version): - Logger.logException("w", "Local material {} has invalid version '{}'." - .format(material["id"], material.version)) - continue + material["version"] = int(material["version"]) - if material.GUID not in result or material.version > result.get(material.GUID).version: - result[material.GUID] = material + # Create a new local material + local_material = LocalMaterial(**material) + + 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"])) except ValueError: Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index f4604580fe..ff896683e1 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -19,7 +19,7 @@ from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob @patch("UM.Resources.Resources.getAllResourcesOfType", lambda _: ["/materials/generic_pla_white.xml.fdm_material"]) class TestSendMaterialJob(TestCase): _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white", - "base_file": "generic_pla_white", "setting_version": 5, "name": "White PLA", + "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": "1", "color_code": "#ffffff", "description": "Test PLA White", "adhesion_info": "Use glue.", "approximate_diameter": "3", @@ -27,7 +27,7 @@ class TestSendMaterialJob(TestCase): "definition": "fdmprinter", "compatible": True} _LOCAL_MATERIAL_BLACK = {"type": "material", "status": "unknown", "id": "generic_pla_black", - "base_file": "generic_pla_black", "setting_version": 5, "name": "Yellow CPE", + "base_file": "generic_pla_black", "setting_version": "5", "name": "Yellow CPE", "brand": "Ultimaker", "material": "CPE", "color_name": "Black", "GUID": "5fbb362a-41f9-4818-bb43-15ea6df34aa4", "version": "1", "color_code": "#000000", "description": "Test PLA Black", "adhesion_info": "Use glue.", "approximate_diameter": "3", From 7b0f8882a2715d4beb9378646bc915e77c8f4963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Wed, 21 Nov 2018 11:01:26 +0100 Subject: [PATCH 23/34] Reverted models to namedtuples from collections because NamedTuple is a Python3.6 feature --- plugins/UM3NetworkPrinting/src/Models.py | 54 ++++++++++++------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index e84a39db5a..d5e1007555 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,33 +1,33 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import NamedTuple +from collections import namedtuple -ClusterMaterial = NamedTuple("ClusterMaterial", [ - ("guid", str), - ("material", str), - ("brand", str), - ("version", int), - ("color", str), - ("density", str), +ClusterMaterial = namedtuple('ClusterMaterial', [ + 'guid', # Type: str + 'material', # Type: str + 'brand', # Type: str + 'version', # Type: int + 'color', # Type: str + 'density' # Type: str ]) -LocalMaterial = NamedTuple("LocalMaterial", [ - ("GUID", str), - ("id", str), - ("type", str), - ("status", str), - ("base_file", str), - ("setting_version", int), - ("version", int), - ("name", str), - ("brand", str), - ("material", str), - ("color_name", str), - ("color_code", str), - ("description", str), - ("adhesion_info", str), - ("approximate_diameter", str), - ("properties", str), - ("definition", str), - ("compatible", str), +LocalMaterial = namedtuple('LocalMaterial', [ + 'GUID', # Type: str + 'id', # Type: str + 'type', # Type: str + 'status', # Type: str + 'base_file', # Type: str + 'setting_version', # Type: int + 'version', # Type: int + 'name', # Type: str + 'brand', # Type: str + 'material', # Type: str + 'color_name', # Type: str + 'color_code', # Type: str + 'description', # Type: str + 'adhesion_info', # Type: str + 'approximate_diameter', # Type: str + 'properties', # Type: str + 'definition', # Type: str + 'compatible' # Type: str ]) From 7e3f86f0913b7432c46b20507e33be57b7a1d514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Thu, 22 Nov 2018 09:37:47 +0100 Subject: [PATCH 24/34] Moved some of the mocks to class level because they are used in every test method --- .../tests/TestSendMaterialJob.py | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index ff896683e1..548704fd33 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -17,6 +17,8 @@ from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob 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): _LOCAL_MATERIAL_WHITE = {"type": "material", "status": "unknown", "id": "generic_pla_white", "base_file": "generic_pla_white", "setting_version": "5", "name": "White PLA", @@ -52,16 +54,13 @@ class TestSendMaterialJob(TestCase): "density": 1.00 } - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - def test_run(self, device_mock): + def test_run(self, device_mock, reply_mock): 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) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withFailedRequest(self, reply_mock, device_mock): reply_mock.attribute.return_value = 404 job = SendMaterialJob(device_mock) @@ -71,8 +70,6 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") @@ -84,8 +81,6 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 remote_material_without_guid = self._REMOTE_MATERIAL_WHITE.copy() @@ -101,10 +96,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, application_mock, container_registry_mock, + reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) @@ -125,10 +118,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withNoUpdate(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withNoUpdate(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" @@ -150,10 +141,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withUpdatedMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withUpdatedMaterial(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" @@ -181,10 +170,8 @@ class TestSendMaterialJob(TestCase): @patch("cura.Settings.CuraContainerRegistry") @patch("cura.CuraApplication") - @patch("plugins.UM3NetworkPrinting.src.ClusterUM3OutputDevice") - @patch("PyQt5.QtNetwork.QNetworkReply") - def test__onGetRemoteMaterials_withNewMaterial(self, reply_mock, device_mock, application_mock, - container_registry_mock): + def test__onGetRemoteMaterials_withNewMaterial(self, application_mock, container_registry_mock, reply_mock, + device_mock): application_mock.getContainerRegistry.return_value = container_registry_mock device_mock.createFormPart.return_value = "_xXx_" From 352427e4601204851daba848599153ebab3f8c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Thu, 22 Nov 2018 10:01:15 +0100 Subject: [PATCH 25/34] Moved exception handling closer to the cause of error --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index ee8dd8042d..72269040e7 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -46,9 +46,8 @@ class SendMaterialJob(Job): # Collect materials from the printer's reply and send the missing ones if needed. try: remote_materials_by_guid = self._parseReply(reply) - self._sendMissingMaterials(remote_materials_by_guid) - except json.JSONDecodeError: - Logger.logException("w", "Error parsing materials from printer") + if remote_materials_by_guid: + self._sendMissingMaterials(remote_materials_by_guid) except TypeError: Logger.logException("w", "Error parsing materials from printer") @@ -153,12 +152,14 @@ class SendMaterialJob(Job): # Parses the reply to a "/materials" request to the printer # # \return a dictionary of ClusterMaterial objects by GUID - # \throw json.JSONDecodeError Raised when the reply does not contain a valid json string # \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 = json.loads(reply.readAll().data().decode("utf-8")) - return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + try: + remote_materials = json.loads(reply.readAll().data().decode("utf-8")) + return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + except json.JSONDecodeError: + Logger.logException("w", "Error parsing materials from printer") ## Retrieves a list of local materials # From bb5c0326de589ee09b2398b74a542f838d6ea8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 23 Nov 2018 09:20:19 +0100 Subject: [PATCH 26/34] Used duoble quotes iso single quotes --- plugins/UM3NetworkPrinting/src/Models.py | 52 ++++++++++++------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index d5e1007555..bcdeb8299c 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -2,32 +2,32 @@ # Cura is released under the terms of the LGPLv3 or higher. from collections import namedtuple -ClusterMaterial = namedtuple('ClusterMaterial', [ - 'guid', # Type: str - 'material', # Type: str - 'brand', # Type: str - 'version', # Type: int - 'color', # Type: str - 'density' # Type: str +ClusterMaterial = namedtuple("ClusterMaterial", [ + "guid", # Type: str + "material", # Type: str + "brand", # Type: str + "version", # Type: int + "color", # Type: str + "density" # Type: str ]) -LocalMaterial = namedtuple('LocalMaterial', [ - 'GUID', # Type: str - 'id', # Type: str - 'type', # Type: str - 'status', # Type: str - 'base_file', # Type: str - 'setting_version', # Type: int - 'version', # Type: int - 'name', # Type: str - 'brand', # Type: str - 'material', # Type: str - 'color_name', # Type: str - 'color_code', # Type: str - 'description', # Type: str - 'adhesion_info', # Type: str - 'approximate_diameter', # Type: str - 'properties', # Type: str - 'definition', # Type: str - 'compatible' # Type: str +LocalMaterial = namedtuple("LocalMaterial", [ + "GUID", # Type: str + "id", # Type: str + "type", # Type: str + "status", # Type: str + "base_file", # Type: str + "setting_version", # Type: int + "version", # Type: int + "name", # Type: str + "brand", # Type: str + "material", # Type: str + "color_name", # Type: str + "color_code", # Type: str + "description", # Type: str + "adhesion_info", # Type: str + "approximate_diameter", # Type: str + "properties", # Type: str + "definition", # Type: str + "compatible" # Type: str ]) From 294527f7febf96d81f4bcdf62b142a552d58fbbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 23 Nov 2018 09:21:09 +0100 Subject: [PATCH 27/34] Review changes --- .../UM3NetworkPrinting/src/SendMaterialJob.py | 19 ++++++------ .../tests/TestSendMaterialJob.py | 29 +++++++++++++------ 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 72269040e7..48760af28e 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -7,6 +7,7 @@ from typing import Dict, TYPE_CHECKING, Set 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 @@ -27,7 +28,6 @@ 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: @@ -44,12 +44,9 @@ class SendMaterialJob(Job): return # Collect materials from the printer's reply and send the missing ones if needed. - try: - remote_materials_by_guid = self._parseReply(reply) - if remote_materials_by_guid: - self._sendMissingMaterials(remote_materials_by_guid) - except TypeError: - Logger.logException("w", "Error parsing materials from printer") + remote_materials_by_guid = self._parseReply(reply) + if remote_materials_by_guid: + self._sendMissingMaterials(remote_materials_by_guid) ## Determine which materials should be updated and send them to the printer. # @@ -158,8 +155,12 @@ class SendMaterialJob(Job): try: remote_materials = json.loads(reply.readAll().data().decode("utf-8")) return {material["guid"]: ClusterMaterial(**material) for material in remote_materials} + except UnicodeDecodeError: + Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") except json.JSONDecodeError: - Logger.logException("w", "Error parsing materials from printer") + Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") + except TypeError: + Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.") ## Retrieves a list of local materials # @@ -168,7 +169,7 @@ class SendMaterialJob(Job): # \return a dictionary of LocalMaterial objects by GUID def _getLocalMaterials(self) -> Dict[str, LocalMaterial]: result = {} # type: Dict[str, LocalMaterial] - container_registry = self._application.getContainerRegistry() + container_registry = Application.getInstance().getContainerRegistry() material_containers = container_registry.findContainersMetadata(type = "material") # Find the latest version of all material containers in the registry. diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index 548704fd33..f5a475b3ab 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -8,7 +8,7 @@ from unittest.mock import patch, call from PyQt5.QtCore import QByteArray from UM.MimeTypeDatabase import MimeType -from cura.CuraApplication import CuraApplication +from UM.Application import Application from plugins.UM3NetworkPrinting.src.SendMaterialJob import SendMaterialJob @@ -70,6 +70,17 @@ class TestSendMaterialJob(TestCase): self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) + def test__onGetRemoteMaterials_withWrongEncoding(self, reply_mock, device_mock): + reply_mock.attribute.return_value = 200 + reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("cp500")) + job = SendMaterialJob(device_mock) + job._onGetRemoteMaterials(reply_mock) + + # We expect the reply to be called once to try to get the printers from the list (readAll()). + # Given that the parsing fails we do no expect the device to be called for any follow up. + self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) + self.assertEqual(0, device_mock.createFormPart.call_count) + def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(b"Six sick hicks nick six slick bricks with picks and sticks.") @@ -95,7 +106,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @patch("UM.Application") def test__onGetRemoteMaterials_withInvalidVersionInLocalMaterial(self, application_mock, container_registry_mock, reply_mock, device_mock): reply_mock.attribute.return_value = 200 @@ -107,7 +118,7 @@ class TestSendMaterialJob(TestCase): application_mock.getContainerRegistry.return_value = container_registry_mock - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -117,7 +128,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @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 @@ -129,7 +140,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -140,7 +151,7 @@ class TestSendMaterialJob(TestCase): self.assertEqual(0, device_mock.postFormWithParts.call_count) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @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 @@ -154,7 +165,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_WHITE]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) @@ -169,7 +180,7 @@ class TestSendMaterialJob(TestCase): device_mock.method_calls) @patch("cura.Settings.CuraContainerRegistry") - @patch("cura.CuraApplication") + @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 @@ -182,7 +193,7 @@ class TestSendMaterialJob(TestCase): reply_mock.attribute.return_value = 200 reply_mock.readAll.return_value = QByteArray(json.dumps([self._REMOTE_MATERIAL_BLACK]).encode("ascii")) - with mock.patch.object(CuraApplication, "getInstance", new = lambda: application_mock): + with mock.patch.object(Application, "getInstance", new = lambda: application_mock): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) From 76542a82d5a19b3dd9ea2a3f2c785d8fd956fc57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Fri, 23 Nov 2018 10:33:57 +0100 Subject: [PATCH 28/34] Removed the asserts on internals --- .../tests/TestSendMaterialJob.py | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py index f5a475b3ab..b669eb192a 100644 --- a/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/tests/TestSendMaterialJob.py @@ -1,4 +1,5 @@ # Copyright (c) 2018 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import io import json @@ -66,8 +67,7 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - # We expect the error string to be retrieved and the device not to be called for any follow up. - self.assertEqual([call.attribute(0), call.errorString()], reply_mock.method_calls) + # 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): @@ -76,9 +76,7 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that the parsing fails we do no expect the device to be called for any follow up. - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) def test__onGetRemoteMaterials_withBadJsonAnswer(self, reply_mock, device_mock): @@ -87,9 +85,7 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that the parsing fails we do no expect the device to be called for any follow up. - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) def test__onGetRemoteMaterials_withMissingGuidInRemoteMaterial(self, reply_mock, device_mock): @@ -100,9 +96,7 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - # We expect the reply to be called once to try to get the printers from the list (readAll()). # Given that parsing fails we do not expect the device to be called for any follow up. - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") @@ -122,9 +116,6 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) - self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) @patch("cura.Settings.CuraContainerRegistry") @@ -144,9 +135,6 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) - self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) self.assertEqual(0, device_mock.createFormPart.call_count) self.assertEqual(0, device_mock.postFormWithParts.call_count) @@ -169,9 +157,6 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) - self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) self.assertEqual(1, device_mock.createFormPart.call_count) self.assertEqual(1, device_mock.postFormWithParts.call_count) self.assertEquals( @@ -197,9 +182,6 @@ class TestSendMaterialJob(TestCase): job = SendMaterialJob(device_mock) job._onGetRemoteMaterials(reply_mock) - self.assertEqual([call.attribute(0), call.readAll()], reply_mock.method_calls) - self.assertEqual([call.getContainerRegistry()], application_mock.method_calls) - self.assertEqual([call.findContainersMetadata(type = "material")], container_registry_mock.method_calls) self.assertEqual(1, device_mock.createFormPart.call_count) self.assertEqual(1, device_mock.postFormWithParts.call_count) self.assertEquals( From 68a90ec510b15d41ba585f013c7d9c5fe52fb94f Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 26 Nov 2018 14:08:21 +0100 Subject: [PATCH 29/34] Use simple models instead of namedtuples Named tuples would throw a TypeError if an unknown attribute was set, but we just want to ignore those --- plugins/UM3NetworkPrinting/src/Models.py | 67 ++++++++++++++---------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index bcdeb8299c..e2ad411e90 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -1,33 +1,42 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from collections import namedtuple -ClusterMaterial = namedtuple("ClusterMaterial", [ - "guid", # Type: str - "material", # Type: str - "brand", # Type: str - "version", # Type: int - "color", # Type: str - "density" # Type: str -]) -LocalMaterial = namedtuple("LocalMaterial", [ - "GUID", # Type: str - "id", # Type: str - "type", # Type: str - "status", # Type: str - "base_file", # Type: str - "setting_version", # Type: int - "version", # Type: int - "name", # Type: str - "brand", # Type: str - "material", # Type: str - "color_name", # Type: str - "color_code", # Type: str - "description", # Type: str - "adhesion_info", # Type: str - "approximate_diameter", # Type: str - "properties", # Type: str - "definition", # Type: str - "compatible" # Type: str -]) +## Base model that maps kwargs to instance attributes. +class BaseModel: + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + +## Class representing a material that was fetched from the cluster API. +class ClusterMaterial(BaseModel): + def __init__(self, **kwargs): + self.guid = None # type: str + self.material = None # type: str + self.brand = None # type: str + self.version = None # type: int + self.color = None # type: str + self.density = None # type: str + super().__init__(**kwargs) + + +## Class representing a local material that was fetched from the container registry. +class LocalMaterial(BaseModel): + def __init__(self, **kwargs): + self.GUID = None # type: str + self.id = None # type: str + self.type = None # type: str + self.status = None # type: str + self.base_file = None # type: str + self.setting_version = None # type: int + self.version = None # type: int + self.brand = None # type: str + self.material = None # type: str + self.color_name = None # type: str + self.color_code = None # type: str + self.description = None # type: str + self.adhesion_info = None # type: str + self.approximate_diameter = None # type: str + self.definition = None # type: str + self.compatible = None # type: bool + super().__init__(**kwargs) From a382b77eaa81998d5a413a019701137117b45eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marijn=20De=C3=A9?= Date: Mon, 26 Nov 2018 14:30:17 +0100 Subject: [PATCH 30/34] Added validation to the models --- plugins/UM3NetworkPrinting/src/Models.py | 14 ++++++++++++++ plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index e2ad411e90..d0708c8127 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -6,6 +6,10 @@ class BaseModel: def __init__(self, **kwargs): self.__dict__.update(kwargs) + self.validate() + + def validate(self): + pass ## Class representing a material that was fetched from the cluster API. @@ -19,6 +23,10 @@ class ClusterMaterial(BaseModel): self.density = None # type: str super().__init__(**kwargs) + def validate(self): + if not self.guid: + raise ValueError("guid is required on ClusterMaterial") + ## Class representing a local material that was fetched from the container registry. class LocalMaterial(BaseModel): @@ -40,3 +48,9 @@ class LocalMaterial(BaseModel): self.definition = None # type: str self.compatible = None # type: bool super().__init__(**kwargs) + + def validate(self): + if not self.GUID: + raise ValueError("guid is required on LocalMaterial") + if not self.id: + raise ValueError("id is required on LocalMaterial") diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 48760af28e..6f33e75ee1 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -159,8 +159,8 @@ class SendMaterialJob(Job): Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") except json.JSONDecodeError: Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") - except TypeError: - Logger.log("e", "Request material storage on printer: Printer's answer was missing GUIDs.") + except ValueError: + Logger.log("e", "Request material storage on printer: Printer's answer was missing a value.") ## Retrieves a list of local materials # From da5683c876682019fe2360cf56fd27afe9f39844 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 26 Nov 2018 15:30:30 +0100 Subject: [PATCH 31/34] add typing to models --- plugins/UM3NetworkPrinting/src/Models.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index d0708c8127..2a34e41f86 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -4,17 +4,17 @@ ## Base model that maps kwargs to instance attributes. class BaseModel: - def __init__(self, **kwargs): + def __init__(self, **kwargs) -> None: self.__dict__.update(kwargs) self.validate() - def validate(self): + def validate(self) -> None: pass ## Class representing a material that was fetched from the cluster API. class ClusterMaterial(BaseModel): - def __init__(self, **kwargs): + def __init__(self, **kwargs) -> None: self.guid = None # type: str self.material = None # type: str self.brand = None # type: str @@ -23,14 +23,14 @@ class ClusterMaterial(BaseModel): self.density = None # type: str super().__init__(**kwargs) - def validate(self): + def validate(self) -> None: if not self.guid: raise ValueError("guid is required on ClusterMaterial") ## Class representing a local material that was fetched from the container registry. class LocalMaterial(BaseModel): - def __init__(self, **kwargs): + def __init__(self, **kwargs) -> None: self.GUID = None # type: str self.id = None # type: str self.type = None # type: str @@ -49,7 +49,7 @@ class LocalMaterial(BaseModel): self.compatible = None # type: bool super().__init__(**kwargs) - def validate(self): + def validate(self) -> None: if not self.GUID: raise ValueError("guid is required on LocalMaterial") if not self.id: From e4d8fb36abccd5e1a5f7f68bef086ae82ec3b9a4 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 26 Nov 2018 15:53:04 +0100 Subject: [PATCH 32/34] Add more typing as per request from @sedwards2009 --- plugins/UM3NetworkPrinting/src/Models.py | 35 ++++++++---------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index 2a34e41f86..5ef44bc006 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -14,43 +14,30 @@ class BaseModel: ## Class representing a material that was fetched from the cluster API. class ClusterMaterial(BaseModel): - def __init__(self, **kwargs) -> None: - self.guid = None # type: str - self.material = None # type: str - self.brand = None # type: str - self.version = None # type: int - self.color = None # type: str - self.density = None # type: str + def __init__(self, guid = str, version = str, **kwargs) -> None: + self.guid = guid # type: str + self.version = version # type: int super().__init__(**kwargs) def validate(self) -> None: if not self.guid: raise ValueError("guid is required on ClusterMaterial") + if not self.version: + raise ValueError("version is required on ClusterMaterial") ## Class representing a local material that was fetched from the container registry. class LocalMaterial(BaseModel): - def __init__(self, **kwargs) -> None: - self.GUID = None # type: str - self.id = None # type: str - self.type = None # type: str - self.status = None # type: str - self.base_file = None # type: str - self.setting_version = None # type: int - self.version = None # type: int - self.brand = None # type: str - self.material = None # type: str - self.color_name = None # type: str - self.color_code = None # type: str - self.description = None # type: str - self.adhesion_info = None # type: str - self.approximate_diameter = None # type: str - self.definition = None # type: str - self.compatible = None # type: bool + def __init__(self, GUID = str, id = str, version = str, **kwargs) -> None: + self.GUID = GUID # type: str + self.id = id # type: str + self.version = version # type: int super().__init__(**kwargs) def validate(self) -> None: if not self.GUID: raise ValueError("guid is required on LocalMaterial") + if not self.version: + raise ValueError("version is required on LocalMaterial") if not self.id: raise ValueError("id is required on LocalMaterial") From 6506596eceeb241decdc8456f8d2d3d21a9b140e Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 26 Nov 2018 16:19:01 +0100 Subject: [PATCH 33/34] Fix typing syntax --- plugins/UM3NetworkPrinting/src/Models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Models.py b/plugins/UM3NetworkPrinting/src/Models.py index 5ef44bc006..2bcac70766 100644 --- a/plugins/UM3NetworkPrinting/src/Models.py +++ b/plugins/UM3NetworkPrinting/src/Models.py @@ -14,7 +14,7 @@ class BaseModel: ## Class representing a material that was fetched from the cluster API. class ClusterMaterial(BaseModel): - def __init__(self, guid = str, version = str, **kwargs) -> None: + def __init__(self, guid: str, version: int, **kwargs) -> None: self.guid = guid # type: str self.version = version # type: int super().__init__(**kwargs) @@ -28,7 +28,7 @@ class ClusterMaterial(BaseModel): ## Class representing a local material that was fetched from the container registry. class LocalMaterial(BaseModel): - def __init__(self, GUID = str, id = str, version = str, **kwargs) -> None: + def __init__(self, GUID: str, id: str, version: int, **kwargs) -> None: self.GUID = GUID # type: str self.id = id # type: str self.version = version # type: int From efd5f3799bdb1c156722046bc2056b961d217f4d Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Mon, 26 Nov 2018 16:31:32 +0100 Subject: [PATCH 34/34] Also catch TypeError now that we have explicit arguments --- plugins/UM3NetworkPrinting/src/SendMaterialJob.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py index 6f33e75ee1..f536fad49a 100644 --- a/plugins/UM3NetworkPrinting/src/SendMaterialJob.py +++ b/plugins/UM3NetworkPrinting/src/SendMaterialJob.py @@ -160,7 +160,9 @@ class SendMaterialJob(Job): except json.JSONDecodeError: Logger.log("e", "Request material storage on printer: I didn't understand the printer's answer.") except ValueError: - Logger.log("e", "Request material storage on printer: Printer's answer was missing a value.") + Logger.log("e", "Request material storage on printer: Printer's answer had an incorrect value.") + except TypeError: + Logger.log("e", "Request material storage on printer: Printer's answer was missing a required value.") ## Retrieves a list of local materials # @@ -189,5 +191,7 @@ class SendMaterialJob(Job): Logger.logException("w", "Local material {} has missing values.".format(material["id"])) except ValueError: Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) + except TypeError: + Logger.logException("w", "Local material {} has invalid values.".format(material["id"])) return result