From 27b40a2880fbd9e5c840f39cf9f5b42409bf9bd7 Mon Sep 17 00:00:00 2001 From: Bennett Lynch Date: Mon, 12 Jul 2021 17:47:35 -0700 Subject: [PATCH 1/5] Add support for marshalling lists of strings in HTTP headers... ...for JSON/XML protocols. ## Motivation and Context We currently lack support for marshalling lists of strings (and enums) in HTTP headers. Other IDLs and modeling languages, e.g., Smithy, do support such bindings and services may come to expect such support: https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html#httpheader-trait ## Description * Add support for marshalling a list of strings to relevant JSON and XML marshalling classes * Consistent with the existing XML HeaderUnmarshaller map implementation, unmarshalling only supports string value types (this can be extended in the future if needed) * Marshalling implementations are changed to use appendHeader rather than putHeader * Add convenience method SdkField#getRequiredTrait(..) to support the common use case of ensuring a trait is present; update other existing use cases to utilize this method * Update relevant test classes to correctly recognize headers as a Map> * Only wrap marshalling assertion exceptions as runtime when needed, minimizing stack trace noise ## Testing New tests added to: * rest-json-input.json * rest-json-output.json * rest-xml-input.json * rest-xml-output.json --- .../internal/marshall/HeaderMarshaller.java | 17 +++++++++-- .../marshall/JsonProtocolMarshaller.java | 1 + .../unmarshall/HeaderUnmarshaller.java | 9 ++++++ .../unmarshall/JsonProtocolUnmarshaller.java | 1 + .../internal/marshall/HeaderMarshaller.java | 22 +++++++++++++- .../marshall/QueryParamMarshaller.java | 3 +- .../marshall/XmlPayloadMarshaller.java | 7 ++--- .../marshall/XmlProtocolMarshaller.java | 1 + .../unmarshall/HeaderUnmarshaller.java | 8 +++++ .../unmarshall/XmlProtocolUnmarshaller.java | 1 + .../software/amazon/awssdk/core/SdkField.java | 17 +++++++++++ .../asserts/marshalling/HeadersAssertion.java | 12 +++++--- .../marshalling/MarshallingAssertion.java | 4 ++- .../awssdk/protocol/model/GivenResponse.java | 7 +++-- .../runners/UnmarshallingTestRunner.java | 4 ++- .../suites/cases/rest-json-input.json | 30 +++++++++++++++++++ .../suites/cases/rest-json-output.json | 28 +++++++++++++++++ .../protocol/suites/cases/rest-xml-input.json | 30 +++++++++++++++++++ .../suites/cases/rest-xml-output.json | 28 +++++++++++++++++ .../codegen-resources/restjson/service-2.json | 5 ++++ .../codegen-resources/restxml/service-2.json | 5 ++++ 21 files changed, 221 insertions(+), 19 deletions(-) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java index 8d7fac6d7d21..007a1fc0d0fd 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java @@ -17,9 +17,12 @@ import java.nio.charset.StandardCharsets; import java.time.Instant; +import java.util.List; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkField; +import software.amazon.awssdk.core.protocol.MarshallLocation; import software.amazon.awssdk.core.traits.JsonValueTrait; +import software.amazon.awssdk.core.traits.ListTrait; import software.amazon.awssdk.protocols.core.ValueToStringConverter; import software.amazon.awssdk.utils.BinaryUtils; @@ -45,6 +48,17 @@ public final class HeaderMarshaller { public static final JsonMarshaller INSTANT = new SimpleHeaderMarshaller<>(JsonProtocolMarshaller.INSTANT_VALUE_TO_STRING); + public static final JsonMarshaller> LIST = (list, context, paramName, sdkField) -> { + if (list.isEmpty()) { + return; + } + SdkField memberFieldInfo = sdkField.getRequiredTrait(ListTrait.class).memberFieldInfo(); + for (Object listValue : list) { + JsonMarshaller marshaller = context.marshallerRegistry().getMarshaller(MarshallLocation.HEADER, listValue); + marshaller.marshall(listValue, context, paramName, memberFieldInfo); + } + }; + private HeaderMarshaller() { } @@ -58,8 +72,7 @@ private SimpleHeaderMarshaller(ValueToStringConverter.ValueToString converter @Override public void marshall(T val, JsonMarshallerContext context, String paramName, SdkField sdkField) { - context.request().putHeader(paramName, converter.convert(val, sdkField)); + context.request().appendHeader(paramName, converter.convert(val, sdkField)); } } - } diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java index a7d883c17289..22dd827eaa21 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java @@ -110,6 +110,7 @@ private static JsonMarshallerRegistry createMarshallerRegistry() { .headerMarshaller(MarshallingType.FLOAT, HeaderMarshaller.FLOAT) .headerMarshaller(MarshallingType.BOOLEAN, HeaderMarshaller.BOOLEAN) .headerMarshaller(MarshallingType.INSTANT, HeaderMarshaller.INSTANT) + .headerMarshaller(MarshallingType.LIST, HeaderMarshaller.LIST) .headerMarshaller(MarshallingType.NULL, JsonMarshaller.NULL) .queryParamMarshaller(MarshallingType.STRING, QueryParamMarshaller.STRING) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/HeaderUnmarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/HeaderUnmarshaller.java index cd088e2c6e7c..8289f931a91c 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/HeaderUnmarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/HeaderUnmarshaller.java @@ -15,14 +15,18 @@ package software.amazon.awssdk.protocols.json.internal.unmarshall; +import static java.util.stream.Collectors.toList; + import java.nio.charset.StandardCharsets; import java.time.Instant; +import java.util.List; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkField; import software.amazon.awssdk.core.traits.JsonValueTrait; import software.amazon.awssdk.protocols.core.StringToValueConverter; import software.amazon.awssdk.protocols.json.internal.dom.SdkJsonNode; import software.amazon.awssdk.utils.BinaryUtils; +import software.amazon.awssdk.utils.http.SdkHttpUtils; /** * Header unmarshallers for all the simple types we support. @@ -39,6 +43,11 @@ final class HeaderUnmarshaller { public static final JsonUnmarshaller BOOLEAN = new SimpleHeaderUnmarshaller<>(StringToValueConverter.TO_BOOLEAN); public static final JsonUnmarshaller FLOAT = new SimpleHeaderUnmarshaller<>(StringToValueConverter.TO_FLOAT); + // Only supports string value type + public static final JsonUnmarshaller> LIST = (context, jsonContent, field) -> { + return SdkHttpUtils.allMatchingHeaders(context.response().headers(), field.locationName()).collect(toList()); + }; + private HeaderUnmarshaller() { } 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 3d075d39b692..c95cd0b42314 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 @@ -79,6 +79,7 @@ private static JsonUnmarshallerRegistry createUnmarshallerRegistry( .headerUnmarshaller(MarshallingType.BOOLEAN, HeaderUnmarshaller.BOOLEAN) .headerUnmarshaller(MarshallingType.INSTANT, HeaderUnmarshaller.createInstantHeaderUnmarshaller(instantStringToValue)) .headerUnmarshaller(MarshallingType.FLOAT, HeaderUnmarshaller.FLOAT) + .headerUnmarshaller(MarshallingType.LIST, HeaderUnmarshaller.LIST) .payloadUnmarshaller(MarshallingType.STRING, new SimpleTypeJsonUnmarshaller<>(StringToValueConverter.TO_STRING)) .payloadUnmarshaller(MarshallingType.INTEGER, new SimpleTypeJsonUnmarshaller<>(StringToValueConverter.TO_INTEGER)) diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java index 8d377395d488..57ab9668ddc4 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java @@ -16,10 +16,12 @@ package software.amazon.awssdk.protocols.xml.internal.marshall; import java.time.Instant; +import java.util.List; import java.util.Map; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkField; import software.amazon.awssdk.core.protocol.MarshallLocation; +import software.amazon.awssdk.core.traits.ListTrait; import software.amazon.awssdk.protocols.core.ValueToStringConverter; @SdkInternalApi @@ -66,6 +68,24 @@ protected boolean shouldEmit(Map map) { } }; + public static final XmlMarshaller> LIST = new SimpleHeaderMarshaller>(null) { + @Override + public void marshall(List list, XmlMarshallerContext context, String paramName, SdkField> sdkField) { + if (!shouldEmit(list)) { + return; + } + SdkField memberFieldInfo = sdkField.getRequiredTrait(ListTrait.class).memberFieldInfo(); + for (Object listValue : list) { + XmlMarshaller marshaller = context.marshallerRegistry().getMarshaller(MarshallLocation.HEADER, listValue); + marshaller.marshall(listValue, context, paramName, memberFieldInfo); + } + } + + @Override + protected boolean shouldEmit(List list) { + return list != null && !list.isEmpty(); + } + }; private HeaderMarshaller() { } @@ -83,7 +103,7 @@ public void marshall(T val, XmlMarshallerContext context, String paramName, SdkF return; } - context.request().putHeader(paramName, converter.convert(val, sdkField)); + context.request().appendHeader(paramName, converter.convert(val, sdkField)); } protected boolean shouldEmit(T val) { diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/QueryParamMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/QueryParamMarshaller.java index 43bde6641d14..5c354009fe9e 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/QueryParamMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/QueryParamMarshaller.java @@ -60,8 +60,7 @@ public final class QueryParamMarshaller { return; } - MapTrait mapTrait = sdkField.getOptionalTrait(MapTrait.class) - .orElseThrow(() -> new IllegalStateException("SdkField of list type is missing List trait")); + MapTrait mapTrait = sdkField.getRequiredTrait(MapTrait.class); SdkField valueField = mapTrait.valueFieldInfo(); for (Map.Entry entry : map.entrySet()) { diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlPayloadMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlPayloadMarshaller.java index ebf247c23812..a81d8b02d6ea 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlPayloadMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlPayloadMarshaller.java @@ -81,9 +81,7 @@ public void marshall(List val, XmlMarshallerContext context, String paramName @Override public void marshall(List list, XmlMarshallerContext context, String paramName, SdkField> sdkField, ValueToStringConverter.ValueToString> converter) { - ListTrait listTrait = sdkField - .getOptionalTrait(ListTrait.class) - .orElseThrow(() -> new IllegalStateException(paramName + " member is missing ListTrait")); + ListTrait listTrait = sdkField.getRequiredTrait(ListTrait.class); if (!listTrait.isFlattened()) { context.xmlGenerator().startElement(paramName); @@ -125,8 +123,7 @@ protected boolean shouldEmit(List list, String paramName) { public void marshall(Map map, XmlMarshallerContext context, String paramName, SdkField> sdkField, ValueToStringConverter.ValueToString> converter) { - MapTrait mapTrait = sdkField.getOptionalTrait(MapTrait.class) - .orElseThrow(() -> new IllegalStateException(paramName + " member is missing MapTrait")); + MapTrait mapTrait = sdkField.getRequiredTrait(MapTrait.class); for (Map.Entry entry : map.entrySet()) { context.xmlGenerator().startElement("entry"); diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlProtocolMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlProtocolMarshaller.java index 0095e27e3860..f80dd3c7eafb 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlProtocolMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlProtocolMarshaller.java @@ -176,6 +176,7 @@ private static XmlMarshallerRegistry createMarshallerRegistry() { .headerMarshaller(MarshallingType.BOOLEAN, HeaderMarshaller.BOOLEAN) .headerMarshaller(MarshallingType.INSTANT, HeaderMarshaller.INSTANT) .headerMarshaller(MarshallingType.MAP, HeaderMarshaller.MAP) + .headerMarshaller(MarshallingType.LIST, HeaderMarshaller.LIST) .headerMarshaller(MarshallingType.NULL, XmlMarshaller.NULL) .queryParamMarshaller(MarshallingType.STRING, QueryParamMarshaller.STRING) diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/HeaderUnmarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/HeaderUnmarshaller.java index 08a2da0981aa..13f135057ae1 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/HeaderUnmarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/HeaderUnmarshaller.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.protocols.xml.internal.unmarshall; +import static java.util.stream.Collectors.toList; import static software.amazon.awssdk.utils.StringUtils.replacePrefixIgnoreCase; import static software.amazon.awssdk.utils.StringUtils.startsWithIgnoreCase; @@ -26,6 +27,7 @@ import software.amazon.awssdk.core.SdkField; import software.amazon.awssdk.protocols.core.StringToValueConverter; import software.amazon.awssdk.protocols.query.unmarshall.XmlElement; +import software.amazon.awssdk.utils.http.SdkHttpUtils; @SdkInternalApi public final class HeaderUnmarshaller { @@ -39,6 +41,7 @@ public final class HeaderUnmarshaller { public static final XmlUnmarshaller INSTANT = new SimpleHeaderUnmarshaller<>(XmlProtocolUnmarshaller.INSTANT_STRING_TO_VALUE); + // Only supports string value type public static final XmlUnmarshaller> MAP = ((context, content, field) -> { Map result = new HashMap<>(); context.response().headers().entrySet().stream() @@ -48,6 +51,11 @@ public final class HeaderUnmarshaller { return result; }); + // Only supports string value type + public static final XmlUnmarshaller> LIST = (context, content, field) -> { + return SdkHttpUtils.allMatchingHeaders(context.response().headers(), field.locationName()).collect(toList()); + }; + private HeaderUnmarshaller() { } diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java index d65dbacc708c..cbd4b081a559 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java @@ -150,6 +150,7 @@ private static XmlUnmarshallerRegistry createUnmarshallerRegistry() { .headerUnmarshaller(MarshallingType.INSTANT, HeaderUnmarshaller.INSTANT) .headerUnmarshaller(MarshallingType.FLOAT, HeaderUnmarshaller.FLOAT) .headerUnmarshaller(MarshallingType.MAP, HeaderUnmarshaller.MAP) + .headerUnmarshaller(MarshallingType.LIST, HeaderUnmarshaller.LIST) .payloadUnmarshaller(MarshallingType.STRING, XmlPayloadUnmarshaller.STRING) .payloadUnmarshaller(MarshallingType.INTEGER, XmlPayloadUnmarshaller.INTEGER) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java index e43d94f5a2da..161754efa1f7 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkField.java @@ -122,6 +122,23 @@ public Optional getOptionalTrait(Class clzz) { return Optional.ofNullable((T) traits.get(clzz)); } + /** + * Gets the trait of the specified class, or throw {@link IllegalStateException} if not available. + * + * @param clzz Trait class to get. + * @param Type of trait. + * @return Trait instance. + * @throws IllegalStateException if trait is not present. + */ + @SuppressWarnings("unchecked") + public T getRequiredTrait(Class clzz) throws IllegalStateException { + T trait = (T) traits.get(clzz); + if (trait == null) { + throw new IllegalStateException(memberName + " member is missing " + clzz.getSimpleName()); + } + return trait; + } + /** * Checks if a given {@link Trait} is present on the field. * diff --git a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/HeadersAssertion.java b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/HeadersAssertion.java index c4eb088fa845..07b3f7ec1ea6 100644 --- a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/HeadersAssertion.java +++ b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/HeadersAssertion.java @@ -17,6 +17,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import com.github.tomakehurst.wiremock.http.HttpHeaders; import com.github.tomakehurst.wiremock.verification.LoggedRequest; @@ -28,11 +29,11 @@ */ public class HeadersAssertion extends MarshallingAssertion { - private Map contains; + private Map> contains; private List doesNotContain; - public void setContains(Map contains) { + public void setContains(Map> contains) { this.contains = contains; } @@ -51,8 +52,11 @@ protected void doAssert(LoggedRequest actual) throws Exception { } private void assertHeadersContains(HttpHeaders actual) { - contains.entrySet().forEach(e -> { - assertEquals(e.getValue(), actual.getHeader(e.getKey()).firstValue()); + contains.forEach((expectedKey, expectedValues) -> { + assertTrue(String.format("Header '%s' was expected to be present. Actual headers: %s", expectedKey, actual), + actual.getHeader(expectedKey).isPresent()); + List actualValues = actual.getHeader(expectedKey).values(); + assertEquals(expectedValues, actualValues); }); } diff --git a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/MarshallingAssertion.java b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/MarshallingAssertion.java index 9ec645a77e6c..767e5e59cf83 100644 --- a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/MarshallingAssertion.java +++ b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/marshalling/MarshallingAssertion.java @@ -29,9 +29,11 @@ public abstract class MarshallingAssertion { * @throws AssertionError If any assertions fail */ public final void assertMatches(LoggedRequest actual) throws AssertionError { - // Catches the exception to play nicer with lambda's + // Wrap checked exceptions to play nicer with lambda's try { doAssert(actual); + } catch (Error | RuntimeException e) { + throw e; } catch (Exception e) { throw new RuntimeException(e); } diff --git a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/model/GivenResponse.java b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/model/GivenResponse.java index bc35f4050be7..5dfbd15e6a81 100644 --- a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/model/GivenResponse.java +++ b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/model/GivenResponse.java @@ -16,13 +16,14 @@ package software.amazon.awssdk.protocol.model; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.List; import java.util.Map; public class GivenResponse { @JsonProperty(value = "status_code") private Integer statusCode; - private Map headers; + private Map> headers; private String body; public Integer getStatusCode() { @@ -33,11 +34,11 @@ public void setStatusCode(Integer statusCode) { this.statusCode = statusCode; } - public Map getHeaders() { + public Map> getHeaders() { return headers; } - public void setHeaders(Map headers) { + public void setHeaders(Map> headers) { this.headers = headers; } 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 7dbd9846b0b4..4560184e88aa 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 @@ -99,7 +99,9 @@ private ResponseDefinitionBuilder toResponseBuilder(GivenResponse givenResponse) ResponseDefinitionBuilder responseBuilder = aResponse().withStatus(200); if (givenResponse.getHeaders() != null) { - givenResponse.getHeaders().forEach(responseBuilder::withHeader); + givenResponse.getHeaders().forEach((key, values) -> { + responseBuilder.withHeader(key, values.toArray(new String[0])); + }); } if (givenResponse.getStatusCode() != null) { responseBuilder.withStatus(givenResponse.getStatusCode()); diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json index 0b1fc0e681c2..d0d09d653709 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json @@ -144,6 +144,36 @@ "uri": "/2016-03-11/operationWithGreedyLabel/pathParamValue/foo/bar/baz" } } + }, + { + "description": "ListOfStrings in header is serialized as multi-valued header", + "given": { + "input": { + "StringMember": "singleValue", + "ListOfStringsMember": [ + "listValueOne", + "listValueTwo" + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "contains": { + "x-amz-string": "singleValue", + "x-amz-string-list": [ + "listValueOne", + "listValueTwo" + ] + } + } + } + } } // TODO This is a post process customization for API Gateway // { 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 940ae3e0bcb4..5a6197b2c625 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 @@ -161,5 +161,33 @@ "QueryParamOne": "Found QueryParamOne in the payload! Yay!" } } + }, + { + "description": "ListOfStrings in multi-valued header is unmarshalled correctly", + "given": { + "response": { + "status_code": 200, + "headers": { + "x-amz-string": "singleValue", + "x-amz-string-list": [ + "listValueOne", + "listValueTwo" + ] + } + } + }, + "when": { + "action": "unmarshall", + "operation": "MembersInHeaders" + }, + "then": { + "deserializedAs": { + "StringMember": "singleValue", + "ListOfStringsMember": [ + "listValueOne", + "listValueTwo" + ] + } + } } ] \ No newline at end of file diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json index 7fefb6cf7de1..76d7d94421d0 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json @@ -388,6 +388,36 @@ "uri": "/2016-03-11/operationWithGreedyLabel/pathParamValue//foo/bar/baz" } } + }, + { + "description": "ListOfStrings in header is serialized as multi-valued header", + "given": { + "input": { + "StringMember": "singleValue", + "ListOfStringsMember": [ + "listValueOne", + "listValueTwo" + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "contains": { + "x-amz-string": "singleValue", + "x-amz-string-list": [ + "listValueOne", + "listValueTwo" + ] + } + } + } + } } // TODO this is not possible, payloads can only be structures or blobs. Only S3 utilizes this // { diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-output.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-output.json index 72cc4a9081a9..7a36f628a32a 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-output.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-output.json @@ -334,5 +334,33 @@ } } } + }, + { + "description": "ListOfStrings in multi-valued header is unmarshalled correctly", + "given": { + "response": { + "status_code": 200, + "headers": { + "x-amz-string": "singleValue", + "x-amz-string-list": [ + "listValueOne", + "listValueTwo" + ] + } + } + }, + "when": { + "action": "unmarshall", + "operation": "MembersInHeaders" + }, + "then": { + "deserializedAs": { + "StringMember": "singleValue", + "ListOfStringsMember": [ + "listValueOne", + "listValueTwo" + ] + } + } } ] 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 72596a8d6e5c..6701ab2fbb17 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 @@ -505,6 +505,11 @@ "location":"header", "locationName":"x-amz-string" }, + "ListOfStringsMember":{ + "shape":"ListOfStrings", + "location":"header", + "locationName":"x-amz-string-list" + }, "BooleanMember":{ "shape":"Boolean", "location":"header", diff --git a/test/protocol-tests/src/main/resources/codegen-resources/restxml/service-2.json b/test/protocol-tests/src/main/resources/codegen-resources/restxml/service-2.json index cac67bf99cae..b95077656e46 100644 --- a/test/protocol-tests/src/main/resources/codegen-resources/restxml/service-2.json +++ b/test/protocol-tests/src/main/resources/codegen-resources/restxml/service-2.json @@ -268,6 +268,11 @@ "location":"header", "locationName":"x-amz-string" }, + "ListOfStringsMember":{ + "shape":"ListOfStrings", + "location":"header", + "locationName":"x-amz-string-list" + }, "BooleanMember":{ "shape":"Boolean", "location":"header", From 96eee61371cee987a7e098cdeae1cb93c6b857e7 Mon Sep 17 00:00:00 2001 From: Bennett Lynch Date: Mon, 12 Jul 2021 17:51:43 -0700 Subject: [PATCH 2/5] Add support for marshalling lists of strings in HTTP headers... ...for JSON/XML protocols. ## Motivation and Context We currently lack support for marshalling lists of strings (and enums) in HTTP headers. Other IDLs and modeling languages, e.g., Smithy, do support such bindings and services may come to expect such support: https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html#httpheader-trait ## Description * Add support for marshalling a list of strings to relevant JSON and XML marshalling classes * Consistent with the existing XML HeaderUnmarshaller map implementation, unmarshalling only supports string value types (this can be extended in the future if needed) * Marshalling implementations are changed to use appendHeader rather than putHeader * Add convenience method SdkField#getRequiredTrait(..) to support the common use case of ensuring a trait is present; update other existing use cases to utilize this method * Update relevant test classes to correctly recognize headers as a Map> * Only wrap marshalling assertion exceptions as runtime when needed, minimizing stack trace noise ## Testing New tests added to: * rest-json-input.json * rest-json-output.json * rest-xml-input.json * rest-xml-output.json --- .changes/next-release/feature-AWSSDKforJavav2-641dd1e.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-641dd1e.json diff --git a/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json b/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json new file mode 100644 index 000000000000..c241539f9ac7 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json @@ -0,0 +1,6 @@ +{ + "category": "AWS SDK for Java v2", + "contributor": "Bennett-Lynch", + "type": "feature", + "description": "Add support for marshalling lists of strings in HTTP headers" +} From 717c9553e8320b35203df647b9a0e7976170c7f6 Mon Sep 17 00:00:00 2001 From: Bennett Lynch Date: Tue, 13 Jul 2021 13:42:07 -0700 Subject: [PATCH 3/5] Update pull request template to place motivation before description Pull requests read more intuitively when they begin with the motivation before diving into a detailed description. --- .../protocols/json/internal/marshall/HeaderMarshaller.java | 4 +++- .../protocols/xml/internal/marshall/HeaderMarshaller.java | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java index 007a1fc0d0fd..caf9445671a4 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java @@ -15,6 +15,8 @@ package software.amazon.awssdk.protocols.json.internal.marshall; +import static software.amazon.awssdk.utils.CollectionUtils.isNullOrEmpty; + import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.List; @@ -49,7 +51,7 @@ public final class HeaderMarshaller { = new SimpleHeaderMarshaller<>(JsonProtocolMarshaller.INSTANT_VALUE_TO_STRING); public static final JsonMarshaller> LIST = (list, context, paramName, sdkField) -> { - if (list.isEmpty()) { + if (isNullOrEmpty(list)) { return; } SdkField memberFieldInfo = sdkField.getRequiredTrait(ListTrait.class).memberFieldInfo(); diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java index 57ab9668ddc4..272ca80b3b11 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java @@ -15,6 +15,8 @@ package software.amazon.awssdk.protocols.xml.internal.marshall; +import static software.amazon.awssdk.utils.CollectionUtils.isNullOrEmpty; + import java.time.Instant; import java.util.List; import java.util.Map; @@ -64,7 +66,7 @@ public void marshall(Map map, XmlMarshallerContext context, String pa @Override protected boolean shouldEmit(Map map) { - return map != null && !map.isEmpty(); + return !isNullOrEmpty(map); } }; @@ -83,7 +85,7 @@ public void marshall(List list, XmlMarshallerContext context, String paramNam @Override protected boolean shouldEmit(List list) { - return list != null && !list.isEmpty(); + return !isNullOrEmpty(list); } }; From 6ac3549952296b9c46ea0cfbe9487a68aa813594 Mon Sep 17 00:00:00 2001 From: Bennett Lynch Date: Tue, 13 Jul 2021 13:54:37 -0700 Subject: [PATCH 4/5] Update pull request template to place motivation before description Pull requests read more intuitively when they begin with the motivation before diving into a detailed description. --- .changes/next-release/feature-AWSSDKforJavav2-641dd1e.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json b/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json index c241539f9ac7..5a3fde415aef 100644 --- a/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json +++ b/.changes/next-release/feature-AWSSDKforJavav2-641dd1e.json @@ -1,6 +1,6 @@ { "category": "AWS SDK for Java v2", - "contributor": "Bennett-Lynch", + "contributor": "", "type": "feature", "description": "Add support for marshalling lists of strings in HTTP headers" } From 1b24486043d4867700a25905de21fa8a5c3ede76 Mon Sep 17 00:00:00 2001 From: Bennett Lynch Date: Wed, 14 Jul 2021 12:58:12 -0700 Subject: [PATCH 5/5] Update pull request template to place motivation before description Pull requests read more intuitively when they begin with the motivation before diving into a detailed description. --- .../internal/marshall/HeaderMarshaller.java | 2 + .../internal/marshall/HeaderMarshaller.java | 2 + .../suites/cases/rest-json-input.json | 89 +++++++++++++++++++ .../protocol/suites/cases/rest-xml-input.json | 89 +++++++++++++++++++ 4 files changed, 182 insertions(+) diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java index caf9445671a4..69d2e75ea87b 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/HeaderMarshaller.java @@ -51,6 +51,8 @@ public final class HeaderMarshaller { = new SimpleHeaderMarshaller<>(JsonProtocolMarshaller.INSTANT_VALUE_TO_STRING); public static final JsonMarshaller> LIST = (list, context, paramName, sdkField) -> { + // Null or empty lists cannot be meaningfully (or safely) represented in an HTTP header message since header-fields must + // typically have a non-empty field-value. https://datatracker.ietf.org/doc/html/rfc7230#section-3.2 if (isNullOrEmpty(list)) { return; } diff --git a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java index 272ca80b3b11..102d4239d3c8 100644 --- a/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java +++ b/core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/HeaderMarshaller.java @@ -85,6 +85,8 @@ public void marshall(List list, XmlMarshallerContext context, String paramNam @Override protected boolean shouldEmit(List list) { + // Null or empty lists cannot be meaningfully (or safely) represented in an HTTP header message since header-fields + // must typically have a non-empty field-value. https://datatracker.ietf.org/doc/html/rfc7230#section-3.2 return !isNullOrEmpty(list); } }; diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json index d0d09d653709..9bd43b7b9e56 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json @@ -174,6 +174,95 @@ } } } + }, + { + "description": "Null string header member is not serialized", + "given": { + "input": { + "StringMember": null + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string" ] + } + } + } + }, + { + "description": "Null list header member is not serialized", + "given": { + "input": { + "ListOfStringsMember": null + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string-list" ] + } + } + } + }, + { + "description": "List header member with only null value is not serialized", + "given": { + "input": { + "ListOfStringsMember": [ + null + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string-list" ] + } + } + } + }, + { + "description": "List header member's null elements are not serialized", + "given": { + "input": { + "ListOfStringsMember": [ + "listValueOne", + null + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "contains": { + "x-amz-string-list": [ + "listValueOne" + ] + } + } + } + } } // TODO This is a post process customization for API Gateway // { diff --git a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json index 76d7d94421d0..69eceade6c46 100644 --- a/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json +++ b/test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json @@ -418,6 +418,95 @@ } } } + }, + { + "description": "Null string header member is not serialized", + "given": { + "input": { + "StringMember": null + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string" ] + } + } + } + }, + { + "description": "Null list header member is not serialized", + "given": { + "input": { + "ListOfStringsMember": null + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string-list" ] + } + } + } + }, + { + "description": "List header member with only null value is not serialized", + "given": { + "input": { + "ListOfStringsMember": [ + null + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "doesNotContain": [ "x-amz-string-list" ] + } + } + } + }, + { + "description": "List header member's null elements are not serialized", + "given": { + "input": { + "ListOfStringsMember": [ + "listValueOne", + null + ] + } + }, + "when": { + "action": "marshall", + "operation": "MembersInHeaders" + }, + "then": { + "serializedAs": { + "uri": "/2016-03-11/membersInHeaders", + "headers": { + "contains": { + "x-amz-string-list": [ + "listValueOne" + ] + } + } + } + } } // TODO this is not possible, payloads can only be structures or blobs. Only S3 utilizes this // {