diff BUGFIX user-ordered replace operation

Author: Ardival

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);

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 =
+            "<df xmlns=\"urn:libyang:tests:defaults\" xmlns:yang=\"urn:ietf:params:xml:ns:yang:1\" yang:operation=\"none\">\n"
+            "  <person yang:operation=\"delete\" yang:orig-key=\"[name='alice'][surname='chalice']\">"
+            "    <name>roland</name>"
+            "    <surname>doland</surname>"
+            "  </person>"
+            "  <person yang:operation=\"replace\" yang:orig-key=\"\" yang:key=\"[name='bob'][surname='drop']\">"
+            "    <name>jake</name>"
+            "    <surname>fake</surname>"
+            "  </person>"
+            "  <person yang:operation=\"replace\" yang:orig-key=\"[name='jake'][surname='fake']\" yang:key=\"[name='alice'][surname='chalice']\">"
+            "    <name>bob</name>"
+            "    <surname>drop</surname>"
+            "  </person>"
+            "</df>\n";
+
+    char *diff_xml2 =
+            "<df xmlns=\"urn:libyang:tests:defaults\" xmlns:yang=\"urn:ietf:params:xml:ns:yang:1\" yang:operation=\"none\">\n"
+            "  <person yang:operation=\"replace\" yang:orig-key=\"[name='alice'][surname='chalice']\" yang:key=\"\">"
+            "    <name>bob</name>"
+            "    <surname>drop</surname>"
+            "  </person>"
+            "</df>\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 =
+            "<df xmlns=\"urn:libyang:tests:defaults\" xmlns:yang=\"urn:ietf:params:xml:ns:yang:1\" yang:operation=\"none\">\n"
+            "  <person yang:operation=\"delete\" yang:orig-key=\"[name='alice'][surname='chalice']\">\n"
+            "    <name>roland</name>\n"
+            "    <surname>doland</surname>\n"
+            "  </person>\n"
+            "  <person yang:operation=\"replace\" yang:orig-key=\"\" yang:key=\"[name='bob'][surname='drop']\">\n"
+            "    <name>jake</name>\n"
+            "    <surname>fake</surname>\n"
+            "  </person>\n"
+            "</df>\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);