Skip to content

Commit

Permalink
Fixed: [Clixon backend transaction callback fails for empty types](#360)
Browse files Browse the repository at this point in the history 
Fixed: [Clixon backend transactions for choice/case is not logical](#361)
  • Loading branch information
@olofhagsand
olofhagsand committed Sep 7, 2022
1 parent ddf0150 commit 15fcae3
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 47 deletions.
19 changes: 8 additions & 11 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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)
Expand Down
54 changes: 31 additions & 23 deletions lib/src/clixon_xml_map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions lib/src/clixon_xml_sort.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
127 changes: 114 additions & 13 deletions test/test_transaction.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fyang=$dir/trans.yang
flog=$dir/backend.log
touch $flog

# Used as a trigger for user-validittion errors, eg <a>$errnr</a> = <a>42</a> is invalid
# Used as a trigger for user-validation errors, eg <a>$errnr</a> = <a>42</a> is invalid
errnr=42

cat <<EOF > $fyang
Expand Down Expand Up @@ -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
Expand All @@ -82,7 +108,7 @@ cat <<EOF > $cfg
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
<CLICON_SOCK>$dir/$APPNAME.sock</CLICON_SOCK>
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
</clixon-config>
EOF

Expand All @@ -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]:"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><first>true</first></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf commit same"
Expand All @@ -324,27 +348,104 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
new "Set second choice"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><second>true</second></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

# choice chanmge with same value did not show up in log
new "netconf commit second"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

let nr++
let nr++

let line+=12
# check complete

for op in begin validate complete commit commit_done; do
checklog "$nr main_$op change: <first>true</first><second>true</second>" $line
checklog "$nr main_$op del: <first>true</first>" $line
let line++
checklog "$nr main_$op add: <second>true</second>" $line
let line++
checklog "$nr nacm_$op change: <first>true</first><second>true</second>" $line
checklog "$nr nacm_$op del: <first>true</first>" $line
let line++
checklog "$nr nacm_$op add: <second>true</second>" $line
let line++
done

# End is special because change does not have old element
checklog "$nr main_end change: <second>true</second>" $line
checklog "$nr main_end add: <second>true</second>" $line
let line++
# This check does not work if MOVE_TRANS_END is set
checklog "$nr nacm_end change: <second>true</second>" $line
checklog "$nr nacm_end add: <second>true</second>" $line
let line++

new "9. Choice/case with empty type"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><aaa/></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf commit first"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "Set second choice"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><ccc/></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf commit second"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

let nr++
let nr++

let line+=12

for op in begin validate complete commit commit_done; do
checklog "$nr main_$op del: <aaa/>" $line
let line++
checklog "$nr main_$op add: <ccc/>" $line
let line++
checklog "$nr nacm_$op del: <aaa/>" $line
let line++
checklog "$nr nacm_$op add: <ccc/>" $line
let line++
done

# End is special because change does not have old element
checklog "$nr main_end add: <ccc/>" $line
let line++

checklog "$nr nacm_end add: <ccc/>" $line
let line++

#---------------------------------------------

new "10. Choice/case with anydata"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><ddd><foo>a</foo></ddd></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf commit first"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "Set second choice"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><fff><bar/></fff></x></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf commit second"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

let nr++
let nr++

let line+=12

for op in begin validate complete commit commit_done; do
checklog "$nr main_$op del: <ddd><foo>a</foo></ddd>" $line
let line++
checklog "$nr main_$op add: <fff><bar/></fff>" $line
let line++
checklog "$nr nacm_$op del: <ddd><foo>a</foo></ddd>" $line
let line++
checklog "$nr nacm_$op add: <fff><bar/></fff>" $line
let line++
done

# End is special because change does not have old element
checklog "$nr main_end add: <fff><bar/></fff>" $line
let line++

checklog "$nr nacm_end add: <fff><bar/></fff>" $line
let line++

if [ $BE -ne 0 ]; then
new "Kill backend"
# Check if premature kill
Expand Down

0 comments on commit 15fcae3

Please sign in to comment.