Fix XML namespace serialization to only add xmlns when needed#3
Fix XML namespace serialization to only add xmlns when needed#3mischadiehm wants to merge 1 commit into
Conversation
The fields_to_elements method now accepts a parent_namespace parameter and only adds xmlns declarations when the child's namespace differs from the parent's namespace. This ensures proper namespace inheritance in XML output. Changes: - Add parent_namespace parameter to fields_to_elements() - Only add nsmap when parent_namespace is None or differs from self.namespace - Implement data_root wrapper in model_dump_xml_string() - Use __class__.model_fields to avoid Pydantic deprecation warnings - Remove unused imports (ruff lint fixes) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| elements: list[etree._Element] = [] | ||
|
|
||
| # Get field info from model class (not instance to avoid deprecation warning) | ||
| for field_name, field_info in self.__class__.model_fields.items(): |
There was a problem hiding this comment.
Accessing model_fields is deprecated and also does not support excluding defaults or None. And not really a reason to access it over __class__
| continue | ||
|
|
||
| # Get the XML element name from alias or field name | ||
| xml_name = field_info.alias if field_info.alias else field_name |
There was a problem hiding this comment.
Not sure why the AI thinks it is a good idea to extract the namespace from the field alias rather than using the classvar. There is a reason the class vars exist, and the alias could be wrong. I don't see it as clean software engineering to magically extract information from a field that is not designed for it.
There was a problem hiding this comment.
okay I see why it does it. In case the var name would be different to the field name. this could happen if the field name has a dash in it's name 🤔
| xml_name = field_info.alias if field_info.alias else field_name | ||
| # Strip prefix (e.g., "configuration:devicename" -> "devicename") | ||
| if ":" in xml_name: | ||
| xml_name = xml_name.split(":", 1)[1] |
There was a problem hiding this comment.
I am not a fan of this at all
| container_name=xml_name, | ||
| parent_namespace=self.namespace, | ||
| ) | ||
| elements.extend(child_elements) |
There was a problem hiding this comment.
Funny that it uses now the classvar
| root = etree.Element(container_name) | ||
| for elem in elements: | ||
| root.append(elem) | ||
| return [root] |
There was a problem hiding this comment.
it works but could be done without nesting if's and for loop. The extra loop is not sexy
|
|
||
| xml_name = field_info.alias if field_info.alias else field_name | ||
| if ":" in xml_name: | ||
| xml_name = xml_name.split(":", 1)[1] |
| if ":" in xml_name: | ||
| xml_name = xml_name.split(":", 1)[1] | ||
|
|
||
| if isinstance(value, XMLPydantifyModel): |
There was a problem hiding this comment.
not really needed as the check is done in fields_to_elements
|
|
||
| # Fallback: return self as root | ||
| elements = self.fields_to_elements(container_name=self.prefix) | ||
| return elements[0] if elements else etree.Element(self.prefix) |
There was a problem hiding this comment.
Not sure what the idea of the use of self.prefix is, but this does not make sense at all for me
2 participants
Summary
fields_to_elements()to only addxmlnsdeclaration when namespace differs from parentdata_rootwrapper inmodel_dump_xml_string()for NETCONF base namespace__class__.model_fieldsto avoid Pydantic deprecation warningsProblem
The current implementation incorrectly adds xmlns declarations on every element. It should only add xmlns when the child's namespace differs from the parent's.
Solution
Added
parent_namespaceparameter tofields_to_elements():nsmap={None: self.namespace}whenparent_namespace is Noneorself.namespace != parent_namespaceparent_namespace=self.namespacein all recursive callsTest plan
test_augment_xml_dump- validates different-namespace behavior (xmlns on each element where namespace changes)test_import_uses_xml_dump- validates same-namespace behavior (xmlns only on root, children inherit)🤖 Generated with Claude Code