Fix PATCH target with a single forward slash

The targetSchemaNode is set to the root in JsonPatchBodyReader if
'target' is '/' regardless of the URI path which causes failure in
processing a PATCH request with 'target' of '/' if the URI goes deeper
than the top level data nodes.

Consolidate the processing of target in ParserIdentifier and update
inference based on the effective target node whether target is '/' or
not. And add test cases to cover the failure scenarios.

JIRA: NETCONF-1095
Change-Id: I8ca81dd47e01805cfd45c45ec3fe795fece62697
Signed-off-by: Sangwook Ha <sangwook.ha@verizon.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/AbstractPatchBodyReader.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/AbstractPatchBodyReader.java
index 9bf6b41464744040496d5a667ccc369f91d6965e..5ab7a750579314ab45360de148f9094684962fc4 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/AbstractPatchBodyReader.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/AbstractPatchBodyReader.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.restconf.nb.rfc8040.jersey.providers.patch;
 
+import static com.google.common.base.Verify.verify;
+
 import org.opendaylight.mdsal.dom.api.DOMMountPointService;
 import org.opendaylight.restconf.common.context.InstanceIdentifierContext;
 import org.opendaylight.restconf.common.patch.PatchContext;
@@ -34,8 +36,14 @@ abstract class AbstractPatchBodyReader extends AbstractIdentifierAwareJaxRsProvi
     }
 
     static final YangInstanceIdentifier parsePatchTarget(final InstanceIdentifierContext context, final String target) {
-        final var schemaContext = context.getSchemaContext();
         final var urlPath = context.getInstanceIdentifier();
+        if (target.equals("/")) {
+            verify(!urlPath.isEmpty(),
+                "target resource of URI must not be a datastore resource when target is '/'");
+            return urlPath;
+        }
+
+        final var schemaContext = context.getSchemaContext();
         final String targetUrl;
         if (urlPath.isEmpty()) {
             targetUrl = target.startsWith("/") ? target.substring(1) : target;

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReader.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReader.java
index 4347fd2ea69c27e89ac963ce3d4c73130b7039d4..ac9a7b82b7e208eb7c4f182b94818ea0854aaac0 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReader.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReader.java
@@ -48,7 +48,6 @@ import org.opendaylight.yangtools.yang.data.impl.schema.ResultAlreadySetExceptio
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree;
 import org.opendaylight.yangtools.yang.model.api.SchemaNode;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
-import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack.Inference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -222,24 +221,17 @@ public class JsonPatchBodyReader extends AbstractPatchBodyReader {
                     break;
                 case "target":
                     // target can be specified completely in request URI
-                    final String target = in.nextString();
-                    if (target.equals("/")) {
-                        edit.setTarget(path.getInstanceIdentifier());
-                        edit.setTargetSchemaNode(SchemaInferenceStack.of(path.getSchemaContext()).toInference());
-                    } else {
-                        edit.setTarget(parsePatchTarget(path, target));
-
-                        final var stack = schemaTree.enterPath(edit.getTarget()).orElseThrow().stack();
-                        if (!stack.isEmpty()) {
-                            stack.exit();
-                        }
+                    edit.setTarget(parsePatchTarget(path, in.nextString()));
+                    final var stack = schemaTree.enterPath(edit.getTarget()).orElseThrow().stack();
+                    if (!stack.isEmpty()) {
+                        stack.exit();
+                    }
 
-                        if (!stack.isEmpty()) {
-                            final EffectiveStatement<?, ?> parentStmt = stack.currentStatement();
-                            verify(parentStmt instanceof SchemaNode, "Unexpected parent %s", parentStmt);
-                        }
-                        edit.setTargetSchemaNode(stack.toInference());
+                    if (!stack.isEmpty()) {
+                        final EffectiveStatement<?, ?> parentStmt = stack.currentStatement();
+                        verify(parentStmt instanceof SchemaNode, "Unexpected parent %s", parentStmt);
                     }
+                    edit.setTargetSchemaNode(stack.toInference());
 
                     break;
                 case "value":

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReader.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReader.java
index 8856c54d8393ee810867fca5e4b120ec1206cc14..7552cc652a31a816183ada1dd15b033cd4ed0dbd 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReader.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReader.java
@@ -30,14 +30,12 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.patch.
 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;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
 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;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree;
-import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack.Inference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
@@ -89,24 +87,15 @@ public class XmlPatchBodyReader extends AbstractPatchBodyReader {
             final Element firstValueElement = values != null ? values.get(0) : null;
 
             // find complete path to target, it can be also empty (only slash)
-            final YangInstanceIdentifier targetII;
-            final Inference inference;
-            if (target.equals("/")) {
-                targetII = pathContext.getInstanceIdentifier();
-                inference = pathContext.inference();
-            } else {
-                // interpret as simple context
-                targetII = parsePatchTarget(pathContext, target);
-
-                // move schema node
-                final var lookup = DataSchemaContextTree.from(pathContext.getSchemaContext())
-                    .enterPath(targetII).orElseThrow();
-
-                final var stack = lookup.stack();
-                inference = stack.toInference();
-                if (!stack.isEmpty()) {
-                    stack.exit();
-                }
+            final var targetII = parsePatchTarget(pathContext, target);
+            // move schema node
+            final var lookup = DataSchemaContextTree.from(pathContext.getSchemaContext())
+                .enterPath(targetII).orElseThrow();
+
+            final var stack = lookup.stack();
+            final var inference = stack.toInference();
+            if (!stack.isEmpty()) {
+                stack.exit();
             }
 
             if (requiresValue(oper)) {

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReaderTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReaderTest.java
index ab6de83cbdfedf177de5be1925ed256373c1564b..fdffeb2d0476819a9ec09f93bb57d7b5d2ec5999 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReaderTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/JsonPatchBodyReaderTest.java
@@ -188,6 +188,97 @@ public class JsonPatchBodyReaderTest extends AbstractBodyReaderTest {
         checkPatchContext(returnValue);
     }
 
+    /**
+     * Test of Yang Patch on the top-level container with the full path in the URI and "/" in 'target'.
+     */
+    @Test
+    public void modulePatchTargetTopLevelContainerWithFullPathURITest() throws Exception {
+        mockBodyReader("instance-identifier-patch-module:patch-cont", jsonToPatchBodyReader, false);
+        final var inputStream = new ByteArrayInputStream("""
+            {
+                "ietf-yang-patch:yang-patch": {
+                    "patch-id": "test-patch",
+                    "comment": "Test patch applied to the top-level container with '/' in target",
+                    "edit": [
+                        {
+                            "edit-id": "edit1",
+                            "operation": "replace",
+                            "target": "/",
+                            "value": {
+                                "patch-cont": {
+                                    "my-list1": [
+                                        {
+                                            "name": "my-leaf-set",
+                                            "my-leaf11": "leaf-a",
+                                            "my-leaf12": "leaf-b"
+                                        }
+                                    ]
+                                }
+                            }
+                        }
+                    ]
+                }
+            }""".getBytes(StandardCharsets.UTF_8));
+        final PatchContext returnValue = jsonToPatchBodyReader.readFrom(null, null, null, mediaType, null, inputStream);
+        checkPatchContext(returnValue);
+        assertEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(PATCH_CONT_QNAME))
+            .withChild(Builders.mapBuilder()
+                .withNodeIdentifier(new NodeIdentifier(MY_LIST1_QNAME))
+                .withChild(Builders.mapEntryBuilder()
+                    .withNodeIdentifier(NodeIdentifierWithPredicates.of(MY_LIST1_QNAME, LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF11_QNAME, "leaf-a"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF12_QNAME, "leaf-b"))
+                    .build())
+                .build())
+            .build(), returnValue.getData().get(0).getNode());
+    }
+
+    /**
+     * Test of Yang Patch on the second-level list with the full path in the URI and "/" in 'target'.
+     */
+    @Test
+    public void modulePatchTargetSecondLevelListWithFullPathURITest() throws Exception {
+        mockBodyReader("instance-identifier-patch-module:patch-cont/my-list1=my-leaf-set",
+                jsonToPatchBodyReader, false);
+        final var inputStream = new ByteArrayInputStream("""
+            {
+                "ietf-yang-patch:yang-patch": {
+                    "patch-id": "test-patch",
+                    "comment": "Test patch applied to the second-level list with '/' in target",
+                    "edit": [
+                        {
+                            "edit-id": "edit1",
+                            "operation": "replace",
+                            "target": "/",
+                            "value": {
+                                "my-list1": [
+                                    {
+                                        "name": "my-leaf-set",
+                                        "my-leaf11": "leaf-a",
+                                        "my-leaf12": "leaf-b"
+                                    }
+                                ]
+                            }
+                        }
+                    ]
+                }
+            }""".getBytes(StandardCharsets.UTF_8));
+        final PatchContext returnValue = jsonToPatchBodyReader.readFrom(null, null, null, mediaType, null, inputStream);
+        checkPatchContext(returnValue);
+        assertEquals(Builders.mapBuilder()
+            .withNodeIdentifier(new NodeIdentifier(MY_LIST1_QNAME))
+            .withChild(Builders.mapEntryBuilder()
+                    .withNodeIdentifier(NodeIdentifierWithPredicates.of(
+                            MY_LIST1_QNAME, LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF11_QNAME, "leaf-a"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF12_QNAME, "leaf-b"))
+                    .build())
+            .build(), returnValue.getData().get(0).getNode());
+    }
+
     /**
      * Test of Yang Patch on the top augmented element.
      */

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReaderTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReaderTest.java
index 683370e1fd4be110ab765f8a8b41de4a1694791f..73ed79d3ac4b63612bbd0c75bc258b262072b530 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReaderTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/patch/XmlPatchBodyReaderTest.java
@@ -17,6 +17,7 @@ import javax.ws.rs.core.MediaType;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
+import org.opendaylight.restconf.common.patch.PatchContext;
 import org.opendaylight.restconf.nb.rfc8040.jersey.providers.test.AbstractBodyReaderTest;
 import org.opendaylight.restconf.nb.rfc8040.jersey.providers.test.XmlBodyReaderTest;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
@@ -135,6 +136,86 @@ public class XmlPatchBodyReaderTest extends AbstractBodyReaderTest {
         checkPatchContext(xmlToPatchBodyReader.readFrom(null, null, null, mediaType, null, inputStream));
     }
 
+    /**
+     * Test of Yang Patch on the top-level container with the full path in the URI and "/" in 'target'.
+     */
+    @Test
+    public void modulePatchTargetTopLevelContainerWithFullPathURITest() throws Exception {
+        mockBodyReader("instance-identifier-patch-module:patch-cont", xmlToPatchBodyReader, false);
+        final var inputStream = new ByteArrayInputStream("""
+            <yang-patch xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-patch">
+                <patch-id>test-patch</patch-id>
+                <comment>Test patch applied to the top-level container with '/' in target</comment>
+                <edit>
+                    <edit-id>edit1</edit-id>
+                    <operation>replace</operation>
+                    <target>/</target>
+                    <value>
+                        <patch-cont xmlns="instance:identifier:patch:module">
+                            <my-list1>
+                                <name>my-leaf-set</name>
+                                <my-leaf11>leaf-a</my-leaf11>
+                                <my-leaf12>leaf-b</my-leaf12>
+                            </my-list1>
+                        </patch-cont>
+                    </value>
+                </edit>
+            </yang-patch>
+            """.getBytes(StandardCharsets.UTF_8));
+        final PatchContext returnValue = xmlToPatchBodyReader.readFrom(null, null, null, mediaType, null, inputStream);
+        checkPatchContext(returnValue);
+        assertEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(PATCH_CONT_QNAME))
+            .withChild(Builders.mapBuilder()
+                .withNodeIdentifier(new NodeIdentifier(MY_LIST1_QNAME))
+                .withChild(Builders.mapEntryBuilder()
+                    .withNodeIdentifier(NodeIdentifierWithPredicates.of(MY_LIST1_QNAME, LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(LEAF_NAME_QNAME, "my-leaf-set"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF11_QNAME, "leaf-a"))
+                    .withChild(ImmutableNodes.leafNode(MY_LEAF12_QNAME, "leaf-b"))
+                    .build())
+                .build())
+            .build(), returnValue.getData().get(0).getNode());
+    }
+
+    /**
+     * Test of Yang Patch on the second-level list with the full path in the URI and "/" in 'target'.
+     */
+    @Test
+    public void modulePatchTargetSecondLevelListWithFullPathURITest() throws Exception {
+        mockBodyReader("instance-identifier-patch-module:patch-cont/my-list1=my-leaf-set",
+                xmlToPatchBodyReader, false);
+        final var inputStream = new ByteArrayInputStream("""
+            <yang-patch xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-patch">
+                <patch-id>test-patch</patch-id>
+                <comment>Test patch applied to the second-level list with '/' in target</comment>
+                <edit>
+                    <edit-id>edit1</edit-id>
+                    <operation>replace</operation>
+                    <target>/</target>
+                    <value>
+                        <my-list1 xmlns="instance:identifier:patch:module">
+                            <name>my-leaf-set</name>
+                            <my-leaf11>leaf-a</my-leaf11>
+                            <my-leaf12>leaf-b</my-leaf12>
+                        </my-list1>
+                    </value>
+                </edit>
+            </yang-patch>
+            """.getBytes(StandardCharsets.UTF_8));
+        final PatchContext returnValue = xmlToPatchBodyReader.readFrom(null, null, null, mediaType, null, inputStream);
+        checkPatchContext(returnValue);
+        assertEquals(Builders.mapBuilder()
+            .withNodeIdentifier(new NodeIdentifier(MY_LIST1_QNAME))
+            .withChild(Builders.mapEntryBuilder()
+                .withNodeIdentifier(NodeIdentifierWithPredicates.of(MY_LIST1_QNAME, LEAF_NAME_QNAME, "my-leaf-set"))
+                .withChild(ImmutableNodes.leafNode(LEAF_NAME_QNAME, "my-leaf-set"))
+                .withChild(ImmutableNodes.leafNode(MY_LEAF11_QNAME, "leaf-a"))
+                .withChild(ImmutableNodes.leafNode(MY_LEAF12_QNAME, "leaf-b"))
+                .build())
+            .build(), returnValue.getData().get(0).getNode());
+    }
+
     /**
      * Test of Yang Patch on the top augmented element.
      */

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/test/AbstractBodyReaderTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/test/AbstractBodyReaderTest.java
index a3ee31338d8e8c8a99f6f0bda143cd5757a9d019..450b717bd31362890315963cb194346e077dca05 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/test/AbstractBodyReaderTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/test/AbstractBodyReaderTest.java
@@ -54,8 +54,16 @@ public abstract class AbstractBodyReaderTest {
     protected static final QName LIST_LEAF2_QNAME = QName.create("list:ns", "leaf2").intern();
     protected static final QName CHOICE_CONT_QNAME = QName.create("choice:ns", "case-cont1").intern();
     protected static final QName CASE_LEAF1_QNAME = QName.create("choice:ns", "case-leaf1").intern();
+    protected static final QName PATCH_CONT_QNAME = QName.create("instance:identifier:patch:module",
+            "2015-11-21", "patch-cont").intern();
+    protected static final QName MY_LIST1_QNAME = QName.create("instance:identifier:patch:module",
+            "2015-11-21", "my-list1").intern();
     protected static final QName LEAF_NAME_QNAME = QName.create("instance:identifier:patch:module",
             "2015-11-21", "name").intern();
+    protected static final QName MY_LEAF11_QNAME = QName.create("instance:identifier:patch:module",
+            "2015-11-21", "my-leaf11").intern();
+    protected static final QName MY_LEAF12_QNAME = QName.create("instance:identifier:patch:module",
+            "2015-11-21", "my-leaf12").intern();
 
     protected final MediaType mediaType;
     protected final DatabindProvider databindProvider;