Skip to content

Commit bce130d

Browse files
dagnirdagnir
authored
Merge branch 'master' into additional-retry-codes
2 parents 2971134 + c44951d commit bce130d

File tree

7 files changed

+108
-26
lines changed

7 files changed

+108
-26
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-00b4b93.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "bugfix",
4+
"description": "Fix unmarshalling for models with xml attributes. See [#1488](https://github.com/aws/aws-sdk-java-v2/issues/1488)."
5+
}

‎core/protocols/aws-query-protocol/src/main/java/software/amazon/awssdk/protocols/query/unmarshall/XmlDomParser.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616
package software.amazon.awssdk.protocols.query.unmarshall;
1717

1818
import java.io.InputStream;
19+
import java.util.HashMap;
20+
import java.util.Iterator;
21+
import java.util.Map;
1922
import javax.xml.stream.XMLEventReader;
2023
import javax.xml.stream.XMLInputFactory;
2124
import javax.xml.stream.XMLStreamException;
25+
import javax.xml.stream.events.Attribute;
2226
import javax.xml.stream.events.StartElement;
2327
import javax.xml.stream.events.XMLEvent;
2428
import software.amazon.awssdk.annotations.SdkProtectedApi;
@@ -59,6 +63,11 @@ public static XmlElement parse(InputStream inputStream) {
5963
private static XmlElement parseElement(StartElement startElement, XMLEventReader reader) throws XMLStreamException {
6064
XmlElement.Builder elementBuilder = XmlElement.builder()
6165
.elementName(startElement.getName().getLocalPart());
66+
67+
if (startElement.getAttributes().hasNext()) {
68+
parseAttributes(startElement, elementBuilder);
69+
}
70+
6271
XMLEvent nextEvent;
6372
do {
6473
nextEvent = reader.nextEvent();
@@ -71,6 +80,21 @@ private static XmlElement parseElement(StartElement startElement, XMLEventReader
7180
return elementBuilder.build();
7281
}
7382

83+
/**
84+
* Parse the attributes of the element.
85+
*/
86+
@SuppressWarnings("unchecked")
87+
private static void parseAttributes(StartElement startElement, XmlElement.Builder elementBuilder) {
88+
Iterator<Attribute> iterator = startElement.getAttributes();
89+
Map<String, String> attributes = new HashMap<>();
90+
iterator.forEachRemaining(a -> {
91+
String key = a.getName().getPrefix() + ":" + a.getName().getLocalPart();
92+
attributes.put(key, a.getValue());
93+
});
94+
95+
elementBuilder.attributes(attributes);
96+
}
97+
7498
/**
7599
* Reads all characters until the next end element event.
76100
*

‎core/protocols/aws-query-protocol/src/main/java/software/amazon/awssdk/protocols/query/unmarshall/XmlElement.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Collections;
2020
import java.util.HashMap;
2121
import java.util.List;
22+
import java.util.Map;
2223
import java.util.Optional;
2324
import software.amazon.awssdk.annotations.SdkProtectedApi;
2425
import software.amazon.awssdk.core.exception.SdkClientException;
@@ -35,12 +36,14 @@ public final class XmlElement {
3536
private final HashMap<String, List<XmlElement>> childrenByElement;
3637
private final List<XmlElement> children;
3738
private final String textContent;
39+
private final Map<String, String> attributes;
3840

3941
private XmlElement(Builder builder) {
4042
this.elementName = builder.elementName;
4143
this.childrenByElement = new HashMap<>(builder.childrenByElement);
4244
this.children = Collections.unmodifiableList(new ArrayList<>(builder.children));
4345
this.textContent = builder.textContent;
46+
this.attributes = Collections.unmodifiableMap(new HashMap<>(builder.attributes));
4447
}
4548

4649
/**
@@ -109,6 +112,20 @@ public String textContent() {
109112
return textContent;
110113
}
111114

115+
/**
116+
* Retrieves an optional attribute by attribute name.
117+
*/
118+
public Optional<String> getOptionalAttributeByName(String attribute) {
119+
return Optional.ofNullable(attributes.get(attribute));
120+
}
121+
122+
/**
123+
* Retrieves the attributes associated with the element
124+
*/
125+
public Map<String, String> attributes() {
126+
return attributes;
127+
}
128+
112129
/**
113130
* @return New {@link Builder} instance.
114131
*/
@@ -129,9 +146,10 @@ public static XmlElement empty() {
129146
public static final class Builder {
130147

131148
private String elementName;
132-
private final HashMap<String, List<XmlElement>> childrenByElement = new HashMap<>();
149+
private final Map<String, List<XmlElement>> childrenByElement = new HashMap<>();
133150
private final List<XmlElement> children = new ArrayList<>();
134151
private String textContent = "";
152+
private Map<String, String> attributes = new HashMap<>();
135153

136154
private Builder() {
137155
}
@@ -153,6 +171,11 @@ public Builder textContent(String textContent) {
153171
return this;
154172
}
155173

174+
public Builder attributes(Map<String, String> attributes) {
175+
this.attributes = attributes;
176+
return this;
177+
}
178+
156179
public XmlElement build() {
157180
return new XmlElement(this);
158181
}

‎core/protocols/aws-query-protocol/src/test/java/software/amazon/awssdk/protocols/query/XmlDomParserTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,22 @@ public void simpleXmlDocument_ParsedCorrectly() {
3333
.isEqualTo("42");
3434
}
3535

36+
@Test
37+
public void xmlWithAttributes_ParsedCorrectly() {
38+
String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
39+
+ "<Struct xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:type=\"foo\" xsi:nil=\"bar\">"
40+
+ " <stringMember>stringVal</stringMember>"
41+
+ "</Struct>";
42+
XmlElement element = XmlDomParser.parse(new StringInputStream(xml));
43+
assertThat(element.elementName()).isEqualTo("Struct");
44+
assertThat(element.children()).hasSize(1);
45+
assertThat(element.getElementsByName("stringMember"))
46+
.hasSize(1);
47+
assertThat(element.attributes()).hasSize(2);
48+
assertThat(element.getOptionalAttributeByName("xsi:type").get()).isEqualTo("foo");
49+
assertThat(element.getOptionalAttributeByName("xsi:nil").get()).isEqualTo("bar");
50+
}
51+
3652
@Test
3753
public void multipleElementsWithSameName_ParsedCorrectly() {
3854
String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"

‎core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import software.amazon.awssdk.core.protocol.MarshallingType;
3030
import software.amazon.awssdk.core.traits.PayloadTrait;
3131
import software.amazon.awssdk.core.traits.TimestampFormatTrait;
32+
import software.amazon.awssdk.core.traits.XmlAttributeTrait;
3233
import software.amazon.awssdk.http.SdkHttpFullResponse;
3334
import software.amazon.awssdk.protocols.core.StringToInstant;
3435
import software.amazon.awssdk.protocols.core.StringToValueConverter;
@@ -79,12 +80,18 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
7980
XmlUnmarshaller<Object> unmarshaller = REGISTRY.getUnmarshaller(field.location(), field.marshallingType());
8081

8182
if (root != null && field.location() == MarshallLocation.PAYLOAD) {
82-
List<XmlElement> element = isExplicitPayloadMember(field) ?
83-
singletonList(root) :
84-
root.getElementsByName(field.unmarshallLocationName());
85-
if (!CollectionUtils.isNullOrEmpty(element)) {
86-
Object unmarshalled = unmarshaller.unmarshall(context, element, (SdkField<Object>) field);
87-
field.set(sdkPojo, unmarshalled);
83+
if (isAttribute(field)) {
84+
root.getOptionalAttributeByName(field.unmarshallLocationName())
85+
.ifPresent(e -> field.set(sdkPojo, e));
86+
} else {
87+
List<XmlElement> element = isExplicitPayloadMember(field) ?
88+
singletonList(root) :
89+
root.getElementsByName(field.unmarshallLocationName());
90+
91+
if (!CollectionUtils.isNullOrEmpty(element)) {
92+
Object unmarshalled = unmarshaller.unmarshall(context, element, (SdkField<Object>) field);
93+
field.set(sdkPojo, unmarshalled);
94+
}
8895
}
8996
} else {
9097
Object unmarshalled = unmarshaller.unmarshall(context, null, (SdkField<Object>) field);
@@ -94,6 +101,10 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
94101
return (SdkPojo) ((Buildable) sdkPojo).build();
95102
}
96103

104+
private boolean isAttribute(SdkField<?> field) {
105+
return field.containsTrait(XmlAttributeTrait.class);
106+
}
107+
97108
private boolean isExplicitPayloadMember(SdkField<?> field) {
98109
return field.containsTrait(PayloadTrait.class);
99110
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/AclIntegrationTest.java

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,19 @@
1616
package software.amazon.awssdk.services.s3;
1717

1818

19+
import static org.assertj.core.api.Assertions.assertThat;
1920
import static org.junit.Assert.assertNotNull;
2021
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
2122

22-
import java.io.File;
23-
import java.util.List;
2423
import java.util.function.Consumer;
25-
import java.util.stream.Collectors;
2624
import org.junit.AfterClass;
2725
import org.junit.BeforeClass;
2826
import org.junit.Test;
2927
import software.amazon.awssdk.core.sync.RequestBody;
3028
import software.amazon.awssdk.services.s3.model.AccessControlPolicy;
3129
import software.amazon.awssdk.services.s3.model.GetBucketAclResponse;
3230
import software.amazon.awssdk.services.s3.model.GetObjectAclResponse;
33-
import software.amazon.awssdk.services.s3.model.Grant;
3431
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
35-
import software.amazon.awssdk.services.s3.model.Type;
3632

3733
public class AclIntegrationTest extends S3IntegrationTestBase {
3834

@@ -55,10 +51,12 @@ public static void deleteAllBuckets() {
5551
}
5652

5753
@Test
58-
public void putObjectAcl() {
54+
public void putGetObjectAcl() {
5955
GetObjectAclResponse objectAcl = s3.getObjectAcl(b -> b.bucket(BUCKET).key(KEY));
56+
GetObjectAclResponse objectAclAsyncResponse = s3Async.getObjectAcl(b -> b.bucket(BUCKET).key(KEY)).join();
57+
assertThat(objectAcl.equalsBySdkFields(objectAclAsyncResponse)).isTrue();
6058
Consumer<AccessControlPolicy.Builder> aclBuilder = a -> a.owner(objectAcl.owner())
61-
.grants(addGranteeType(objectAcl.grants()));
59+
.grants(objectAcl.grants());
6260

6361

6462
assertNotNull(s3.putObjectAcl(b -> b.bucket(BUCKET)
@@ -71,24 +69,16 @@ public void putObjectAcl() {
7169
}
7270

7371
@Test
74-
public void putBucketAcl() {
72+
public void putGetBucketAcl() {
7573
GetBucketAclResponse bucketAcl = s3.getBucketAcl(b -> b.bucket(BUCKET));
74+
GetBucketAclResponse bucketAclAsyncResponse = s3Async.getBucketAcl(b -> b.bucket(BUCKET)).join();
75+
assertThat(bucketAcl.equalsBySdkFields(bucketAclAsyncResponse)).isTrue();
7676
Consumer<AccessControlPolicy.Builder> aclBuilder = a -> a.owner(bucketAcl.owner())
77-
.grants(addGranteeType(bucketAcl.grants()));
77+
.grants(bucketAcl.grants());
7878
assertNotNull(s3.putBucketAcl(b -> b.bucket(BUCKET)
7979
.accessControlPolicy(aclBuilder)));
8080
assertNotNull(s3Async.putBucketAcl(b -> b.bucket(BUCKET)
8181
.accessControlPolicy(aclBuilder)).join());
8282

8383
}
84-
85-
//TODO: remove this once we fix the unmarshalling issue for Grant#type
86-
private List<Grant> addGranteeType(List<Grant> grants) {
87-
return grants.stream().map(g -> g.toBuilder().grantee(g.grantee()
88-
.toBuilder()
89-
.type(Type.CANONICAL_USER)
90-
.build()).build())
91-
.collect(Collectors.toList());
92-
}
93-
9484
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/AclTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
1919
import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor;
2020
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
2122
import static com.github.tomakehurst.wiremock.client.WireMock.put;
2223
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
2324
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
25+
import static org.assertj.core.api.Assertions.assertThat;
2426

2527
import com.github.tomakehurst.wiremock.junit.WireMockRule;
2628
import com.github.tomakehurst.wiremock.matching.ContainsPattern;
@@ -33,6 +35,7 @@
3335
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
3436
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
3537
import software.amazon.awssdk.regions.Region;
38+
import software.amazon.awssdk.services.s3.model.GetBucketAclResponse;
3639
import software.amazon.awssdk.services.s3.model.Grant;
3740
import software.amazon.awssdk.services.s3.model.Permission;
3841
import software.amazon.awssdk.services.s3.model.PutBucketAclRequest;
@@ -71,6 +74,16 @@ public void putBucketAcl_marshalling() {
7174
verify(anyRequestedFor(anyUrl()).withRequestBody(new ContainsPattern(MOCK_ACL_RESPONSE)));
7275
}
7376

77+
@Test
78+
public void getBucketAcl_shouldUnmarshallCorrectly() {
79+
stubFor(get(anyUrl())
80+
.willReturn(aResponse().withBody(MOCK_ACL_RESPONSE).withStatus(200)));
81+
82+
GetBucketAclResponse bucketAcl = s3Client.getBucketAcl(b -> b.bucket("test"));
83+
assertThat(bucketAcl.owner()).isEqualTo(request().accessControlPolicy().owner());
84+
assertThat(bucketAcl.grants()).isEqualTo(request().accessControlPolicy().grants());
85+
}
86+
7487
private PutBucketAclRequest request() {
7588

7689
List<Grant> grants = new ArrayList<>();

0 commit comments

Comments
 (0)