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

Previous fix only for some sub-cases. New fix should work for all reecursive cases

apps/cli/cli_main.c

@@ -426,7 +426,7 @@ usage(clicon_handle h,
             "\t-m <mode>\tSpecify plugin syntax mode\n"
 	    "\t-q \t\tQuiet mode, dont print greetings or prompt, terminate on ctrl-C\n"
 	    "\t-p <dir>\tYang directory path (see CLICON_YANG_DIR)\n"
-	    "\t-G \t\tPrint auo-cli CLI syntax generated from YANG\n"
+	    "\t-G \t\tPrint auto-cli CLI syntax generated from YANG\n"
 	    "\t-L \t\tDebug print dynamic CLI syntax including completions and expansions\n"
 	    "\t-l <s|e|o|n|f<file>> \tLog on (s)yslog, std(e)rr, std(o)ut, (n)one or (f)ile (stderr is default)\n"
 	    "\t-y <file>\tOverride yang spec file (dont include .yang suffix)\n"

lib/src/clixon_datastore_write.c

@@ -274,58 +274,66 @@ check_when_condition(cxobj              *x0p,
     goto done;
 }
 
-/*! Given choice/case, remove all other cases.
+/*! Get cloxest yang case and choice, if any
  */
 static int
-choice_delete_existing_children(cxobj     *x0,
+choice_case_get(yang_stmt  *yc,
-				yang_stmt *y1c,
+		yang_stmt **ycase,
-				yang_stmt *y1case,
+		yang_stmt **ychoice)
-				yang_stmt *y1choice)
 {
-    int        retval = -1;
+    yang_stmt *yp;
-    cxobj     *x0c;
+
-    yang_stmt *y0c;
+    if ((yp = yang_parent_get(yc)) == NULL)
-    yang_stmt *y0case;
+	return 0;
-    yang_stmt *y0choice;
+    if (yang_keyword_get(yp) == Y_CASE){
-    cxobj     *x0prev;
+	*ycase = yp;
-    yang_stmt *y0p;
+	*ychoice = yang_parent_get(yp);
-    
+	return 1;
-    /* Now traverse existing tree and compare with choice yang structure of added tree */
-    x0prev = NULL;
-    x0c = NULL;
-    while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) {
-	if ((y0c = xml_spec(x0c)) == NULL ||
-	    yang_parent_get(y0c) == NULL){
-	    x0prev = x0c;
-	    continue;
-	}
-	y0p = yang_parent_get(y0c);
-	if (yang_keyword_get(y0p) == Y_CASE){
-	    y0case = y0p;
-	    y0choice = yang_parent_get(y0case);
-	}
-	else if (yang_keyword_get(y0p) == Y_CHOICE){
-	    y0case = NULL;
-	    y0choice = y0p;
-	}
-	else{
-	    x0prev = x0c;
-	    continue;
-	}
-	if (y0choice == y1choice){
-	    if ((y0case == NULL && y0c != y1c) ||
-		y0case != y1case){
-		if (xml_purge(x0c) < 0)
-		    goto done;
-		x0c = x0prev;
-		continue;
-	    }
-	}
-	x0prev = x0c;
     }
-    retval = 0;
+    else if (yang_keyword_get(yp) == Y_CHOICE){
- done:
+	*ycase = NULL;
-    return retval;
+	*ychoice = yp;
+	return 1;
+    }
+    return 0;
+}
+
+/*! Check if x0/y0 is part of other choice/case than y1 recursively , if so purge 
+ * @retval 0 No, y0 it is not in other case than y1
+ * @retval 1 yes, y0 is in other case than y1
+ */
+static int
+choice_is_other(yang_stmt *y0c,
+		yang_stmt *y0case,
+		yang_stmt *y0choice,
+		yang_stmt *y1c,
+		yang_stmt *y1case,
+		yang_stmt *y1choice)
+{
+    yang_stmt *ycase;
+    yang_stmt *ychoice;
+
+    if (y0choice == y1choice){
+	if ((y0case == NULL && y0c != y1c) ||
+	    y0case != y1case){
+	    return 1;
+	}
+    }
+    else {
+	/* First recurse y0 */
+	if (choice_case_get(y0choice, &ycase, &ychoice)){
+	    if (choice_is_other(y0choice, ycase, ychoice,
+				y1c, y1case, y1choice) == 1)
+		return 1;
+	}
+	/* Second  recurse y1 */
+	if (choice_case_get(y1choice, &ycase, &ychoice)){
+	    if (choice_is_other(y0choice, y0case, y0choice,
+				y1choice, ycase, ychoice) == 1)
+		return 1;
+	}
+    }
+    return 0;
 }
 
 /*! Check if choice nodes and implicitly remove all other cases.
@@ -346,31 +354,41 @@ choice_delete_existing_children(cxobj     *x0,
  * other cases inside the choice.
  */
 static int
-check_delete_existing_case(cxobj     *x0,
+choice_delete_other(cxobj     *x0,
-    			   yang_stmt *y1c)
+		    yang_stmt *y1c)
 {
     int        retval = -1;
-    yang_stmt *yp;
+    cxobj     *x0c;
-    yang_stmt *y1case;
+    cxobj     *x0prev;
-    yang_stmt *y1choice;
+    yang_stmt *y0c;
-
+    yang_stmt *y0case;
-    if ((yp = yang_parent_get(y1c)) == NULL)
+    yang_stmt *y0choice;
+    yang_stmt *y1case = NULL;
+    yang_stmt *y1choice = NULL;
+    
+    if (choice_case_get(y1c, &y1case, &y1choice) == 0)
 	goto ok;
-    if (yang_keyword_get(yp) == Y_CASE){
+    x0prev = NULL;
-	y1case = yp;
+    x0c = NULL;
-	y1choice = yang_parent_get(y1case);
+    while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) {
+	if ((y0c = xml_spec(x0c)) == NULL ||
+	    yang_parent_get(y0c) == NULL){
+	    x0prev = x0c;
+	    continue;
+	}
+	if (choice_case_get(y0c, &y0case, &y0choice) == 0){
+	    x0prev = x0c;
+	    continue;
+	}
+	/* Check if x0/y0 is part of other choice/case than y1 recursively , if so purge */
+	if (choice_is_other(y0c, y0case, y0choice, y1c, y1case, y1choice) == 1){
+	    if (xml_purge(x0c) < 0)
+		goto done;
+	    x0c = x0prev;
+		continue;
+	}
+	x0prev = x0c;
     }
-    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:
@@ -827,8 +845,8 @@ text_modify(clicon_handle       h,
 			       x1cname, yang_find_mynamespace(y0));
 		    goto done;
 		}
-		/* Check if existing case should be deleted */
+		/* Check if existing choice/case should be deleted */
-		if (check_delete_existing_case(x0, yc) < 0)
+		if (choice_delete_other(x0, yc) < 0)
 		    goto done;
 		/* See if there is a corresponding node in the base tree */
 		x0c = NULL;

test/test_choice_recursive.sh

@@ -40,12 +40,16 @@ module system{
 	choice top{
 	    case topA {
 		choice A{
-		    leaf A1x{
+		    case A1{
+ 		      leaf A1x{
 			type string;
-		    }
+		      }
-		    leaf A2x{
+                    }
+		    case A2{
+  		      leaf A2x{
 			type string;
-		    }
+		      }
+                    }
 		}
 		leaf Ay{
 		    type string;
@@ -112,17 +116,41 @@ fi
 new "wait backend"
 wait_backend
 
-new "cli set 2nd A stmt"
+new "cli set 1st A stmt"
-expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay foo)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c A1x aaa)" 0 "^$"
 
 new "show config"
-expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><Ay>foo</Ay></c>$"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A1x>aaa</A1x></c>$"
+
+new "cli set 2nd A stmt"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c A2x bbb)" 0 "^$"
+
+new "show config, only A2x"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x></c>$"
+
+new "cli set 3rd A stmt"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay ccc)" 0 "^$"
+
+new "show config: A2x + Ay"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x><Ay>ccc</Ay></c>$"
 
 new "cli set 1st B stmt"
-expectpart "$($clixon_cli -1 -f $cfg -l o set c B1x bar)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c B1x ddd)" 0 "^$"
 
-new "show config, Ay removed"
+new "show config: B1x"
-expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><B1x>bar</B1x></c>$" --not-- "<Ay>foo</Ay>"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><B1x>ddd</B1x></c>$"
+
+new "cli set 3rd A stmt"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay ccc)" 0 "^$"
+
+new "show config: Ay"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><Ay>ccc</Ay></c>$"
+
+new "cli set 3rd B stmt"
+expectpart "$($clixon_cli -1 -f $cfg -l o set c By fff)" 0 "^$"
+
+new "show config: By"
+expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><By>fff</By></c>$"
 
 if [ $BE -ne 0 ]; then
     new "Kill backend"