CESNET / libyang

YANG data modeling language library
BSD 3-Clause "New" or "Revised" License
368 stars 292 forks source link

XPath is not evaluated over anydata nodes #2301

Open ahassany opened 1 month ago

ahassany commented 1 month ago

I using latest libyang from master to query YANG data trees with XPath, but I noticed XPath evaluation is skipping the anydata nodes. Is that by design or there's a bug somewhere?

To clarify my issue. In the following example, I'm trying to use XPath over a yang push notification. What comes after is defined as anydata in the schema. I can query everything for the following xpath /ietf-yang-push:push-change-update/ietf-yang-push:datastore-changes/ietf-yang-push:yang-patch/ietf-yang-push:edit/ietf-yang-push:value but it returns nothing for this xpath /ietf-yang-push:push-change-update/ietf-yang-push:datastore-changes/ietf-yang-push:yang-patch/ietf-yang-push:edit/ietf-yang-push:value/ietf-interfaces:interfaces

#include <libyang/libyang.h>
#include <stdint.h>
#include <stdio.h>

const char *notification_xml =
    "<notification xmlns=\"urn:ietf:params:xml:ns:netconf:notification:1.0\">"
    "  <eventTime>2023-08-25T10:00:00Z</eventTime>"
    "<push-change-update"
    "        xmlns=\"urn:ietf:params:xml:ns:yang:ietf-yang-push\">"
    "    <id>89</id>"
    "    <datastore-changes>"
    "        <yang-patch>"
    "            <patch-id>0</patch-id>"
    "            <edit>"
    "                <edit-id>edit1</edit-id>"
    "                <operation>replace</operation>"
    "                <target>/ietf-interfaces:interfaces</target>"
    "                <value>"
    "                    <interfaces"
    "                            xmlns=\"urn:ietf:params:xml:ns:yang:ietf-interfaces\">"
    "                        <interface>"
    "                            <name>eth0</name>"
    "                            <oper-status>down</oper-status>"
    "                        </interface>"
    "                    </interfaces>"
    "                </value>"
    "            </edit>"
    "        </yang-patch>"
    "    </datastore-changes>"
    "</push-change-update>"
    "</notification>";

int main()
{
    struct ly_ctx *ctx;
    struct lyd_node *tree;
    struct lyd_node *notification = NULL, *op = NULL;
    LY_ERR ret;

    // Initialize context
    ret = ly_ctx_new("~/yangmodels/yang/standard/ietf/RFC/", LY_CTX_ALL_IMPLEMENTED, &ctx);
    if (ret != LY_SUCCESS)
    {
        fprintf(stderr, "Failed to create context: %s\n", ly_errmsg(ctx));
        return 1;
    }

    /* Load ietf-interfaces module */
    const struct lys_module *ietf_interfaces_mod;
    ietf_interfaces_mod = ly_ctx_load_module(ctx, "ietf-interfaces", NULL, NULL);
    if (!ietf_interfaces_mod)
    {
        fprintf(stderr, "Failed to load ietf-interfaces module: %s\n", ly_errmsg(ctx));
        ly_ctx_destroy(ctx);
        return 1;
    }

    const struct lys_module *ietf_yang_push_mod;
    const char *features[] = {"on-change", NULL};
    ietf_yang_push_mod = ly_ctx_load_module(ctx, "ietf-yang-push", NULL, features);
    if (!ietf_yang_push_mod)
    {
        fprintf(stderr, "Failed to load ietf-yang-push module: %s\n", ly_errmsg(ctx));
        ly_ctx_destroy(ctx);
        return 1;
    }

    /* Parse the XML notification using lyd_parse_op */
    struct ly_in *in;
    ret = ly_in_new_memory(notification_xml, &in);
    if (ret != LY_SUCCESS)
    {
        fprintf(stderr, "Failed to create input handler: %s\n", ly_errmsg(ctx));
        ly_ctx_destroy(ctx);
        return 1;
    }

    ret = lyd_parse_op(ctx, NULL, in, LYD_XML, LYD_TYPE_NOTIF_NETCONF, &notification, &op);
    ly_in_free(in, 0);

    if (ret != LY_SUCCESS) {
        fprintf(stderr, "Failed to parse notification: %s\n", ly_errmsg(ctx));
        ly_ctx_destroy(ctx);
        return 1;
    }

    if (!notification) {
        fprintf(stderr, "Parsing succeeded but produced no data\n");
        ly_ctx_destroy(ctx);
        return 1;
    }

    struct ly_set *set = NULL;
    // XPATH till the first any data
    lyd_find_xpath(op, "/ietf-yang-push:push-change-update/ietf-yang-push:datastore-changes/ietf-yang-push:yang-patch/ietf-yang-push:edit/ietf-yang-push:value", &set);
    printf("Before any data found count: %d, and size: %d\n", set->count, set->size);
    for (uint32_t i=0; i < set->count; i++) {
        char *x;
        lyd_print_mem(&x, set->dnodes[i], LYD_JSON, 0);
        printf("OUT: %s\n", x);
    }
    ly_set_free(set, NULL);

    // XPATH till after the anydata node
    lyd_find_xpath(op, "/ietf-yang-push:push-change-update/ietf-yang-push:datastore-changes/ietf-yang-push:yang-patch/ietf-yang-push:edit/ietf-yang-push:value/ietf-interfaces:interfaces", &set);
    printf("In anydata found count: %d, and size: %d\n", set->count, set->size);
    for (uint32_t i=0; i < set->count; i++) {
        char *x;
        lyd_print_mem(&x, set->dnodes[i], LYD_JSON, 0);
        printf("OUT: %s\n", x);
    }
    ly_set_free(set, NULL);

    // Clean up
    lyd_free_all(op);
    lyd_free_all(notification);
    ly_ctx_destroy(ctx);
    return 0;
}

The output is:

Before any data found count: 1, and size: 1
OUT: {
  "ietf-yang-push:value": {
    "ietf-interfaces:interfaces": {
      "interface": [
        {
          "name": "eth0",
          "oper-status": "down"
        }
      ]
    }
  }
}

In anydata found count: 0, and size: 0
jktjkt commented 1 month ago

I think that XPath evaluation doesn't cross anyxml or anydata boundaries. In our code, if the struct lyd_node_any::value_type is LYD_ANYDATA_DATATREE, we access its .value.tree and that one effectively becomes a root node, see this unit test in our C++ bindings. I see that we have no tests for XPath evaluation "below" that node, though.

OTOH, when I read the docs, I see that lyd_find_path ignores "opaque" nodes, while lyd_find_xpath is said to support them...

ahassany commented 1 month ago

Thank you for the quick response, and thank you for the C++ reference I need to spend more time with it as I'm not very familiar with the C++ bindings.

for lyd_find_xpath I think it's a bug since the doc states that it crosses the opaque while the implementation doesn't work as expected.

michalvasko commented 1 month ago

You are considering any nodes as opaque nodes, they are not the same. When documentation mentions opaque nodes, it refers to opaque nodes that are part of the data tree, not those part of any nodes. The content of any nodes is not directly traversed/considered by any libyang functions.

We can discuss changing this but there are some problems bound to arise. For example, paths that cross module top-level nodes inside the path are supported for schema-mount nodes, which could then be confused with paths referencing nodes in any nodes.

ahassany commented 1 month ago

Thank you Michal for the explanation, however, The content of any nodes is not directly traversed/considered by any libyang functions. is a strong statement, at least the print statements traverses the any nodes. I only understand schema-mount on high-level but not sure how it's implemented in libyang.

So, I'm trying to dig a bit in the code just to understand better what's going, I noticed that in line https://github.com/CESNET/libyang/blob/master/src/tree_data_new.c#L289 the parent for any tree is set to NULL .. I hope I'm looking at the right place.

So my question, why is the parent of any is null? not the any data node? or it's parent?

michalvasko commented 1 month ago

at least the print statements traverses the any nodes.

Fair enough, printer does traverse these nodes.

why is the parent of any is null?

Because the contents of any nodes are treated as a separate data tree. They have no parent and no node has child set to this data tree. You need to manually go into the any node data tree.

ahassany commented 1 month ago

Is there a fundamental reason for that? or just implementation details? When parsing YANG data, I would assume yang will have one data tree not many.

michalvasko commented 1 month ago

The main reason is that a standard YANG data should begin with instances of top-level schema nodes and then include any descendants as children. In this case there would, again, be top-level nodes, which is confusing and may even cause ambiguities when referencing these nodes.

Also, this rule is broken because of schema-mount but it could not be implemented any other way.