diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c4f28b3..a6f4c689 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ ## 7.2.0 Expected: October 2024 +### Features + +* List pagination: added sort-by parameter + ### C/CLI-API changes on existing features Developers may need to change their code diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c index 77f4cf03..636deac0 100644 --- a/apps/backend/backend_get.c +++ b/apps/backend/backend_get.c @@ -531,27 +531,27 @@ get_list_pagination(clixon_handle h, cbuf *cbret ) { - int retval = -1; - uint32_t offset = 0; - uint32_t limit = 0; - cbuf *cbpath = NULL; - int list_config; - yang_stmt *ylist; - cxobj *xerr = NULL; - cbuf *cbmsg = NULL; /* For error msg */ - cxobj *xret = NULL; - char *xpath2; /* With optional pagination predicate */ - uint32_t iddb; /* DBs lock, if any */ - int locked; - cbuf *cberr = NULL; - cxobj **xvec = NULL; - size_t xlen; - int ret; + int retval = -1; + uint32_t offset = 0; + uint32_t limit = 0; + cbuf *cbpath = NULL; + int list_config; + yang_stmt *ylist; + cxobj *xerr = NULL; + cbuf *cbmsg = NULL; /* For error msg */ + cxobj *xret = NULL; + char *xpath2; /* With optional pagination predicate */ + uint32_t iddb; /* DBs lock, if any */ + int locked; + cbuf *cberr = NULL; + cxobj **xvec = NULL; + size_t xlen; + int ret; + cxobj *x; + char *sort_by = NULL; #ifdef NOTYET - cxobj *x; - char *direction = NULL; - char *sort = NULL; - char *where = NULL; + char *direction = NULL; + char *where = NULL; #endif if (cbret == NULL){ @@ -606,16 +606,16 @@ get_list_pagination(clixon_handle h, goto ok; } } - /* sort */ - if (ret && (x = xml_find_type(xe, NULL, "sort-by", CX_ELMNT)) != NULL) - sort = xml_body(x); - if (sort) { - /* XXX: nothing yet */ - } /* where */ if (ret && (x = xml_find_type(xe, NULL, "where", CX_ELMNT)) != NULL) where = xml_body(x); -#endif +#endif /* NOTYET */ + /* sort-by */ + if (ret && (x = xml_find_type(xe, NULL, "sort-by", CX_ELMNT)) != NULL){ + sort_by = xml_body(x); + if (strcmp(sort_by, "none") == 0) + sort_by = NULL; + } if (ret == 0) goto ok; /* Read config */ @@ -664,7 +664,6 @@ get_list_pagination(clixon_handle h, if (ret == 0){ if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) goto done; - goto ok; } break; @@ -674,10 +673,32 @@ get_list_pagination(clixon_handle h, break; }/* switch content */ + /* Read state */ + switch (content){ + case CONTENT_CONFIG: /* config data only */ + break; + case CONTENT_ALL: /* both config and state */ + case CONTENT_NONCONFIG: /* state data only */ + if (list_config == 0) + break; + if ((ret = get_statedata(h, xpath?xpath:"/", nsc, &xret)) < 0) + goto done; + if (ret == 0){ /* Error from callback (error in xret) */ + if (clixon_xml2cbuf(cbret, xret, 0, 0, NULL, -1, 0) < 0) + goto done; + goto ok; + } + /* Add defaults to state data. This consumes some cycles */ + /* Ensure all state-data is report-all */ + if (xml_global_defaults(h, xret, nsc, xpath, yspec, 1) < 0) + goto done; + /* Apply default values */ + if (xml_default_recurse(xret, 1, 0) < 0) + goto done; + } if (list_config){ #ifdef LIST_PAGINATION_REMAINING /* Get total/remaining - * XXX: Works only for cache */ if ((xcache = xmldb_cache_get(h, db)) != NULL){ if (xpath_count(xcache, nsc, xpath, &total) < 0) @@ -731,6 +752,12 @@ get_list_pagination(clixon_handle h, /* Help function to filter out anything that is outside of xpath */ if (filter_xpath_again(h, yspec, xret, xvec, xlen, xpath, nsc) < 0) goto done; + if (sort_by){ + cxobj *x, *xp; + if ((x = xpath_first(xret, nsc, "%s", xpath?xpath:"/")) != NULL && + (xp = xml_parent(x)) != NULL) + xml_sort_by(xp, sort_by); // XXX sort_by + } #ifdef LIST_PAGINATION_REMAINING /* Add remaining attribute Sec 3.1.5: Any list or leaf-list that is limited includes, on the first element in the result set, @@ -793,29 +820,27 @@ get_common(clixon_handle h, cbuf *cbret ) { - int retval = -1; - cxobj *xfilter; - char *xpath = NULL; - cxobj *xret = NULL; - char *username; - cvec *nsc0 = NULL; /* Create a netconf namespace context from filter */ - cvec *nsc = NULL; - char *attr; - int32_t depth = -1; /* Nr of levels to print, -1 is all, 0 is none */ - yang_stmt *yspec; - cxobj *xerr = NULL; - int ret; - char *reason = NULL; - cbuf *cbmsg = NULL; /* For error msg */ - char *xpath0; - char *xpath01 = NULL; - cbuf *cbreason = NULL; - int list_pagination = 0; - cxobj **xvec = NULL; - size_t xlen; - cxobj *xfind; - uint32_t offset = 0; - uint32_t limit = 0; + int retval = -1; + cxobj *xfilter; + char *xpath = NULL; + cxobj *xret = NULL; + char *username; + cvec *nsc0 = NULL; /* Create a netconf namespace context from filter */ + cvec *nsc = NULL; + char *attr; + int32_t depth = -1; /* Nr of levels to print, -1 is all, 0 is none */ + yang_stmt *yspec; + cxobj *xerr = NULL; + int ret; + char *reason = NULL; + cbuf *cbmsg = NULL; /* For error msg */ + char *xpath0; + char *xpath01 = NULL; + cbuf *cbreason = NULL; + cxobj **xvec = NULL; + size_t xlen; + cxobj *xlpg; + cxobj *xlpg2 = NULL; withdefaults_type wdef; char *wdefstr; @@ -862,33 +887,26 @@ get_common(clixon_handle h, } if ((wdefstr = xml_find_body(xe, "with-defaults")) != NULL) wdef = withdefaults_str2int(wdefstr); - /* Check if list pagination */ - if ((xfind = xml_find_type(xe, NULL, "list-pagination", CX_ELMNT)) != NULL){ - /* with non-presence list-pagination, use ad-hoc algorithm to determine - * whether list-pagination is enabled: - * offset!=0 && limit!=unbounded - */ - /* offset */ - if ((ret = element2value(h, xfind, "offset", "none", cbret, &offset)) < 0) - goto done; - /* limit */ - if (ret && (ret = element2value(h, xfind, "limit", "unbounded", cbret, &limit)) < 0) - goto done; - if (ret == 0) - goto ok; - list_pagination = (offset != 0 || limit != 0); - } - /* Sanity check for list pagination: path must be a list/leaf-list, if it is, - * check config/state + /* How to check if list-pagination? + * Problem is clixon expands messages on entry and pagination default values + non-presence cont + * Therefore reverse expands by removing defaults/nopresence and see if it is empty + * If it is empty, all are default values and is regular get */ - if (list_pagination){ - if (get_list_pagination(h, ce, - xfind, - content, db, - depth, yspec, xpath, nsc, username, wdef, - cbret) < 0) + if ((xlpg = xml_find_type(xe, NULL, "list-pagination", CX_ELMNT)) != NULL){ + + if ((xlpg2 = xml_dup(xlpg)) == NULL) goto done; - goto ok; + if (xml_default_nopresence(xlpg2, 2, 0) < 0) + goto done; + if (xml_child_nr_type(xlpg2, CX_ELMNT) != 0) { + if (get_list_pagination(h, ce, + xlpg, + content, db, + depth, yspec, xpath, nsc, username, wdef, + cbret) < 0) + goto done; + goto ok; + } } /* Read configuration */ switch (content){ @@ -1022,6 +1040,8 @@ get_common(clixon_handle h, retval = 0; done: clixon_debug(CLIXON_DBG_BACKEND | CLIXON_DBG_DETAIL, "retval:%d", retval); + if (xlpg2) + xml_free(xlpg2); if (xvec) free(xvec); if (xret) diff --git a/lib/clixon/clixon_plugin.h b/lib/clixon/clixon_plugin.h index 75afd40b..bf2d6655 100644 --- a/lib/clixon/clixon_plugin.h +++ b/lib/clixon/clixon_plugin.h @@ -279,7 +279,7 @@ typedef int (datastore_upgrade_t)(clixon_handle h, const char *db, cxobj *xt, mo /*! YANG schema mount * * Given an XML mount-point xt, return XML yang-lib modules-set - * Return yanglib as XML tree on the RFC8525 form: + * Return yanglib as XML tree on the RFC8528 form: * * * ... diff --git a/lib/clixon/clixon_xml_sort.h b/lib/clixon/clixon_xml_sort.h index fe8ae4f7..c4457ea3 100644 --- a/lib/clixon/clixon_xml_sort.h +++ b/lib/clixon/clixon_xml_sort.h @@ -42,7 +42,8 @@ * Prototypes */ int xml_cmp(cxobj *x1, cxobj *x2, int same, int skip1, char *expl); -int xml_sort(cxobj *x0); +int xml_sort(cxobj *x); +int xml_sort_by(cxobj *x, char *indexvar); int xml_sort_recurse(cxobj *xn); int xml_insert(cxobj *xp, cxobj *xc, enum insert_type ins, char *key_val, cvec *nsckey); int xml_sort_verify(cxobj *x, void *arg); diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index e1492781..e84724e1 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -41,6 +41,7 @@ #endif #include +#define __USE_GNU /* for qsort_r */ #include #include #include @@ -166,7 +167,7 @@ xml_cv_cache_clear(cxobj *xt) * @param[in] same If set, x1 and x2 are member of same parent & enumeration * is used (see explanation below) * @param[in] skip1 Key matching skipped for keys not in x1 (see explanation) - * @param[in] explicit For list nodes, use explicit index variables, not keys + * @param[in] indexvar For (leaf)list nodes, use explicit index variable xpaths * @retval 0 If equal * @retval <0 If x1 is less than x2 * @retval >0 If x1 is greater than x2 @@ -273,6 +274,7 @@ xml_cmp(cxobj *x1, * existing list. */ if (same && + indexvar == NULL && ( #ifndef STATE_ORDERED_BY_SYSTEM yang_config(y1)==0 || @@ -283,8 +285,25 @@ xml_cmp(cxobj *x1, } switch (yang_keyword_get(y1)){ case Y_LEAF_LIST: /* Match with name and value */ +#ifdef XML_EXPLICIT_INDEX + if (indexvar){ + if ((x1b = xpath_first(x1, 0, "%s", indexvar)) != NULL) + b1 = xml_body(x1b); + else + b1 = NULL; + if ((x2b = xpath_first(x2, 0, "%s", indexvar)) != NULL) + b2 = xml_body(x2b); + else + b2 = NULL; + } + else { + b1 = xml_body(x1); + b2 = xml_body(x2); + } +#else b1 = xml_body(x1); b2 = xml_body(x2); +#endif if (b1 == NULL && b2 == NULL) ; else if (b1 == NULL) @@ -309,8 +328,8 @@ xml_cmp(cxobj *x1, case Y_LIST: /* Match with key values */ if (indexvar != NULL){ #ifdef XML_EXPLICIT_INDEX - x1b = xml_find(x1, indexvar); - x2b = xml_find(x2, indexvar); + x1b = xpath_first(x1, 0, "%s", indexvar); + x2b = xpath_first(x2, 0, "%s", indexvar); if (x1b == NULL && x2b == NULL) ; else if (x1b == NULL) @@ -396,19 +415,35 @@ xml_cmp(cxobj *x1, * @note args are pointer to pointers, to fit into qsort cmp function */ static int -xml_cmp_qsort(const void* arg1, - const void* arg2) +xml_cmp_qsort(const void *arg1, + const void *arg2, + void *indexvar) { - return xml_cmp(*(struct xml**)arg1, *(struct xml**)arg2, 1, 0, NULL); + return xml_cmp(*(struct xml**)arg1, *(struct xml**)arg2, 1, 0, indexvar); +} + +/*! Sort children of an XML node using an index + * + * @param[in] x XML node + * @param[in] indexvar Descendant-schema-nodeid + * @retval 0 OK, all nodes traversed (subparts may have been skipped) + */ +int +xml_sort_by(cxobj *x, + char *indexvar) +{ + xml_enumerate_children(x); /* This is to make sorting "stable", ie not change existing order */ + qsort_r(xml_childvec_get(x), xml_child_nr(x), sizeof(cxobj *), xml_cmp_qsort, indexvar); + return 0; } /*! Sort children of an XML node * * Assume populated by yang spec. - * @param[in] x0 XML node - * @retval 1 OK, aborted on first fn returned 1 - * @retval 0 OK, all nodes traversed (subparts may have been skipped) - * @retval -1 Error, aborted at first error encounter + * @param[in] x XML node + * @retval 1 OK, aborted on first fn returned 1 + * @retval 0 OK, all nodes traversed (subparts may have been skipped) + * @retval -1 Error, aborted at first error encounter * @see xml_apply - typically called by recursive apply function * @see xml_sort_verify */ @@ -423,7 +458,7 @@ xml_sort(cxobj *x) return 1; #endif xml_enumerate_children(x); /* This is to make sorting "stable", ie not change existing order */ - qsort(xml_childvec_get(x), xml_child_nr(x), sizeof(cxobj *), xml_cmp_qsort); + qsort_r(xml_childvec_get(x), xml_child_nr(x), sizeof(cxobj *), xml_cmp_qsort, NULL); return 0; } diff --git a/lib/src/clixon_xml_vec.c b/lib/src/clixon_xml_vec.c index 60838b74..b78d6588 100644 --- a/lib/src/clixon_xml_vec.c +++ b/lib/src/clixon_xml_vec.c @@ -34,6 +34,7 @@ ***** END LICENSE BLOCK ***** * Clixon XML object vectors + * Note these are used only occasionally, instead the more primitive cxobj** is mostly used. */ #ifdef HAVE_CONFIG_H diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 0f8e839a..5ccfd0d8 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -508,7 +508,7 @@ yang_when_nsc_set(yang_stmt *ys, return retval; } -/*! Get yang filename for error/debug purpose +/*! Get yang filename for error/debug purpose (only modules) * * @param[in] ys Yang statement * @retval filename @@ -3053,7 +3053,7 @@ ys_populate2(yang_stmt *ys, default: break; } - /* RFC 8525 Yang schema mount flag for optimization */ + /* RFC 8528 Yang schema mount flag for optimization */ if ((ret = yang_schema_mount_point0(ys)) < 0) goto done; if (ret == 1) diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index 8eae46d6..c4eb2a64 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -527,7 +527,6 @@ ys_iskey(yang_stmt *y, return 0; } - /*! Helper function to yang_expand_grouping * * @param[in] yn Yang parent node of uses ststement @@ -549,6 +548,7 @@ yang_expand_uses_node(yang_stmt *yn, yang_stmt *yg; /* grouping child */ yang_stmt *yr; /* refinement */ yang_stmt *yp; + yang_stmt *ym; int glen; size_t size; int j; @@ -564,8 +564,13 @@ yang_expand_uses_node(yang_stmt *yn, if (ys_grouping_resolve(ys, prefix, id, &ygrouping) < 0) goto done; if (ygrouping == NULL){ - clixon_log(NULL, LOG_NOTICE, "%s: Yang error : grouping \"%s\" not found in module \"%s\"", - __FUNCTION__, yang_argument_get(ys), yang_argument_get(ys_module(ys))); + if ((ym = ys_module(yn)) != NULL) + clixon_err(OE_YANG, 0, "Yang error : grouping \"%s\" not found in module \"%s\" in file: %s:%d", + yang_argument_get(ys), yang_argument_get(ys_module(ys)), + yang_filename_get(ym), yang_linenum_get(yn)); + else + clixon_err(OE_YANG, 0, "Yang error : grouping \"%s\" not found in module \"%s\"", + yang_argument_get(ys), yang_argument_get(ys_module(ys))); goto done; } /* Check so that this uses statement is not a descendant of the grouping diff --git a/lib/src/clixon_yang_schema_mount.c b/lib/src/clixon_yang_schema_mount.c index 94ee9cc7..73b89598 100644 --- a/lib/src/clixon_yang_schema_mount.c +++ b/lib/src/clixon_yang_schema_mount.c @@ -31,7 +31,7 @@ ***** END LICENSE BLOCK ***** - * RFC 8525 Yang schema mount support + * RFC 8528 Yang schema mount support * * Extend a container with ietf-yang-schema-mount:mount-point. * Structure of mount-points in YANG anc config: @@ -118,7 +118,7 @@ #include "clixon_xml_nsctx.h" #include "clixon_yang_schema_mount.h" -/*! Check if YANG node is a RFC 8525 YANG schema mount +/*! Check if YANG node is a RFC 8528 YANG schema mount * * Check if: * - y is CONTAINER or LIST, AND @@ -126,7 +126,7 @@ * - the extension label matches y (see note below) * If so, then return 1 * @param[in] y Yang statement - * @retval 1 Yes, y is a RFC 8525 YANG mount-point + * @retval 1 Yes, y is a RFC 8528 YANG mount-point * @retval 0 No, y is not * @retval -1 Error * @note That this may be a restriction on the usage of "label". The RFC is somewhat unclear. diff --git a/test/test_pagination_extra.sh b/test/test_pagination_extra.sh new file mode 100755 index 00000000..731d7178 --- /dev/null +++ b/test/test_pagination_extra.sh @@ -0,0 +1,251 @@ +#!/usr/bin/env bash +# List pagination tests according to draft-ietf-netconf-list-pagination-04 +# sort-by and where in Appendix A.3.5 +# Only NETCONF, see more extensive testng in _draft test + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + +APPNAME=example + +cfg=$dir/conf.xml +fexample=$dir/example-social.yang +fstate=$dir/mystate.xml + +# Common example-module spec (fexample must be set) +. ./example_social.sh + +# Validate internal state xml +: ${validatexml:=false} + +cat < $cfg + + $cfg + ietf-netconf:startup + ${YANG_INSTALLDIR} + $IETFRFC + $dir + /usr/local/var/run/$APPNAME.sock + /usr/local/lib/$APPNAME/backend + /usr/local/var/run/$APPNAME.pidfile + $dir + json + true + $APPNAME + /usr/local/lib/$APPNAME/cli + /usr/local/lib/$APPNAME/clispec + $validatexml + +EOF + +# See draft-wwlh-netconf-list-pagination-00 A.2 (except stats and audit-log) +cat <<'EOF' > $dir/startup_db +{"config": + { + "example-social:members": { + "member": [ + { + "member-id": "bob", + "email-address": "bob@example.com", + "password": "$0$1543", + "avatar": "BASE64VALUE=", + "tagline": "Here and now, like never before.", + "posts": { + "post": [ + { + "timestamp": "2020-08-14T03:32:25Z", + "body": "Just got in." + }, + { + "timestamp": "2020-08-14T03:33:55Z", + "body": "What's new?" + }, + { + "timestamp": "2020-08-14T03:34:30Z", + "body": "I'm bored..." + } + ] + }, + "favorites": { + "decimal64-numbers": ["3.14159", "2.71828"] + } + }, + { + "member-id": "eric", + "email-address": "eric@example.com", + "password": "$0$1543", + "avatar": "BASE64VALUE=", + "tagline": "Go to bed with dreams; wake up with a purpose.", + "following": ["alice"], + "posts": { + "post": [ + { + "timestamp": "2020-09-17T18:02:04Z", + "title": "Son, brother, husband, father", + "body": "What's your story?" + } + ] + }, + "favorites": { + "bits": ["two", "one", "zero"] + } + }, + { + "member-id": "alice", + "email-address": "alice@example.com", + "password": "$0$1543", + "avatar": "BASE64VALUE=", + "tagline": "Every day is a new day", + "privacy-settings": { + "hide-network": false, + "post-visibility": "public" + }, + "following": ["bob", "eric", "lin"], + "posts": { + "post": [ + { + "timestamp": "2020-07-08T13:12:45Z", + "title": "My first post", + "body": "Hiya all!" + }, + { + "timestamp": "2020-07-09T01:32:23Z", + "title": "Sleepy...", + "body": "Catch y'all tomorrow." + } + ] + }, + "favorites": { + "uint8-numbers": [17, 13, 11, 7, 5, 3], + "int8-numbers": [-5, -3, -1, 1, 3, 5] + } + }, + { + "member-id": "lin", + "email-address": "lin@example.com", + "password": "$0$1543", + "privacy-settings": { + "hide-network": true, + "post-visibility": "followers-only" + }, + "following": ["joe", "eric", "alice"] + }, + { + "member-id": "joe", + "email-address": "joe@example.com", + "password": "$0$1543", + "avatar": "BASE64VALUE=", + "tagline": "Greatness is measured by courage and heart.", + "privacy-settings": { + "post-visibility": "unlisted" + }, + "following": ["bob"], + "posts": { + "post": [ + { + "timestamp": "2020-10-17T18:02:04Z", + "body": "What's your status?" + } + ] + } + } + ] + } + } +} +EOF + +# See draft-wwlh-netconf-list-pagination-00 A.2 (only stats and audit-log) +cat< $fstate + + + bob + + 2020-08-14T03:30:00Z + standard + 2020-08-14T03:34:30Z + + + + eric + + 2020-09-17T19:38:32Z + pro + 2020-09-17T18:02:04Z + + + + alice + + 2020-07-08T12:38:32Z + admin + 2021-04-01T02:51:11Z + + + + lin + + 2020-07-09T12:38:32Z + standard + 2021-04-01T02:51:11Z + + + + joe + + 2020-10-08T12:38:32Z + pro + 2021-04-01T02:51:11Z + + + +EOF + +new "test params: -f $cfg -s startup -- -sS $fstate" + +if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + sudo pkill -f clixon_backend # to be sure + + new "start backend -s startup -f $cfg -- -sS $fstate" + start_backend -s startup -f $cfg -- -sS $fstate +fi + +new "wait backend" +wait_backend + +# new "A.3.5 The sort-by parameter" + +new "A.3.5.1.1. type is a leaf-list" +# 3,5,7,11,13,17 +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "." "alice357111317" + +new "A.3.5.1.2. type is a list and sort-by node is a direct descendent" +# alice, bob, eric, joe, lin +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "member-id" "alice.*bob.*eric.*joe.*lin" + +new "A.3.5.1.3. type is a list and sort-by node is an indirect descendent" +# alice, lin, bob, eric, joe +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "stats/joined" "alice.*lin.*bob.*eric.*joe" + +if [ $BE -ne 0 ]; then + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg +fi + +unset validatexml + +rm -rf $dir + +new "endtest" +endtest