diff --git a/CHANGELOG.md b/CHANGELOG.md index f0afd172..ce4e843a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [Nested YANG choice does not work #342](https://github.com/clicon/clixon/issues/342) * Fixed: [YANG if-feature does not support nested boolean expression](https://github.com/clicon/clixon/issues/341) * Fixed: [RPC edit-config payloads are not fully validated](https://github.com/clicon/clixon/issues/337) diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index b9ca299f..fe6bdf42 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -274,50 +274,22 @@ 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. +/*! Given choice/case, remove all other cases. */ static int -check_delete_existing_case(cxobj *x0, - yang_stmt *y1c) +choice_delete_existing_children(cxobj *x0, + yang_stmt *y1c, + yang_stmt *y1case, + yang_stmt *y1choice) { int retval = -1; - yang_stmt *yp; - yang_stmt *y0p; - yang_stmt *y0case; - yang_stmt *y1case; - yang_stmt *y0choice; - yang_stmt *y1choice; - yang_stmt *y0c; cxobj *x0c; + yang_stmt *y0c; + yang_stmt *y0case; + yang_stmt *y0choice; 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; + yang_stmt *y0p; + /* Now traverse existing tree and compare with choice yang structure of added tree */ x0prev = NULL; x0c = NULL; @@ -351,6 +323,54 @@ check_delete_existing_case(cxobj *x0, } x0prev = x0c; } + retval = 0; + done: + return retval; +} + +/*! 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 *y1case; + yang_stmt *y1choice; + + 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; + if (choice_delete_existing_children(x0, y1c, y1case, y1choice) < 0) + goto done; + /* Recursive call */ + if (check_delete_existing_case(x0, y1choice) < 0) + goto done; ok: retval = 0; done: @@ -772,7 +792,7 @@ text_modify(clicon_handle h, /* First pass: Loop through children of the x1 modification tree * collect matching nodes from x0 in x0vec (no changes to x0 children) */ - if ((x0vec = calloc(xml_child_nr(x1), sizeof(x1))) == NULL){ + if ((x0vec = calloc(xml_child_nr_type(x1, CX_ELMNT), sizeof(x1))) == NULL){ clicon_err(OE_UNIX, errno, "calloc"); goto done; } diff --git a/test/test_choice_recursive.sh b/test/test_choice_recursive.sh new file mode 100755 index 00000000..e9b48a17 --- /dev/null +++ b/test/test_choice_recursive.sh @@ -0,0 +1,141 @@ +#!/usr/bin/env bash +# Test hierarchical choices +# See eg https://github.com/clicon/clixon/issues/342 + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + +APPNAME=example + +cfg=$dir/choice.xml +clidir=$dir/cli +fyang=$dir/type.yang + +test -d ${clidir} || rm -rf ${clidir} +mkdir $clidir + +cat < $cfg + + $cfg + clixon-restconf:allow-auth-none + $dir + ${YANG_INSTALLDIR} + $fyang + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + $clidir + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + +EOF + +cat < $fyang +module system{ + yang-version 1.1; + namespace "urn:example:config"; + prefix ex; + container c { + choice top{ + case topA { + choice A{ + leaf A1x{ + type string; + } + leaf A2x{ + type string; + } + } + leaf Ay{ + type string; + } + } + case topB{ + choice B{ + case B1{ + leaf B1x{ + type string; + } + } + case B2{ + leaf B2x{ + type string; + } + } + } + leaf By{ + type string; + } + } + } + } +} +EOF + +cat < $clidir/ex.cli +# Clixon example specification +CLICON_MODE="example"; +CLICON_PROMPT="%U@%H %W> "; +CLICON_PLUGIN="example_cli"; + +# Autocli syntax tree operations +set @datamodel, cli_auto_set(); +delete("Delete a configuration item") { + @datamodel, cli_auto_del(); + all("Delete whole candidate configuration"), delete_all("candidate"); +} +validate("Validate changes"), cli_validate(); +commit("Commit the changes"), cli_commit(); +quit("Quit"), cli_quit(); +discard("Discard edits (rollback 0)"), discard_changes(); + +show("Show a particular state of the system"){ + configuration("Show configuration"), cli_auto_show("datamodel", "candidate", "xml", false, false); +} +EOF + +new "test params: -f $cfg" + +if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + sudo pkill -f clixon_backend # to be sure + + new "start backend -s init -f $cfg" + start_backend -s init -f $cfg +fi + +new "wait backend" +wait_backend + +new "cli set 2nd A stmt" +expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay foo)" 0 "^$" + +new "show config" +expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^foo$" + +new "cli set 1st B stmt" +expectpart "$($clixon_cli -1 -f $cfg -l o set c B1x bar)" 0 "^$" + +new "show config, Ay removed" +expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^bar$" --not-- "foo" + +if [ $BE -ne 0 ]; then + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg +fi + +rm -rf $dir + +new "endtest" +endtest