diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f86dacf..1360ad07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,22 +36,13 @@ * [3.3.2](#332) Aug 27 2017 * [3.3.1](#331) June 7 2017 -## with-defaults branch - -### New features - -* With-defaults RFC6243 - * see [Netconf With-defaults Capability](https://github.com/clicon/clixon/issues/262) - -### Corrected Bugs - -* Fixed: [with-defaults=trim does not work due to dodgy handling of state data marked as default](https://github.com/clicon/clixon/issues/348) - ## 5.9.0 Expected: September 2022 ### New features +* With-defaults RFC6243 + * see [Netconf With-defaults Capability](https://github.com/clicon/clixon/issues/262) * RESTCONF call home according to RFC 8071 * clixon-restconf.yang extended with callhome inspired by ietf-restconf-server.yang * See e.g., draft-ietf-netconf-restconf-client-server-26.txt @@ -65,6 +56,9 @@ Expected: September 2022 Users may have to change how they access the system +* Backend transaction plugins: edit of choice node will always result in a "del/add" event for all edits of change nodes, never a "change" event. + * Before, some cases were using a "change" event if the "yang ordering" happended to be the same. + * See more details in: [Clixon backend transactions for choice/case is not logical](https://github.com/clicon/clixon/issues/361) * Constraints on number of elements have been made stricter (ie unique, min/max-elements) * Usecases that passed previously may now return error * This includes: @@ -73,6 +67,9 @@ Users may have to change how they access the system ### Corrected Bugs +* Fixed: [Clixon backend transactions for choice/case is not logical](https://github.com/clicon/clixon/issues/361) +* Fixed: [Clixon backend transaction callback fails for empty types](https://github.com/clicon/clixon/issues/360) +* Fixed: [with-defaults=trim does not work due to dodgy handling of state data marked as default](https://github.com/clicon/clixon/issues/348) * Fixed: [YANG ordering fails for nested choice and action](https://github.com/clicon/clixon/issues/356) * Fixed: [YANG min-elements within non-presence container does not work](https://github.com/clicon/clixon/issues/355) * Fixed: [Issues with ietf-snmp modules](https://github.com/clicon/clixon/issues/353) diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index b271715d..d5f6f00f 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -310,7 +310,8 @@ xml_diff1(cxobj *x0, int retval = -1; cxobj *x0c = NULL; /* x0 child */ cxobj *x1c = NULL; /* x1 child */ - yang_stmt *yc; + yang_stmt *yc0; + yang_stmt *yc1; char *b1; char *b2; int eq; @@ -352,29 +353,36 @@ xml_diff1(cxobj *x0, /* xml-spec NULL could happen with anydata children for example, * if so, continute compare children but without yang */ - yc = xml_spec(x0c); - if (yc && yang_keyword_get(yc) == Y_LEAF){ - /* if x0c and x1c are leafs w bodies, then they may be changed */ - b1 = xml_body(x0c); - b2 = xml_body(x1c); - if (b1 == NULL && b2 == NULL) - ; - else if (b1 == NULL || b2 == NULL - || strcmp(b1, b2) != 0 - || strcmp(xml_name(x0c), xml_name(x1c)) != 0 /* Ex: choice { a:bool; b:bool } */ - ){ - if (cxvec_append(x0c, changed_x0, changedlen) < 0) - goto done; - (*changedlen)--; /* append two vectors */ - if (cxvec_append(x1c, changed_x1, changedlen) < 0) - goto done; - } + yc0 = xml_spec(x0c); + yc1 = xml_spec(x1c); + if (yc0 && yc1 && yc0 != yc1){ /* choice */ + if (cxvec_append(x0c, x0vec, x0veclen) < 0) + goto done; + if (cxvec_append(x1c, x1vec, x1veclen) < 0) + goto done; } - else if (xml_diff1(x0c, x1c, - x0vec, x0veclen, - x1vec, x1veclen, - changed_x0, changed_x1, changedlen)< 0) - goto done; + else + if (yc0 && yang_keyword_get(yc0) == Y_LEAF){ + /* if x0c and x1c are leafs w bodies, then they may be changed */ + b1 = xml_body(x0c); + b2 = xml_body(x1c); + if (b1 == NULL && b2 == NULL) + ; + else if (b1 == NULL || b2 == NULL + || strcmp(b1, b2) != 0 + ){ + if (cxvec_append(x0c, changed_x0, changedlen) < 0) + goto done; + (*changedlen)--; /* append two vectors */ + if (cxvec_append(x1c, changed_x1, changedlen) < 0) + goto done; + } + } + else if (xml_diff1(x0c, x1c, + x0vec, x0veclen, + x1vec, x1veclen, + changed_x0, changed_x1, changedlen)< 0) + goto done; } x0c = xml_child_each(x0, x0c, CX_ELMNT); x1c = xml_child_each(x1, x1c, CX_ELMNT); diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index 065f9696..d1b81e7e 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -258,6 +258,7 @@ xml_cmp(cxobj *x1, /* XXX handle errors */ yi1 = yang_order(y1); yi2 = yang_order(y2); + /* this is for choice */ if ((equal = yi1-yi2) != 0) goto done; } diff --git a/test/test_transaction.sh b/test/test_transaction.sh index 69c19fa0..fc83e622 100755 --- a/test/test_transaction.sh +++ b/test/test_transaction.sh @@ -28,7 +28,7 @@ fyang=$dir/trans.yang flog=$dir/backend.log touch $flog -# Used as a trigger for user-validittion errors, eg $errnr = 42 is invalid +# Used as a trigger for user-validation errors, eg $errnr = 42 is invalid errnr=42 cat < $fyang @@ -64,7 +64,33 @@ module trans{ leaf second { type boolean; } + } + choice cempty { + case a{ + leaf aaa { + type empty; + } } + case b{ + leaf bbb { + type boolean; + } + leaf ccc { + type empty; + } + } + } + choice canydata { + case a{ + anydata ddd; + } + case b{ + leaf eee { + type boolean; + } + anydata fff; + } + } } } EOF @@ -82,7 +108,7 @@ cat < $cfg $APPNAME $dir/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile - /usr/local/var/$APPNAME + $dir EOF @@ -93,7 +119,7 @@ function checklog(){ s=$1 # statement l0=$2 # linenr new "Check $s in log" -# echo "grep \"transaction_log $s line:$l0\" $flog" + echo "grep \"transaction_log $s line:$l0\" $flog" t=$(grep -n "transaction_log $s" $flog) if [ -z "$t" ]; then echo -e "\e[31m\nError in Test$testnr [$testname]:" @@ -125,7 +151,7 @@ if [ $BE -ne 0 ]; then if [ $? -ne 0 ]; then err fi - new "start backend -s init -f $cfg -l f$flog -- -t -v /x/y[a=$errnr]" + new "start backend -s init -f $cfg -l f$flog -- -t -v \"/x/y[a='$errnr']\"" start_backend -s init -f $cfg -l f$flog -- -t -v "/x/y[a='$errnr']" # -t means transaction logging fi @@ -313,9 +339,7 @@ for op in begin validate complete commit commit_done end; do let line++ done -# Variant check that only b,c - -new "8. Set first choice" +new "8. Choice" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "true" "" "" new "netconf commit same" @@ -324,27 +348,104 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " new "Set second choice" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "true" "" "" -# choice chanmge with same value did not show up in log new "netconf commit second" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + let nr++ let nr++ let line+=12 -# check complete + for op in begin validate complete commit commit_done; do - checklog "$nr main_$op change: truetrue" $line + checklog "$nr main_$op del: true" $line let line++ - checklog "$nr nacm_$op change: truetrue" $line + checklog "$nr main_$op add: true" $line + let line++ + checklog "$nr nacm_$op del: true" $line + let line++ + checklog "$nr nacm_$op add: true" $line let line++ done # End is special because change does not have old element -checklog "$nr main_end change: true" $line +checklog "$nr main_end add: true" $line let line++ # This check does not work if MOVE_TRANS_END is set -checklog "$nr nacm_end change: true" $line +checklog "$nr nacm_end add: true" $line let line++ + +new "9. Choice/case with empty type" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "netconf commit first" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "Set second choice" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "netconf commit second" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +let nr++ +let nr++ + +let line+=12 + +for op in begin validate complete commit commit_done; do + checklog "$nr main_$op del: " $line + let line++ + checklog "$nr main_$op add: " $line + let line++ + checklog "$nr nacm_$op del: " $line + let line++ + checklog "$nr nacm_$op add: " $line + let line++ +done + +# End is special because change does not have old element +checklog "$nr main_end add: " $line +let line++ + +checklog "$nr nacm_end add: " $line +let line++ + +#--------------------------------------------- + +new "10. Choice/case with anydata" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "a" "" "" + +new "netconf commit first" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "Set second choice" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "netconf commit second" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + +let nr++ +let nr++ + +let line+=12 + +for op in begin validate complete commit commit_done; do + checklog "$nr main_$op del: a" $line + let line++ + checklog "$nr main_$op add: " $line + let line++ + checklog "$nr nacm_$op del: a" $line + let line++ + checklog "$nr nacm_$op add: " $line + let line++ +done + +# End is special because change does not have old element +checklog "$nr main_end add: " $line +let line++ + +checklog "$nr nacm_end add: " $line +let line++ + if [ $BE -ne 0 ]; then new "Kill backend" # Check if premature kill