diff --git a/CHANGELOG.md b/CHANGELOG.md index 63321267..b80e4c9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [Problem deleting non-last list element if ordered-by user](https://github.com/clicon/clixon/issues/475) * Fixed: [Tab completion mounted devices with lists](https://github.com/clicon/clixon-controller/issues/72) * Fixed: kill-session cleanup when client none existant, and for all db:s * Fixed: [Using the characters '<' and '>' might cause an invalid diff](https://github.com/clicon/clixon-controller/issues/73) diff --git a/lib/src/clixon_text_syntax.c b/lib/src/clixon_text_syntax.c index 8f8b7a13..e3de7f3b 100644 --- a/lib/src/clixon_text_syntax.c +++ b/lib/src/clixon_text_syntax.c @@ -78,6 +78,9 @@ #define TEXT_TOP_SYMBOL "top" +/* Forward */ +static int text_diff2cbuf(cbuf *cb, cxobj *x0, cxobj *x1, int level, int skiptop); + /*! x is element and has eactly one child which in turn has none * * @see child_type in clixon_json.c @@ -563,6 +566,67 @@ text_diff_keys(cbuf *cb, return 0; } +/*! Handle order-by user(leaf)list for text_diff2cbuf + * + * @param[out] cb CLIgen buffer + * @param[in] x0 First XML tree + * @param[in] x1 Second XML tree + * @param[in] x0c Start of sublist in first XML tree + * @param[in] x1c Start of sublist in second XML tree + * @param[in] yc Yang of x0c/x1c + * @param[in] level How many spaces to insert before each line + * @param[in] skiptop 0: Include top object 1: Skip top-object, only children, + * @retval 0 Ok + * @retval -1 Error + * @see xml_diff_ordered_by_user + * @see text_diff2cbuf_ordered_by_user + */ +static int +text_diff2cbuf_ordered_by_user(cbuf *cb, + cxobj *x0, + cxobj *x1, + cxobj *x0c, + cxobj *x1c, + yang_stmt *yc, + int level, + int skiptop) +{ + int retval = 1; + cxobj *xi; + cxobj *xj; + + xj = x1c; + do { + xml_flag_set(xj, XML_FLAG_ADD); + } while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + /* If in both sets, unmark add/del */ + xi = x0c; + do { + xml_flag_set(xi, XML_FLAG_DEL); + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD) && + xml_cmp(xi, xj, 0, 0, NULL) == 0){ + /* Unmark node in x0 and x1 */ + xml_flag_reset(xi, XML_FLAG_DEL); + xml_flag_reset(xj, XML_FLAG_ADD); + if (text_diff2cbuf(cb, xi, xj, level+1, 0) < 0) + goto done; + break; + } + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc); + + retval = 0; + done: + return retval; +} + /*! Print TEXT diff of two cxobj trees into a cbuf * * YANG dependent @@ -589,6 +653,7 @@ text_diff_keys(cbuf *cb, * + value [ * + 97 * - 99 + * @see xml_diff2cbuf */ static int text_diff2cbuf(cbuf *cb, @@ -611,6 +676,8 @@ text_diff2cbuf(cbuf *cb, char *prefix = NULL; int leafl = 0; // XXX char *leaflname = NULL; // XXX + cxobj *xi; + cxobj *xj; level1 = level*PRETTYPRINT_INDENT; if ((y0 = xml_spec(x0)) != NULL){ @@ -660,8 +727,58 @@ text_diff2cbuf(cbuf *cb, eq = xml_cmp(x0c, x1c, 0, 0, NULL); yc0 = xml_spec(x0c); yc1 = xml_spec(x1c); + if (eq && yc0 && yc1 && yang_find(yc0, Y_ORDERED_BY, "user")){ + if (text_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, yc0, + level, skiptop) < 0) + goto done; + /* Add all in x0 marked as DELETE in x0vec + * Flags can remain: XXX should apply to all + */ + xi = x0c; + do { + if (xml_flag(xi, XML_FLAG_DEL)){ + xml_flag_reset(xi, XML_FLAG_DEL); + if (nr==0 && skiptop==0){ + cprintf(cb, "%*s", level1, ""); + if (prefix) + cprintf(cb, "%s:", prefix); + cprintf(cb, "%s", xml_name(x0)); + text_diff_keys(cb, x0, y0); + cprintf(cb, " {\n"); + nr++; + } + if (text2cbuf(cb, xi, level+1, "-", 0, &leafl, &leaflname) < 0) + goto done; + } + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc0); + x0c = xi; - if (eq < 0){ + /* Add all in x1 marked as ADD in x1vec */ + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD)){ + xml_flag_reset(xj, XML_FLAG_ADD); + if (nr==0 && skiptop==0){ + cprintf(cb, "%*s", level1, ""); + if (prefix) + cprintf(cb, "%s:", prefix); + cprintf(cb, "%s", xml_name(x1)); + text_diff_keys(cb, x1, y0); + cprintf(cb, " {\n"); + nr++; + } + if (text2cbuf(cb, xj, level+1, "+", 0, &leafl, &leaflname) < 0) + goto done; + } + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc1); + x1c = xj; + continue; + } + else if (eq < 0){ if (nr==0 && skiptop==0){ cprintf(cb, "%*s", level1, ""); if (prefix) diff --git a/lib/src/clixon_xml_io.c b/lib/src/clixon_xml_io.c index ca39b1e9..5ee9b0be 100644 --- a/lib/src/clixon_xml_io.c +++ b/lib/src/clixon_xml_io.c @@ -652,7 +652,7 @@ _xml_parse(const char *str, if (xml_sort_recurse(xt) < 0) goto done; retval = 1; - done: + done: clixon_xml_parsel_exit(&xy); if (xy.xy_parse_string != NULL) free(xy.xy_parse_string); @@ -955,6 +955,67 @@ xml_diff_context(cbuf *cb, return retval; } +/*! Handle order-by user(leaf)list for xml_diff2cbuf + * + * @param[out] cb CLIgen buffer + * @param[in] x0 First XML tree + * @param[in] x1 Second XML tree + * @param[in] x0c Start of sublist in first XML tree + * @param[in] x1c Start of sublist in second XML tree + * @param[in] yc Yang of x0c/x1c + * @param[in] level How many spaces to insert before each line + * @param[in] skiptop 0: Include top object 1: Skip top-object, only children, + * @retval 0 Ok + * @retval -1 Error + * @see xml_diff_ordered_by_user + * @see text_diff2cbuf_ordered_by_user + */ +static int +xml_diff2cbuf_ordered_by_user(cbuf *cb, + cxobj *x0, + cxobj *x1, + cxobj *x0c, + cxobj *x1c, + yang_stmt *yc, + int level, + int skiptop) +{ + int retval = 1; + cxobj *xi; + cxobj *xj; + + xj = x1c; + do { + xml_flag_set(xj, XML_FLAG_ADD); + } while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + /* If in both sets, unmark add/del */ + xi = x0c; + do { + xml_flag_set(xi, XML_FLAG_DEL); + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD) && + xml_cmp(xi, xj, 0, 0, NULL) == 0){ + /* Unmark node in x0 and x1 */ + xml_flag_reset(xi, XML_FLAG_DEL); + xml_flag_reset(xj, XML_FLAG_ADD); + if (xml_diff2cbuf(cb, xi, xj, level+1, 0) < 0) + goto done; + break; + } + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc); + + retval = 0; + done: + return retval; +} + /*! Print XML diff of two cxobj trees into a cbuf * * YANG dependent @@ -974,6 +1035,7 @@ xml_diff_context(cbuf *cb, * @endcode * @see xml_diff which returns diff sets * @see clixon_compare_xmls which uses files and is independent of YANG + * @see text_diff2cbuf */ int xml_diff2cbuf(cbuf *cb, @@ -995,6 +1057,8 @@ xml_diff2cbuf(cbuf *cb, int eq; int nr=0; int level1; + cxobj *xi; + cxobj *xj; level1 = level*PRETTYPRINT_INDENT; y0 = xml_spec(x0); @@ -1030,7 +1094,52 @@ xml_diff2cbuf(cbuf *cb, } /* Both x0c and x1c exists, check if they are yang-equal. */ eq = xml_cmp(x0c, x1c, 0, 0, NULL); - if (eq < 0){ + yc0 = xml_spec(x0c); + yc1 = xml_spec(x1c); + if (eq && yc0 && yc1 && yang_find(yc0, Y_ORDERED_BY, "user")){ + if (xml_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, yc0, + level, skiptop) < 0) + goto done; + /* Add all in x0 marked as DELETE in x0vec + * Flags can remain: XXX should apply to all + */ + xi = x0c; + do { + if (xml_flag(xi, XML_FLAG_DEL)){ + xml_flag_reset(xi, XML_FLAG_DEL); + if (nr==0 && skiptop==0){ + xml_diff_context(cb, x0, level1); + xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT); + nr++; + } + if (clixon_xml2cbuf(cb, xi, level+1, 1, "-", -1, 0) < 0) + goto done; + } + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc0); + x0c = xi; + + /* Add all in x1 marked as ADD in x1vec */ + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD)){ + xml_flag_reset(xj, XML_FLAG_ADD); + if (nr==0 && skiptop==0){ + xml_diff_context(cb, x1, level1); + xml_diff_keys(cb, x1, y0, (level+1)*PRETTYPRINT_INDENT); + nr++; + } + if (clixon_xml2cbuf(cb, xj, level+1, 1, "+", -1, 0) < 0) + goto done; + } + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc1); + x1c = xj; + continue; + } + else if (eq < 0){ if (nr==0 && skiptop==0){ xml_diff_context(cb, x0, level1); xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT); @@ -1056,8 +1165,6 @@ xml_diff2cbuf(cbuf *cb, /* xml-spec NULL could happen with anydata children for example, * if so, continute compare children but without yang */ - yc0 = xml_spec(x0c); - yc1 = xml_spec(x1c); if (yc0 && yc1 && yc0 != yc1){ /* choice */ if (nr==0 && skiptop==0){ xml_diff_context(cb, x0, level1); @@ -1145,6 +1252,7 @@ xml_diff2cbuf(cbuf *cb, * @endcode * @see xml_diff which returns diff sets * @see clixon_compare_xmls which uses files and is independent of YANG + * @see clixon_text_diff2cbuf */ int clixon_xml_diff2cbuf(cbuf *cb, diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index d3f47145..db1dd011 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -93,6 +93,11 @@ typedef struct { yang_stmt *mt_yc; } merge_twophase; +/* Forward declaration */ +static int xml_diff1(cxobj *x0, cxobj *x1, cxobj ***x0vec, int *x0veclen, + cxobj ***x1vec, int *x1veclen, + cxobj ***changed_x0, cxobj ***changed_x1, int *changedlen); + /*! Is attribute and is either of form xmlns="", or xmlns:x="" */ int isxmlns(cxobj *x) @@ -276,6 +281,73 @@ cvec2xml_1(cvec *cvv, return retval; } +/*! Handle order-by user(leaf)list for xml_diff + * + * Loop over sublists started by x0c and x1c respectively until end or yang is no longer yc + * First mark all in x0 as DELETE and all x1 as ADD + * Then find all equal nodes and unmark them + * After the function, there will be nodes in x0 marked with DEL and nodes in x1 marked as ADD + * @param[in] x0 First XML tree + * @param[in] x1 Second XML tree + * @param[in] x0c Start of sublist in first XML tree + * @param[in] x1c Start of sublist in second XML tree + * @param[in] yc Yang of ordered-by user (leaf)list + * @retval 0 Ok + * @retval -1 rror + */ +static int +xml_diff_ordered_by_user(cxobj *x0, + cxobj *x1, + cxobj *x0c, + cxobj *x1c, + yang_stmt *yc, + cxobj ***x0vec, + int *x0veclen, + cxobj ***x1vec, + int *x1veclen, + cxobj ***changed_x0, + cxobj ***changed_x1, + int *changedlen + ) +{ + int retval = -1; + cxobj *xi; + cxobj *xj; + + xj = x1c; + do { + xml_flag_set(xj, XML_FLAG_ADD); + } while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + /* If in both sets, unmark add/del */ + xi = x0c; + do { + xml_flag_set(xi, XML_FLAG_DEL); + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD) && + xml_cmp(xi, xj, 0, 0, NULL) == 0){ + /* Unmark node in x0 and x1 */ + xml_flag_reset(xi, XML_FLAG_DEL); + xml_flag_reset(xj, XML_FLAG_ADD); + if (xml_diff1(xi, xj, + x0vec, x0veclen, + x1vec, x1veclen, + changed_x0, changed_x1, changedlen) < 0) + goto done; + break; + } + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc); + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc); + retval = 0; + done: + return retval; +} + /*! Recursive help function to compute differences between two xml trees * * @param[in] x0 First XML tree @@ -302,6 +374,7 @@ cvec2xml_1(cvec *cvv, * perspective, ie both have the same yang spec, if they are lists, they have the * the same keys. NOT that the values are equal! * @see xml_diff API function, this one is internal and recursive + * @note reordering in ordered-by user is NOT supported */ static int xml_diff1(cxobj *x0, @@ -322,6 +395,8 @@ xml_diff1(cxobj *x0, char *b0; char *b1; int eq; + cxobj *xi; + cxobj *xj; /* Traverse x0 and x1 in lock-step */ x0c = x1c = NULL; @@ -344,7 +419,41 @@ xml_diff1(cxobj *x0, } /* Both x0c and x1c exists, check if they are yang-equal. */ eq = xml_cmp(x0c, x1c, 0, 0, NULL); - if (eq < 0){ + yc0 = xml_spec(x0c); + yc1 = xml_spec(x1c); + /* override ordered-by user with special look-ahead checks */ + if (eq && yc0 && yc1 && yang_find(yc0, Y_ORDERED_BY, "user")){ + if (xml_diff_ordered_by_user(x0, x1, x0c, x1c, yc0, + x0vec, x0veclen, x1vec, x1veclen, + changed_x0, changed_x1, changedlen) < 0) + goto done; + /* Add all in x0 marked as DELETE in x0vec + * Flags can remain: XXX should apply to all + */ + xi = x0c; + do { + if (xml_flag(xi, XML_FLAG_DEL)){ + if (cxvec_append(xi, x0vec, x0veclen) < 0) + goto done; + } + } + while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && + xml_spec(xi) == yc0); + x0c = xi; + + /* Add all in x1 marked as ADD in x1vec */ + xj = x1c; + do { + if (xml_flag(xj, XML_FLAG_ADD)) + if (cxvec_append(xj, x1vec, x1veclen) < 0) + goto done; + } + while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && + xml_spec(xj) == yc1); + x1c = xj; + continue; + } + else if (eq < 0){ if (cxvec_append(x0c, x0vec, x0veclen) < 0) goto done; x0c = xml_child_each(x0, x0c, CX_ELMNT); @@ -360,8 +469,6 @@ xml_diff1(cxobj *x0, /* xml-spec NULL could happen with anydata children for example, * if so, continute compare children but without yang */ - yc0 = xml_spec(x0c); - yc1 = xml_spec(x1c); if (yc0 && yc1 && yc0 != yc1){ /* choice */ if (cxvec_append(x0c, x0vec, x0veclen) < 0) goto done; @@ -1233,9 +1340,9 @@ xml_merge1(cxobj *x0, /* the target */ for (i=0; i" "" "ebarafoo71fiebbar42fumcfoodfie" +new "netconf discard" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "" + +new "add y0: a e b c d" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "aebcd" "" "" + +new "netconf commit" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "" + +# remove ordered-by user https://github.com/clicon/clixon/issues/475 +KEY=e +new "rm $KEY" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "none$KEY" "" "" + +new "check ordered-by-user: a b c d" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "abcd" + +new "show compare xml" +expectpart "$($clixon_cli -1 -f $cfg show compare xml)" 0 "\- e" --not-- "a" "b" "c" "d" "+ e" + +new "show compare curly" +expectpart "$($clixon_cli -1 -f $cfg show compare text)" 0 "\- order-example:y0" "\- e" --not-- "+ order-example:y0" "\- a" "+ e" + +new "validate" +expectpart "$($clixon_cli -1 -f $cfg validate)" 0 "^$" + +new "netconf discard" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "" + +# remove ordered-by user https://github.com/clicon/clixon/issues/475 +KEY=b +new "rm $KEY" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "none$KEY" "" "" + +new "check ordered-by-user: a e c d" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "aecd" + +new "show compare 2" +expectpart "$($clixon_cli -1 -f $cfg show compare xml)" 0 "\- b" --not-- "a" "c" "d" "+ b" + +new "netconf discard" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "" "" "" + + if [ $BE -ne 0 ]; then new "Kill backend" # Check if premature kill