diff --git a/CHANGELOG.md b/CHANGELOG.md
index ce4e843a..01b9f230 100644
--- a/CHANGELOG.md
+++ b/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)
diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c
index 85e80724..d38b8c7f 100644
--- a/lib/src/clixon_validate.c
+++ b/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 */
diff --git a/test/test_choice.sh b/test/test_choice.sh
index 20ac6be7..4c8e6d27 100755
--- a/test/test_choice.sh
+++ b/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" "
new "netconf discard-changes"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" ""
+new "netconf set non-mandatory leaf"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "xxx" "" ""
+
+new "netconf validate missing"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "applicationmissing-elementa2errorMandatory variable a2 in module system"
+
+new "netconf set mandatory leaf"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "yyy" "" ""
+
+new "netconf validate ok"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" ""
+
+new "netconf discard-changes"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" ""
+
if [ $RC -ne 0 ]; then
new "Kill restconf daemon"
stop_restconf
diff --git a/test/test_choice_recursive.sh b/test/test_choice_recursive.sh
index 65c86fab..e880b3e5 100755
--- a/test/test_choice_recursive.sh
+++ b/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" "" "" ""
+
new "show config"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^aaa$"
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" "" "" ""
+
new "show config, only A2x"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^bbb$"
@@ -143,6 +150,9 @@ expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^" "" "applicationmissing-elementAerrorMandatory variable A in module system"
+
new "show config: Ay"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^ccc$"