lib: implement missing YANG choice/case statements.

Signed-off-by: Christian Hopps <chopps@labn.net>

diff --git a/lib/northbound_oper.c b/lib/northbound_oper.c
index b5201c6306c8be205e2579b45f711a39ee216496..b796c9d04c6ac970cac9c66a43b6f2363863a5cf 100644 (file)
--- a/lib/northbound_oper.c
+++ b/lib/northbound_oper.c
@@ -69,8 +69,10 @@ struct nb_op_node_info {
  * @xpath: current xpath representing the node_info stack.
  * @xpath_orig: the original query string from the user
  * @node_infos: the container stack for the walk from root to current
- * @schema_path: the schema nodes for each node in the query string.
- # @query_tokstr: the query string tokenized with NUL bytes.
+ * @schema_path: the schema nodes along the path indicated by the query string.
+ *               this will include the choice and case nodes which are not
+ *               present in the query string.
+ * @query_tokstr: the query string tokenized with NUL bytes.
  * @query_tokens: the string pointers to each query token (node).
  * @non_specific_predicate: tracks if a query_token is non-specific predicate.
  * @walk_root_level: The topmost specific node, +1 is where we start walking.
@@ -204,6 +206,14 @@ static void ys_pop_inner(struct nb_op_yield_state *ys)
        ys_trim_xpath(ys);
 }
 
+static void ys_free_inner(struct nb_op_yield_state *ys,
+                         struct nb_op_node_info *ni)
+{
+       if (!CHECK_FLAG(ni->schema->nodetype, LYS_CASE | LYS_CHOICE))
+               lyd_free_tree(&ni->inner->node);
+       ni->inner = NULL;
+}
+
 static void nb_op_get_keys(struct lyd_node_inner *list_node,
                           struct yang_list_keys *keys)
 {
@@ -254,8 +264,7 @@ static bool __move_back_to_next(struct nb_op_yield_state *ys, int i)
         * The i'th node has been lost after a yield so trim it from the tree
         * now.
         */
-       lyd_free_tree(&ni->inner->node);
-       ni->inner = NULL;
+       ys_free_inner(ys, ni);
        ni->list_entry = NULL;
 
        /*
@@ -296,7 +305,7 @@ static void nb_op_resume_data_tree(struct nb_op_yield_state *ys)
                ni = &ys->node_infos[i];
                nn = ni->schema->priv;
 
-               if (CHECK_FLAG(ni->schema->nodetype, LYS_CONTAINER))
+               if (!CHECK_FLAG(ni->schema->nodetype, LYS_LIST))
                        continue;
 
                assert(ni->list_entry != NULL ||
@@ -418,7 +427,8 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys,
        /* Assert that we are walking the rightmost branch */
        assert(!inner->parent || &inner->node == inner->parent->child->prev);
 
-       if (CHECK_FLAG(inner->schema->nodetype, LYS_CONTAINER)) {
+       if (CHECK_FLAG(inner->schema->nodetype,
+                      LYS_CASE | LYS_CHOICE | LYS_CONTAINER)) {
                /* containers have only zero or one child on a branch of a tree */
                inner = (struct lyd_node_inner *)inner->child;
                assert(!inner || inner->prev == &inner->node);
@@ -502,9 +512,12 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys)
                        ret = NB_ERR_NOT_FOUND;
                return ret;
        }
-       while (node &&
-              !CHECK_FLAG(node->schema->nodetype, LYS_CONTAINER | LYS_LIST))
+
+       /* Move up to the container if on a leaf currently. */
+       if (node &&
+           !CHECK_FLAG(node->schema->nodetype, LYS_CONTAINER | LYS_LIST))
                node = &node->parent->node;
+       assert(CHECK_FLAG(node->schema->nodetype, LYS_CONTAINER | LYS_LIST));
        if (!node)
                return NB_ERR_NOT_FOUND;
 
@@ -512,7 +525,6 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys)
        for (len = 1; inner->parent; len++)
                inner = inner->parent;
 
-
        darr_append_nz_mt(ys->node_infos, len, MTYPE_NB_NODE_INFOS);
 
        /*
@@ -761,7 +773,7 @@ static const struct lysc_node *nb_op_sib_next(struct nb_op_yield_state *ys,
        /*
         * If the node info stack is shorter than the schema path then we are
         * doign specific query still on the node from the schema path (should
-        * match) so just return NULL.
+        * match) so just return NULL (i.e., don't process siblings)
         */
        if (darr_len(ys->schema_path) > darr_len(ys->node_infos))
                return NULL;
@@ -826,7 +838,10 @@ static const struct lysc_node *nb_op_sib_first(struct nb_op_yield_state *ys,
         * base of the user query, return the next schema node from the query
         * string (schema_path).
         */
-       assert(darr_last(ys->node_infos) != NULL && darr_last(ys->node_infos)->schema == parent);
+       if (darr_last(ys->node_infos) != NULL &&
+           !CHECK_FLAG(darr_last(ys->node_infos)->schema->nodetype,
+                       LYS_CASE | LYS_CHOICE))
+               assert(darr_last(ys->node_infos)->schema == parent);
        if (darr_lasti(ys->node_infos) < ys->query_base_level)
                return ys->schema_path[darr_lasti(ys->node_infos) + 1];
 
@@ -937,28 +952,30 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
                if (!sib) {
                        /*
                         * We've reached the end of the siblings inside a
-                        * containing node; either a container or a specific
-                        * list node entry.
+                        * containing node; either a container, case, choice, or
+                        * a specific list node entry.
                         *
-                        * We handle container node inline; however, for lists
-                        * we are only done with a specific entry and need to
-                        * move to the next element on the list so we drop down
-                        * into the switch for that case.
+                        * We handle case/choice/container node inline; however,
+                        * for lists we are only done with a specific entry and
+                        * need to move to the next element on the list so we
+                        * drop down into the switch for that case.
                         */
 
                        /* Grab the containing node. */
                        sib = ni->schema;
 
-                       if (sib->nodetype == LYS_CONTAINER) {
+                       if (CHECK_FLAG(sib->nodetype,
+                                      LYS_CASE | LYS_CHOICE | LYS_CONTAINER)) {
                                /* If we added an empty container node (no
                                 * children) and it's not a presence container
                                 * or it's not backed by the get_elem callback,
                                 * remove the node from the tree.
                                 */
-                               if (!lyd_child(&ni->inner->node) &&
+                               if (sib->nodetype == LYS_CONTAINER &&
+                                   !lyd_child(&ni->inner->node) &&
                                    !nb_op_empty_container_ok(sib, ys->xpath,
                                                              ni->list_entry))
-                                       lyd_free_tree(&ni->inner->node);
+                                       ys_free_inner(ys, ni);
 
                                /* If we have returned to our original walk base,
                                 * then we are done with the walk.
@@ -995,11 +1012,13 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
                         */
                }
 
-               /* TODO: old code checked for "first" here and skipped if set */
                if (CHECK_FLAG(sib->nodetype,
                               LYS_LEAF | LYS_LEAFLIST | LYS_CONTAINER))
                        xpath_child = nb_op_get_child_path(ys->xpath, sib,
                                                           xpath_child);
+               else if (CHECK_FLAG(sib->nodetype, LYS_CASE | LYS_CHOICE))
+                       darr_in_strdup(xpath_child, ys->xpath);
+
                nn = sib->priv;
 
                switch (sib->nodetype) {
@@ -1032,27 +1051,31 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
                                goto done;
                        sib = nb_op_sib_next(ys, sib);
                        continue;
+               case LYS_CASE:
+               case LYS_CHOICE:
                case LYS_CONTAINER:
                        if (CHECK_FLAG(nn->flags, F_NB_NODE_CONFIG_ONLY)) {
                                sib = nb_op_sib_next(ys, sib);
                                continue;
                        }
 
-                       node = NULL;
-                       err = lyd_new_inner(&ni->inner->node, sib->module,
-                                           sib->name, false, &node);
-                       if (err) {
-                               ret = NB_ERR_RESOURCE;
-                               goto done;
+                       if (sib->nodetype != LYS_CONTAINER) {
+                               /* Case/choice use parent inner. */
+                               node = &ni->inner->node;
+                       } else {
+                               err = lyd_new_inner(&ni->inner->node,
+                                                   sib->module, sib->name,
+                                                   false, &node);
+                               if (err) {
+                                       ret = NB_ERR_RESOURCE;
+                                       goto done;
+                               }
                        }
 
-                       /* push this container node on top of the stack */
+                       /* push this choice/container node on top of the stack */
                        ni = darr_appendz(ys->node_infos);
                        ni->inner = (struct lyd_node_inner *)node;
-                       ni->schema = node->schema;
-                       ni->niters = 0;
-                       ni->nents = 0;
-                       ni->has_lookup_next = false;
+                       ni->schema = sib;
                        ni->lookup_next_ok = ni[-1].lookup_next_ok;
                        ni->list_entry = ni[-1].list_entry;
 
@@ -1279,7 +1302,7 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
                              lysc_is_key((*darr_last(ys->schema_path)))) &&
                            /* is this at or below the base? */
                            darr_ilen(ys->node_infos) <= ys->query_base_level)
-                               lyd_free_tree(&ni->inner->node);
+                               ys_free_inner(ys, ni);
 
 
                        if (!list_entry) {
@@ -1433,12 +1456,6 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
                        sib = nb_op_sib_first(ys, sib);
                        continue;
 
-               case LYS_CHOICE:
-                       /* Container type with no data */
-                       /*FALLTHROUGH*/
-               case LYS_CASE:
-                       /* Container type with no data */
-                       /*FALLTHROUGH*/
                default:
                        /*FALLTHROUGH*/
                case LYS_ANYXML:
@@ -1534,8 +1551,7 @@ static void nb_op_trim_yield_state(struct nb_op_yield_state *ys)
 
        DEBUGD(&nb_dbg_events, "NB oper-state: deleting tree at level %d", i);
        __free_siblings(&ni->inner->node);
-       lyd_free_tree(&ni->inner->node);
-       ni->inner = NULL;
+       ys_free_inner(ys, ni);
 
        while (--i > 0) {
                DEBUGD(&nb_dbg_events,
@@ -1648,6 +1664,17 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys,
                int nlen = strlen(name);
                int mnlen = 0;
 
+               /*
+                * Technically the query_token for choice/case should probably be pointing at
+                * the child (leaf) rather than the parent (container), however,
+                * we only use these for processing list nodes so KISS.
+                */
+               if (CHECK_FLAG(ys->schema_path[i]->nodetype,
+                              LYS_CASE | LYS_CHOICE)) {
+                       ys->query_tokens[i] = ys->query_tokens[i - 1];
+                       continue;
+               }
+
                while (true) {
                        s2 = strstr(s, name);
                        if (!s2)

diff --git a/tests/lib/northbound/test_oper_data.c b/tests/lib/northbound/test_oper_data.c
index 9ac75da4fef832ebe32cb7195cbe17d7aa129858..8f7e7c5f8c7170b3a668bc0435a77445839347e9 100644 (file)
--- a/tests/lib/northbound/test_oper_data.c
+++ b/tests/lib/northbound/test_oper_data.c
@@ -200,6 +200,25 @@ static struct yang_data *frr_test_module_vrfs_vrf_routes_route_active_get_elem(
        return NULL;
 }
 
+
+/*
+ * XPath: /frr-test-module:frr-test-module/c1value
+ */
+static struct yang_data *
+frr_test_module_c1value_get_elem(struct nb_cb_get_elem_args *args)
+{
+       return yang_data_new_uint8(args->xpath, 21);
+}
+
+/*
+ * XPath: /frr-test-module:frr-test-module/c2cont/c2value
+ */
+static struct yang_data *
+frr_test_module_c2cont_c2value_get_elem(struct nb_cb_get_elem_args *args)
+{
+       return yang_data_new_uint32(args->xpath, 0xAB010203);
+}
+
 /* clang-format off */
 const struct frr_yang_module_info frr_test_module_info = {
        .name = "frr-test-module",
@@ -243,6 +262,14 @@ const struct frr_yang_module_info frr_test_module_info = {
                        .xpath = "/frr-test-module:frr-test-module/vrfs/vrf/routes/route/active",
                        .cbs.get_elem = frr_test_module_vrfs_vrf_routes_route_active_get_elem,
                },
+               {
+                       .xpath = "/frr-test-module:frr-test-module/c1value",
+                       .cbs.get_elem = frr_test_module_c1value_get_elem,
+               },
+               {
+                       .xpath = "/frr-test-module:frr-test-module/c2cont/c2value",
+                       .cbs.get_elem = frr_test_module_c2cont_c2value_get_elem,
+               },
                {
                        .xpath = NULL,
                },

diff --git a/tests/lib/northbound/test_oper_data.refout b/tests/lib/northbound/test_oper_data.refout
index 57ecd2f0a0a1b0a0424c768100aaa9a51486821a..aa930fe127bea42c5dd723588acca2d00f4e06fe 100644 (file)
--- a/tests/lib/northbound/test_oper_data.refout
+++ b/tests/lib/northbound/test_oper_data.refout
@@ -112,6 +112,10 @@ test# show yang operational-data /frr-test-module:frr-test-module
           }\r
         }\r
       ]\r
+    },\r
+    "c1value": 21,\r
+    "c2cont": {\r
+      "c2value": 2868969987\r
     }\r
   }\r
 }\r

diff --git a/yang/frr-test-module.yang b/yang/frr-test-module.yang
index d6e7188240684319e22d0e2e5a454fedbb5df656..6cc60e866533fdd7fcbc9c328b2735cfbe116670 100644 (file)
--- a/yang/frr-test-module.yang
+++ b/yang/frr-test-module.yang
@@ -82,5 +82,23 @@ module frr-test-module {
         }
       }
     }
+    choice achoice {
+      description "a choice statement";
+      case case1 {
+        leaf c1value {
+          type uint8;
+          description "A uint8 value for case 1";
+        }
+      }
+      case case2 {
+        container c2cont {
+          description "case 2 container";
+          leaf c2value {
+            type uint32;
+            description "A uint32 value for case 2";
+          }
+        }
+      }
+    }
   }
 }