From 20fc26b8ba50f057e1a97c0b85546f4f3ba3d0f6 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Fri, 8 Mar 2019 13:32:36 +0100 Subject: [PATCH 1/3] Correct setting version for definition files Otherwise the container provider actually rejects loading it completely. Contributes to issue CURA-6270. --- resources/definitions/fdmextruder.def.json | 2 +- resources/definitions/fdmprinter.def.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/definitions/fdmextruder.def.json b/resources/definitions/fdmextruder.def.json index 0af1e68075..d624e37785 100644 --- a/resources/definitions/fdmextruder.def.json +++ b/resources/definitions/fdmextruder.def.json @@ -6,7 +6,7 @@ "type": "extruder", "author": "Ultimaker", "manufacturer": "Unknown", - "setting_version": 1, + "setting_version": 6, "visible": false, "position": "0" }, diff --git a/resources/definitions/fdmprinter.def.json b/resources/definitions/fdmprinter.def.json index 295108f27b..0852e05fe8 100644 --- a/resources/definitions/fdmprinter.def.json +++ b/resources/definitions/fdmprinter.def.json @@ -7,7 +7,7 @@ "author": "Ultimaker", "category": "Other", "manufacturer": "Unknown", - "setting_version": 1, + "setting_version": 6, "file_formats": "text/x-gcode;application/x-stl-ascii;application/x-stl-binary;application/x-wavefront-obj;application/x3g", "visible": false, "has_materials": true, From 3c779b58de8e9caf39e442f20d15966b3297ff99 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Fri, 8 Mar 2019 13:34:16 +0100 Subject: [PATCH 2/3] Specialise _isMetadataValid for Cura to check setting_version Because we need to reject loading metadata for containers whose setting_version is incorrect. Contributes to issue CURA-6270. --- cura/Settings/CuraContainerRegistry.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/cura/Settings/CuraContainerRegistry.py b/cura/Settings/CuraContainerRegistry.py index 9f44d075e0..6804becf4e 100644 --- a/cura/Settings/CuraContainerRegistry.py +++ b/cura/Settings/CuraContainerRegistry.py @@ -1,11 +1,11 @@ -# Copyright (c) 2018 Ultimaker B.V. +# Copyright (c) 2019 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import os import re import configparser -from typing import cast, Dict, Optional +from typing import Any, cast, Dict, Optional from PyQt5.QtWidgets import QMessageBox from UM.Decorators import override @@ -327,6 +327,23 @@ class CuraContainerRegistry(ContainerRegistry): self._registerSingleExtrusionMachinesExtruderStacks() self._connectUpgradedExtruderStacksToMachines() + ## Check if the metadata for a container is okay before adding it. + # + # This overrides the one from UM.Settings.ContainerRegistry because we + # also require that the setting_version is correct. + @override(ContainerRegistry) + def _isMetadataValid(self, metadata: Optional[Dict[str, Any]]) -> bool: + if metadata is None: + return False + if "setting_version" not in metadata: + return False + try: + if int(metadata["setting_version"]) != cura.CuraApplication.CuraApplication.SettingVersion: + return False + except ValueError: #Not parsable as int. + return False + return True + ## Update an imported profile to match the current machine configuration. # # \param profile The profile to configure. From e53eb70cad590cd9aad3040d02ebe40f66121d6a Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Fri, 8 Mar 2019 14:07:10 +0100 Subject: [PATCH 3/3] Add tests for validating metadata before loading Contributes to issue CURA-6270. --- tests/Settings/TestCuraContainerRegistry.py | 62 ++++++++++++++++++++- tests/Settings/conftest.py | 6 +- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/tests/Settings/TestCuraContainerRegistry.py b/tests/Settings/TestCuraContainerRegistry.py index af478c3b2b..9ec729b5ce 100644 --- a/tests/Settings/TestCuraContainerRegistry.py +++ b/tests/Settings/TestCuraContainerRegistry.py @@ -1,7 +1,8 @@ -# Copyright (c) 2018 Ultimaker B.V. +# Copyright (c) 2019 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import os #To find the directory with test files and find the test files. +import pytest #To parameterize tests. import unittest.mock #To mock and monkeypatch stuff. from UM.Settings.DefinitionContainer import DefinitionContainer @@ -97,3 +98,62 @@ def test_addContainerBadSettingVersion(container_registry, definition_container) container_registry.addContainer(instance) mock_super_add_container.assert_not_called() #Should not get passed on to UM.Settings.ContainerRegistry.addContainer, because the setting_version doesn't match its definition! + +test_loadMetaDataValidation_data = [ + { + "id": "valid_container", + "is_valid": True, + "metadata": { + "id": "valid_container", + "setting_version": None, #The tests sets this to the current version so it's always correct. + "foo": "bar" + } + }, + { + "id": "wrong_setting_version", + "is_valid": False, + "metadata": { + "id": "wrong_setting_version", + "setting_version": "5", + "foo": "bar" + } + }, + { + "id": "missing_setting_version", + "is_valid": False, + "metadata": { + "id": "missing_setting_version", + "foo": "bar" + } + }, + { + "id": "unparsable_setting_version", + "is_valid": False, + "metadata": { + "id": "unparsable_setting_version", + "setting_version": "Not an integer!", + "foo": "bar" + } + } +] + +@pytest.mark.parametrize("parameters", test_loadMetaDataValidation_data) +def test_loadMetadataValidation(container_registry, definition_container, parameters): + from cura.CuraApplication import CuraApplication + definition_container.getMetaData()["setting_version"] = CuraApplication.SettingVersion + container_registry.addContainer(definition_container) + if "setting_version" in parameters["metadata"] and parameters["metadata"]["setting_version"] is None: #Signal that the setting_version must be set to the currently correct version. + parameters["metadata"]["setting_version"] = CuraApplication.SettingVersion + + mock_provider = unittest.mock.MagicMock() + mock_provider.getAllIds = unittest.mock.MagicMock(return_value = [parameters["id"]]) + mock_provider.loadMetadata = unittest.mock.MagicMock(return_value = parameters["metadata"]) + container_registry._providers = [mock_provider] + + container_registry.loadAllMetadata() #Run the test. + + if parameters["is_valid"]: + assert parameters["id"] in container_registry.metadata + assert container_registry.metadata[parameters["id"]] == parameters["metadata"] + else: + assert parameters["id"] not in container_registry.metadata \ No newline at end of file diff --git a/tests/Settings/conftest.py b/tests/Settings/conftest.py index c2d8854f05..d7494dabf8 100644 --- a/tests/Settings/conftest.py +++ b/tests/Settings/conftest.py @@ -1,5 +1,5 @@ -# Copyright (c) 2018 Ultimaker B.V. -# Uranium is released under the terms of the LGPLv3 or higher. +# Copyright (c) 2019 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. # The purpose of this class is to create fixtures or methods that can be shared among all settings tests. @@ -49,6 +49,6 @@ def global_stack(definition_changes_container) -> GlobalStack: # There is a restriction here that the definition changes cannot be an empty container. Added in CURA-5281 @pytest.fixture() def extruder_stack(definition_changes_container) -> ExtruderStack: - extruder_stack= ExtruderStack("TestExtruderStack") + extruder_stack = ExtruderStack("TestExtruderStack") extruder_stack._containers[cura.Settings.CuraContainerStack._ContainerIndexes.DefinitionChanges] = definition_changes_container return extruder_stack \ No newline at end of file