diff --git a/Addon.py b/Addon.py index 3daad4d1..c687898f 100644 --- a/Addon.py +++ b/Addon.py @@ -78,6 +78,12 @@ "web": "Web", } +# The package metadata content types that have a dedicated category in the Addon Manager's +# filter list. Every other content type, whether it is the standard "other" type or a type +# introduced after this version of the Addon Manager was released, is shown in the "Other" +# category. +CATEGORIZED_CONTENT_TYPES = frozenset(["workbench", "macro", "preferencepack", "bundle"]) + class Addon: """Encapsulates information about a FreeCAD addon""" @@ -484,17 +490,21 @@ def contains_macro(self) -> bool: return True return self.contains_packaged_content("macro") + def packaged_content_types(self) -> Set[str]: + """The content types declared by this package's metadata. Empty for anything that is + not a package.""" + if self.repo_type != Addon.Kind.PACKAGE: + return set() + if self.metadata is None: + fci.Console.PrintLog( + f"Addon Manager internal error: lost metadata for package {self.name}\n" + ) + return set() + return set(self.metadata.content) + def contains_packaged_content(self, content_type: str): """Determine if the package contains content_type""" - if self.repo_type == Addon.Kind.PACKAGE: - if self.metadata is None: - fci.Console.PrintLog( - f"Addon Manager internal error: lost metadata for package {self.name}\n" - ) - return False - content = self.metadata.content - return content_type in content - return False + return content_type in self.packaged_content_types() def contains_preference_pack(self) -> bool: """Determine if this package contains a preference pack""" @@ -505,8 +515,9 @@ def contains_bundle(self) -> bool: return self.contains_packaged_content("bundle") def contains_other(self) -> bool: - """Determine if this package contains an "other" content item""" - return self.contains_packaged_content("other") + """Determine if this package contains an "other" content item, or any content type that + this version of the Addon Manager does not have a category for.""" + return bool(self.packaged_content_types() - CATEGORIZED_CONTENT_TYPES) def walk_dependency_tree(self, all_repos: Dict[str, "Addon"], deps: Dependencies): """Compute the total dependency tree for this repo (recursive) diff --git a/AddonManagerTest/app/test_addon.py b/AddonManagerTest/app/test_addon.py index f7abc78e..a3006c03 100644 --- a/AddonManagerTest/app/test_addon.py +++ b/AddonManagerTest/app/test_addon.py @@ -158,6 +158,20 @@ def test_contains_functions(self): # There is no equivalent for preference packs, they are always accompanied by a # metadata file + def test_contains_other_includes_unrecognized_content(self): + addon = Addon( + "FreeCAD", + "https://github.com/FreeCAD/FreeCAD", + Addon.Status.NOT_INSTALLED, + "master", + ) + addon.load_metadata_file(os.path.join(self.test_dir, "unrecognized_content_only.xml")) + self.assertFalse(addon.contains_workbench()) + self.assertFalse(addon.contains_macro()) + self.assertFalse(addon.contains_preference_pack()) + self.assertFalse(addon.contains_bundle()) + self.assertTrue(addon.contains_other()) + def test_create_from_macro(self): macro_file = os.path.join(self.test_dir, "DoNothing.FCMacro") macro = Macro("DoNothing") diff --git a/AddonManagerTest/app/test_metadata.py b/AddonManagerTest/app/test_metadata.py index 911e770d..c739f01d 100644 --- a/AddonManagerTest/app/test_metadata.py +++ b/AddonManagerTest/app/test_metadata.py @@ -404,11 +404,22 @@ def test_parse_content_valid(self, mock_create_node): mock_create_node.reset_mock() @patch("addonmanager_metadata.MetadataReader._create_node") - def test_parse_content_invalid(self, mock_create_node): - content_item = "no_such_content_type" - tree_mock = [self.given_mock_tree_node(content_item, None)] + def test_parse_content_unknown_type(self, mock_create_node): + """Content types that this version of the Addon Manager does not know about are + still parsed, so that the metadata standard can be extended.""" + tree_mock = [self.given_mock_tree_node("no_such_content_type", None)] metadata_mock = MagicMock() amm.MetadataReader._parse_content("", metadata_mock, tree_mock) + mock_create_node.assert_called_once() + + @patch("addonmanager_metadata.MetadataReader._create_node") + def test_parse_content_foreign_namespace(self, mock_create_node): + """Elements from some other namespace are not content types and are skipped.""" + tree_mock = [self.given_mock_tree_node("{http://example.com/}workbench", None)] + metadata_mock = MagicMock() + amm.MetadataReader._parse_content( + "{https://wiki.freecad.org/Package_Metadata}", metadata_mock, tree_mock + ) mock_create_node.assert_not_called() @@ -502,6 +513,15 @@ def test_other(self): self.assertIn("other", metadata.content) self.assertEqual(len(metadata.content["other"]), 1) + def test_unrecognized_content_type_is_retained(self): + filename = os.path.join(self.test_data_dir, "unrecognized_content_only.xml") + metadata = amm.MetadataReader.from_file(filename) + self.assertIn("contenttypefromthefuture", metadata.content) + self.assertEqual(len(metadata.content["contenttypefromthefuture"]), 1) + self.assertEqual( + "Some Future Content", metadata.content["contenttypefromthefuture"][0].name + ) + def test_content_combination(self): filename = os.path.join(self.test_data_dir, "combination.xml") metadata = amm.MetadataReader.from_file(filename) diff --git a/AddonManagerTest/data/unrecognized_content_only.xml b/AddonManagerTest/data/unrecognized_content_only.xml new file mode 100644 index 00000000..aba0c551 --- /dev/null +++ b/AddonManagerTest/data/unrecognized_content_only.xml @@ -0,0 +1,20 @@ + + + Test Unrecognized Content + A package.xml file for unit testing. + 1.0.1 + 2022-01-07 + FreeCAD Developer + LGPL-2.1 + https://github.com/chennes/FreeCAD-Package + https://github.com/chennes/FreeCAD-Package/blob/main/README.md + + + + Some Future Content + A content type that this version of the Addon Manager knows nothing about. + TagFromTheFuture + + + + diff --git a/addonmanager_metadata.py b/addonmanager_metadata.py index 0b92ff66..52eb53ce 100644 --- a/addonmanager_metadata.py +++ b/addonmanager_metadata.py @@ -379,15 +379,17 @@ def _parse_dependency(child: ET.Element) -> Dependency: @staticmethod def _parse_content(namespace: str, metadata: Metadata, root: ET.Element): - """Given a content node, loop over its children, and if they are a recognized - element type, recurse into each one to parse it.""" - known_content_types = ["workbench", "macro", "preferencepack", "bundle", "other"] + """Given a content node, loop over its children and recurse into each one to parse it. + Every child element is treated as a content type, including types that this version of + the Addon Manager does not know about, so that new types added to the package metadata + standard are still available to callers.""" for child in root: + if not isinstance(child.tag, str) or not child.tag.startswith(namespace): + continue content_type = child.tag[len(namespace) :] - if content_type in known_content_types: - if content_type not in metadata.content: - metadata.content[content_type] = [] - metadata.content[content_type].append(MetadataReader._create_node(namespace, child)) + metadata.content.setdefault(content_type, []).append( + MetadataReader._create_node(namespace, child) + ) @staticmethod def _create_node(namespace, child) -> Metadata: