Skip to content

Commit 77be6a4

Browse files
dagnirdagnir
authored
Don't create object if no implicit members (#2773)
This fixes an edge case in the REST-JSON marshaller: if the request has neither an explicit payload nor implicit payload members (i.e. members not explicitly bound to an location), then the marshaller should not create an create an object. i.e. the request body should be "", and not "{}".
1 parent a5f185a commit 77be6a4

File tree

13 files changed

+142
-34
lines changed

13 files changed

+142
-34
lines changed

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/ShapeModel.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,17 @@ public List<MemberModel> getUnboundMembers() {
187187
List<MemberModel> unboundMembers = new ArrayList<>();
188188
if (members != null) {
189189
for (MemberModel member : members) {
190-
if (member.getHttp().getLocation() == null) {
190+
if (member.getHttp().getLocation() == null && !member.getHttp().getIsPayload()) {
191191
if (hasPayloadMember) {
192+
// There is an explicit payload, but this unbound
193+
// member isn't it.
194+
// Note: Somewhat unintuitive, explicit payloads don't
195+
// have an explicit location; they're identified by
196+
// the payload HTTP trait being true.
192197
throw new IllegalStateException(String.format(
193-
"C2J Shape %s has both an explicit payload member and unbound (no explicit location) members. "
194-
+ "This is undefined behavior, verify the correctness of the C2J model", c2jName));
198+
"C2J Shape %s has both an explicit payload member and unbound (no explicit location) member, %s."
199+
+ " This is undefined behavior, verify the correctness of the C2J model.",
200+
c2jName, member.getName()));
195201
}
196202
unboundMembers.add(member);
197203
}
@@ -221,7 +227,12 @@ public List<MemberModel> getUnboundEventMembers() {
221227
public boolean hasPayloadMembers() {
222228
return hasPayloadMember ||
223229
getExplicitEventPayloadMember() != null ||
224-
!getUnboundMembers().isEmpty() ||
230+
hasImplicitPayloadMembers();
231+
232+
}
233+
234+
public boolean hasImplicitPayloadMembers() {
235+
return !getUnboundMembers().isEmpty() ||
225236
(isEvent() && !getUnboundEventMembers().isEmpty());
226237
}
227238

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/transform/protocols/JsonMarshallerSpec.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ protected FieldSpec operationInfoField() {
9595
.add(".httpMethod($T.$L)", SdkHttpMethod.class, shapeModel.getMarshaller().getVerb())
9696
.add(".hasExplicitPayloadMember($L)", shapeModel.isHasPayloadMember() ||
9797
shapeModel.getExplicitEventPayloadMember() != null)
98+
.add(".hasImplicitPayloadMembers($L)", shapeModel.hasImplicitPayloadMembers())
9899
.add(".hasPayloadMembers($L)", shapeModel.hasPayloadMembers());
99100

100101
if (StringUtils.isNotBlank(shapeModel.getMarshaller().getTarget())) {

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/alltypesrequestmarshaller.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
@SdkInternalApi
2020
public class AllTypesRequestMarshaller implements Marshaller<AllTypesRequest> {
2121
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder().requestUri("/")
22-
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasPayloadMembers(true).build();
22+
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasImplicitPayloadMembers(true)
23+
.hasPayloadMembers(true).build();
2324

2425
private final BaseAwsJsonProtocolFactory protocolFactory;
2526

@@ -32,11 +33,10 @@ public SdkHttpFullRequest marshall(AllTypesRequest allTypesRequest) {
3233
Validate.paramNotNull(allTypesRequest, "allTypesRequest");
3334
try {
3435
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
35-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3637
return protocolMarshaller.marshall(allTypesRequest);
3738
} catch (Exception e) {
3839
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
3940
}
4041
}
4142
}
42-

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/eventstreamoperationrequestmarshaller.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
@SdkInternalApi
2020
public class EventStreamOperationRequestMarshaller implements Marshaller<EventStreamOperationRequest> {
2121
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder()
22-
.requestUri("/2016-03-11/eventStreamOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(true)
23-
.hasPayloadMembers(true).hasEventStreamingInput(true).build();
22+
.requestUri("/2016-03-11/eventStreamOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(true)
23+
.hasImplicitPayloadMembers(false).hasPayloadMembers(true).hasEventStreamingInput(true).build();
2424

2525
private final BaseAwsJsonProtocolFactory protocolFactory;
2626

@@ -33,7 +33,7 @@ public SdkHttpFullRequest marshall(EventStreamOperationRequest eventStreamOperat
3333
Validate.paramNotNull(eventStreamOperationRequest, "eventStreamOperationRequest");
3434
try {
3535
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
36-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3737
return protocolMarshaller.marshall(eventStreamOperationRequest);
3838
} catch (Exception e) {
3939
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/eventstreamoperationwithonlyinputrequestmarshaller.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
*/
1818
@Generated("software.amazon.awssdk:codegen")
1919
@SdkInternalApi
20-
public class EventStreamOperationWithOnlyInputRequestMarshaller implements
21-
Marshaller<EventStreamOperationWithOnlyInputRequest> {
20+
public class EventStreamOperationWithOnlyInputRequestMarshaller implements Marshaller<EventStreamOperationWithOnlyInputRequest> {
2221
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder()
23-
.requestUri("/2016-03-11/EventStreamOperationWithOnlyInput").httpMethod(SdkHttpMethod.POST)
24-
.hasExplicitPayloadMember(false).hasPayloadMembers(true).hasEventStreamingInput(true).build();
22+
.requestUri("/2016-03-11/EventStreamOperationWithOnlyInput").httpMethod(SdkHttpMethod.POST)
23+
.hasExplicitPayloadMember(false).hasImplicitPayloadMembers(true).hasPayloadMembers(true).hasEventStreamingInput(true)
24+
.build();
2525

2626
private final BaseAwsJsonProtocolFactory protocolFactory;
2727

@@ -34,11 +34,10 @@ public SdkHttpFullRequest marshall(EventStreamOperationWithOnlyInputRequest even
3434
Validate.paramNotNull(eventStreamOperationWithOnlyInputRequest, "eventStreamOperationWithOnlyInputRequest");
3535
try {
3636
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
37-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
37+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3838
return protocolMarshaller.marshall(eventStreamOperationWithOnlyInputRequest);
3939
} catch (Exception e) {
4040
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
4141
}
4242
}
4343
}
44-

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/nestedcontainersrequestmarshaller.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
@SdkInternalApi
2020
public class NestedContainersRequestMarshaller implements Marshaller<NestedContainersRequest> {
2121
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder().requestUri("/")
22-
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasPayloadMembers(true).build();
22+
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasImplicitPayloadMembers(true)
23+
.hasPayloadMembers(true).build();
2324

2425
private final BaseAwsJsonProtocolFactory protocolFactory;
2526

@@ -32,11 +33,10 @@ public SdkHttpFullRequest marshall(NestedContainersRequest nestedContainersReque
3233
Validate.paramNotNull(nestedContainersRequest, "nestedContainersRequest");
3334
try {
3435
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
35-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3637
return protocolMarshaller.marshall(nestedContainersRequest);
3738
} catch (Exception e) {
3839
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
3940
}
4041
}
4142
}
42-

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/operationwithnoinputoroutputrequestmarshaller.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
*/
1818
@Generated("software.amazon.awssdk:codegen")
1919
@SdkInternalApi
20-
public class OperationWithNoInputOrOutputRequestMarshaller implements
21-
Marshaller<OperationWithNoInputOrOutputRequest> {
20+
public class OperationWithNoInputOrOutputRequestMarshaller implements Marshaller<OperationWithNoInputOrOutputRequest> {
2221
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder().requestUri("/")
23-
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasPayloadMembers(false).build();
22+
.httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false).hasImplicitPayloadMembers(false)
23+
.hasPayloadMembers(false).build();
2424

2525
private final BaseAwsJsonProtocolFactory protocolFactory;
2626

@@ -33,11 +33,10 @@ public SdkHttpFullRequest marshall(OperationWithNoInputOrOutputRequest operation
3333
Validate.paramNotNull(operationWithNoInputOrOutputRequest, "operationWithNoInputOrOutputRequest");
3434
try {
3535
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
36-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3737
return protocolMarshaller.marshall(operationWithNoInputOrOutputRequest);
3838
} catch (Exception e) {
3939
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
4040
}
4141
}
4242
}
43-

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/streaminginputoperationrequestmarshaller.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
@SdkInternalApi
2020
public class StreamingInputOperationRequestMarshaller implements Marshaller<StreamingInputOperationRequest> {
2121
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder()
22-
.requestUri("/2016-03-11/streamingInputOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(true)
23-
.hasPayloadMembers(true).hasStreamingInput(true).build();
22+
.requestUri("/2016-03-11/streamingInputOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(true)
23+
.hasImplicitPayloadMembers(false).hasPayloadMembers(true).hasStreamingInput(true).build();
2424

2525
private final BaseAwsJsonProtocolFactory protocolFactory;
2626

@@ -33,11 +33,10 @@ public SdkHttpFullRequest marshall(StreamingInputOperationRequest streamingInput
3333
Validate.paramNotNull(streamingInputOperationRequest, "streamingInputOperationRequest");
3434
try {
3535
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
36-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3737
return protocolMarshaller.marshall(streamingInputOperationRequest);
3838
} catch (Exception e) {
3939
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
4040
}
4141
}
4242
}
43-

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/transform/streamingoutputoperationrequestmarshaller.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
@SdkInternalApi
2020
public class StreamingOutputOperationRequestMarshaller implements Marshaller<StreamingOutputOperationRequest> {
2121
private static final OperationInfo SDK_OPERATION_BINDING = OperationInfo.builder()
22-
.requestUri("/2016-03-11/streamingOutputOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false)
23-
.hasPayloadMembers(false).build();
22+
.requestUri("/2016-03-11/streamingOutputOperation").httpMethod(SdkHttpMethod.POST).hasExplicitPayloadMember(false)
23+
.hasImplicitPayloadMembers(false).hasPayloadMembers(false).build();
2424

2525
private final BaseAwsJsonProtocolFactory protocolFactory;
2626

@@ -33,11 +33,10 @@ public SdkHttpFullRequest marshall(StreamingOutputOperationRequest streamingOutp
3333
Validate.paramNotNull(streamingOutputOperationRequest, "streamingOutputOperationRequest");
3434
try {
3535
ProtocolMarshaller<SdkHttpFullRequest> protocolMarshaller = protocolFactory
36-
.createProtocolMarshaller(SDK_OPERATION_BINDING);
36+
.createProtocolMarshaller(SDK_OPERATION_BINDING);
3737
return protocolMarshaller.marshall(streamingOutputOperationRequest);
3838
} catch (Exception e) {
3939
throw SdkClientException.builder().message("Unable to marshall request to JSON: " + e.getMessage()).cause(e).build();
4040
}
4141
}
4242
}
43-

‎core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public class JsonProtocolMarshaller implements ProtocolMarshaller<SdkHttpFullReq
6262
private final String contentType;
6363
private final AwsJsonProtocolMetadata protocolMetadata;
6464
private final boolean hasExplicitPayloadMember;
65+
private final boolean hasImplicitPayloadMembers;
6566
private final boolean hasStreamingInput;
6667

6768
private final JsonMarshallerContext marshallerContext;
@@ -78,6 +79,7 @@ public class JsonProtocolMarshaller implements ProtocolMarshaller<SdkHttpFullReq
7879
this.contentType = contentType;
7980
this.protocolMetadata = protocolMetadata;
8081
this.hasExplicitPayloadMember = operationInfo.hasExplicitPayloadMember();
82+
this.hasImplicitPayloadMembers = operationInfo.hasImplicitPayloadMembers();
8183
this.hasStreamingInput = operationInfo.hasStreamingInput();
8284
this.hasEventStreamingInput = operationInfo.hasEventStreamingInput();
8385
this.hasEvent = operationInfo.hasEvent();
@@ -167,7 +169,8 @@ private SdkHttpFullRequest.Builder fillBasicRequestParams(OperationInfo operatio
167169
* members bound to the payload will be added as fields to this object.
168170
*/
169171
private void startMarshalling() {
170-
if (!hasExplicitPayloadMember) {
172+
// Create the implicit request object if needed.
173+
if (needTopLevelJsonObject()) {
171174
jsonGenerator.writeStartObject();
172175
}
173176
}
@@ -220,7 +223,7 @@ private SdkHttpFullRequest finishMarshalling() {
220223
// Content may already be set if the payload is binary data.
221224
if (request.contentStreamProvider() == null) {
222225
// End the implicit request object if needed.
223-
if (!hasExplicitPayloadMember) {
226+
if (needTopLevelJsonObject()) {
224227
jsonGenerator.writeEndObject();
225228
}
226229

@@ -263,4 +266,10 @@ private void marshallField(SdkField<?> field, Object val) {
263266
MARSHALLER_REGISTRY.getMarshaller(field.location(), field.marshallingType(), val)
264267
.marshall(val, marshallerContext, field.locationName(), (SdkField<Object>) field);
265268
}
269+
270+
private boolean needTopLevelJsonObject() {
271+
return AwsJsonProtocol.AWS_JSON.equals(protocolMetadata.protocol())
272+
|| (!hasExplicitPayloadMember && hasImplicitPayloadMembers);
273+
274+
}
266275
}

‎core/protocols/protocol-core/src/main/java/software/amazon/awssdk/protocols/core/OperationInfo.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public final class OperationInfo {
3131
private final String apiVersion;
3232
private final boolean hasExplicitPayloadMember;
3333
private final boolean hasPayloadMembers;
34+
private final boolean hasImplicitPayloadMembers;
3435
private final boolean hasStreamingInput;
3536
private final boolean hasEventStreamingInput;
3637
private final boolean hasEvent;
@@ -42,6 +43,7 @@ private OperationInfo(Builder builder) {
4243
this.operationIdentifier = builder.operationIdentifier;
4344
this.apiVersion = builder.apiVersion;
4445
this.hasExplicitPayloadMember = builder.hasExplicitPayloadMember;
46+
this.hasImplicitPayloadMembers = builder.hasImplicitPayloadMembers;
4547
this.hasPayloadMembers = builder.hasPayloadMembers;
4648
this.hasStreamingInput = builder.hasStreamingInput;
4749
this.additionalMetadata = builder.additionalMetadata.build();
@@ -95,6 +97,14 @@ public boolean hasPayloadMembers() {
9597
return hasPayloadMembers;
9698
}
9799

100+
/**
101+
* @return True if the operation has members that are not explicitly bound to a marshalling location, and thus are
102+
* implicitly bound to the body.
103+
*/
104+
public boolean hasImplicitPayloadMembers() {
105+
return hasImplicitPayloadMembers;
106+
}
107+
98108
/**
99109
* @return True if the operation has streaming input.
100110
*/
@@ -144,6 +154,7 @@ public static final class Builder {
144154
private String operationIdentifier;
145155
private String apiVersion;
146156
private boolean hasExplicitPayloadMember;
157+
private boolean hasImplicitPayloadMembers;
147158
private boolean hasPayloadMembers;
148159
private boolean hasStreamingInput;
149160
private boolean hasEventStreamingInput;
@@ -183,6 +194,11 @@ public Builder hasPayloadMembers(boolean hasPayloadMembers) {
183194
return this;
184195
}
185196

197+
public Builder hasImplicitPayloadMembers(boolean hasImplicitPayloadMembers) {
198+
this.hasImplicitPayloadMembers = hasImplicitPayloadMembers;
199+
return this;
200+
}
201+
186202
public Builder hasStreamingInput(boolean hasStreamingInput) {
187203
this.hasStreamingInput = hasStreamingInput;
188204
return this;

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,60 @@
219219
}
220220
}
221221
},
222+
{
223+
"description": "NoPayloadGet",
224+
"given": {
225+
"input": {
226+
}
227+
},
228+
"when": {
229+
"action": "marshall",
230+
"operation": "NoPayloadPost"
231+
},
232+
"then": {
233+
"serializedAs": {
234+
"uri": "/no-payload",
235+
"method": "POST",
236+
"headers": {
237+
"doesNotContain": [
238+
"Content-Type"
239+
]
240+
},
241+
"body": {
242+
"equals": ""
243+
}
244+
}
245+
}
246+
},
247+
{
248+
"description": "NoPayloadGetWithHeader",
249+
"given": {
250+
"input": {
251+
"testId": "t-12345"
252+
}
253+
},
254+
"when": {
255+
"action": "marshall",
256+
"operation": "NoPayloadPost"
257+
},
258+
"then": {
259+
"serializedAs": {
260+
"uri": "/no-payload",
261+
"method": "POST",
262+
"headers": {
263+
"contains": {
264+
"x-amz-test-id": "t-12345"
265+
},
266+
"doesNotContain": [
267+
"Content-Type"
268+
]
269+
},
270+
"body": {
271+
"equals": ""
272+
}
273+
}
274+
}
275+
},
222276
{
223277
"description": "TestBodyNoParams",
224278
"given": {

0 commit comments

Comments
 (0)