Fixed: [Clixon backend transaction callback fails for empty types](https://github.com/clicon/clixon/issues/360)

Fixed: [Clixon backend transactions for choice/case is not logical](https://github.com/clicon/clixon/issues/361)

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)

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);
+	    yc0 = xml_spec(x0c);
-	    if (yc && yang_keyword_get(yc) == Y_LEAF){
+	    yc1 = xml_spec(x1c);
-		/* if x0c and x1c are leafs w bodies, then they may be changed */
+	    if (yc0 && yc1 && yc0 != yc1){ /* choice */
-		b1 = xml_body(x0c);
+		if (cxvec_append(x0c, x0vec, x0veclen) < 0) 
-		b2 = xml_body(x1c);
+		    goto done;
-		if (b1 == NULL && b2 == NULL)
+		if (cxvec_append(x1c, x1vec, x1veclen) < 0) 
-		    ;
+		    goto done;
-		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;
-		}
 	    }
-	    else if (xml_diff1(x0c, x1c,   
+	    else
-			       x0vec, x0veclen, 
+		if (yc0 && yang_keyword_get(yc0) == Y_LEAF){
-			       x1vec, x1veclen, 
+		    /* if x0c and x1c are leafs w bodies, then they may be changed */
-			       changed_x0, changed_x1, changedlen)< 0)
+		    b1 = xml_body(x0c);
-		goto done;
+		    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);

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;
     }

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 <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
@@ -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 <<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
 
@@ -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. Choice"
-
-new "8. Set first 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"
@@ -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 nacm_$op change: <first>true</first><second>true</second>" $line
+    checklog "$nr main_$op add: <second>true</second>" $line
+    let 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