diff --git a/src/diff.c b/src/diff.c index e22f7c949..e14f808da 100644 --- a/src/diff.c +++ b/src/diff.c @@ -377,13 +377,49 @@ lyd_diff_dup(const struct lyd_node *node, enum lyd_diff_op op, struct lyd_node * return LY_SUCCESS; } +void +lyd_diff_find_node(struct lyd_node *diff, const struct lyd_node *node, struct lyd_node **diff_parent, + struct lyd_node **match) +{ + struct lyd_node *siblings, *m = NULL, *dparent = NULL; + const struct lyd_node *parent = NULL; + + siblings = diff; + do { + /* find next node parent */ + parent = node; + while (parent->parent && (!dparent || (parent->parent->schema != dparent->schema))) { + parent = parent->parent; + } + + if (lysc_is_dup_inst_list(parent->schema)) { + /* assume it never exists, we are not able to distinguish whether it does or not */ + m = NULL; + break; + } + + /* check whether it exists in the diff */ + if (lyd_find_sibling_first(siblings, parent, &m)) { + break; + } + + /* another parent found */ + dparent = m; + + /* move down in the diff */ + siblings = lyd_child_no_keys(m); + } while (parent != node); + + *diff_parent = dparent; + *match = (m && (parent == node)) ? m : NULL; +} + LY_ERR lyd_diff_add(const struct lyd_node *node, enum lyd_diff_op op, const char *orig_default, const char *orig_value, const char *key, const char *value, const char *position, const char *orig_key, const char *orig_position, struct lyd_node **diff, struct lyd_node **diff_node) { - struct lyd_node *dup, *siblings, *match = NULL, *diff_parent = NULL, *elem; - const struct lyd_node *parent = NULL; + struct lyd_node *dup, *match = NULL, *diff_parent = NULL, *elem; enum lyd_diff_op cur_op; struct lyd_meta *meta; ly_bool found; @@ -410,35 +446,11 @@ lyd_diff_add(const struct lyd_node *node, enum lyd_diff_op op, const char *orig_ *diff_node = NULL; } - /* find the first existing parent */ - siblings = *diff; - do { - /* find next node parent */ - parent = node; - while (parent->parent && (!diff_parent || (parent->parent->schema != diff_parent->schema))) { - parent = parent->parent; - } - - if (lysc_is_dup_inst_list(parent->schema)) { - /* assume it never exists, we are not able to distinguish whether it does or not */ - match = NULL; - break; - } - - /* check whether it exists in the diff */ - if (lyd_find_sibling_first(siblings, parent, &match)) { - break; - } - - /* another parent found */ - diff_parent = match; - - /* move down in the diff */ - siblings = lyd_child_no_keys(match); - } while (parent != node); + lyd_diff_find_node(*diff, node, &diff_parent, &match); - if (match && (parent == node)) { + if (match) { /* special case when there is already an operation on our descendant */ + diff_parent = match; assert(!lyd_diff_get_op(diff_parent, &cur_op, NULL)); /* move it to the end where it is expected (matters for user-ordered lists) */ @@ -525,6 +537,26 @@ lyd_diff_add(const struct lyd_node *node, enum lyd_diff_op op, const char *orig_ return LY_SUCCESS; } +LY_ERR +lyd_diff_add_explicit_op(const struct lyd_node *node, enum lyd_diff_op op, const char *key, const char *value, + const char *position, struct lyd_node **diff) +{ + struct lyd_node *dup = NULL; + struct lyd_meta *meta; + + LY_CHECK_RET(lyd_diff_add(node, op, NULL, NULL, key, value, position, NULL, NULL, diff, &dup)); + + /* ::lyd_diff_add() omits an operation a parent already states, a validation diff states it + * on every node */ + lyd_diff_find_meta(dup, "operation", &meta, NULL); + if (!meta) { + LY_CHECK_RET(lyd_new_meta(NULL, dup, NULL, "yang:operation", lyd_diff_op2str(op), + LYD_NEW_VAL_STORE_ONLY, NULL)); + } + + return LY_SUCCESS; +} + /** * @brief Get a userord entry for a specific user-ordered list/leaf-list. Create if does not exist yet. * @@ -2700,6 +2732,69 @@ lyd_diff_is_redundant(struct lyd_node *diff) return 0; } +/** + * @brief Check whether a diff subtree was created by validation only, so that deleting the + * corresponding data nodes cancels it out. + * + * @param[in] diff_node Diff subtree to check. + * @return Whether it can be dropped. + */ +static ly_bool +lyd_diff_val_subtree_created(const struct lyd_node *diff_node) +{ + const struct lyd_node *elem; + struct lyd_meta *meta; + struct lyd_attr *attr; + + LYD_TREE_DFS_BEGIN(diff_node, elem) { + if (!elem->schema) { + /* cannot reason about opaque nodes */ + return 0; + } + + lyd_diff_find_meta(elem, "operation", &meta, &attr); + if (attr) { + return 0; + } + if (meta && (lyd_diff_str2op(lyd_get_meta_value(meta)) != LYD_DIFF_OP_CREATE)) { + return 0; + } + + /* a non-default term was set explicitly, deleting it is a real change */ + if ((elem->schema->nodetype & LYD_NODE_TERM) && !(elem->flags & LYD_DEFAULT)) { + return 0; + } + + LYD_TREE_DFS_END(diff_node, elem); + } + + return 1; +} + +LY_ERR +lyd_diff_val_del_created(struct lyd_node *diff_node, struct lyd_node **diff) +{ + struct lyd_node *parent; + enum lyd_diff_op op; + + LY_CHECK_RET(lyd_diff_get_op(diff_node, &op, NULL)); + if ((op != LYD_DIFF_OP_CREATE) || !lyd_diff_val_subtree_created(diff_node)) { + return LY_ENOT; + } + + /* drop it, then any ancestor left without a change */ + do { + parent = lyd_parent(diff_node); + if (diff_node == *diff) { + *diff = (*diff)->next; + } + lyd_free_tree(diff_node); + diff_node = parent; + } while (diff_node && lyd_diff_is_redundant(diff_node)); + + return LY_SUCCESS; +} + /** * @brief Merge all diff metadata found on a source diff node. * diff --git a/src/diff.h b/src/diff.h index b7f360496..85545f1b7 100644 --- a/src/diff.h +++ b/src/diff.h @@ -40,6 +40,17 @@ enum lyd_diff_op { LYD_DIFF_OP_NONE /**< No change of an existing inner node or default flag change of a term node. */ }; +/** + * @brief Find a node and its deepest existing ancestor in a diff. + * + * @param[in] diff Diff to search (first sibling). + * @param[in] node Data node to look for. + * @param[out] diff_parent Deepest ancestor of @p node in @p diff, NULL if none. + * @param[out] match Diff node of @p node, NULL if not present. + */ +void lyd_diff_find_node(struct lyd_node *diff, const struct lyd_node *node, struct lyd_node **diff_parent, + struct lyd_node **match); + /** * @brief Add a new change into diff. * @@ -60,4 +71,30 @@ LIBYANG_API_DECL LY_ERR lyd_diff_add(const struct lyd_node *node, enum lyd_diff_ const char *orig_value, const char *key, const char *value, const char *position, const char *orig_key, const char *orig_position, struct lyd_node **diff, struct lyd_node **diff_node); +/** + * @brief Add a new change into diff, always stating the operation on the added node. + * + * Unlike ::lyd_diff_add(), sets the operation metadata even when a parent already states it. + * + * @param[in] node Node (subtree) to add into diff. + * @param[in] op Operation to set. + * @param[in] key Key metadata to set. + * @param[in] value Value metadata to set. + * @param[in] position Position metadata to set. + * @param[in,out] diff Diff to append to. + * @return LY_ERR value. + */ +LY_ERR lyd_diff_add_explicit_op(const struct lyd_node *node, enum lyd_diff_op op, const char *key, + const char *value, const char *position, struct lyd_node **diff); + +/** + * @brief Drop a diff subtree that validation created, when its data nodes are being deleted again. + * + * @param[in] diff_node Diff node of the data node being deleted. + * @param[in,out] diff Diff @p diff_node belongs to. + * @return LY_SUCCESS if the subtree was dropped. + * @return LY_ENOT if it has to be merged instead. + */ +LY_ERR lyd_diff_val_del_created(struct lyd_node *diff_node, struct lyd_node **diff); + #endif /* LY_DIFF_H_ */ diff --git a/src/validation.c b/src/validation.c index d43176443..eb03b1818 100644 --- a/src/validation.c +++ b/src/validation.c @@ -188,7 +188,7 @@ LY_ERR lyd_val_diff_add(const struct lyd_node *node, enum lyd_diff_op op, struct lyd_node **diff) { LY_ERR ret = LY_SUCCESS; - struct lyd_node *new_diff = NULL; + struct lyd_node *new_diff = NULL, *diff_parent, *match; const struct lyd_node *prev_inst; char *key = NULL, *value = NULL, *position = NULL; size_t buflen = 0, bufused = 0; @@ -240,6 +240,24 @@ lyd_val_diff_add(const struct lyd_node *node, enum lyd_diff_op op, struct lyd_no } } + /* appending reuses the ancestors already in the diff, unlike building a one-node diff and + * merging it; merge only to reconcile an operation already recorded on this node */ + lyd_diff_find_node(*diff, node, &diff_parent, &match); + if (!match) { + ret = lyd_diff_add_explicit_op(node, op, key, value, position, diff); + goto cleanup; + } + + /* a create followed by a delete cancels out; merging would reconcile the whole subtree to + * "none" only to drop it as redundant */ + if (op == LYD_DIFF_OP_DELETE) { + ret = lyd_diff_val_del_created(match, diff); + if (ret != LY_ENOT) { + goto cleanup; + } + ret = LY_SUCCESS; + } + /* create new diff tree */ LY_CHECK_GOTO(ret = lyd_diff_add(node, op, NULL, NULL, key, value, position, NULL, NULL, &new_diff, NULL), cleanup); diff --git a/tests/perf/perf.c b/tests/perf/perf.c index c3c43085e..52fa70368 100644 --- a/tests/perf/perf.c +++ b/tests/perf/perf.c @@ -342,6 +342,78 @@ setup_data_offset_tree(const struct lys_module *mod, uint32_t count, struct test } /* TEST CB */ +static LY_ERR +setup_data_dflt_tree(const struct lys_module *mod, uint32_t count, struct test_state *state) +{ + const struct lys_module *dflt_mod; + struct lyd_node *parent; + char buf[64]; + uint32_t i; + LY_ERR r; + + state->count = count; + + dflt_mod = ly_ctx_get_module_implemented(mod->ctx, "perf_dflt"); + if (!dflt_mod) { + return LY_ENOTFOUND; + } + state->mod = dflt_mod; + + for (i = 0; i < count; ++i) { + sprintf(buf, "/perf_dflt:cont/lst[k='%" PRIu32 "']", i); + if ((r = lyd_new_path(state->data1, mod->ctx, buf, NULL, 0, &parent))) { + return r; + } + if (!state->data1) { + state->data1 = parent; + } + + sprintf(buf, "/perf_dflt:cont/lst[k='%" PRIu32 "']/type", i); + if ((r = lyd_new_path(state->data1, mod->ctx, buf, "a", 0, NULL))) { + return r; + } + } + + return LY_SUCCESS; +} + +static LY_ERR +test_validate_dflt(struct test_state *state, struct timespec *ts_start, struct timespec *ts_end, uint32_t *size) +{ + LY_ERR r; + + *size = 0; + TEST_START(ts_start); + + if ((r = lyd_validate_all(&state->data1, NULL, LYD_VALIDATE_PRESENT, NULL))) { + return r; + } + + TEST_END(ts_end); + + return LY_SUCCESS; +} + +static LY_ERR +test_validate_dflt_diff(struct test_state *state, struct timespec *ts_start, struct timespec *ts_end, uint32_t *size) +{ + struct lyd_node *diff = NULL; + LY_ERR r; + + *size = 0; + TEST_START(ts_start); + + if ((r = lyd_validate_all(&state->data1, NULL, LYD_VALIDATE_PRESENT, &diff))) { + return r; + } + + TEST_END(ts_end); + + lyd_free_siblings(diff); + + return LY_SUCCESS; +} + static LY_ERR test_create_new_text(struct test_state *state, struct timespec *ts_start, struct timespec *ts_end, uint32_t *size) { @@ -820,6 +892,8 @@ struct test tests[] = { {"create new text", setup_basic, test_create_new_text}, {"create path", setup_basic, test_create_path}, {"validate", setup_data_single_tree, test_validate}, + {"validate defaults", setup_data_dflt_tree, test_validate_dflt}, + {"validate defaults diff", setup_data_dflt_tree, test_validate_dflt_diff}, {"parse xml mem validate", setup_data_single_tree, test_parse_xml_mem_validate}, {"parse xml mem no validate", setup_data_single_tree, test_parse_xml_mem_no_validate}, {"parse xml file no validate format", setup_data_single_tree, test_parse_xml_file_no_validate_format}, @@ -887,6 +961,10 @@ main(int argc, char **argv) ret = LY_ENOTFOUND; goto cleanup; } + if (!ly_ctx_load_module(ctx, "perf_dflt", NULL, NULL)) { + ret = LY_ENOTFOUND; + goto cleanup; + } /* tests */ name_len = 0; diff --git a/tests/perf/perf_dflt.yang b/tests/perf/perf_dflt.yang new file mode 100644 index 000000000..57a60dd7a --- /dev/null +++ b/tests/perf/perf_dflt.yang @@ -0,0 +1,450 @@ +module perf_dflt { + yang-version 1.1; + namespace "urn:sysrepo:tests:perf-dflt"; + prefix pd; + + description + "Every list instance offers four alternative sets of defaults gated on the same leaf, + so only one set can ever apply. Validation materialises the defaults of all four + before resolving the when conditions, then deletes the three that do not apply."; + + container cont { + list lst { + key "k"; + + leaf k { + type uint32; + } + + leaf type { + type enumeration { + enum "a"; + enum "b"; + enum "c"; + enum "d"; + } + } + + container set-a { + when "../type = 'a'"; + + leaf d1 { + type uint32; + default 1; + } + + leaf d2 { + type uint32; + default 2; + } + + leaf d3 { + type uint32; + default 3; + } + + leaf d4 { + type uint32; + default 4; + } + + leaf d5 { + type uint32; + default 5; + } + + leaf d6 { + type uint32; + default 6; + } + + leaf d7 { + type uint32; + default 7; + } + + leaf d8 { + type uint32; + default 8; + } + + leaf d9 { + type uint32; + default 9; + } + + leaf d10 { + type uint32; + default 10; + } + + leaf d11 { + type uint32; + default 11; + } + + leaf d12 { + type uint32; + default 12; + } + + leaf d13 { + type uint32; + default 13; + } + + leaf d14 { + type uint32; + default 14; + } + + leaf d15 { + type uint32; + default 15; + } + + leaf d16 { + type uint32; + default 16; + } + + leaf d17 { + type uint32; + default 17; + } + + leaf d18 { + type uint32; + default 18; + } + + leaf d19 { + type uint32; + default 19; + } + + leaf d20 { + type uint32; + default 20; + } + + } + + container set-b { + when "../type = 'b'"; + + leaf d1 { + type uint32; + default 1; + } + + leaf d2 { + type uint32; + default 2; + } + + leaf d3 { + type uint32; + default 3; + } + + leaf d4 { + type uint32; + default 4; + } + + leaf d5 { + type uint32; + default 5; + } + + leaf d6 { + type uint32; + default 6; + } + + leaf d7 { + type uint32; + default 7; + } + + leaf d8 { + type uint32; + default 8; + } + + leaf d9 { + type uint32; + default 9; + } + + leaf d10 { + type uint32; + default 10; + } + + leaf d11 { + type uint32; + default 11; + } + + leaf d12 { + type uint32; + default 12; + } + + leaf d13 { + type uint32; + default 13; + } + + leaf d14 { + type uint32; + default 14; + } + + leaf d15 { + type uint32; + default 15; + } + + leaf d16 { + type uint32; + default 16; + } + + leaf d17 { + type uint32; + default 17; + } + + leaf d18 { + type uint32; + default 18; + } + + leaf d19 { + type uint32; + default 19; + } + + leaf d20 { + type uint32; + default 20; + } + + } + + container set-c { + when "../type = 'c'"; + + leaf d1 { + type uint32; + default 1; + } + + leaf d2 { + type uint32; + default 2; + } + + leaf d3 { + type uint32; + default 3; + } + + leaf d4 { + type uint32; + default 4; + } + + leaf d5 { + type uint32; + default 5; + } + + leaf d6 { + type uint32; + default 6; + } + + leaf d7 { + type uint32; + default 7; + } + + leaf d8 { + type uint32; + default 8; + } + + leaf d9 { + type uint32; + default 9; + } + + leaf d10 { + type uint32; + default 10; + } + + leaf d11 { + type uint32; + default 11; + } + + leaf d12 { + type uint32; + default 12; + } + + leaf d13 { + type uint32; + default 13; + } + + leaf d14 { + type uint32; + default 14; + } + + leaf d15 { + type uint32; + default 15; + } + + leaf d16 { + type uint32; + default 16; + } + + leaf d17 { + type uint32; + default 17; + } + + leaf d18 { + type uint32; + default 18; + } + + leaf d19 { + type uint32; + default 19; + } + + leaf d20 { + type uint32; + default 20; + } + + } + + container set-d { + when "../type = 'd'"; + + leaf d1 { + type uint32; + default 1; + } + + leaf d2 { + type uint32; + default 2; + } + + leaf d3 { + type uint32; + default 3; + } + + leaf d4 { + type uint32; + default 4; + } + + leaf d5 { + type uint32; + default 5; + } + + leaf d6 { + type uint32; + default 6; + } + + leaf d7 { + type uint32; + default 7; + } + + leaf d8 { + type uint32; + default 8; + } + + leaf d9 { + type uint32; + default 9; + } + + leaf d10 { + type uint32; + default 10; + } + + leaf d11 { + type uint32; + default 11; + } + + leaf d12 { + type uint32; + default 12; + } + + leaf d13 { + type uint32; + default 13; + } + + leaf d14 { + type uint32; + default 14; + } + + leaf d15 { + type uint32; + default 15; + } + + leaf d16 { + type uint32; + default 16; + } + + leaf d17 { + type uint32; + default 17; + } + + leaf d18 { + type uint32; + default 18; + } + + leaf d19 { + type uint32; + default 19; + } + + leaf d20 { + type uint32; + default 20; + } + + } + + } + } +}