Fix YANG-PATCH Leaf-list request 69/116369/3
authorPeter Suna <peter.suna@pantheon.tech>
Thu, 17 Apr 2025 09:08:48 +0000 (11:08 +0200)
committerIvan Hrasko <ivan.hrasko@pantheon.tech>
Mon, 28 Apr 2025 07:18:52 +0000 (07:18 +0000)
If a YANG-PATCH request is sent to populate a single Leaf-list element,
we receive an IllegalArgumentException with the following messages
Create operation:
LeafSetEntryNode is not valid for parent
Merge operation:
Invalid nesting of data.

Similar to List use parent path for leaf-list requests
to resolve this issue.

JIRA: NETCONF-1453
Change-Id: I4d16d803f524b9ace86850023ed95afe4dd12c5e
Signed-off-by: Peter Suna <peter.suna@pantheon.tech>
protocol/restconf-server-api/src/main/java/org/opendaylight/restconf/server/api/JsonPatchBody.java
protocol/restconf-server-api/src/main/java/org/opendaylight/restconf/server/api/XmlPatchBody.java
protocol/restconf-server-jaxrs/src/test/java/org/opendaylight/restconf/server/jaxrs/NC1453Test.java [new file with mode: 0644]
protocol/restconf-server-jaxrs/src/test/resources/nc1453/foo.yang [new file with mode: 0644]

index 197d19dbfd608e40ee6479dfa178f2371a385438..21e6b74d2665323ec790dc4a416731355f5d54aa 100644 (file)
@@ -28,6 +28,7 @@ import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.codec.gson.JSONCodecFactory;
 import org.opendaylight.yangtools.yang.data.codec.gson.JsonParserStream;
@@ -279,9 +280,10 @@ public final class JsonPatchBody extends PatchBody {
                 return new PatchEntity(edit.getId(), edit.getOperation(), edit.getTarget());
             }
 
-            // for lists allow to manipulate with list items through their parent
+            // for lists/leaf-list allow to manipulate with items through their parent
             final YangInstanceIdentifier targetNode;
-            if (edit.getTarget().getLastPathArgument() instanceof NodeIdentifierWithPredicates) {
+            if (edit.getTarget().getLastPathArgument() instanceof NodeIdentifierWithPredicates
+                    || edit.getTarget().getLastPathArgument() instanceof NodeWithValue<?>) {
                 targetNode = edit.getTarget().getParent();
             } else {
                 targetNode = edit.getTarget();
index 85d5666aea8720e8fb824063d9de7aa90ddfaeb8..d467b66ad37331bb6f2d330e198ed4dc7a50cf56 100644 (file)
@@ -23,6 +23,7 @@ import org.opendaylight.yangtools.util.xml.UntrustedXML;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.codec.xml.XmlParserStream;
 import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNormalizedNodeStreamWriter;
 import org.opendaylight.yangtools.yang.data.impl.schema.NormalizationResultHolder;
@@ -83,8 +84,9 @@ public final class XmlPatchBody extends PatchBody {
                 xmlParser.traverse(new DOMSource(firstValueElement));
 
                 final var result = resultHolder.getResult().data();
-                // for lists allow to manipulate with list items through their parent
-                if (targetPath.getLastPathArgument() instanceof NodeIdentifierWithPredicates) {
+                // for lists/leaf-list allow to manipulate with items through their parent
+                if (targetPath.getLastPathArgument() instanceof NodeIdentifierWithPredicates
+                        || targetPath.getLastPathArgument() instanceof NodeWithValue<?>) {
                     entities.add(new PatchEntity(editId, oper, targetPath.getParent(), result));
                 } else {
                     entities.add(new PatchEntity(editId, oper, targetPath, result));
diff --git a/protocol/restconf-server-jaxrs/src/test/java/org/opendaylight/restconf/server/jaxrs/NC1453Test.java b/protocol/restconf-server-jaxrs/src/test/java/org/opendaylight/restconf/server/jaxrs/NC1453Test.java
new file mode 100644 (file)
index 0000000..576ee33
--- /dev/null
@@ -0,0 +1,286 @@
+/*
+ * Copyright (c) 2025 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.restconf.server.jaxrs;
+
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFalseFluentFuture;
+
+import java.util.function.Consumer;
+import javax.ws.rs.container.AsyncResponse;
+import javax.ws.rs.core.MultivaluedHashMap;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.opendaylight.mdsal.common.api.CommitInfo;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction;
+import org.opendaylight.restconf.server.spi.YangPatchStatusBody;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
+import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode;
+import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
+import org.opendaylight.yangtools.yang.data.spi.node.ImmutableNodes;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+class NC1453Test extends AbstractRestconfTest {
+    private static final EffectiveModelContext MODEL_CONTEXT = YangParserTestUtils
+        .parseYangResourceDirectory("/nc1453");
+    private static final YangInstanceIdentifier BAZ_LIST_PATH = YangInstanceIdentifier.of(
+        new NodeIdentifier(QName.create("urn:foo", "foo")),
+        new NodeIdentifier(QName.create("urn:foo", "baz-list")),
+        new NodeWithValue<>(QName.create("urn:foo", "baz-list"), "delta"));
+    private static final LeafSetEntryNode<Object> BAZ_LIST_NODE = ImmutableNodes.newLeafSetEntryBuilder()
+        .withNodeIdentifier(new NodeWithValue<>(QName.create("urn:foo", "baz-list"), "delta"))
+        .withValue("delta")
+        .build();
+    private static final NormalizedNode BAZ_NODE = ImmutableNodes.newSystemLeafSetBuilder()
+        .withNodeIdentifier(NodeIdentifier.create(QName.create("urn:foo", "baz-list")))
+        .withChild(BAZ_LIST_NODE).build();
+
+    @Mock
+    private DOMDataTreeReadWriteTransaction tx;
+
+    @BeforeEach
+    void beforeEach() {
+        doReturn(tx).when(dataBroker).newReadWriteTransaction();
+        doNothing().when(tx).merge(any(), any(), any());
+        doReturn(CommitInfo.emptyFluentFuture()).when(tx).commit();
+        doReturn(new MultivaluedHashMap<>()).when(uriInfo).getQueryParameters();
+    }
+
+    @Override
+    EffectiveModelContext modelContext() {
+        return MODEL_CONTEXT;
+    }
+
+    @Test
+    void testMergeLeafList() {
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangJsonPATCH(
+            stringInputStream("""
+                {
+                    "ietf-yang-patch:yang-patch": {
+                       "patch-id": "patch1",
+                       "edit": [
+                         {
+                           "edit-id": "edit1",
+                           "operation": "merge",
+                           "target": "/foo:foo/baz-list=delta",
+                           "value": {
+                             "foo:baz-list": [
+                               "delta"
+                             ]
+                           }
+                         }
+                       ]
+                    }
+                }"""), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).merge(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH.getParent(), BAZ_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    @Test
+    void testCreateLeafList() {
+        doReturn(immediateFalseFluentFuture()).when(tx).exists(any(), any());
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangJsonPATCH(
+            stringInputStream("""
+                {
+                    "ietf-yang-patch:yang-patch": {
+                       "patch-id": "patch1",
+                       "edit": [
+                         {
+                           "edit-id": "edit1",
+                           "operation": "create",
+                           "target": "/foo:foo/baz-list=delta",
+                           "value": {
+                             "foo:baz-list": [
+                               "delta"
+                             ]
+                           }
+                         }
+                       ]
+                    }
+                }"""), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).put(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH, BAZ_LIST_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    @Test
+    void testReplaceLeafList() {
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangJsonPATCH(
+            stringInputStream("""
+                {
+                    "ietf-yang-patch:yang-patch": {
+                       "patch-id": "patch1",
+                       "edit": [
+                         {
+                           "edit-id": "edit1",
+                           "operation": "replace",
+                           "target": "/foo:foo/baz-list=delta",
+                           "value": {
+                             "foo:baz-list": [
+                               "delta"
+                             ]
+                           }
+                         }
+                       ]
+                    }
+                }"""), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).put(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH, BAZ_LIST_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    @Test
+    void testMergeLeafListXml() {
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangXmlPATCH(
+            stringInputStream("""
+                <yang-patch xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-patch">
+                  <patch-id>patch1</patch-id>
+                  <edit>
+                    <edit-id>edit1</edit-id>
+                    <operation>merge</operation>
+                    <target>/foo:foo/baz-list=delta</target>
+                    <value>
+                      <baz-list xmlns="urn:foo">delta</baz-list>
+                    </value>
+                  </edit>
+                </yang-patch>
+            """), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).merge(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH.getParent(), BAZ_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    @Test
+    void testCreateLeafListXml() {
+        doReturn(immediateFalseFluentFuture()).when(tx).exists(any(), any());
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangXmlPATCH(
+            stringInputStream("""
+                <yang-patch xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-patch">
+                  <patch-id>patch1</patch-id>
+                  <edit>
+                    <edit-id>edit1</edit-id>
+                    <operation>create</operation>
+                    <target>/foo:foo/baz-list=delta</target>
+                    <value>
+                      <baz-list xmlns="urn:foo">delta</baz-list>
+                    </value>
+                  </edit>
+                </yang-patch>
+            """), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).put(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH, BAZ_LIST_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    @Test
+    void testReplaceLeafListXml() {
+        // Send YANG-PATCH request.
+        final var body = assertPatchStatus(200, ar -> restconf.dataYangXmlPATCH(
+            stringInputStream("""
+                <yang-patch xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-patch">
+                  <patch-id>patch1</patch-id>
+                  <edit>
+                    <edit-id>edit1</edit-id>
+                    <operation>replace</operation>
+                    <target>/foo:foo/baz-list=delta</target>
+                    <value>
+                      <baz-list xmlns="urn:foo">delta</baz-list>
+                    </value>
+                  </edit>
+                </yang-patch>
+                """), uriInfo, sc, ar));
+
+        // Verify that the correct node with the path is attempted to be stored in the datastore.
+        verify(tx).put(LogicalDatastoreType.CONFIGURATION, BAZ_LIST_PATH, BAZ_LIST_NODE);
+
+        // Verify response.
+        assertFormat("""
+            {
+              "ietf-yang-patch:yang-patch-status": {
+                "patch-id": "patch1",
+                "ok": [
+                  null
+                ]
+              }
+            }""", body::formatToJSON, true);
+    }
+
+    private static YangPatchStatusBody assertPatchStatus(final int status, final Consumer<AsyncResponse> invocation) {
+        return assertInstanceOf(YangPatchStatusBody.class, assertFormattableBody(status, invocation));
+    }
+}
diff --git a/protocol/restconf-server-jaxrs/src/test/resources/nc1453/foo.yang b/protocol/restconf-server-jaxrs/src/test/resources/nc1453/foo.yang
new file mode 100644 (file)
index 0000000..65e7dd2
--- /dev/null
@@ -0,0 +1,10 @@
+module foo {
+  namespace "urn:foo";
+  prefix foo;
+
+  container foo {
+    leaf-list baz-list {
+      type string;
+    }
+  }
+}