Skip to content

WIP feat(yang-push): xpath normalization - #7

Open
rodonile wants to merge 5 commits into
xpath-namespacefrom
xpath-normalization
Open

WIP feat(yang-push): xpath normalization#7
rodonile wants to merge 5 commits into
xpath-namespacefrom
xpath-normalization

Conversation

@rodonile

@rodonile rodonile commented Aug 14, 2026

Copy link
Copy Markdown
Owner

This PR depends on network-analytics#43.

Summary

Normalizes datastore-xpath-filter targets — from both NETCONF/XML
subscription fetches and JSON-encoded SubscriptionStarted/
SubscriptionModified notifications — to RFC 8641's module-name-qualified,
prefix-on-change form, and adds a diagnostic that warns when a
target xpath doesn't resolve against the loaded schema or isn't in libyang's
canonical form.

Motivation

Different publishers/transports encode the same xpath target
differently: some use declared xmlns prefixes, some use bare
module-name prefixes, some repeat the prefix on every step instead
of only on module change. This caused issues to non-YANG-aware
post processing engines that are relying on xpath matching
for filtering messages.

Changes

  • Consolidated the scattered XPath 1.0 text-parsing helpers into a
    new crates/netconf-proto/src/xpath.rs module.
  • Added DatastoreXPathFilter::normalize_path, the canonicalization
    engine (bails safely to the original path on anything unsupported).
  • Wired it in on both ingestion paths: NETCONF/XML fetches (resolving
    declared prefixes via the router's YANG library) and JSON-encoded
    SubscriptionStarted/Modified targets.
  • Added check_xpath_target_resolves, a diagnostic that warns when a
    target xpath doesn't resolve against the loaded schema or isn't in
    libyang's canonical form.

@rodonile rodonile changed the title Xpath normalization WIP xpath normalization Aug 14, 2026
@rodonile rodonile changed the title WIP xpath normalization WIP feat(yang-push): xpath normalization Aug 14, 2026
@rodonile
rodonile force-pushed the xpath-normalization branch 2 times, most recently from ae0cf99 to 06692a6 Compare August 18, 2026 14:46
@rodonile
rodonile force-pushed the xpath-normalization branch from 06692a6 to 28b002b Compare August 20, 2026 14:23
@rodonile
rodonile force-pushed the xpath-normalization branch 2 times, most recently from 4ee03e6 to c4de624 Compare August 25, 2026 15:15
Move find_xpath_prefixes out of xml_utils.rs (a broad, unrelated
XML-parsing grab-bag) into a new xpath.rs module, the shared home
for XPath 1.0 subset text utilities. Register the module in
lib.rs and update its two call sites.

Pure refactor, no behavior change. Sets up xpath.rs as the target
for the normalize_path engine added next.
Add DatastoreXPathFilter::normalize_path: converts an xpath to
RFC 8641's canonical, module-name-qualified, prefix-on-change
form (matches libyang's SchemaPathFormat::DATA), whether the
source path uses declared xmlns prefixes or bare module-name
prefixes.

Backed by three new pure helpers in xpath.rs: split_location_path,
parse_node_test, is_ncname. Bails to None (caller keeps the
original path) for unsupported XPath 1.0 constructs or an
unresolvable declared prefix.

Not wired into any caller yet.
Apply DatastoreXPathFilter::normalize_path to the datastore
xpath filter fetched via get_yang_push_subscription_by_id, so
the cached target is always in canonical module-name-qualified
form regardless of how the device encoded prefixes (declared
xmlns vs. bare module name).

Falls back to the original path (with a warning) when the path
can't be confidently normalized.
JSON-encoded SubscriptionStarted/Modified notifications carry
their datastore-xpath-filter as a plain string with no xmlns
table. Normalize it the same way as the NETCONF/XML path, in
build_subscription_info, via normalize_json_target_xpath.

Needs no schema access: passing an empty namespace table makes
normalize_path treat every prefix as already-resolved. Reduces
noise in the canonical-form diagnostic and avoids spurious
subscription-changed refetches from pure prefix-style variance.
Add check_xpath_target_resolves: after a schema loads, evaluate
the subscription's datastore-xpath-filter against the libyang
context and warn if it does not resolve to a schema node, or
resolves but isn't in libyang's canonical (SchemaPathFormat::
DATA) form. Diagnostic only, never mutates the target.

Move xpath_diff and strip_xpath_predicates into netconf-proto's
xpath module so they're reusable and directly testable.
@rodonile
rodonile force-pushed the xpath-normalization branch from c4de624 to 6a66ed9 Compare August 25, 2026 16:12
@rodonile
rodonile requested a balanced review from Copilot August 26, 2026 05:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new normalization logic currently treats // (descendant axis) as acceptable input (instead of bailing as documented), and strip_xpath_predicates can silently truncate malformed inputs, leading to incorrect normalization/diagnostics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment on lines +417 to +420
// Empty segment: leading '/' (absolute) or '//' (descendant).
if seg.is_empty() {
continue;
}
Comment on lines +193 to +216
pub fn strip_xpath_predicates(path: &str) -> String {
let mut out = String::with_capacity(path.len());
let mut depth: u32 = 0;
let mut in_single = false;
let mut in_double = false;
for c in path.chars() {
if depth == 0 {
if c == '[' {
depth = 1;
} else {
out.push(c);
}
} else {
match c {
'\'' if !in_double => in_single = !in_single,
'"' if !in_single => in_double = !in_double,
'[' if !in_single && !in_double => depth += 1,
']' if !in_single && !in_double => depth -= 1,
_ => {}
}
}
}
out
}
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