Fixed: [Nested YANG choice does not work #342](https://github.com/clicon/clixon/issues/342)

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)
 

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.
+/*! Given choice/case, 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,
+choice_delete_existing_children(cxobj     *x0,
-    			   yang_stmt *y1c)
+				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;
-
+    yang_stmt *y0p;
-    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;
     /* 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;
 	    }

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 <<EOF > $cfg
+<clixon-config xmlns="http://clicon.org/config">
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
+  <CLICON_FEATURE>clixon-restconf:allow-auth-none</CLICON_FEATURE> <!-- Use auth-type=none -->
+  <CLICON_YANG_DIR>$dir</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
+  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
+  <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
+  <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
+  <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
+  <CLICON_CLISPEC_DIR>$clidir</CLICON_CLISPEC_DIR>
+  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
+  <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
+  <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+</clixon-config>
+EOF
+
+cat <<EOF > $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 <<EOF > $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 "^<c xmlns=\"urn:example:config\"><Ay>foo</Ay></c>$"
+
+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 "^<c xmlns=\"urn:example:config\"><B1x>bar</B1x></c>$" --not-- "<Ay>foo</Ay>"
+
+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