xml_cmp() compares numeric nodes based on string value [https://github.com/clicon/clixon/issues/64]

CHANGELOG.md

@@ -142,7 +142,8 @@
   * <!DOCTYPE (ie DTD) is not supported.
 
 ### Corrected Bugs
-* xml_cmp() respects 'ordered-by user' for state nodes, which violates RFC 7950 [https://github.com/clicon/clixon/issues/63. (Thanks JDL)
+* xml_cmp() compares numeric nodes based on string value [https://github.com/clicon/clixon/issues/64]
+* xml_cmp() respects 'ordered-by user' for state nodes, which violates RFC 7950 [https://github.com/clicon/clixon/issues/63]. (Thanks JDL)
 * XML<>JSON conversion problems [https://github.com/clicon/clixon/issues/66]
   * CDATA sections stripped from XML when converted to JSON
 * Restconf returns error when RPC generates "ok" reply [https://github.com/clicon/clixon/issues/69]

lib/clixon/clixon_xml.h

@@ -130,6 +130,8 @@ int       xml_childvec_set(cxobj *x, int len);
 cxobj    *xml_new(char *name, cxobj *xn_parent, yang_stmt *spec);
 yang_stmt *xml_spec(cxobj *x);
 int       xml_spec_set(cxobj *x, yang_stmt *spec);
+cg_var   *xml_cv(cxobj *x);
+int       xml_cv_set(cxobj *x, cg_var *cv);
 cxobj    *xml_find(cxobj *xn_parent, char *name);
 
 int       xml_addsub(cxobj *xp, cxobj *xc);

lib/src/clixon_xml.c

@@ -126,6 +126,8 @@ struct xml{
     int               x_flags;      /* Flags according to XML_FLAG_* */
     yang_stmt        *x_spec;       /* Pointer to specification, eg yang, by 
 				       reference, dont free */
+    cg_var           *x_cv;         /* Cached value as cligen variable 
+                                       (eg xml_cmp) */
 };
 
 /*
@@ -758,6 +760,34 @@ xml_spec_set(cxobj     *x,
     return 0;
 }
 
+/*! Return (cached)  cligen variable value of xml node
+ * @param[in]  x    XML node (body and leaf/leaf-list)
+ * @retval     cv   CLIgen variable containing value of x body
+ * @retval     NULL
+ * @note only applicable if x is body and has yang-spec and is leaf or leaf-list
+ */
+cg_var *
+xml_cv(cxobj *x)
+{
+    return x->x_cv;
+}
+
+/*! Return (cached) cligen variable value of xml node
+ * @param[in]  x   XML node (body and leaf/leaf-list)
+ * @param[in]  cv  CLIgen variable containing value of x body
+ * @retval     0   OK
+ * @note only applicable if x is body and has yang-spec and is leaf or leaf-list
+ */
+int
+xml_cv_set(cxobj  *x, 
+	   cg_var *cv)
+{
+    if (x->x_cv)
+	cv_free(x->x_cv);
+    x->x_cv = cv;
+    return 0;
+}
+
 /*! Find an XML node matching name among a parent's children.
  *
  * Get first XML node directly under x_up in the xml hierarchy with
@@ -1229,6 +1259,8 @@ xml_free(cxobj *x)
     }
     if (x->x_childvec)
 	free(x->x_childvec);
+    if (x->x_cv)
+	cv_free(x->x_cv);
     free(x);
     return 0;
 }

lib/src/clixon_xml_sort.c

@@ -60,14 +60,75 @@
 #include "clixon_hash.h"
 #include "clixon_handle.h"
 #include "clixon_yang.h"
+#include "clixon_yang_type.h"
 #include "clixon_xml.h"
 #include "clixon_options.h"
 #include "clixon_xml_map.h"
 #include "clixon_xml_sort.h"
 
-/*
+/*! Get xml body value as cligen variable
- * Variables
+ * @param[in]  x   XML node (body and leaf/leaf-list)
+ * @param[out] cvp Pointer to cligen variable containing value of x body
+ * @retval     0   OK, cvp contains cv or NULL
+ * @retval    -1   Error
+ * @note only applicable if x is body and has yang-spec and is leaf or leaf-list
+ * Move to clixon_xml.c?
  */
+static int
+xml_cv_cache(cxobj   *x,
+	     char    *body,
+	     cg_var **cvp)
+{
+    int          retval = -1;
+    cg_var      *cv = NULL;
+    yang_stmt   *y;
+    yang_stmt   *yrestype;
+    enum cv_type cvtype;
+    int          ret;
+    char        *reason=NULL;
+    int          options = 0;
+    uint8_t      fraction = 0;
+	    
+    if ((cv = xml_cv(x)) != NULL)
+	goto ok;
+    if ((y = xml_spec(x)) == NULL)
+	goto done;
+    if (yang_type_get(y, NULL, &yrestype, &options, NULL, NULL, &fraction) < 0)
+	goto done;
+    yang2cv_type(yrestype->ys_argument, &cvtype);
+    if (cvtype==CGV_ERR){
+	clicon_err(OE_YANG, errno, "yang->cligen type %s mapping failed",
+		   yrestype->ys_argument);
+	goto done;
+    }
+    if ((cv = cv_new(cvtype)) == NULL){
+	clicon_err(OE_YANG, errno, "cv_new");
+	goto done;
+    }
+    if (cvtype == CGV_DEC64)
+	cv_dec64_n_set(cv, fraction);
+	
+    if ((ret = cv_parse1(body, cv, &reason)) < 0){
+	clicon_err(OE_YANG, errno, "cv_parse1");
+	goto done;
+    }
+    if (ret == 0){
+	clicon_err(OE_YANG, EINVAL, "cv parse error: %s\n", reason);
+	goto done;
+    }
+    if (xml_cv_set(x, cv) < 0)
+	goto done;
+ ok:
+    *cvp = cv;
+    cv = NULL;
+    retval = 0;
+ done:
+    if (reason)
+	free(reason);
+    if (cv)
+	cv_free(cv);
+    return retval;
+}
 
 /*! Given a child name and an XML object, return yang stmt of child
  * If no xml parent, find root yang stmt matching name
@@ -150,6 +211,8 @@ xml_cmp(const void* arg1,
     char       *b1;
     char       *b2;
     char       *keyname;
+    cg_var     *cv1; 
+    cg_var     *cv2;
 
     assert(x1&&x2);
     y1 = xml_spec(x1);
@@ -164,7 +227,7 @@ xml_cmp(const void* arg1,
     }
     /* Now y1==y2, same Yang spec, can only be list or leaf-list,
      * But first check exceptions, eg config false or ordered-by user
-     * otherwuse sort according to key
+     * otherwise sort according to key
      */
     if (yang_config(y1)==0 ||
 	yang_find((yang_node*)y1, Y_ORDERED_BY, "user") != NULL)
@@ -175,8 +238,13 @@ xml_cmp(const void* arg1,
 	    equal = -1;
 	else if ((b2 = xml_body(x2)) == NULL)
 	    equal = 1;
-	else
+	else{
-	    equal = strcmp(b1, b2);
+	    if (xml_cv_cache(x1, b1, &cv1) < 0)
+		goto done;
+	    if (xml_cv_cache(x2, b2, &cv2) < 0)
+		goto done;
+	    equal = cv_cmp(cv1, cv2);
+	}
 	break;
     case Y_LIST: /* Match with key values 
 		  * Use Y_LIST cache (see struct yang_stmt)
@@ -200,7 +268,10 @@ xml_cmp(const void* arg1,
 }
 
 /*! Compare xml object
- * @param[in]  yangi    Yang order
+ * @param[in]  x        XML node to compare with
+ * @param[in]  y        The yang spec of x
+ * @param[in]  name     Name to compare with x
+ * @param[in]  keyword  Yang keyword (stmt type) to compare w x/y
  * @param[in]  keynr    Length of keyvec/keyval vector when applicable
  * @param[in]  keyvec   Array of of yang key identifiers
  * @param[in]  keyval   Array of of yang key values
@@ -209,6 +280,7 @@ xml_cmp(const void* arg1,
  * @retval <0  if arg1 is less than arg2
  * @retval >0  if arg1 is greater than arg2
  * @see xml_cmp   Similar, but for two objects
+ * @note Does not care about y type of value as xml_cmp
  */
 static int
 xml_cmp1(cxobj        *x,
@@ -220,12 +292,15 @@ xml_cmp1(cxobj        *x,
 	 char        **keyval,
 	 int          *userorder)
 {
-    char *b;
+    char      *b;
-    int   i;
+    int        i;
-    char *keyname;
+    char      *keyname;
-    char *key;
+    char      *key;
-    int   match = 0;
+    int        match = 0;
 
+    /* state data = userorder */
+    if (userorder && yang_config(y)==0)
+	*userorder=1;
     /* Check if same yang spec (order in yang stmt list) */
     switch (keyword){
     case Y_CONTAINER: /* Match with name */
@@ -257,6 +332,7 @@ xml_cmp1(cxobj        *x,
     default:
 	break;
     }
+    // done:
     return match; /* should not reach here */
 }
 
@@ -603,7 +679,6 @@ match_base_child(cxobj      *x0,
     yang_node *yp; /* yang parent */
     
     *x0cp = NULL; /* init return value */
-#if 1
     /* 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
@@ -619,7 +694,6 @@ match_base_child(cxobj      *x0,
 	*x0cp = x0c;
 	goto ok; /* What to do if not found? */
     }
-#endif
     switch (yc->ys_keyword){
     case Y_CONTAINER: 	/* Equal regardless */
     case Y_LEAF: 	/* Equal regardless */

test/test_order.sh

@@ -46,7 +46,7 @@ module order-example{
     namespace "urn:example:order";
     prefix ex;
     import clixon-example { /* for state callback */
-	prefix ex;
+       prefix ex;
     }
     container c{
       leaf d{
@@ -84,6 +84,23 @@ module order-example{
         type string;
       } 
     }
+    container types{
+      description "For testing ordering using other types than strings";
+      leaf-list strings{
+        type string;
+        ordered-by system;
+      }
+      leaf-list ints{
+        type int32;
+        ordered-by system;
+      }
+      leaf-list decs{
+        type decimal64{
+           fraction-digits 3;
+        }
+        ordered-by system;
+      }
+    }
 }
 EOF
 
@@ -184,6 +201,19 @@ expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 '<rpc><edit-config><target><can
 new "verify list user order (as entered)"
 expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 '<rpc><get-config><source><candidate/></source><filter type="xpath" select="/y2"/></get-config></rpc>]]>]]>' '^<rpc-reply><data><y2 xmlns="urn:example:order"><k>c</k><a>bar</a></y2><y2 xmlns="urn:example:order"><k>b</k><a>foo</a></y2><y2 xmlns="urn:example:order"><k>a</k><a>fie</a></y2></data></rpc-reply>]]>]]>$'
 
+#-- order by type rather than strings.
+# there are three lists: strings, ints, and decimal64
+# the strings is there for comparison
+new "add type ordered entries"
+expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 '<rpc><edit-config><target><candidate/></target><config><types xmlns="urn:example:order">
+<ints>10</ints><ints>2</ints><ints>1</ints>
+<strings>10</strings><strings>2</strings><strings>1</strings>
+<decs>10.0</decs><decs>2.0</decs><decs>1.0</decs>
+</types></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+
+new "get type ordered entries"
+expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 '<rpc><get-config><source><candidate/></source><filter type="xpath" select="/types"/></get-config></rpc>]]>]]>' '^<rpc-reply><data><types xmlns="urn:example:order"><strings>1</strings><strings>10</strings><strings>2</strings><ints>1</ints><ints>2</ints><ints>10</ints><decs>1.0</decs><decs>2.0</decs><decs>10.0</decs></types></data></rpc-reply>]]>]]>$'
+
 if [ $BE -eq 0 ]; then
     exit # BE
 fi