REST-JSON: Unmarshall URI members from body

When unmarshalling, shapes that have marshalling locations set for a URI
component such as path or query paramter, they are located in the payload on
responses.

Author: dagnir

.changes/next-release/bugfix-AWSSDKforJavav2-bf4eb0a.json

@@ -0,0 +1,5 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "description": "For JSON protocols, when unmarshalling a response, if a member is declared to be located in the URI, the member is treated as being located in the payload instead."
+}

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/MarshallerUtil.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.protocols.json.internal;
+
+import software.amazon.awssdk.annotations.SdkInternalApi;
+import software.amazon.awssdk.core.protocol.MarshallLocation;
+
+@SdkInternalApi
+public final class MarshallerUtil {
+    private MarshallerUtil() {
+    }
+
+    /**
+     * @return true if the location is in the URI, false otherwise.
+     */
+    public static boolean locationInUri(MarshallLocation location) {
+        switch (location) {
+            case PATH:
+            case QUERY_PARAM:
+                return true;
+            default:
+                return false;
+        }
+    }
+}

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonProtocolUnmarshaller.java

@@ -38,6 +38,7 @@
 import software.amazon.awssdk.http.SdkHttpFullResponse;
 import software.amazon.awssdk.protocols.core.StringToInstant;
 import software.amazon.awssdk.protocols.core.StringToValueConverter;
+import software.amazon.awssdk.protocols.json.internal.MarshallerUtil;
 import software.amazon.awssdk.protocols.json.internal.dom.JsonDomParser;
 import software.amazon.awssdk.protocols.json.internal.dom.SdkJsonNode;
 import software.amazon.awssdk.utils.builder.Buildable;
@@ -163,7 +164,7 @@ public T unmarshall(JsonUnmarshallerContext context,
 
     public <TypeT extends SdkPojo> TypeT unmarshall(SdkPojo sdkPojo,
                             SdkHttpFullResponse response) throws IOException {
-        if (hasPayloadMembers(sdkPojo) && !hasExplicitBlobPayloadMember(sdkPojo)) {
+        if (hasPayloadMembersOnUnmarshall(sdkPojo) && !hasExplicitBlobPayloadMember(sdkPojo)) {
             SdkJsonNode jsonNode = parser.parse(ReleasableInputStream.wrap(response.content().orElse(null)).disableClose());
             return unmarshall(sdkPojo, response, jsonNode);
         } else {
@@ -181,10 +182,11 @@ private static boolean isExplicitPayloadMember(SdkField<?> f) {
         return f.containsTrait(PayloadTrait.class);
     }
 
-    private boolean hasPayloadMembers(SdkPojo sdkPojo) {
+    private boolean hasPayloadMembersOnUnmarshall(SdkPojo sdkPojo) {
         return sdkPojo.sdkFields()
-                      .stream()
-                      .anyMatch(f -> f.location() == MarshallLocation.PAYLOAD);
+                .stream()
+                .anyMatch(f -> f.location() == MarshallLocation.PAYLOAD
+                        || MarshallerUtil.locationInUri(f.location()));
     }
 
     public <TypeT extends SdkPojo> TypeT unmarshall(SdkPojo sdkPojo,

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonUnmarshallerContext.java

@@ -20,6 +20,7 @@
 import software.amazon.awssdk.core.protocol.MarshallLocation;
 import software.amazon.awssdk.core.protocol.MarshallingType;
 import software.amazon.awssdk.http.SdkHttpFullResponse;
+import software.amazon.awssdk.protocols.json.internal.MarshallerUtil;
 
 /**
  * Dependencies needed by implementations of {@link JsonUnmarshaller}.
@@ -51,6 +52,11 @@ public SdkHttpFullResponse response() {
      * @throws SdkClientException if no unmarshaller is found.
      */
     public JsonUnmarshaller<Object> getUnmarshaller(MarshallLocation location, MarshallingType<?> marshallingType) {
+        // A member being in the URI on a response is nonsensical; when a member is declared to be somewhere in the URI,
+        // it should be found in the payload on response
+        if (MarshallerUtil.locationInUri(location)) {
+            location = MarshallLocation.PAYLOAD;
+        }
         return unmarshallerRegistry.getUnmarshaller(location, marshallingType);
     }
 

test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/runners/UnmarshallingTestRunner.java

@@ -20,9 +20,9 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
 import com.github.tomakehurst.wiremock.client.WireMock;
-import java.lang.reflect.Method;
 import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
 import software.amazon.awssdk.codegen.model.intermediate.Metadata;
 import software.amazon.awssdk.core.sync.ResponseTransformer;
@@ -31,6 +31,7 @@
 import software.amazon.awssdk.protocol.model.GivenResponse;
 import software.amazon.awssdk.protocol.model.TestCase;
 import software.amazon.awssdk.protocol.reflect.ClientReflector;
+import software.amazon.awssdk.protocol.reflect.ShapeModelReflector;
 import software.amazon.awssdk.utils.IoUtils;
 
 /**
@@ -51,13 +52,14 @@ class UnmarshallingTestRunner {
     void runTest(TestCase testCase) throws Exception {
         resetWireMock(testCase.getGiven().getResponse());
         String operationName = testCase.getWhen().getOperationName();
+        ShapeModelReflector shapeModelReflector = createShapeModelReflector(testCase);
         if (!hasStreamingMember(operationName)) {
-            Object actualResult = clientReflector.invokeMethod(testCase, createRequestObject(operationName));
+            Object actualResult = clientReflector.invokeMethod(testCase, shapeModelReflector.createShapeObject());
             testCase.getThen().getUnmarshallingAssertion().assertMatches(createContext(operationName), actualResult);
         } else {
             CapturingResponseTransformer responseHandler = new CapturingResponseTransformer();
             Object actualResult = clientReflector
-                    .invokeStreamingMethod(testCase, createRequestObject(operationName), responseHandler);
+                    .invokeStreamingMethod(testCase, shapeModelReflector.createShapeObject(), responseHandler);
             testCase.getThen().getUnmarshallingAssertion()
                     .assertMatches(createContext(operationName, responseHandler.captured), actualResult);
         }
@@ -112,35 +114,11 @@ private ResponseDefinitionBuilder toResponseBuilder(GivenResponse givenResponse)
         return responseBuilder;
     }
 
-    /**
-     * @return An empty request object to call the operation method with.
-     */
-    private Object createRequestObject(String operationName) throws Exception {
-        String requestClassName = getModelFqcn(getOperationRequestClassName(operationName));
-
-        Class<?> requestClass = Class.forName(requestClassName);
-
-        Method builderMethod = null;
-
-        try {
-            builderMethod = requestClass.getDeclaredMethod("builder");
-        } catch (NoSuchMethodException ignored) {
-            // Ignored
-        }
-
-        if (builderMethod != null) {
-            builderMethod.setAccessible(true);
-
-            Object builderInstance = builderMethod.invoke(null);
-
-            Method buildMethod = builderInstance.getClass().getDeclaredMethod("build");
-            buildMethod.setAccessible(true);
-
-            return buildMethod.invoke(builderInstance);
-        } else {
-            return requestClass.newInstance();
-        }
-
+    private ShapeModelReflector createShapeModelReflector(TestCase testCase) {
+        String operationName = testCase.getWhen().getOperationName();
+        String requestClassName = getOperationRequestClassName(operationName);
+        JsonNode input = testCase.getGiven().getInput();
+        return new ShapeModelReflector(model, requestClassName, input);
     }
 
     private UnmarshallingTestContext createContext(String operationName) {
@@ -154,14 +132,6 @@ private UnmarshallingTestContext createContext(String operationName, String stre
                 .withStreamedResponse(streamedResponse);
     }
 
-    /**
-     * @param simpleClassName Class name to fully qualify.
-     * @return Fully qualified name of class in the client's model package.
-     */
-    private String getModelFqcn(String simpleClassName) {
-        return String.format("%s.%s", metadata.getFullModelPackageName(), simpleClassName);
-    }
-
     /**
      * @return Name of the request class that corresponds to the given operation.
      */

test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-output.json

@@ -114,5 +114,27 @@
         "StatusCodeMember": 201
       }
     }
+  },
+  {
+    "description": "Members declared to be in the query parameters or path on response are unmarshalled from the payload",
+    "given": {
+      "input": {
+        "PathParam": "param"
+      },
+      "response": {
+        "status_code": 200,
+        "body": "{\"PathParam\": \"Found PathParam in the payload! Yay!\", \"QueryParamOne\":\"Found QueryParamOne in the payload! Yay!\"}"
+      }
+    },
+    "when": {
+      "action": "unmarshall",
+      "operation": "MultiLocationOperation"
+    },
+    "then": {
+      "deserializedAs": {
+        "PathParam": "Found PathParam in the payload! Yay!",
+        "QueryParamOne": "Found QueryParamOne in the payload! Yay!"
+      }
+    }
   }
 ]

test/protocol-tests/src/main/resources/codegen-resources/restjson/service-2.json

@@ -101,15 +101,17 @@
         "method":"GET",
         "requestUri":"/2016-03-11/membersInQueryParams?StaticQueryParam=foo"
       },
-      "input":{"shape":"MembersInQueryParamsInput"}
+      "input":{"shape":"MembersInQueryParamsInput"},
+      "output":{"shape":"MembersInQueryParamsInput"}
     },
     "MultiLocationOperation":{
       "name":"MultiLocationOperation",
       "http":{
         "method":"POST",
         "requestUri":"/2016-03-11/multiLocationOperation/{PathParam}"
       },
-      "input":{"shape":"MultiLocationOperationInput"}
+      "input":{"shape":"MultiLocationOperationInput"},
+      "output":{"shape":"MultiLocationOperationInput"}
     },
     "NestedContainers":{
       "name":"NestedContainers",