Skip to content

fix(data-mapper-v2): resolve XML attribute paths when loading saved maps#8770

Merged
takyyon merged 2 commits intomainfrom
fix/data-mapper-attribute-deserialization
Feb 4, 2026
Merged

fix(data-mapper-v2): resolve XML attribute paths when loading saved maps#8770
takyyon merged 2 commits intomainfrom
fix/data-mapper-attribute-deserialization

Conversation

@takyyon
Copy link
Contributor

@takyyon takyyon commented Jan 31, 2026

Commit Type

  • fix - Bug fix
  • feature
  • refactor
  • perf
  • docs
  • test
  • chore

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Fixes bug where Data Mapper fails to load saved maps containing XML attribute bindings within loops. The error "Source schema node not found, or invalid custom value for key './@id'" occurred because relative attribute paths (./@AttributeName) were not being properly resolved during deserialization.

Root cause: When constructing the schema lookup path, ./@Attr was concatenated as /loop/path/./@Attr instead of /loop/path/@Attr.

Impact of Change

  • Users: Saved data maps with XML attribute bindings in loops will now load correctly without errors
  • Developers: Small change to MapDefinitionDeserializer.ts - normalizes ./@ paths by stripping ./ prefix
  • System: No performance or architecture impact

Test Plan

  • Unit tests added/updated - Re-enabled previously skipped test "creates a looping connection w/ index variable, conditional, and relative attribute path"
  • Existing deserializer tests pass (67 tests)
  • Existing serializer tests pass (33 tests)
  • Full data-mapper-v2 test suite passes (220 tests)
  • Manual verification with customer's XSD/JSON schemas (pending)

Contributors

No additional contributors.

🤖 Generated with Claude Code

When deserializing data maps containing XML attribute bindings within loops,
the relative attribute path format './@AttributeName' was not being properly
resolved back to the full schema node key. This caused "Source schema node
not found" errors when reloading saved maps.

The fix normalizes './@' prefixed paths by stripping the './' before
constructing the full schema path lookup.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 31, 2026 02:13
@github-actions
Copy link

github-actions bot commented Jan 31, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(data-mapper-v2): resolve XML attribute paths when loading saved maps
  • Issue: None — title is clear, concise, and follows conventional commit style (scope + short description).
  • Recommendation: Keep as-is. Optionally include an issue number if this addresses a tracked bug (e.g., (fixes #1234)).

Commit Type

  • Properly selected (fix).
  • Only one box selected, which is correct for this change.

Risk Level

  • The PR body selects Low and the repo labels include risk:low. The code diff is small (4 additions, 3 deletions across 2 files) and the change is narrowly scoped to normalizing relative attribute paths in the deserializer — the assigned risk level is appropriate.

What & Why

  • Current: Fixes bug where Data Mapper fails to load saved maps containing XML attribute bindings in loops by normalizing './@attr' to '@attr' during deserialization.
  • Issue: None — explanation is clear and includes root cause.
  • Recommendation: Optionally add a short reference to a failing test or issue number that originally demonstrated this behavior for future traceability.

Impact of Change

  • The Impact section is present and aligns with the diff.
  • Recommendation: Minor suggestions to make it even clearer:
    • Users: Saved data maps with XML attribute bindings in loops will now load correctly (already present).
    • Developers: Call out the exact file changed (MapDefinitionDeserializer.ts) and mention the test that was re-enabled/unskipped (MapDefinitionDeserializer.spec.ts).
    • System: No performance or architecture impact (already stated).

Test Plan

  • Unit test update is visible in the diff: a previously skipped test was re-enabled (creates a looping connection w/ index variable, conditional, and relative attribute path). That matches the claim "Unit tests added/updated".
  • The PR body also claims the broader suites pass — ensure CI is green before merging. If CI does not run here, please run the suite locally and attach CI logs or a short note in the PR.
  • Recommendation: If not already covered by the re-enabled test, consider adding a focused unit test asserting the normalization behavior for a standalone case (input key startsWith './@' leads to lookup of '@attr' without './'). This makes the regression explicit and easier to maintain.

Contributors

  • Statement present: "No additional contributors." That is fine. (Optional: credit any reviewer or PM if they contributed to the design/discussion.)

Screenshots/Videos

  • Not applicable for this non-visual change. Good to leave blank.

Summary Table

Section Status Recommendation
Title Keep as-is; consider adding issue ref if available.
Commit Type Good — fix selected.
Risk Level risk:low label is present and matches body.
What & Why Clear; optionally add issue/test reference.
Impact of Change Good; optionally name changed file(s) explicitly.
Test Plan Unit test un-skipped in diff; ensure CI passes and consider a focused test for normalization.
Contributors Fine.
Screenshots/Videos N/A for this change.

Final notes

  • The PR title and body comply with the template and accurately reflect the code changes in the diff. The risk level is appropriate (low) given the small, targeted change.
  • Before merging, please ensure the CI test run is green and (if possible) include a short CI status note in the PR. If you can, add a small unit test that explicitly asserts that keys beginning with ./@ are normalized to @ when looked up — this will make the fix more robust and clearly documented in the test suite.

Please update the PR with the CI result or an additional focused test if convenient, then proceed with merge. Thank you for the clear description and small, focused change!


Last updated: Wed, 04 Feb 2026 21:07:25 GMT

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where the Data Mapper v2 fails to load saved maps containing XML attribute bindings within loops. The error occurred because relative attribute paths (./@AttributeName) were not being properly resolved during deserialization, resulting in malformed paths like /loop/path/./@Attr instead of /loop/path/@Attr.

Changes:

  • Added normalization logic to strip the ./ prefix from ./@ attribute paths when resolving relative keys within loops
  • Re-enabled a previously skipped test case that validates this exact scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
libs/data-mapper-v2/src/mapHandling/MapDefinitionDeserializer.ts Added path normalization in getSourceNodeForRelativeKeyInLoop() to handle ./@ attribute patterns correctly by stripping the ./ prefix before path construction
libs/data-mapper-v2/src/mapHandling/test/MapDefinitionDeserializer.spec.ts Re-enabled test case that validates looping connections with index variables, conditionals, and relative attribute paths

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@takyyon takyyon added the risk:low Low risk change with minimal impact label Feb 4, 2026
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

📊 Coverage check completed. See workflow run for details.

@takyyon takyyon merged commit 3a74489 into main Feb 4, 2026
13 checks passed
@takyyon takyyon deleted the fix/data-mapper-attribute-deserialization branch February 4, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants