Skip to content

fix(data-impl): validate unique in lists without schema paths - #53

Merged
lllyfeng merged 3 commits into
yang-central:mainfrom
Deepya1502:work/unique-null-schema-path
Oct 9, 2026
Merged

lllyfeng merged 3 commits into
yang-central:mainfrom
Deepya1502:work/unique-null-schema-path

Conversation

@Deepya1502

Copy link
Copy Markdown
Contributor

Summary

  • Calculate relative unique-leaf paths from schema ancestors when a list has no absolute schema path, instead of throwing a NullPointerException.
  • Add an anydata payload-list fixture with nested unique "details/name" and test both duplicate and distinct values.

This follows up the separate unique null-schema-path review on #50; it does not change anydata strictness.

Validation

  • mvn -q -pl yangkit-data-json-codec -am -Dtest=AnydataValidationOptionsJsonCodecTest,JsonCodecNotificationsTest -Dsurefire.failIfNoSpecifiedTests=false test

@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? Also, a reminder of the open question from my earlier review: why does getSchemaPath() return null for this list? It should already be set by the SCHEMA_TREE phase, so I'd like to understand the root cause before approving. Recording a validation error instead of throwing IllegalArgumentException when no ancestor is found would also match the rest of the codebase better.

M S Madhavan Iyer and others added 3 commits October 9, 2026 12:56
Derive unique leaf paths from schema ancestors when a list has no absolute schema path. Add payload-list regression tests for duplicate and distinct nested unique values.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Delegate schema build phases to SchemaNodeSupport after the composition refactor. Restore structure and generated operation child paths, remove the unique ancestor fallback, and record unresolved unique paths as validation errors. Cover lifecycle paths and unique validation regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Combine the unique details leaf with upstream's existing item list, preserving mandatory and cardinality constraints. Supply valid required fields in unique regression inputs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Deepya1502
Deepya1502 force-pushed the work/unique-null-schema-path branch from 9993deb to 527ac90 Compare October 9, 2026 07:29
@Deepya1502

Copy link
Copy Markdown
Contributor Author

Thanks for the comment @lllyfeng , You’re right: this list should already have a schema path after  SCHEMA_TREE . I traced the null back to the inheritance-to-composition refactor in  c18dd66d :  DataDefinitionImpl  delegates schema getters to  SchemaNodeSupport , but its build lifecycle never calls  SchemaNodeSupport.buildSelf() . The phase completes without assigning paths. The paths are already null before anydata deserialization, and the codec reuses the original schema nodes, so this is not specific to anydata.
I’ve prepared a local fix restoring that lifecycle delegation and removed the ancestor-walking workaround. I also addressed the related lifecycle gaps for operations and structures. Unique validation now records an  OPERATION_FAILED  error for missing paths or invalid ancestry rather than throwing, while continuing validation.
Regression tests cover the expected schema paths, duplicate/distinct unique values, missing paths, invalid ancestry, and continued validation. These and the full Maven package build pass locally.
Rebased the code onto latest main to resolve conflicts.

@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.

Thanks @Deepya1502, great root-cause analysis. Restoring the SchemaNodeSupport build lifecycle fixes the null schema paths introduced by c18dd66, which also caused an NPE in unique validation for any list on main. The tests fail on main and pass here, and CI is green. Approving and merging.

Two non-blocking follow-ups:

  1. Please update the PR title/description: the ancestor-walking workaround is gone, so it should describe restoring the schema path build lifecycle and fixing the unique-validation NPE (regression from c18dd66).
  2. When a unique constraint can't be resolved, matchUnique records the same OPERATION_FAILED once per list entry. It would be cleaner to resolve the unique paths once before the entry loop and record a single error. A small follow-up PR would be welcome.

@lllyfeng
lllyfeng merged commit 573a6f1 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