diff BUGFIX user-ordered replace operation

Author: Ardival

src/diff.c

@@ -2004,18 +2004,92 @@ 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.
+ *
+ * @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 *metas[2];
+    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 != NULL) {
+        diff_iter = diff_iter->prev;
+        metas[0] = lyd_find_meta((*diff)->meta, mod, "key");
+        metas[1] = lyd_find_meta(diff_iter->meta, mod, "orig-key");
+
+        /* If keys don't match, skip - not a candidate for cyclic change */
+        if (strcmp(lyd_get_meta_value(metas[0]), lyd_get_meta_value(metas[1]))) {
+            continue;
+        }
+
+        metas[0] = lyd_find_meta((*diff)->meta, mod, "orig-key");
+        metas[1] = lyd_find_meta(diff_iter->meta, mod, "key");
+
+        /* Store string values of metadata to compare later */
+        name = lyd_get_meta_value(metas[0]);
+        name_iter = lyd_get_meta_value(metas[1]);
+
+        if ((ret = lyd_path_list_predicate(*diff, &buff1, &bufflen1, &buffused1, 0))) {
+            return ret;
+        }
+
+        if ((ret = lyd_path_list_predicate(diff_iter, &buff2, &bufflen2, &buffused2, 0))) {
+            return ret;
+        }
+
+        /* 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))) {
+                return ret;
+            }
+
+            lyd_free_all(*diff);
+            free(buff1);
+            free(buff2);
+            *diff = NULL;
+            return ret;
+        }
+
+        free(buff1);
+        buff1 = NULL;
+        bufflen1 = 0;
+        buffused1 = 0;
+        free(buff2);
+        buff2 = NULL;
+        bufflen2 = 0;
+        buffused2 = 0;
+    }
+    return ret;
+}
+
 /**
  * @brief Update operations on a diff node when the new operation is REPLACE.
  *
- * @param[in] diff_match Node from the diff.
+ * @param[in,out] diff_node Node from the diff.
  * @param[in] cur_op Current operation of @p diff_match.
  * @param[in] src_diff Current source diff node.
  * @return LY_ERR value.
  */
 static LY_ERR
-lyd_diff_merge_replace(struct lyd_node *diff_match, enum lyd_diff_op cur_op, const struct lyd_node *src_diff)
+lyd_diff_merge_replace(struct lyd_node **diff_node, enum lyd_diff_op cur_op, const struct lyd_node *src_diff)
 {
     LY_ERR ret;
+    struct lyd_node *diff_match = *diff_node;
     const char *str_val, *meta_name, *orig_meta_name;
     struct lyd_meta *meta;
     const struct lys_module *mod;
@@ -2047,6 +2121,14 @@ 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));
+
+            /** 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_node, mod));
+            }
+
             break;
         case LYS_LEAF:
             /* replaced with the exact same value, impossible */
@@ -2720,7 +2802,13 @@ lyd_diff_merge_r(const struct lyd_node *src_diff, struct lyd_node *diff_parent,
         /* merge operations */
         switch (src_op) {
         case LYD_DIFF_OP_REPLACE:
-            ret = lyd_diff_merge_replace(diff_node, cur_op, src_diff);
+            ret = lyd_diff_merge_replace(&diff_node, cur_op, src_diff);
+
+            /* diff_node was unlinked and do not need any further changes */
+            if (!diff_node) {
+                return LY_SUCCESS;
+            }
+
             break;
         case LYD_DIFF_OP_CREATE:
             if ((cur_op == LYD_DIFF_OP_CREATE) && lysc_is_dup_inst_list(diff_node->schema)) {

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