diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-bf4eb0a.json b/.changes/next-release/bugfix-AWSSDKforJavav2-bf4eb0a.json new file mode 100644 index 000000000000..da363de01b49 --- /dev/null +++ b/.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." +} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/MarshallerUtil.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/MarshallerUtil.java new file mode 100644 index 000000000000..fc5625289e61 --- /dev/null +++ b/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; + } + } +} diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonProtocolUnmarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonProtocolUnmarshaller.java index 68aa66bd8a5c..0b6bb2577207 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonProtocolUnmarshaller.java +++ b/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 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 unmarshall(SdkPojo sdkPojo, diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonUnmarshallerContext.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonUnmarshallerContext.java index e740350fc0ab..69fea61aa9e1 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonUnmarshallerContext.java +++ b/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 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); } diff --git a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/runners/UnmarshallingTestRunner.java b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/runners/UnmarshallingTestRunner.java index 46d1cd0a96cf..7dbd9846b0b4 100644 --- a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/runners/UnmarshallingTestRunner.java +++ b/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. */ diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-output.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-output.json index 75fc74bf9229..2ea331b183e4 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-output.json +++ b/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!" + } + } } ] \ No newline at end of file diff --git a/test/protocol-tests/src/main/resources/codegen-resources/restjson/service-2.json b/test/protocol-tests/src/main/resources/codegen-resources/restjson/service-2.json index 34cadfef1655..8beae49151fe 100644 --- a/test/protocol-tests/src/main/resources/codegen-resources/restjson/service-2.json +++ b/test/protocol-tests/src/main/resources/codegen-resources/restjson/service-2.json @@ -101,7 +101,8 @@ "method":"GET", "requestUri":"/2016-03-11/membersInQueryParams?StaticQueryParam=foo" }, - "input":{"shape":"MembersInQueryParamsInput"} + "input":{"shape":"MembersInQueryParamsInput"}, + "output":{"shape":"MembersInQueryParamsInput"} }, "MultiLocationOperation":{ "name":"MultiLocationOperation", @@ -109,7 +110,8 @@ "method":"POST", "requestUri":"/2016-03-11/multiLocationOperation/{PathParam}" }, - "input":{"shape":"MultiLocationOperationInput"} + "input":{"shape":"MultiLocationOperationInput"}, + "output":{"shape":"MultiLocationOperationInput"} }, "NestedContainers":{ "name":"NestedContainers",