From 77e1a78bbea3277bfc63de6ce52b03ca5bfaf9f5 Mon Sep 17 00:00:00 2001 From: Juraj Budai Date: Tue, 14 Oct 2025 14:45:22 +0200 Subject: [PATCH] diff BUGFIX user-ordered replace operation --- src/diff.c | 92 +++++++++++++++++++++++++++++++++++ tests/utests/data/test_diff.c | 62 +++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/src/diff.c b/src/diff.c index 9f5fe05d5..1b9950566 100644 --- a/src/diff.c +++ b/src/diff.c @@ -2004,6 +2004,86 @@ lyd_diff_change_op(struct lyd_node *node, enum lyd_diff_op op) } } +/** + * @brief In user-ordered lists, certain operations on sibling nodes can result in logically identical changes. + * However, applying the first change may cause the second one to fail. + * To prevent this, the affected node is unlinked and freed. + * + * @param[in,out] diff The node whose metadata has been modified. + * @param[in] mod The YANG module associated with the metadata. + * @return LY_ERR value. + */ +static LY_ERR +lyd_diff_find_and_unlink_cyclic_nodes(struct lyd_node **diff, const struct lys_module *mod) +{ + LY_ERR ret = LY_SUCCESS; + struct lyd_meta *meta1, *meta2; + struct lyd_node *diff_iter = *diff; + char *buff1 = NULL, *buff2 = NULL; + const char *name = NULL, *name_iter = NULL; + size_t bufflen1 = 0, buffused1 = 0; + size_t bufflen2 = 0, buffused2 = 0; + + /* itereate throught previous nodes and look for logically identical changes */ + while (diff_iter->prev->next) { + diff_iter = diff_iter->prev; + + meta1 = lyd_find_meta((*diff)->meta, mod, "key"); + meta2 = lyd_find_meta(diff_iter->meta, mod, "orig-key"); + + name = lyd_get_meta_value(meta1); + name_iter = lyd_get_meta_value(meta2); + + if (!name || !name_iter) { + continue; + } + + /* if keys don't match, skip - not a candidate for cyclic change */ + if (strcmp(name, name_iter)) { + continue; + } + + meta1 = lyd_find_meta((*diff)->meta, mod, "orig-key"); + meta2 = lyd_find_meta(diff_iter->meta, mod, "key"); + + /* store string values of metadata to compare later */ + name = lyd_get_meta_value(meta1); + name_iter = lyd_get_meta_value(meta2); + + /* reuse buffers by resetting used size */ + buffused1 = buffused2 = 0; + + if ((ret = lyd_path_list_predicate(*diff, &buff1, &bufflen1, &buffused1, 0))) { + goto cleanup; + } + + if ((ret = lyd_path_list_predicate(diff_iter, &buff2, &bufflen2, &buffused2, 0))) { + goto cleanup; + } + + if (!name || !name_iter) { + continue; + } + + /* compare path predicates with metadata - check if this is a reversed operation */ + if (!strcmp(buff1, name_iter) && !strcmp(buff2, name)) { + + /* found a cyclic change - remove and free the node */ + if ((ret = lyd_unlink_tree(*diff))) { + goto cleanup; + } + + lyd_free_tree(*diff); + *diff = NULL; + goto cleanup; + } + } +cleanup: + free(buff1); + free(buff2); + return ret; +} + /** * @brief Update operations on a diff node when the new operation is REPLACE. * @@ -2047,6 +2127,7 @@ lyd_diff_merge_replace(struct lyd_node *diff_match, enum lyd_diff_op cur_op, con meta = lyd_find_meta(src_diff->meta, mod, meta_name); LY_CHECK_ERR_RET(!meta, LOGERR_META(ctx, meta_name, src_diff), LY_EINVAL); LY_CHECK_RET(lyd_dup_meta_single(meta, diff_match, NULL)); + break; case LYS_LEAF: /* replaced with the exact same value, impossible */ @@ -2461,6 +2542,17 @@ lyd_diff_is_redundant(struct lyd_node *diff) orig_meta_name = "orig-value"; } + /** userordered lists can have different nodes that lead to identical changes. + * Only one node will stay other is unlinked + */ + if (!strcmp(meta_name, "key")) { + LY_CHECK_RET(lyd_diff_find_and_unlink_cyclic_nodes(&diff, mod), 0); + if (!diff) { + return 0; + } + + } + /* check for redundant move */ orig_val_meta = lyd_find_meta(diff->meta, mod, orig_meta_name); val_meta = lyd_find_meta(diff->meta, mod, meta_name); diff --git a/tests/utests/data/test_diff.c b/tests/utests/data/test_diff.c index 59ec8a9aa..003d6a308 100644 --- a/tests/utests/data/test_diff.c +++ b/tests/utests/data/test_diff.c @@ -90,6 +90,10 @@ const char *schema = " leaf l3 {type string;}" " }" " }" + " list person {key \"name surname\"; ordered-by user;" + " leaf name {type string;}" + " leaf surname {type string;}" + " }" " leaf-list dllist {type uint8; default \"1\"; default \"2\"; default \"3\";}" " list list {key \"name\";" " leaf name {type string;}" @@ -1418,6 +1422,63 @@ test_metadata(void **state) TEST_DIFF_3(xml1, xml2, xml3, LYD_DIFF_META, out_diff_1, out_diff_2, out_merge); } +static void +test_userord_conflicting_replace(void **state) +{ + (void) state; + struct lyd_node *diff_tree1 = NULL, *diff_tree2 = NULL, *final_diff = NULL; + char *final_xml; + + char *diff_xml1 = + "\n" + " " + " roland" + " doland" + " " + " " + " jake" + " fake" + " " + " " + " bob" + " drop" + " " + "\n"; + + char *diff_xml2 = + "\n" + " " + " bob" + " drop" + " " + "\n"; + + CHECK_PARSE_LYD(diff_xml1, diff_tree1); + CHECK_PARSE_LYD(diff_xml2, diff_tree2); + lyd_diff_merge_all(&final_diff, diff_tree1, 0); + lyd_diff_merge_all(&final_diff, diff_tree2, 0); + + lyd_print_mem(&final_xml, final_diff, LYD_XML, LYD_PRINT_WITHSIBLINGS); + + char *result_xml = + "\n" + " \n" + " roland\n" + " doland\n" + " \n" + " \n" + " jake\n" + " fake\n" + " \n" + "\n"; + + assert_string_equal(final_xml, result_xml); + free(final_xml); + lyd_free_all(diff_tree1); + lyd_free_all(diff_tree2); + lyd_free_all(final_diff); +} + int main(void) { @@ -1442,6 +1503,7 @@ main(void) UTEST(test_state_llist, setup), UTEST(test_wd, setup), UTEST(test_metadata, setup), + UTEST(test_userord_conflicting_replace, setup), }; return cmocka_run_group_tests(tests, NULL, NULL);