Move patch target resolution 36/111236/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Apr 2024 20:13:54 +0000 (22:13 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Apr 2024 20:15:11 +0000 (22:15 +0200)
We now have a lingua franca between PatchBody and ApiPathNormalizer.
Take advantage of that and define the normalizeSubResource() utility
method.

JIRA: NETCONF-1288
Change-Id: Ie3aa238be767918a7c4c795fda04649823aa2824
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/api/PatchBody.java
restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/ApiPathNormalizer.java

index ca6235091ae47ab967b2ee212f5a84ac8fa07ce2..0820d5b373b355a4e1c4826855abe20f55b41366 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.restconf.server.api;
 
-import static com.google.common.base.Verify.verify;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.text.ParseException;
@@ -40,29 +38,34 @@ public abstract sealed class PatchBody extends AbstractBody permits JsonPatchBod
         throws IOException;
 
     static final YangInstanceIdentifier parsePatchTarget(final DatabindPath.@NonNull Data path, final String target) {
-        final var urlPath = path.instance();
-        if (target.equals("/")) {
-            verify(!urlPath.isEmpty(),
-                "target resource of URI must not be a datastore resource when target is '/'");
-            return urlPath;
-        }
-
-        // FIXME: NETCONF-1157: these two operations should really be a single ApiPathNormalizer step -- but for that
-        //                      we need to switch to ApiPath forms
-        final var pathNormalizer = new ApiPathNormalizer(path.databind());
-        final String targetUrl;
-        if (urlPath.isEmpty()) {
-            targetUrl = target.startsWith("/") ? target.substring(1) : target;
-        } else {
-            targetUrl = pathNormalizer.canonicalize(urlPath).toString() + target;
+        // As per: https://www.rfc-editor.org/rfc/rfc8072#page-18:
+        //
+        //        "Identifies the target data node for the edit
+        //        operation.  If the target has the value '/', then
+        //        the target data node is the target resource.
+        //        The target node MUST identify a data resource,
+        //        not the datastore resource.";
+        //
+        final ApiPath targetPath;
+        try {
+            targetPath = ApiPath.parse(target.startsWith("/") ? target.substring(1) : target);
+        } catch (ParseException e) {
+            throw new RestconfDocumentedException("Failed to parse edit target '" + target + "'",
+                ErrorType.RPC, ErrorTag.MALFORMED_MESSAGE, e);
         }
 
+        final YangInstanceIdentifier result;
         try {
-            return pathNormalizer.normalizeDataPath(ApiPath.parse(targetUrl)).instance();
-        } catch (ParseException | RestconfDocumentedException e) {
-            throw new RestconfDocumentedException("Failed to parse target " + target,
+            result = ApiPathNormalizer.normalizeSubResource(path, targetPath).instance();
+        } catch (RestconfDocumentedException e) {
+            throw new RestconfDocumentedException("Invalid edit target '" + targetPath + "'",
                 ErrorType.RPC, ErrorTag.MALFORMED_MESSAGE, e);
         }
+        if (result.isEmpty()) {
+            throw new RestconfDocumentedException("Target node resource must not be a datastore resource",
+                ErrorType.RPC, ErrorTag.MALFORMED_MESSAGE);
+        }
+        return result;
     }
 
     /**
index 22105bef840185aa74cbf7b36305560c978990f1..b54c081d96324a9514c1dd335759cb7ad7cbad04 100644 (file)
@@ -15,6 +15,7 @@ import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
+import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.List;
 import org.eclipse.jdt.annotation.NonNull;
@@ -210,6 +211,29 @@ public final class ApiPathNormalizer implements PointNormalizer {
             ErrorType.PROTOCOL, ErrorTag.DATA_MISSING);
     }
 
+    public static @NonNull Data normalizeSubResource(final Data resource, final ApiPath subResource) {
+        // If subResource is empty just return the resource
+        final var urlPath = resource.instance();
+        if (subResource.steps().isEmpty()) {
+            return resource;
+        }
+        final var normalizer = new ApiPathNormalizer(resource.databind());
+        if (urlPath.isEmpty()) {
+            // URL indicates the datastore resource, let's just normalize targetPath
+            return normalizer.normalizeDataPath(subResource);
+        }
+
+        // FIXME: We are re-parsing the concatenation. We should provide enough context for the bottom half of
+        //        normalizePath() logic instead
+        final String targetUrl = normalizer.canonicalize(urlPath).toString() + "/" + subResource.toString();
+        try {
+            return normalizer.normalizeDataPath(ApiPath.parse(targetUrl));
+        } catch (ParseException e) {
+            throw new RestconfDocumentedException("Failed to parse target " + targetUrl,
+                ErrorType.PROTOCOL, ErrorTag.MALFORMED_MESSAGE, e);
+        }
+    }
+
     @Override
     public PathArgument normalizePoint(final ApiPath value) {
         final var path = normalizePath(value);