fix: some more improvements

Author: vinitkumar

json2xml/dicttoxml.py

@@ -8,12 +8,16 @@
 from typing import Any, Union
 
 from defusedxml.minidom import parseString
+from xml.parsers.expat import ExpatError
 
 # Create a safe random number generator
 
 # Set up logging
 LOG = logging.getLogger("dicttoxml")
 
+# Module-level set for true uniqueness tracking
+_used_ids: set[str] = set()
+
 
 def make_id(element: str, start: int = 100000, end: int = 999999) -> str:
     """
@@ -41,16 +45,11 @@ def get_unique_id(element: str) -> str:
     Returns:
         str: The unique ID.
     """
-    ids: list[str] = []  # initialize list of unique ids
     this_id = make_id(element)
-    dup = True
-    while dup:
-        if this_id not in ids:
-            dup = False
-            ids.append(this_id)
-        else:
-            this_id = make_id(element)
-    return ids[-1]
+    while this_id in _used_ids:
+        this_id = make_id(element)
+    _used_ids.add(this_id)
+    return this_id
 
 
 ELEMENT = Union[
@@ -77,23 +76,22 @@ def get_xml_type(val: ELEMENT) -> str:
     Returns:
         str: The XML type.
     """
-    if val is not None:
-        if type(val).__name__ in ("str", "unicode"):
-            return "str"
-        if type(val).__name__ in ("int", "long"):
-            return "int"
-        if type(val).__name__ == "float":
-            return "float"
-        if type(val).__name__ == "bool":
-            return "bool"
-        if isinstance(val, numbers.Number):
-            return "number"
-        if isinstance(val, dict):
-            return "dict"
-        if isinstance(val, Sequence):
-            return "list"
-    else:
+    if val is None:
         return "null"
+    if isinstance(val, bool):  # Check bool before int (bool is subclass of int)
+        return "bool"
+    if isinstance(val, int):
+        return "int"
+    if isinstance(val, float):
+        return "float"
+    if isinstance(val, str):
+        return "str"
+    if isinstance(val, numbers.Number):
+        return "number"
+    if isinstance(val, dict):
+        return "dict"
+    if isinstance(val, Sequence):
+        return "list"
     return type(val).__name__
 
 
@@ -102,19 +100,19 @@ def escape_xml(s: str | int | float | numbers.Number) -> str:
     Escape a string for use in XML.
 
     Args:
-        s (str | numbers.Number): The string to escape.
+        s (str | int | float | numbers.Number): The string to escape.
 
     Returns:
         str: The escaped string.
     """
+    s_str = str(s)  # Convert to string once
     if isinstance(s, str):
-        s = str(s)  # avoid UnicodeDecodeError
-        s = s.replace("&", "&")
-        s = s.replace('"', """)
-        s = s.replace("'", "'")
-        s = s.replace("<", "&lt;")
-        s = s.replace(">", "&gt;")
-    return str(s)
+        s_str = s_str.replace("&", "&amp;")
+        s_str = s_str.replace('"', "&quot;")
+        s_str = s_str.replace("'", "&apos;")
+        s_str = s_str.replace("<", "&lt;")
+        s_str = s_str.replace(">", "&gt;")
+    return s_str
 
 
 def make_attrstring(attr: dict[str, Any]) -> str:
@@ -145,37 +143,39 @@ def key_is_valid_xml(key: str) -> bool:
     try:
         parseString(test_xml)
         return True
-    except Exception:  # minidom does not implement exceptions well
+    except (ExpatError, ValueError) as e:
+        LOG.debug(f"Invalid XML name '{key}': {e}")
         return False
 
 
-def make_valid_xml_name(key: str, attr: dict[str, Any]) -> tuple[str, dict[str, Any]]:
+def make_valid_xml_name(key: str | int, attr: dict[str, Any]) -> tuple[str, dict[str, Any]]:
     """Tests an XML name and fixes it if invalid"""
-    key = escape_xml(key)
+    key_str = str(key)  # Ensure we're working with strings
+    key_str = escape_xml(key_str)
     # nothing happens at escape_xml if attr is not a string, we don't
     # need to pass it to the method at all.
     # attr = escape_xml(attr)
 
     # pass through if key is already valid
-    if key_is_valid_xml(key):
-        return key, attr
+    if key_is_valid_xml(key_str):
+        return key_str, attr
 
     # prepend a lowercase n if the key is numeric
-    if isinstance(key, int) or key.isdigit():
-        return f"n{key}", attr
+    if key_str.isdigit():
+        return f"n{key_str}", attr
 
     # replace spaces with underscores if that fixes the problem
-    if key_is_valid_xml(key.replace(" ", "_")):
-        return key.replace(" ", "_"), attr
+    if key_is_valid_xml(key_str.replace(" ", "_")):
+        return key_str.replace(" ", "_"), attr
 
     # allow namespace prefixes + ignore @flat in key
-    if key_is_valid_xml(key.replace(":", "").replace("@flat", "")):
-        return key, attr
+    if key_is_valid_xml(key_str.replace(":", "").replace("@flat", "")):
+        return key_str, attr
 
     # key is still invalid - move it into a name attribute
-    attr["name"] = key
-    key = "key"
-    return key, attr
+    attr["name"] = key_str
+    key_str = "key"
+    return key_str, attr
 
 
 def wrap_cdata(s: str | int | float | numbers.Number) -> str:
@@ -188,6 +188,25 @@ def default_item_func(parent: str) -> str:
     return "item"
 
 
+def _build_namespace_string(xml_namespaces: dict[str, Any]) -> str:
+    """Build XML namespace string from namespace dictionary."""
+    parts = []
+    
+    for prefix, value in xml_namespaces.items():
+        if prefix == 'xsi' and isinstance(value, dict):
+            for schema_att, ns in value.items():
+                if schema_att == 'schemaInstance':
+                    parts.append(f'xmlns:{prefix}="{ns}"')
+                elif schema_att == 'schemaLocation':
+                    parts.append(f'xsi:{schema_att}="{ns}"')
+        elif prefix == 'xmlns':
+            parts.append(f'xmlns="{value}"')
+        else:
+            parts.append(f'xmlns:{prefix}="{value}"')
+    
+    return ' ' + ' '.join(parts) if parts else ''
+
+
 def convert(
     obj: ELEMENT,
     ids: Any,
@@ -262,7 +281,6 @@ def dict2xml_str(
     parse dict2xml
     """
     ids: list[str] = []  # initialize list of unique ids
-    ", ".join(str(key) for key in item)
     subtree = ""  # Initialize subtree with default empty string
 
     if attr_type:
@@ -562,7 +580,7 @@ def dicttoxml(
     item_wrap: bool = True,
     item_func: Callable[[str], str] = default_item_func,
     cdata: bool = False,
-    xml_namespaces: dict[str, Any] = {},
+    xml_namespaces: dict[str, Any] | None = None,
     list_headers: bool = False
 ) -> bytes:
     """
@@ -681,26 +699,11 @@ def dicttoxml(
         <list a="b" c="d"><item>4</item><item>5</item><item>6</item></list>
 
     """
+    if xml_namespaces is None:
+        xml_namespaces = {}
+    
     output = []
-    namespace_str = ""
-    for prefix in xml_namespaces:
-        if prefix == 'xsi':
-            for schema_att in xml_namespaces[prefix]:
-                if schema_att == 'schemaInstance':
-                    ns = xml_namespaces[prefix]['schemaInstance']
-                    namespace_str += f' xmlns:{prefix}="{ns}"'
-                elif schema_att == 'schemaLocation':
-                    ns = xml_namespaces[prefix][schema_att]
-                    namespace_str += f' xsi:{schema_att}="{ns}"'
-
-        elif prefix == 'xmlns':
-            # xmns needs no prefix
-            ns = xml_namespaces[prefix]
-            namespace_str += f' xmlns="{ns}"'
-
-        else:
-            ns = xml_namespaces[prefix]
-            namespace_str += f' xmlns:{prefix}="{ns}"'
+    namespace_str = _build_namespace_string(xml_namespaces)
     if root:
         output.append('<?xml version="1.0" encoding="UTF-8" ?>')
         output_elem = convert(