Fixed: [YANG mandatory statements within case nodes do not work](https://github.com/clicon/clixon/issues/344)

CHANGELOG.md

@@ -110,7 +110,8 @@ 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 mandatory statements within case nodes do not work](https://github.com/clicon/clixon/issues/344)
+* Fixed: [Nested YANG choice does not work](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_validate.c

@@ -609,12 +609,56 @@ check_list_key(cxobj     *xt,
     goto done;
 }
 
+/*! Go through all case:s children, ensure all mandatory nodes are marked, else error. Then clear
+ * @retval     1     Validation OK
+ * @retval     0     Validation failed (xret set)
+ * @retval    -1     Error
+ */
+static int
+choice_mandatory_check(yang_stmt *ycase,
+		       cxobj    **xret)
+{
+    int        retval = -1;
+    yang_stmt *yc = NULL;
+    cbuf      *cb = NULL;
+    int        fail = 0;
+
+    while ((yc = yn_each(ycase, yc)) != NULL) {
+	if (yang_mandatory(yc)){
+	    if (yang_flag_get(yc, YANG_FLAG_MARK))
+		yang_flag_reset(yc, YANG_FLAG_MARK);
+	    else if (fail == 0){
+		fail++;
+		if (xret){
+		    if ((cb = cbuf_new()) == NULL){
+			clicon_err(OE_UNIX, errno, "cbuf_new");
+			goto done;
+		    }
+		    cprintf(cb, "Mandatory variable %s in module %s",
+			    yang_argument_get(yc),
+			    yang_argument_get(ys_module(ycase)));
+		    if (netconf_missing_element_xml(xret, "application", yang_argument_get(yc), cbuf_get(cb)) < 0)
+			goto done;
+		}
+	    }
+	}
+    }
+    if (fail)
+	retval = 0;
+    else
+	retval = 1;
+ done:
+    if (cb)
+	cbuf_free(cb);
+    return retval;
+}
+
 /*! Check if an xml node lacks mandatory children
  * @param[in]  xt    XML node to be validated
  * @param[in]  yt    xt:s yang statement
  * @param[out] xret  Error XML tree. Free with xml_free after use
  * @retval     1     Validation OK
- * @retval     0     Validation failed (cbret set)
+ * @retval     0     Validation failed (xret set)
  * @retval    -1     Error
  */
 static int
@@ -628,6 +672,7 @@ check_mandatory(cxobj     *xt,
     yang_stmt *y;
     yang_stmt *yc;
     yang_stmt *yp;
+    yang_stmt *ycase;
     cbuf      *cb = NULL;
     int        ret;
     
@@ -643,7 +688,71 @@ check_mandatory(cxobj     *xt,
     }
     yc = NULL;
     while ((yc = yn_each(yt, yc)) != NULL) {
-	if (!yang_mandatory(yc))
+	/* Choice is more complex because of choice/case structure and possibly hierarchical */
+	if (yang_keyword_get(yc) == Y_CHOICE){ 
+	    if (yang_mandatory(yc)){
+		x = NULL;
+		while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
+		    if ((y = xml_spec(x)) != NULL &&
+			(yp = yang_choice(y)) != NULL &&
+			yp == yc){
+			break; /* leave loop with x set */
+		    }
+		}
+		if (x == NULL){
+		    /* @see RFC7950: 15.6 Error Message for Data That Violates 
+		     * a Mandatory "choice" Statement */
+		    if (xret && netconf_data_missing_xml(xret, yang_argument_get(yc), NULL) < 0)
+			goto done;
+		    goto fail;
+		}
+	    }
+	    /* RFC7950 7.9.4:
+	     * if this ancestor is a case node, the constraint is
+	     * enforced if any other node from the case exists.
+	     * Algorithm uses a yang mark flag to detect if mandatory xml nodes exist
+	     */
+	    ycase = NULL;
+	    x = NULL;
+	    while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
+		if ((y = xml_spec(x)) != NULL &&
+		    (yp = yang_parent_get(y)) != NULL){
+		    if (yang_keyword_get(yp) == Y_CASE){
+			if (yang_mandatory(y)){
+			    assert(yang_flag_get(y, YANG_FLAG_MARK) == 0);
+			    yang_flag_set(y, YANG_FLAG_MARK);
+			}
+			if (ycase != NULL){
+			    if (yp != ycase){ /* End of case, new case */
+				/* Check and clear marked mandatory */
+				if ((ret = choice_mandatory_check(ycase, xret)) < 0)
+				    goto done;
+				if (ret == 0)
+				    goto fail;
+				ycase = yp;
+			    }
+			}
+			else /* New case */
+			    ycase = yp;
+		    }			
+		    else if (ycase != NULL){ /* End of case */
+			/* Check and clear marked mandatory */
+			if ((ret = choice_mandatory_check(ycase, xret)) < 0)
+			    goto done;
+			if (ret == 0)
+			    goto fail;
+			ycase = NULL;
+		    }
+		}
+	    }
+	    if (ycase){
+		if ((ret = choice_mandatory_check(ycase, xret)) < 0)
+		    goto done;
+		if (ret == 0)
+		    goto fail;
+	    }
+	}
+	if (!yang_mandatory(yc)) /* Rest of yangs are immediate children */
 	    continue;
 	switch (yang_keyword_get(yc)){
 	case Y_CONTAINER:
@@ -670,23 +779,6 @@ check_mandatory(cxobj     *xt,
 		goto fail;
 	    }
 	    break;
-	case Y_CHOICE: /* More complex because of choice/case structure */
-	    x = NULL;
-	    while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
-		if ((y = xml_spec(x)) != NULL &&
-		    (yp = yang_choice(y)) != NULL &&
-		    yp == yc){
-		    break; /* leave loop with x set */
-		}
-	    }
-	    if (x == NULL){
-		/* @see RFC7950: 15.6 Error Message for Data That Violates 
-		 * a Mandatory "choice" Statement */
-		if (xret && netconf_data_missing_xml(xret, yang_argument_get(yc), NULL) < 0)
-		    goto done;
-		goto fail;
-	    }
-	    break;
 	default:
 	    break;
 	} /* switch */

test/test_choice.sh

@@ -126,6 +126,25 @@ module system{
          }
      }
   }
+  /* Case with mandatory leaf */
+  container manleaf {
+     choice name {
+        case a {
+           leaf a1 {
+	      type string;
+           }
+           leaf a2 {
+	      mandatory true;
+	      type string;
+           }
+        }
+        case b {
+           leaf b1 {
+	      type string;
+           }
+        }
+     }
+  }
 }
 EOF
 
@@ -377,6 +396,21 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
 new "netconf discard-changes"
 expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><discard-changes/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
 
+new "netconf set non-mandatory leaf"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><manleaf xmlns=\"urn:example:config\"><a1>xxx</a1></manleaf></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+new "netconf validate missing"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>a2</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory variable a2 in module system</error-message></rpc-error></rpc-reply>"
+
+new "netconf set mandatory leaf"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><manleaf xmlns=\"urn:example:config\"><a2>yyy</a2></manleaf></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+new "netconf validate ok"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+new "netconf discard-changes"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><discard-changes/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
 if [ $RC -ne 0 ]; then
     new "Kill restconf daemon"
     stop_restconf

test/test_choice_recursive.sh

@@ -40,6 +40,7 @@ module system{
 	choice top{
 	    case topA {
 		choice A{
+		mandatory true;
 		    case A1{
  		      leaf A1x{
 			type string;
@@ -119,12 +120,18 @@ wait_backend
 new "cli set 1st A stmt"
 expectpart "$($clixon_cli -1 -f $cfg -l o set c A1x aaa)" 0 "^$"
 
+new "netconf validate ok"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
 new "show config"
 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 "netconf validate ok"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
 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>$"
 
@@ -143,6 +150,9 @@ expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:examp
 new "cli set 3rd A stmt"
 expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay ccc)" 0 "^$"
 
+new "netconf validate fail (choice A is mandatory)"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>A</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory variable A in module system</error-message></rpc-error></rpc-reply>"
+
 new "show config: Ay"
 expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><Ay>ccc</Ay></c>$"