Documented bug [Yang identityref XML encoding is not general #90](https://github.com/clicon/clixon/issues/90)

CHANGELOG.md

@@ -217,6 +217,7 @@
 
 ### Minor changes
 
+* Documented bug [Yang identityref XML encoding is not general #90](https://github.com/clicon/clixon/issues/90)
 * Added new API function `xpath_parse()` to split parsing and xml evaluation.
 * Rewrote `api_path2xpath` to handle namespaces.
 * `api_path2xml_vec` strict mode check added if list key length mismatch 

apps/cli/cli_generate.c

@@ -181,33 +181,56 @@ yang2cli_var_identityref(yang_stmt *ys,
     yang_stmt *ybaseref;
     yang_stmt *ybaseid;
     cg_var    *cv = NULL;
-    char      *name;
-    char      *id;
+    char      *prefix = NULL;
+    char      *id = NULL;
     int        i;
+    cvec      *idrefvec;
+    yang_stmt *ymod;
+    yang_stmt *yprefix;
+    yang_stmt *yspec;
     
     if ((ybaseref = yang_find(ytype, Y_BASE, NULL)) != NULL &&
 	(ybaseid = yang_find_identity(ys, yang_argument_get(ybaseref))) != NULL){
-	if (cvec_len(yang_cvec_get(ybaseid)) > 0){
+	idrefvec = yang_cvec_get(ybaseid);
+	if (cvec_len(idrefvec) > 0){
 	    /* Add a wildchar string first -let validate take it for default prefix */
 	    cprintf(cb, ">");
 	    if (helptext)
 		cprintf(cb, "(\"%s\")", helptext);
 	    cprintf(cb, "|<%s:%s choice:", yang_argument_get(ys), cvtypestr);
+	    yspec = ys_spec(ys);
 	    i = 0;
-	    while ((cv = cvec_each(yang_cvec_get(ybaseid), cv)) != NULL){
-		if (i++)
-		    cprintf(cb, "|"); 
-		name = strdup(cv_name_get(cv));
-		if ((id=strchr(name, ':')) != NULL)
-		    *id = '\0';
-		cprintf(cb, "%s:%s", name, id+1);
-		if (name)
-		    free(name);
-	    }
+	    while ((cv = cvec_each(idrefvec, cv)) != NULL){
+		if (nodeid_split(cv_name_get(cv), &prefix, &id) < 0)
+		    goto done;
+		/* Translate from module-name(prefix) to global prefix
+		 * This is really a kludge for true identityref prefix handling
+		 * IDENTITYREF_KLUDGE 
+		 * This is actually quite complicated: the cli needs to generate
+		 * a netconf statement with correct xmlns binding
+		 */
+		if ((ymod = yang_find_module_by_name(yspec, prefix)) != NULL &&
+		    (yprefix = yang_find(ymod, Y_PREFIX, NULL)) != NULL)
+		    if (i++)
+			cprintf(cb, "|"); 
+		    cprintf(cb, "%s:%s", yang_argument_get(yprefix), id);
+		}
+		if (prefix){
+		    free(prefix);
+		    prefix = NULL;
+		}
+		if (id){
+		    free(id);
+		    id = NULL;
+		}
 	}
     }
     retval = 0;
-    // done:
+ done:
+    if (prefix)
+	free(prefix);
+    if (id)
+	free(id);
     return retval;
 }
     

include/clixon_custom.h

@@ -48,9 +48,13 @@
  */
 #define USE_NETCONF_NS_AS_DEFAULT
 
-/* Use modulename:id instead of prefix:id in derived identityref list
- * Modulenames are global/canonical but prefixes are not.
- * Experimental since there is some mapping between prefixes and module names
- * that needs to be done
+/*! Tag for wrong handling of identityref prefixes (XML encoding)
+ * See https://github.com/clicon/clixon/issues/90
+ * Instead of using generic xmlns prefix bindings, the module's own prefix
+ * is used.
+ * In the CLI generation case, this is actually quite complicated: the cli 
+ * needs to generate a netconf statement with correct xmlns binding.
+ * The easy way to do this is to always generate all prefix/namespace bindings 
+ * on the top-level for the modules involved in the netconf operation.
  */
-#undef USE_IDREF_LIST_MODULE
+#undef IDENTITYREF_KLUDGE

lib/src/clixon_xml_map.c

@@ -330,6 +330,25 @@ validate_leafref(cxobj     *xt,
     goto done;
 }
 
+/* Get module from its own prefix 
+ * This is really not a valid usecase, a kludge for the identityref derived
+ * list workaround (IDENTITYREF_KLUDGE)
+ */ 
+static yang_stmt *
+yang_find_module_by_prefix_yspec(yang_stmt *yspec, 
+				 char      *prefix)
+{
+    yang_stmt *ymod = NULL;
+    yang_stmt *yprefix;
+    
+    while ((ymod = yn_each(yspec, ymod)) != NULL) 
+	if (ymod->ys_keyword == Y_MODULE &&
+	    (yprefix = yang_find(ymod, Y_PREFIX, NULL)) != NULL &&
+	    strcmp(yang_argument_get(yprefix), prefix) == 0)
+	    return ymod;
+    return NULL;
+}
+
 /*! Validate xml node of type identityref, ensure value is a defined identity
  * Check if a given node has value derived from base identity. This is
  * a run-time check necessary when validating eg netconf.
@@ -367,6 +386,7 @@ validate_identityref(cxobj     *xt,
     char       *id = NULL;
     cbuf       *cberr = NULL;
     cbuf       *cb = NULL;
+    cvec       *idrefvec; /* Derived identityref list: (module:id)**/
     
     if ((cb = cbuf_new()) == NULL){
 	clicon_err(OE_UNIX, errno, "cbuf_new"); 
@@ -395,13 +415,15 @@ validate_identityref(cxobj     *xt,
 	goto fail;
     }
 
-#if 0 /* Assume proper namespace, otherwise we assume module prefixes */
+#if 0 /* Assume proper namespace, otherwise we assume module prefixes,
+       * see IDENTITYREF_KLUDGE 
+       */
     {
 	char       *namespace;
 	yang_stmt  *ymod;
 	yang_stmt  *yspec;    
 
-	/* Create an idref as <module>:<id> which is the format of the derived
+	/* Create an idref as <bbmodule>:<id> which is the format of the derived
 	 * identityref list associated with the base identities.
 	 */
 	/* Get namespace (of idref) from xml */
@@ -416,14 +438,16 @@ validate_identityref(cxobj     *xt,
 	cprintf(cb, "%s:%s", yang_argument_get(ymod), id);
     }
 #else
-#ifdef USE_IDREF_LIST_MODULE
     {
 	yang_stmt  *ymod;
 	/* idref from prefix:id to module:id */
 	if (prefix == NULL)
 	    ymod = ys_module(ys);
-	else /* from prefix to name */
-	    ymod = yang_find_module_by_prefix(ys, prefix);
+	else{ /* from prefix to name */
+#if 1 /* IDENTITYREF_KLUDGE  */
+	    ymod = yang_find_module_by_prefix_yspec(ys_spec(ys), prefix);
+#endif
+	}
 	if (ymod == NULL){
 	    cprintf(cberr, "Identityref validation failed, %s not derived from %s", 
 		    node, yang_argument_get(ybaseid));
@@ -433,18 +457,13 @@ validate_identityref(cxobj     *xt,
 	}
 	cprintf(cb, "%s:%s", yang_argument_get(ymod), id);
     }
-#else
-    if (prefix == NULL)
-       cprintf(cb, "%s:%s", yang_find_myprefix(ys), id);
-    else
-       cprintf(cb, "%s:%s", prefix, id);
-#endif
 #endif
     idref = cbuf_get(cb);	
     /* Here check if node is in the derived node list of the base identity 
      * The derived node list is a cvec computed XXX
      */
-    if (cvec_find(yang_cvec_get(ybaseid), idref) == NULL){
+    idrefvec = yang_cvec_get(ybaseid);
+    if (cvec_find(idrefvec, idref) == NULL){
 	cprintf(cberr, "Identityref validation failed, %s not derived from %s", 
 		node, yang_argument_get(ybaseid));
 	if (netconf_operation_failed_xml(xret, "application", cbuf_get(cberr)) < 0)

lib/src/clixon_yang.c

@@ -1616,12 +1616,13 @@ ys_populate_identity(clicon_handle h,
     int             retval = -1;
     yang_stmt      *yc = NULL;
     yang_stmt      *ybaseid;
-    //    yang_stmt      *ymod;
     cg_var         *cv;
     char           *baseid;
     char           *prefix = NULL;
     char           *id = NULL;
     cbuf           *cb = NULL;
+    yang_stmt      *ymod;
+    cvec           *idrefvec; /* Derived identityref list: (module:id)**/
 
     /* Top-call (no recursion) create idref 
      * The idref is (here) in "canonical form": <module>:<id>
@@ -1632,7 +1633,6 @@ ys_populate_identity(clicon_handle h,
 	    clicon_err(OE_UNIX, errno, "cbuf_new"); 
 	    goto done;
 	}
-#if 0 /* Use module:id instead of prefix:id in derived list */
 	if (nodeid_split(yang_argument_get(ys), &prefix, &id) < 0)
 	    goto done;
 	if ((ymod = ys_module(ys)) == NULL){
@@ -1640,16 +1640,6 @@ ys_populate_identity(clicon_handle h,
 	    goto done;
 	}
 	cprintf(cb, "%s:%s", yang_argument_get(ymod), id);
-#else
-	{
-	    if (nodeid_split(yang_argument_get(ys), &prefix, &id) < 0)
-		goto done;
-	    if (prefix)
-		cprintf(cb, "%s:%s", prefix, id);
-	    else
-		cprintf(cb, "%s:%s", yang_find_myprefix(ys), id);
-	}
-#endif
 	idref = cbuf_get(cb);
     }
     /* Iterate through all base statements and check the base identity exists 
@@ -1666,7 +1656,8 @@ ys_populate_identity(clicon_handle h,
 	}
 	//	    continue; /* root identity */
 	/* Check if derived id is already in base identifier */
-	if (cvec_find(ybaseid->ys_cvec, idref) != NULL)
+	idrefvec = yang_cvec_get(ybaseid);
+	if (cvec_find(idrefvec, idref) != NULL)
 	    continue;
 	/* Add derived id to ybaseid */
 	if ((cv = cv_new(CGV_STRING)) == NULL){
@@ -1675,7 +1666,7 @@ ys_populate_identity(clicon_handle h,
 	}
 	/* add prefix */
 	cv_name_set(cv, idref);
-	cvec_append_var(ybaseid->ys_cvec, cv); /* cv copied */
+	cvec_append_var(idrefvec, cv); /* cv copied */
 	if (cv){
 	    cv_free(cv);
 	    cv = NULL;

lib/src/clixon_yang_internal.h

@@ -97,7 +97,7 @@ struct yang_stmt{
 					Y_RANGE: range_min, range_max 
 					Y_LIST: vector of keys
 					Y_TYPE & identity: store all derived 
-					   types as <prefix>:<id> list
+					   types as <module>:<id> list
 				     */
     yang_type_cache   *ys_typecache; /* If ys_keyword==Y_TYPE, cache all typedef data except unions */
     int               _ys_vector_i;   /* internal use: yn_each */