[Backport main] Expand the definition of 'other'

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)

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")

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)

AddonManagerTest/data/unrecognized_content_only.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8" standalone="no" ?>
+<package format="1" xmlns="https://wiki.freecad.org/Package_Metadata">
+	<name>Test Unrecognized Content</name>
+	<description>A package.xml file for unit testing.</description>
+	<version>1.0.1</version>
+	<date>2022-01-07</date>
+	<maintainer email="developer@freecad.org">FreeCAD Developer</maintainer>
+	<license file="LICENSE">LGPL-2.1</license>
+	<url type="repository" branch="main">https://github.com/chennes/FreeCAD-Package</url>
+	<url type="readme">https://github.com/chennes/FreeCAD-Package/blob/main/README.md</url>
+
+	<content>
+		<contenttypefromthefuture>
+			<name>Some Future Content</name>
+			<description>A content type that this version of the Addon Manager knows nothing about.</description>
+			<tag>TagFromTheFuture</tag>
+		</contenttypefromthefuture>
+	</content>
+
+</package>

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: