Skip to content

REST-JSON: Unmarshall URI members from body #1996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-bf4eb0a.json
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
}
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down