Skip to content

Fix wrapper binding when collection item element name is reused - #1224

Merged
tefra merged 1 commit into
tefra:mainfrom
apoorvdarshan:fix-1142-wrapper-reused-item-name
Jul 10, 2026
Merged

Fix wrapper binding when collection item element name is reused#1224
tefra merged 1 commit into
tefra:mainfrom
apoorvdarshan:fix-1142-wrapper-reused-item-name

Conversation

@apoorvdarshan

Copy link
Copy Markdown
Contributor

📒 Description

Resolves #1142

Parsing a document with two wrapped collections on the same dataclass that reuse the same item element name failed. In the issue's example both <Foos> and <Bars> wrap items named <Property>, and parsing raised:

TypeError: Foo.__init__() missing 1 required positional argument: 'foo_id'

🔗 What I've Done

Root cause. A WrapperNode proxies the wrapped item to its parent ElementNode using only the bare item qname (Property). Both the build step (ElementNode.child) and the bind step (ElementNode.bind_object) resolve the field via meta.find_children(qname), which returns every field sharing that qname in declaration order and picks the first workable one. So items from <Bars> were routed into the foos field, and the mismatched item class failed to construct.

Fix. Scope the item lookup to the active wrapper qname:

  • WrapperNode now knows its wrapper element qname and passes it down when proxying to the parent.
  • ElementNode.child filters candidate vars by their wrapper_qname when a wrapper is active, and records the wrapper per item qname (in document order).
  • ElementNode.bind_object pops the recorded wrapper for each parsed item and binds it to the field whose wrapper_qname matches, so sibling wrappers reusing the same item name route to the correct field.

The change is confined to xsdata/formats/dataclass/parsers and leaves the non-wrapper code paths untouched (the wrapper argument defaults to None).

💬 Comments

Verified that serializing the same model already produced the expected XML, so this makes the parse/serialize round trip symmetric.

🛫 Checklist

Tests / Verification

  • Added WrapperTests.test_reused_item_name reproducing the issue. It fails on main with the reported TypeError and passes with this fix.
  • pytest tests/ — 1091 passed, 17 skipped.
  • ruff check / ruff format --check clean on the changed files; mypy reports no new errors.

Disclosure: prepared with AI assistance; reviewed and verified locally.

Two wrapped collections on the same dataclass that reuse the same item
element name (e.g. both <Foos> and <Bars> contain <Property>) failed to
parse. The wrapper node proxied the item to the parent by the bare item
qname, so both build and bind matched the first field sharing that qname,
routing every item into it and raising a TypeError for the required
arguments of the mismatched item class.

Scope the item lookup to the active wrapper qname: WrapperNode now knows
its wrapper element and passes it down, so ElementNode filters candidate
vars by their wrapper_qname when building child nodes and records the
wrapper per item to disambiguate binding.

Fixes tefra#1142.
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (00e6276) to head (1cbd430).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1224   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          116       116           
  Lines         9376      9390   +14     
  Branches      1439      1443    +4     
=========================================
+ Hits          9376      9390   +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@tefra tefra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tefra
tefra merged commit ef191cf into tefra:main Jul 10, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrapper fails if collection items element name is reused

2 participants