Skip to content

Follow standard REST-JSON content-type handling #2765

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 3 commits into from
Oct 13, 2021
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
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-47e105f.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "bugfix",
"description": "Update the REST-JSON marshalling logic to conform to the standard expected behavior WRT to the `Content-Type` of the request."
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
@SdkProtectedApi
public class DefaultJsonContentTypeResolver implements JsonContentTypeResolver {
private static final String REST_JSON_CONTENT_TYPE = "application/json";

private final String prefix;

Expand All @@ -32,7 +33,9 @@ public DefaultJsonContentTypeResolver(String prefix) {

@Override
public String resolveContentType(AwsJsonProtocolMetadata protocolMetadata) {
//Changing this to 'application/json' may break clients expecting 'application/x-amz-json-1.1'
if (AwsJsonProtocol.REST_JSON.equals(protocolMetadata.protocol())) {
return REST_JSON_CONTENT_TYPE;
}
return prefix + protocolMetadata.protocolVersion();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package software.amazon.awssdk.protocols.json.internal.marshall;

import static software.amazon.awssdk.core.internal.util.Mimetype.MIMETYPE_EVENT_STREAM;
import static software.amazon.awssdk.core.protocol.MarshallingType.DOCUMENT;
import static software.amazon.awssdk.http.Header.CHUNKED;
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
import static software.amazon.awssdk.http.Header.CONTENT_TYPE;
Expand Down Expand Up @@ -176,31 +175,40 @@ private void startMarshalling() {
void doMarshall(SdkPojo pojo) {
for (SdkField<?> field : pojo.sdkFields()) {
Object val = field.getValueOrDefault(pojo);
if (isBinary(field, val)) {
request.contentStreamProvider(((SdkBytes) val)::asInputStream);
} else if (isDocumentType(field) && val != null) {
marshalDocumentType(field, val);
} else {
if (val != null && field.containsTrait(PayloadTrait.class)) {
jsonGenerator.writeStartObject();
doMarshall((SdkPojo) val);
jsonGenerator.writeEndObject();
} else {
MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val)
.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);
if (isExplicitBinaryPayload(field)) {
if (val != null) {
request.contentStreamProvider(((SdkBytes) val)::asInputStream);
}
} else if (isExplicitPayloadMember(field)) {
marshallExplicitJsonPayload(field, val);
} else {
marshallField(field, val);
}
}
}

private boolean isBinary(SdkField<?> field, Object val) {
return isExplicitPayloadMember(field) && val instanceof SdkBytes;
private boolean isExplicitBinaryPayload(SdkField<?> field) {
return isExplicitPayloadMember(field) && MarshallingType.SDK_BYTES.equals(field.marshallingType());
}

private boolean isExplicitPayloadMember(SdkField<?> field) {
return field.containsTrait(PayloadTrait.class);
}

private void marshallExplicitJsonPayload(SdkField<?> field, Object val) {
// Explicit JSON payloads are always marshalled as an object,
// even if they're null, in which case it's an empty object.
jsonGenerator.writeStartObject();
if (val != null) {
if (MarshallingType.DOCUMENT.equals(field.marshallingType())) {
marshallField(field, val);
} else {
doMarshall((SdkPojo) val);
}
}
jsonGenerator.writeEndObject();
}

@Override
public SdkHttpFullRequest marshall(SdkPojo pojo) {
startMarshalling();
Expand Down Expand Up @@ -243,27 +251,16 @@ private SdkHttpFullRequest finishMarshalling() {
}
request.removeHeader(CONTENT_LENGTH);
request.putHeader(TRANSFER_ENCODING, CHUNKED);
} else if (contentType != null && !hasStreamingInput && request.contentStreamProvider() != null) {
} else if (contentType != null && !hasStreamingInput && request.headers().containsKey(CONTENT_LENGTH)) {
request.putHeader(CONTENT_TYPE, contentType);
}
}

return request.build();
}

private boolean isDocumentType(SdkField<?> field) {
return DOCUMENT.equals(field.marshallingType());
}

private void marshalDocumentType(SdkField<?> field, Object val) {
boolean isExplicitPayloadField = hasExplicitPayloadMember && field.containsTrait(PayloadTrait.class);
if (isExplicitPayloadField) {
jsonGenerator.writeStartObject();
}
private void marshallField(SdkField<?> field, Object val) {
MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val)
.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);
if (isExplicitPayloadField) {
jsonGenerator.writeEndObject();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public void explicitDocumentOnlyNullPayload() {
WithExplicitDocumentPayloadResponse response =
jsonClient.withExplicitDocumentPayload(c -> c.myDocument(null).accept(AcceptHeader.IMAGE_JPEG));
String syncRequest = getSyncRequestBody();
assertThat(syncRequest).isEmpty();
assertThat(syncRequest).isEqualTo("{}");
SdkHttpRequest sdkHttpRequest = getSyncRequest();
assertThat(sdkHttpRequest.firstMatchingHeader("accept").get()).contains(AcceptHeader.IMAGE_JPEG.toString());
assertThat(response.myDocument()).isNull();
Expand All @@ -236,7 +236,7 @@ public void explicitDocumentOnlyEmptyPayload() {
WithExplicitDocumentPayloadResponse response =
jsonClient.withExplicitDocumentPayload(c -> c.myDocument(null).accept(AcceptHeader.IMAGE_JPEG));
String syncRequest = getSyncRequestBody();
assertThat(syncRequest).isEmpty();
assertThat(syncRequest).isEqualTo("{}");
SdkHttpRequest sdkHttpRequest = getSyncRequest();
assertThat(sdkHttpRequest.firstMatchingHeader("accept").get()).contains(AcceptHeader.IMAGE_JPEG.toString());
assertThat(response.myDocument()).isNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,5 +218,30 @@
}
}
}
},
{
"description": "TestBodyNoParams",
"given": {
"input": {
}
},
"when": {
"action": "marshall",
"operation": "TestBody"
},
"then": {
"serializedAs": {
"uri": "/body",
"method": "POST",
"headers": {
"contains": {
"Content-Type": "application/json"
}
},
"body": {
"jsonEquals": "{}"
}
}
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
}
},
{
"description": "Explicit payload member and no parameters marshalls into an empty request body",
"description": "Explicit payload member and no parameters marshalls into an empty JSON object",
"given": {
"input": {
}
Expand All @@ -71,7 +71,7 @@
"then": {
"serializedAs": {
"body": {
"equals": ""
"jsonEquals": "{}"
}
}
}
Expand Down