Bind Insert point PathArgument earlier

We have a bit of duplicate code in RestconfStrategy, which really should
not be present here. We should be receiving point's value as an
already-baked PathArgument instead.

JIRA: NETCONF-773
Change-Id: I0845ca34a41b0bfe82ada8598cc6a540451c34f0
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/Insert.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/Insert.java
index f3b9076e6d3825295ed5b463e7abb5acfd528208..ec513fed0af5b0d83565740d05b4a6e50fc54e85 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/Insert.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/Insert.java
@@ -9,12 +9,15 @@ package org.opendaylight.restconf.nb.rfc8040;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.annotations.Beta;
 import com.google.common.base.MoreObjects;
 import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.restconf.api.query.InsertParam;
 import org.opendaylight.restconf.api.query.PointParam;
 import org.opendaylight.yangtools.concepts.Immutable;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 
 /**
  * Parser and holder of query parameters from uriInfo for data and datastore modification operations.
@@ -25,15 +28,24 @@ import org.opendaylight.yangtools.concepts.Immutable;
 //                 have a @NonNull PointParam then and there will not be an insert field. We can also ditch toString(),
 //                 as the records will do the right thing.
 public final class Insert implements Immutable {
+    @Beta
+    @NonNullByDefault
+    @FunctionalInterface
+    public interface PointParser {
+
+        PathArgument parseValue(String value);
+    }
+
     private final @NonNull InsertParam insert;
-    private final @Nullable PointParam point;
+    private final @Nullable PathArgument pointArg;
 
-    private Insert(final InsertParam insert, final PointParam point) {
+    private Insert(final InsertParam insert, final PathArgument pointArg) {
         this.insert = requireNonNull(insert);
-        this.point = point;
+        this.pointArg = pointArg;
     }
 
-    public static @Nullable Insert forParams(final @Nullable InsertParam insert, final @Nullable PointParam point) {
+    public static @Nullable Insert forParams(final @Nullable InsertParam insert, final @Nullable PointParam point,
+            final PointParser pointParser) {
         if (insert == null) {
             if (point != null) {
                 throw invalidPointIAE();
@@ -51,7 +63,7 @@ public final class Insert implements Immutable {
                     throw new IllegalArgumentException(
                         "Insert parameter " + insert.paramValue() + " cannot be used without a Point parameter.");
                 }
-                yield new Insert(insert, point);
+                yield new Insert(insert, pointParser.parseValue(point.value()));
             }
             case FIRST, LAST -> {
                 // https://www.rfc-editor.org/rfc/rfc8040#section-4.8.6:
@@ -76,16 +88,16 @@ public final class Insert implements Immutable {
         return insert;
     }
 
-    public @Nullable PointParam point() {
-        return point;
+    public @Nullable PathArgument pointArg() {
+        return pointArg;
     }
 
     @Override
     public String toString() {
         final var helper = MoreObjects.toStringHelper(this).add("insert", insert.paramValue());
-        final var local = point;
+        final var local = pointArg;
         if (local != null) {
-            helper.add("point", local.value());
+            helper.add("point", pointArg);
         }
         return helper.toString();
     }

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java
index b259c5000cdec38b48ffede2a7de1578d86b9ccc..00d2c6b77821f07844ffe9bcd3f8344280133be3 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java
@@ -41,8 +41,10 @@ import org.opendaylight.restconf.nb.rfc8040.legacy.InstanceIdentifierContext;
 import org.opendaylight.restconf.nb.rfc8040.legacy.QueryParameters;
 import org.opendaylight.restconf.nb.rfc8040.utils.parser.NetconfFieldsTranslator;
 import org.opendaylight.restconf.nb.rfc8040.utils.parser.WriterFieldsTranslator;
+import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierDeserializer;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
 
 @Beta
 public final class QueryParams {
@@ -54,6 +56,7 @@ public final class QueryParams {
         InsertParam.uriName, PointParam.uriName,
         // Notifications
         FilterParam.uriName, StartTimeParam.uriName, StopTimeParam.uriName,
+        // ODL extensions
         LeafNodesOnlyParam.uriName, SkipNotificationDataParam.uriName, ChangedLeafNodesOnlyParam.uriName,
         ChildNodesOnlyParam.uriName);
 
@@ -181,7 +184,7 @@ public final class QueryParams {
         return new ReadDataParams(content, depth, fields, withDefaults, prettyPrint);
     }
 
-    public static @Nullable Insert parseInsert(final UriInfo uriInfo) {
+    public static @Nullable Insert parseInsert(final EffectiveModelContext modelContext, final UriInfo uriInfo) {
         InsertParam insert = null;
         PointParam point = null;
 
@@ -207,7 +210,12 @@ public final class QueryParams {
         }
 
         try {
-            return Insert.forParams(insert, point);
+            return Insert.forParams(insert, point,
+                // TODO: instead of a EffectiveModelContext, we should have received
+                //       YangInstanceIdentifierDeserializer.Result, from which we can use to seed the parser. This
+                //       call-site should not support 'yang-ext:mount' and should just reuse DataSchemaContextTree,
+                //       saving a lookup
+                value -> YangInstanceIdentifierDeserializer.create(modelContext, value).path.getLastPathArgument());
         } catch (IllegalArgumentException e) {
             throw new RestconfDocumentedException("Invalid query parameters: " + e.getMessage(), e);
         }

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
index 63cfbb0ca42806f4a7eceffb2353bde4563aabe4..d315c54ca5ed72d3bdf65c9b8078c06f7d12f907 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
@@ -381,8 +381,10 @@ public final class RestconfDataServiceImpl {
     }
 
     private Response putData(final @Nullable String identifier, final UriInfo uriInfo, final ResourceBody body) {
-        final var insert = QueryParams.parseInsert(uriInfo);
-        final var req = bindResourceRequest(identifier, body);
+        final var localModel = databindProvider.currentContext().modelContext();
+        final var context = ParserIdentifier.toInstanceIdentifier(identifier, localModel, mountPointService);
+        final var insert = QueryParams.parseInsert(context.getSchemaContext(), uriInfo);
+        final var req = bindResourceRequest(context, body);
 
         return switch (
             req.strategy().putData(req.path(), req.data(), insert)) {
@@ -498,8 +500,8 @@ public final class RestconfDataServiceImpl {
 
     private Response postData(final Inference inference, final YangInstanceIdentifier parentPath, final ChildBody body,
             final UriInfo uriInfo, final @Nullable DOMMountPoint mountPoint) {
-        final var insert = QueryParams.parseInsert(uriInfo);
         final var modelContext = inference.getEffectiveModelContext();
+        final var insert = QueryParams.parseInsert(modelContext, uriInfo);
         final var strategy = getRestconfStrategy(modelContext, mountPoint);
         var path = parentPath;
         final var payload = body.toPayload(path, inference);
@@ -653,7 +655,10 @@ public final class RestconfDataServiceImpl {
      * @param ar {@link AsyncResponse} which needs to be completed
      */
     private void plainPatchData(final @Nullable String identifier, final ResourceBody body, final AsyncResponse ar) {
-        final var req = bindResourceRequest(identifier, body);
+        final var req = bindResourceRequest(
+            ParserIdentifier.toInstanceIdentifier(identifier, databindProvider.currentContext().modelContext(),
+                mountPointService),
+            body);
         final var future = req.strategy().merge(req.path(), req.data());
 
         Futures.addCallback(future, new FutureCallback<>() {
@@ -669,10 +674,8 @@ public final class RestconfDataServiceImpl {
         }, MoreExecutors.directExecutor());
     }
 
-    private @NonNull ResourceRequest bindResourceRequest(final @Nullable String identifier, final ResourceBody body) {
-        final var dataBind = databindProvider.currentContext();
-        final var context = ParserIdentifier.toInstanceIdentifier(identifier, dataBind.modelContext(),
-            mountPointService);
+    private @NonNull ResourceRequest bindResourceRequest(final InstanceIdentifierContext context,
+            final ResourceBody body) {
         final var inference = context.inference();
         final var path = context.getInstanceIdentifier();
         final var data = body.toNormalizedNode(path, inference, context.getSchemaNode());

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
index c08764a1f4edf017f0a28bc61029630d39f26e09..1b33afda3a2933d769af42febb61a62b8d778ff3 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
@@ -31,7 +31,6 @@ import org.opendaylight.mdsal.dom.api.DOMMountPoint;
 import org.opendaylight.mdsal.dom.api.DOMTransactionChain;
 import org.opendaylight.netconf.dom.api.NetconfDataTreeService;
 import org.opendaylight.restconf.api.query.ContentParam;
-import org.opendaylight.restconf.api.query.PointParam;
 import org.opendaylight.restconf.api.query.WithDefaultsParam;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
 import org.opendaylight.restconf.common.errors.RestconfError;
@@ -41,7 +40,6 @@ import org.opendaylight.restconf.common.patch.PatchContext;
 import org.opendaylight.restconf.common.patch.PatchStatusContext;
 import org.opendaylight.restconf.common.patch.PatchStatusEntity;
 import org.opendaylight.restconf.nb.rfc8040.Insert;
-import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierDeserializer;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.with.defaults.rev110601.WithDefaultsMode;
 import org.opendaylight.yangtools.yang.common.Empty;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
@@ -275,7 +273,7 @@ public abstract class RestconfStrategy {
                 if (readData == null || readData.isEmpty()) {
                     yield replaceAndCommit(tx, path, data);
                 }
-                insertWithPointPut(tx, path, data, verifyNotNull(insert.point()), readData, true);
+                insertWithPointPut(tx, path, data, verifyNotNull(insert.pointArg()), readData, true);
                 yield tx.commit();
             }
             case AFTER -> {
@@ -283,19 +281,17 @@ public abstract class RestconfStrategy {
                 if (readData == null || readData.isEmpty()) {
                     yield replaceAndCommit(tx, path, data);
                 }
-                insertWithPointPut(tx, path, data, verifyNotNull(insert.point()), readData, false);
+                insertWithPointPut(tx, path, data, verifyNotNull(insert.pointArg()), readData, false);
                 yield tx.commit();
             }
         };
     }
 
     private void insertWithPointPut(final RestconfTransaction tx, final YangInstanceIdentifier path,
-            final NormalizedNode data, final @NonNull PointParam point, final NormalizedNodeContainer<?> readList,
+            final NormalizedNode data, final @NonNull PathArgument pointArg, final NormalizedNodeContainer<?> readList,
             final boolean before) {
         tx.remove(path.getParent());
-        // FIXME: this should have happened sooner
-        final var pointArg = YangInstanceIdentifierDeserializer.create(modelContext, point.value()).path
-            .getLastPathArgument();
+
         int lastItemPosition = 0;
         for (var nodeChild : readList.body()) {
             if (nodeChild.name().equals(pointArg)) {
@@ -306,6 +302,7 @@ public abstract class RestconfStrategy {
         if (!before) {
             lastItemPosition++;
         }
+
         int lastInsertedPosition = 0;
         final var emptySubtree = ImmutableNodes.fromInstanceId(modelContext, path.getParent());
         tx.merge(YangInstanceIdentifier.of(emptySubtree.name()), emptySubtree);
@@ -392,7 +389,7 @@ public abstract class RestconfStrategy {
                     tx.replace(path, data);
                 } else {
                     checkItemDoesNotExists(exists(path), path);
-                    insertWithPointPost(tx, path, data, verifyNotNull(insert.point()), readData, grandParent, true);
+                    insertWithPointPost(tx, path, data, verifyNotNull(insert.pointArg()), readData, grandParent, true);
                 }
                 yield tx.commit();
             }
@@ -402,7 +399,7 @@ public abstract class RestconfStrategy {
                     tx.replace(path, data);
                 } else {
                     checkItemDoesNotExists(exists(path), path);
-                    insertWithPointPost(tx, path, data, verifyNotNull(insert.point()), readData, grandParent, false);
+                    insertWithPointPost(tx, path, data, verifyNotNull(insert.pointArg()), readData, grandParent, false);
                 }
                 yield tx.commit();
             }
@@ -502,12 +499,10 @@ public abstract class RestconfStrategy {
     }
 
     private void insertWithPointPost(final RestconfTransaction tx, final YangInstanceIdentifier path,
-            final NormalizedNode data, final PointParam point, final NormalizedNodeContainer<?> readList,
+            final NormalizedNode data, final PathArgument pointArg, final NormalizedNodeContainer<?> readList,
             final YangInstanceIdentifier grandParentPath, final boolean before) {
         tx.remove(grandParentPath);
-        // FIXME: this should have happened sooner
-        final var pointArg = YangInstanceIdentifierDeserializer.create(modelContext, point.value()).path
-            .getLastPathArgument();
+
         int lastItemPosition = 0;
         for (var nodeChild : readList.body()) {
             if (nodeChild.name().equals(pointArg)) {
@@ -518,6 +513,7 @@ public abstract class RestconfStrategy {
         if (!before) {
             lastItemPosition++;
         }
+
         int lastInsertedPosition = 0;
         final var emptySubtree = ImmutableNodes.fromInstanceId(modelContext, grandParentPath);
         tx.merge(YangInstanceIdentifier.of(emptySubtree.name()), emptySubtree);
@@ -525,8 +521,7 @@ public abstract class RestconfStrategy {
             if (lastInsertedPosition == lastItemPosition) {
                 tx.replace(path, data);
             }
-            final YangInstanceIdentifier childPath = grandParentPath.node(nodeChild.name());
-            tx.replace(childPath, nodeChild);
+            tx.replace(grandParentPath.node(nodeChild.name()), nodeChild);
             lastInsertedPosition++;
         }
     }

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java
index a638f261fcc30ae886214c2b784e16397df116d4..a16c1d630073a98c4bb660c6d74a5e175fd2be8c 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java
@@ -32,7 +32,6 @@ import org.opendaylight.restconf.api.query.InsertParam;
 import org.opendaylight.restconf.api.query.RestconfQueryParam;
 import org.opendaylight.restconf.api.query.WithDefaultsParam;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
-import org.opendaylight.restconf.common.errors.RestconfError;
 import org.opendaylight.restconf.nb.rfc8040.legacy.InstanceIdentifierContext;
 import org.opendaylight.restconf.nb.rfc8040.legacy.QueryParameters;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
@@ -60,12 +59,12 @@ public class QueryParamsTest {
      */
     @Test
     public void optionalParamMultipleTest() {
-        final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class,
+        final var ex = assertThrows(RestconfDocumentedException.class,
             () -> QueryParams.optionalParam(ContentParam.uriName, List.of("config", "nonconfig", "all")));
-        final List<RestconfError> errors = ex.getErrors();
+        final var errors = ex.getErrors();
         assertEquals(1, errors.size());
 
-        final RestconfError error = errors.get(0);
+        final var error = errors.get(0);
         assertEquals("Error type is not correct", ErrorType.PROTOCOL, error.getErrorType());
         assertEquals("Error tag is not correct", ErrorTag.INVALID_VALUE, error.getErrorTag());
     }
@@ -77,11 +76,13 @@ public class QueryParamsTest {
     public void checkParametersTypesNegativeTest() {
         assertUnknownParam(QueryParams::newNotificationQueryParams);
         assertUnknownParam(QueryParams::newReadDataParams);
-        assertUnknownParam(QueryParams::parseInsert);
+        assertUnknownParam(uriInfo -> QueryParams.parseInsert(mock(EffectiveModelContext.class), uriInfo));
 
         assertInvalidParam(QueryParams::newNotificationQueryParams, ContentParam.ALL);
         assertInvalidParam(QueryParams::newReadDataParams, InsertParam.LAST);
-        assertInvalidParam(QueryParams::parseInsert, ContentParam.ALL);
+        assertInvalidParam(
+            uriInfo -> QueryParams.parseInsert(mock(EffectiveModelContext.class), uriInfo),
+            ContentParam.ALL);
     }
 
     /**