Merge pull request #1025 from strr/strr/better-namespace-prefixes

Author: austinmatherne-wk

iXBRLViewerPlugin/iXBRLViewer.py

@@ -63,8 +63,15 @@ class NamespaceMap:
     """
 
     def __init__(self) -> None:
-        self.nsmap: dict[str, str] = {}
-        self.prefixmap: dict[str, str] = {}
+        self._nsmap: dict[str, str] = {}
+        self._reportNsMap: dict[str, str] = {}
+
+    @property    
+    def prefixmap(self) -> dict[str, str]:
+        """
+        Get the current prefix map (prefix to namespace URI).
+        """
+        return {p: ns for ns, p in self._nsmap.items()}
 
     def getPrefix(self, ns: str, preferredPrefix: str | None = None) -> str:
         """
@@ -75,27 +82,46 @@ def getPrefix(self, ns: str, preferredPrefix: str | None = None) -> str:
         prefix (or the string "ns")
         """
 
-        prefix = self.nsmap.get(ns, None)
-        if not prefix:
-            if preferredPrefix and preferredPrefix not in self.prefixmap:
-                prefix = preferredPrefix
-            else:
-                p = preferredPrefix if preferredPrefix else "ns"
-                n = 0
-                while f"{p}{n}" in self.prefixmap:
-                    n += 1
+        if (boundPrefix := self._nsmap.get(ns, None)) is not None:
+            return boundPrefix
 
-                prefix = f"{p}{n}"
+        if preferredPrefix is None:
+            # For example, EE2.0 fact values (expanded names) get turned in to
+            # QNames without a prefix but the report will know the prefix
+            preferredPrefix = self._reportNsMap.get(ns, None)
+
+        all_bound_prefixes = self._nsmap.values()
+
+        if preferredPrefix and preferredPrefix not in all_bound_prefixes:
+            chosenPrefix = preferredPrefix
+        else:
+            p = preferredPrefix if preferredPrefix else "ns"
+            n = 0
+            while (chosenPrefix := f"{p}{n}") in all_bound_prefixes:
+                n += 1
 
-            self.prefixmap[prefix] = ns
-            self.nsmap[ns] = prefix
-        return prefix
+        self._nsmap[ns] = chosenPrefix
+        return chosenPrefix
 
     def qname(self, qname: QName) -> str:
         if qname.namespaceURI is None:
             return qname.localName
         return f"{self.getPrefix(qname.namespaceURI, qname.prefix)}:{qname.localName}"
 
+    def clear(self) -> None:
+        """
+        Clear the namespace map.
+        """
+        self._nsmap.clear()
+        self._reportNsMap.clear()
+
+    def stashReportNSMap(self, report: ModelXbrl) -> None:
+        """
+        Stash a copy of Arelle's prefix/namespace map in case it can be used
+        later to find a preferred prefix.
+        """
+        self._reportNsMap = {ns: p for p, ns in report.prefixedNamespaces.items()}
+
 
 class IXBRLViewerBuilderError(Exception):
     pass
@@ -501,6 +527,7 @@ def addSourceReport(self) -> dict[str, list[Any]]:
         return sourceReport
 
     def processModel(self, report: ModelXbrl) -> None:
+        self.nsmap.stashReportNSMap(report)
         self.footnoteRelationshipSet = ModelRelationshipSet(report, "XBRL-footnotes")  # type: ignore[no-untyped-call]
         self.currentTargetReport = self.newTargetReport(getattr(report, "ixdsTarget", None))
         softwareCredits = set()
@@ -600,7 +627,7 @@ def createViewer(
             scriptUrl: str = DEFAULT_JS_FILENAME,
             showValidations: bool = True,
             packageDownloadURL: str | None = None,
-    ) -> 'iXBRLViewer' | None:
+    ) -> iXBRLViewer | None:
         """
         Create an iXBRL file with XBRL data as a JSON blob, and script tags added.
         :param scriptUrl: The `src` value of the script tag that loads the viewer script.

tests/unit_tests/iXBRLViewerPlugin/test_iXBRLViewer.py

@@ -95,6 +95,67 @@ def test_getPrefix_subsequent_call_with_two_namespaces_and_prefix(self):
         result_2 = ns_map.getPrefix(namespace_2)
         assert result_2 == 'ns1'
 
+    def test_stashReportNSMap_uses_report_prefix(self):
+        """
+        After stashing, getPrefix should use the prefix from the report's
+        prefixedNamespaces map as the preferred prefix.
+        """
+        ns_map = NamespaceMap()
+        report = Mock()
+        report.prefixedNamespaces = {"myprefix": "http://example.com"}
+        ns_map.stashReportNSMap(report)
+        assert ns_map.getPrefix("http://example.com") == "myprefix"
+
+    def test_stashReportNSMap_without_stash_uses_generated_prefix(self):
+        """
+        Without stashing, getPrefix should fall back to a generated prefix.
+        """
+        ns_map = NamespaceMap()
+        assert ns_map.getPrefix("http://example.com") == "ns0"
+
+    def test_stashReportNSMap_prefix_differs_from_unstashed(self):
+        """
+        Stashing should produce a different prefix than without stashing for
+        the same namespace.
+        """
+        report = Mock()
+        report.prefixedNamespaces = {"myprefix": "http://example.com"}
+
+        stashed = NamespaceMap()
+        stashed.stashReportNSMap(report)
+
+        unstashed = NamespaceMap()
+
+        assert stashed.getPrefix("http://example.com") != unstashed.getPrefix("http://example.com")
+
+    def test_stashReportNSMap_stashed_prefix_already_taken(self):
+        """
+        If the stashed prefix is already bound to another namespace, getPrefix
+        should fall back to a numbered suffix rather than silently reusing it.
+        """
+        ns_map = NamespaceMap()
+        report = Mock()
+        report.prefixedNamespaces = {"myprefix": "http://example.com"}
+        ns_map.stashReportNSMap(report)
+        # Bind "myprefix" to a different namespace first
+        ns_map.getPrefix("http://other.com", "myprefix")
+        # Now the stashed preferred prefix is taken; should number-suffix
+        assert ns_map.getPrefix("http://example.com") == "myprefix0"
+
+    def test_stashReportNSMap_does_not_affect_already_bound_namespace(self):
+        """
+        If getPrefix was called before stashReportNSMap, the already-bound
+        prefix should be returned unchanged on subsequent calls.
+        """
+        ns_map = NamespaceMap()
+        # Bind the namespace before stashing
+        assert ns_map.getPrefix("http://example.com") == "ns0"
+        report = Mock()
+        report.prefixedNamespaces = {"myprefix": "http://example.com"}
+        ns_map.stashReportNSMap(report)
+        # Stash should not retroactively change the bound prefix
+        assert ns_map.getPrefix("http://example.com") == "ns0"
+
 
 class TestIXBRLViewer:
 
@@ -444,6 +505,11 @@ def urlDocEntry(path, docType, linkQName=None):
             info=info_effect,
             modelDocument=self.modelDocument,
             ixdsTarget=None,
+            prefixedNamespaces={
+                'snake': 'http://example.com/snake',
+                'badger': 'http://example.com/badger',
+                'mushroom': 'http://example.com/mushroom'
+                },
             urlDocs=dict((
                 urlDocEntry('/filesystem/local-inline.htm', Type.INLINEXBRL),
                 urlDocEntry('https://example.com/remote-inline.htm', Type.INLINEXBRL),
@@ -474,6 +540,7 @@ def urlDocEntry(path, docType, linkQName=None):
             fileSource=file_source,
             info=info_effect,
             modelDocument=self.modelDocument,
+            prefixedNamespaces={},
             ixdsTarget=None,
             urlDocs={}
         )
@@ -486,6 +553,7 @@ def urlDocEntry(path, docType, linkQName=None):
             fileSource=file_source,
             info=info_effect,
             modelDocument=self.modelDocumentInlineSet,
+            prefixedNamespaces={},
             ixdsTarget=None,
             urlDocs={}
         )