Skip to content

Support to UnMarshall Blob type payload data in XMlProtocol #2240

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 2 commits into from
Jan 19, 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
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
package software.amazon.awssdk.protocols.xml.internal.unmarshall;

import static java.util.Collections.singletonList;
import static software.amazon.awssdk.protocols.xml.internal.unmarshall.XmlResponseParserUtils.getBlobTypePayloadMemberToUnmarshal;

import java.time.Instant;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.core.SdkField;
import software.amazon.awssdk.core.SdkPojo;
import software.amazon.awssdk.core.protocol.MarshallLocation;
Expand Down Expand Up @@ -76,6 +79,7 @@ public <TypeT extends SdkPojo> TypeT unmarshall(SdkPojo sdkPojo,
}

SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement root) {
Optional<SdkField<?>> payloadMemberAsBlobType = getBlobTypePayloadMemberToUnmarshal(sdkPojo);
for (SdkField<?> field : sdkPojo.sdkFields()) {
XmlUnmarshaller<Object> unmarshaller = REGISTRY.getUnmarshaller(field.location(), field.marshallingType());

Expand All @@ -94,8 +98,15 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
root.getElementsByName(field.unmarshallLocationName());

if (!CollectionUtils.isNullOrEmpty(element)) {
Object unmarshalled = unmarshaller.unmarshall(context, element, (SdkField<Object>) field);
field.set(sdkPojo, unmarshalled);
boolean isFieldBlobTypePayload = payloadMemberAsBlobType.isPresent()
&& payloadMemberAsBlobType.get().equals(field);

if (isFieldBlobTypePayload) {
field.set(sdkPojo, SdkBytes.fromInputStream(context.response().content().get()));
} else {
Object unmarshalled = unmarshaller.unmarshall(context, element, (SdkField<Object>) field);
field.set(sdkPojo, unmarshalled);
}
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@

import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.SdkField;
import software.amazon.awssdk.core.SdkPojo;
import software.amazon.awssdk.core.protocol.MarshallLocation;
import software.amazon.awssdk.core.protocol.MarshallingType;
import software.amazon.awssdk.core.traits.PayloadTrait;
import software.amazon.awssdk.http.AbortableInputStream;
import software.amazon.awssdk.http.SdkHttpFullResponse;
import software.amazon.awssdk.protocols.query.unmarshall.XmlDomParser;
Expand Down Expand Up @@ -49,7 +52,7 @@ public static XmlElement parse(SdkPojo sdkPojo, SdkHttpFullResponse response) {
// In some cases the responseContent is present but empty, so when we are not expecting a body we should
// not attempt to parse it even if the body appears to be present.
if ((!response.isSuccessful() || hasPayloadMembers(sdkPojo)) && responseContent.isPresent() &&
!contentLengthZero(response)) {
!contentLengthZero(response) && !getBlobTypePayloadMemberToUnmarshal(sdkPojo).isPresent()) {
Comment on lines 54 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is long with a lot of nesting. Can we extract it and apply some names to the intermediate values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed context.response().content().isPresent check as we already do in LineNumber 83.
Replaced with local variable

return XmlDomParser.parse(responseContent.get());
} else {
return XmlElement.empty();
Expand All @@ -63,6 +66,21 @@ public static XmlElement parse(SdkPojo sdkPojo, SdkHttpFullResponse response) {
}
}

/**
* Gets the Member which is a Payload and which is of Blob Type.
* @param sdkPojo
* @return Optional of SdkField member if member is Blob type payload else returns Empty.
*/
public static Optional<SdkField<?>> getBlobTypePayloadMemberToUnmarshal(SdkPojo sdkPojo) {
return sdkPojo.sdkFields().stream()
.filter(e -> isExplicitPayloadMember(e))
.filter(f -> f.marshallingType() == MarshallingType.SDK_BYTES).findFirst();
}

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

private static boolean hasPayloadMembers(SdkPojo sdkPojo) {
return sdkPojo.sdkFields().stream()
.anyMatch(f -> f.location() == MarshallLocation.PAYLOAD);
Expand All @@ -71,4 +89,6 @@ private static boolean hasPayloadMembers(SdkPojo sdkPojo) {
private static boolean contentLengthZero(SdkHttpFullResponse response) {
return response.firstMatchingHeader(CONTENT_LENGTH).map(l -> Long.parseLong(l) == 0).orElse(false);
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,26 @@
"deserializedAs": {
}
}
}
},
// TODO header maps, service models supports this but Java SDK does not (not currently exercised)
// TODO header maps with prefix, services models do not support this, only used by S3
// TODO List of strings bound to header, service models supports this, not currently exercised by any service
{
"description": "Operation with explicit payload blob in output is unmarshalled correctly",
"given": {
"response": {
"status_code": 200,
"body": "contents"
}
},
"when": {
"action": "unmarshall",
"operation": "OperationWithExplicitPayloadBlob"
},
"then": {
"deserializedAs": {
"PayloadMember": "contents"
}
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,6 @@
}
}
},
// TODO payload blob and streaming should be moved up to rest-core when Java supports it
{
"description": "Operation with explicit payload blob in output is unmarshalled correctly",
"given": {
"response": {
"status_code": 200,
"body": "contents"
}
},
"when": {
"action": "unmarshall",
"operation": "OperationWithExplicitPayloadBlob"
},
"then": {
"deserializedAs": {
"PayloadMember": "contents"
}
}
},
{
"description": "Operation with streaming payload in output is unmarshalled correctly",
"given": {
Expand Down