Skip to content

fix(xml-codec): reuse parsed list keys during child traversal - #52

Merged
lllyfeng merged 1 commit into
yang-central:mainfrom
Deepya1502:work/xml-existing-list-keys
Oct 9, 2026
Merged

lllyfeng merged 1 commit into
yang-central:mainfrom
Deepya1502:work/xml-existing-list-keys

Conversation

@Deepya1502

Copy link
Copy Markdown
Contributor

Summary

  • Reuse the already-parsed list key leaf when attaching XML list children, avoiding a second key deserialization and keeping the identifier key and data child identical.
  • Record YangDataException from key insertion as a validation record and continue parsing remaining children instead of throwing IllegalStateException.
  • Cover key identity across two list entries and an injected insertion failure that still parses a later child.

Independent follow-up to the list-key handling review on #50; this does not include the separate XML recursion refactor.

Validation

  • mvn -q -pl yangkit-data-xml-codec -am -Dtest=XmlCodecBasicTest,XmlCodecComprehensiveTest -Dsurefire.failIfNoSpecifiedTests=false test

@lllyfeng lllyfeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks @Deepya1502. The fix looks correct and the new test does cover it. This conflicts with #50 (and #51), so please rebase onto main once #50 is merged, before this goes in.

@lllyfeng

lllyfeng commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

#50 is now merged. @Deepya1502, could you please rebase this onto the latest main and resolve the conflicts? I'll merge once CI is green.

Attach the existing identifier key leaf instead of reparsing its XML element. Record add-key failures as validation results and continue processing remaining children; cover key identity and failure handling with XML tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Deepya1502
Deepya1502 force-pushed the work/xml-existing-list-keys branch from c144666 to 05b0d79 Compare October 9, 2026 05:28
@Deepya1502

Copy link
Copy Markdown
Contributor Author

@lllyfeng , rebased the branch onto main and committed the changes, conflicts have been resolved. Please check.
Thanks.

@lllyfeng
lllyfeng merged commit 778f2ca into yang-central:main Oct 9, 2026
1 check 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.

2 participants