diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ecaa987..b28bc19a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,6 +111,8 @@ Users may have to change how they access the system ### Corrected Bugs +* Fixed: [if choice is declared with multiple elements or leaf-list with in a case scope , addition or updation is not happening as expected](https://github.com/clicon/clixon/issues/327) + * This includes several choice/case adjustments to follow RFC 7950 Sec 7.9 better * Fixed: HTTP/1 parse error for '/' path * Fixed: YANG if-feature in config file of disables feature did not work, was always on * This does not apply to the datastore, only the config file itself. diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 841f2d92..20b8d432 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -274,6 +274,86 @@ check_when_condition(cxobj *x0p, goto done; } +/*! Check if choice nodes and implicitly remove all other cases. + * + * Special case is if yc parent (yp) is choice/case + * then find x0 child with same yc even though it does not match lexically + * However this will give another y0c != yc + * @param[in] x0 Base tree node + * @param[in] y1c Yang spec of tree child. If null revert to linear search. + * @retval 0 OK + * @retval -1 Error + * + * From RFC 7950 Sec 7.9 + * Since only one of the choice's cases can be valid at any time in the + * data tree, the creation of a node from one case implicitly deletes + * all nodes from all other cases. If a request creates a node from a + * case, the server will delete any existing nodes that are defined in + * other cases inside the choice. + */ +static int +check_delete_existing_case(cxobj *x0, + yang_stmt *y1c) +{ + int retval = -1; + yang_stmt *yp; + yang_stmt *y0case; + yang_stmt *y1case; + yang_stmt *y0choice; + yang_stmt *y1choice; + yang_stmt *y0c; + cxobj *x0c; + cxobj *x0prev; + + if ((yp = yang_parent_get(y1c)) == NULL) + goto ok; + if (yang_keyword_get(yp) == Y_CASE){ + y1case = yp; + y1choice = yang_parent_get(y1case); + } + else if (yang_keyword_get(yp) == Y_CHOICE){ + y1case = NULL; + y1choice = yp; + } + else + goto ok; + x0prev = NULL; + x0c = NULL; + while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) { + if ((y0c = xml_spec(x0c)) == NULL || + (yp = yang_parent_get(y0c)) == NULL){ + x0prev = x0c; + continue; + } + if (yang_keyword_get(yp) == Y_CASE){ + y0case = yp; + y0choice = yang_parent_get(y0case); + } + else if (yang_keyword_get(yp) == Y_CHOICE){ + y0case = NULL; + y0choice = yp; + } + else{ + x0prev = x0c; + continue; + } + if (y0choice == y1choice){ + if (y0case == NULL || + y0case != y1case){ + if (xml_purge(x0c) < 0) + goto done; + x0c = x0prev; + continue; + } + } + x0prev = x0c; + } + ok: + retval = 0; + done: + return retval; +} + /*! Modify a base tree x0 with x1 with yang spec y according to operation op * @param[in] h Clicon handle * @param[in] x0 Base xml tree (can be NULL in add scenarios) @@ -695,7 +775,7 @@ text_modify(clicon_handle h, } x1c = NULL; i = 0; - while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) { + while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) { x1cname = xml_name(x1c); /* Get yang spec of the child by child matching */ yc = yang_find_datanode(y0, x1cname); @@ -724,16 +804,13 @@ text_modify(clicon_handle h, x1cname, yang_find_mynamespace(y0)); goto done; } + /* Check if existing case should be deleted */ + if (check_delete_existing_case(x0, yc) < 0) + goto done; /* See if there is a corresponding node in the base tree */ x0c = NULL; if (match_base_child(x0, x1c, yc, &x0c) < 0) goto done; - if (x0c && (yc != xml_spec(x0c))){ - /* There is a match but is should be replaced (choice)*/ - if (xml_purge(x0c) < 0) - goto done; - x0c = NULL; - } x0vec[i++] = x0c; /* != NULL if x0c is matching x1c */ } /* Second pass: Loop through children of the x1 modification tree again diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index 4c3a2d6b..b0a1ecc0 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -1128,7 +1128,7 @@ check_list_unique_minmax(cxobj *xt, ych = y; keyw = yang_keyword_get(y); if (keyw != Y_LIST && keyw != Y_LEAF_LIST){ - if (yprev != NULL && y == yprev && yang_choice(y)==NULL){ + if (yprev != NULL && y == yprev){ /* Only lists and leaf-lists are allowed to be many * This checks duplicate container and leafs */ diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index 53f00a0f..3082b835 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -1148,10 +1148,6 @@ match_base_child(cxobj *x0, cg_var *cvi; cxobj *xb; char *keyname; - cxobj *x0c = NULL; - yang_stmt *y0c; - yang_stmt *y0p; - yang_stmt *yp; /* yang parent */ clixon_xvec *xvec = NULL; *x0cp = NULL; /* init return value */ @@ -1160,21 +1156,6 @@ match_base_child(cxobj *x0, *x0cp = xml_find(x0, xml_name(x1c)); goto ok; } - /* Special case is if yc parent (yp) is choice/case - * then find x0 child with same yc even though it does not match lexically - * However this will give another y0c != yc - */ - if ((yp = yang_choice(yc)) != NULL){ - x0c = NULL; - while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) { - if ((y0c = xml_spec(x0c)) != NULL && - (y0p = yang_choice(y0c)) != NULL && - y0p == yp) - break; /* x0c will have a value */ - } - *x0cp = x0c; - goto ok; /* What to do if not found? */ - } switch (yang_keyword_get(yc)){ case Y_CONTAINER: /* Equal regardless */ case Y_LEAF: /* Equal regardless */ diff --git a/test/test_choice.sh b/test/test_choice.sh index 97adda0c..6ec8b26a 100755 --- a/test/test_choice.sh +++ b/test/test_choice.sh @@ -148,7 +148,7 @@ if [ $BE -ne 0 ]; then start_backend -s init -f $cfg fi -new "waiting" +new "wait backend" wait_backend if [ $RC -ne 0 ]; then @@ -158,10 +158,11 @@ if [ $RC -ne 0 ]; then new "start restconf daemon" start_restconf -f $cfg - new "waiting" - wait_restconf fi +new "wait restconf" +wait_restconf + # First vanilla (protocol) case new "netconf validate empty" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" @@ -283,26 +284,32 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " # Choice with multiple items new "netconf choice multiple items first" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " 0 1 2 " "" "" +new "netconf get multiple items" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "012" "" + new "netconf validate multiple ok" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" new "netconf choice multiple items second" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " 0 1 " "" "" -new "netconf validate multiple ok" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" +new "netconf get items second" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "01" "" + +new "netconf validate items second expect fail" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "protocoloperation-failedtoo-many-elementserror/system/mb" new "netconf choice multiple items mix" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " 0 2 2 @@ -311,6 +318,23 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " new "netconf validate multiple error" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "applicationbad-elementmcerrorElement in choice statement already exists" +# Double merge +new "netconf choice single item" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " + 12 +" "" "" + +new "netconf choice merge second item" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " + 99 +" "" "" + +new "netconf get both items" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "1299" "" + +new "netconf validate ok" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + if [ $RC -ne 0 ]; then new "Kill restconf daemon" stop_restconf