Skip to content

Commit 64a0a67

Browse files
zoewanggzoewangg
authored
Refactoring xml unmarshalling response handler to handle SdkPojo (#2854)
1 parent 9a52cf5 commit 64a0a67

File tree

4 files changed

+238
-47
lines changed

4 files changed

+238
-47
lines changed

‎core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/AwsXmlProtocolFactory.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import software.amazon.awssdk.core.internal.http.CombinedResponseHandler;
3535
import software.amazon.awssdk.core.metrics.CoreMetric;
3636
import software.amazon.awssdk.http.SdkHttpFullRequest;
37+
import software.amazon.awssdk.http.SdkHttpFullResponse;
3738
import software.amazon.awssdk.protocols.core.ExceptionMetadata;
3839
import software.amazon.awssdk.protocols.core.OperationInfo;
3940
import software.amazon.awssdk.protocols.core.OperationMetadataAttribute;
@@ -47,6 +48,7 @@
4748
import software.amazon.awssdk.protocols.xml.internal.unmarshall.AwsXmlResponseTransformer;
4849
import software.amazon.awssdk.protocols.xml.internal.unmarshall.AwsXmlUnmarshallingContext;
4950
import software.amazon.awssdk.protocols.xml.internal.unmarshall.XmlProtocolUnmarshaller;
51+
import software.amazon.awssdk.protocols.xml.internal.unmarshall.XmlResponseHandler;
5052

5153
/**
5254
* Factory to generate the various protocol handlers and generators to be used for
@@ -104,10 +106,18 @@ public ProtocolMarshaller<SdkHttpFullRequest> createProtocolMarshaller(Operation
104106
.build();
105107
}
106108

107-
public <T extends AwsResponse> HttpResponseHandler<T> createResponseHandler(Supplier<SdkPojo> pojoSupplier,
108-
XmlOperationMetadata staxOperationMetadata) {
109-
return timeUnmarshalling(new AwsXmlResponseHandler<>(XML_PROTOCOL_UNMARSHALLER, r -> pojoSupplier.get(),
110-
staxOperationMetadata.isHasStreamingSuccessResponse()));
109+
public <T extends SdkPojo> HttpResponseHandler<T> createResponseHandler(Supplier<SdkPojo> pojoSupplier,
110+
XmlOperationMetadata staxOperationMetadata) {
111+
return createResponseHandler(r -> pojoSupplier.get(), staxOperationMetadata);
112+
}
113+
114+
public <T extends SdkPojo> HttpResponseHandler<T> createResponseHandler(Function<SdkHttpFullResponse, SdkPojo> pojoSupplier,
115+
XmlOperationMetadata staxOperationMetadata) {
116+
return timeUnmarshalling(
117+
new AwsXmlResponseHandler<>(
118+
new XmlResponseHandler<>(
119+
XML_PROTOCOL_UNMARSHALLER, pojoSupplier,
120+
staxOperationMetadata.isHasStreamingSuccessResponse())));
111121
}
112122

113123
protected <T extends AwsResponse> Function<AwsXmlUnmarshallingContext, T> createResponseTransformer(

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

Lines changed: 12 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,73 +17,42 @@
1717

1818
import static software.amazon.awssdk.awscore.util.AwsHeader.AWS_REQUEST_ID;
1919

20-
import java.io.IOException;
2120
import java.util.HashMap;
2221
import java.util.Map;
23-
import java.util.function.Function;
2422
import software.amazon.awssdk.annotations.SdkInternalApi;
2523
import software.amazon.awssdk.awscore.AwsResponse;
2624
import software.amazon.awssdk.awscore.AwsResponseMetadata;
2725
import software.amazon.awssdk.awscore.DefaultAwsResponseMetadata;
28-
import software.amazon.awssdk.core.SdkPojo;
29-
import software.amazon.awssdk.core.SdkStandardLogger;
3026
import software.amazon.awssdk.core.http.HttpResponseHandler;
3127
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
3228
import software.amazon.awssdk.http.SdkHttpFullResponse;
3329
import software.amazon.awssdk.http.SdkHttpResponse;
34-
import software.amazon.awssdk.utils.Logger;
3530
import software.amazon.awssdk.utils.http.SdkHttpUtils;
3631

3732
/**
38-
* Response handler for REST-XML services (Cloudfront, Route53, and S3).
33+
* Response handler that adds {@link AwsResponseMetadata} to the response.
3934
*
4035
* @param <T> Indicates the type being unmarshalled by this response handler.
4136
*/
4237
@SdkInternalApi
43-
public final class AwsXmlResponseHandler<T extends AwsResponse> implements HttpResponseHandler<T> {
38+
public final class AwsXmlResponseHandler<T> implements HttpResponseHandler<T> {
4439

45-
private static final Logger log = Logger.loggerFor(AwsXmlResponseHandler.class);
40+
private final HttpResponseHandler<T> delegate;
4641

47-
private final XmlProtocolUnmarshaller unmarshaller;
48-
private final Function<SdkHttpFullResponse, SdkPojo> pojoSupplier;
49-
private final boolean needsConnectionLeftOpen;
50-
51-
public AwsXmlResponseHandler(XmlProtocolUnmarshaller unmarshaller,
52-
Function<SdkHttpFullResponse, SdkPojo> pojoSupplier,
53-
boolean needsConnectionLeftOpen) {
54-
this.unmarshaller = unmarshaller;
55-
this.pojoSupplier = pojoSupplier;
56-
this.needsConnectionLeftOpen = needsConnectionLeftOpen;
42+
public AwsXmlResponseHandler(HttpResponseHandler<T> responseHandler) {
43+
this.delegate = responseHandler;
5744
}
5845

5946
@Override
6047
public T handle(SdkHttpFullResponse response, ExecutionAttributes executionAttributes) throws Exception {
61-
try {
62-
return unmarshallResponse(response);
63-
} finally {
64-
if (!needsConnectionLeftOpen) {
65-
closeStream(response);
66-
}
67-
}
68-
}
48+
T result = delegate.handle(response, executionAttributes);
6949

70-
private void closeStream(SdkHttpFullResponse response) {
71-
response.content().ifPresent(i -> {
72-
try {
73-
i.close();
74-
} catch (IOException e) {
75-
log.warn(() -> "Error closing HTTP content.", e);
76-
}
77-
});
78-
}
50+
if (result instanceof AwsResponse) {
51+
AwsResponseMetadata responseMetadata = generateResponseMetadata(response);
52+
return (T) ((AwsResponse) result).toBuilder().responseMetadata(responseMetadata).build();
53+
}
7954

80-
@SuppressWarnings("unchecked")
81-
private T unmarshallResponse(SdkHttpFullResponse response) throws Exception {
82-
SdkStandardLogger.REQUEST_LOGGER.trace(() -> "Parsing service response XML.");
83-
T result = unmarshaller.unmarshall(pojoSupplier.apply(response), response);
84-
SdkStandardLogger.REQUEST_LOGGER.trace(() -> "Done parsing service response.");
85-
AwsResponseMetadata responseMetadata = generateResponseMetadata(response);
86-
return (T) result.toBuilder().responseMetadata(responseMetadata).build();
55+
return result;
8756
}
8857

8958
/**
@@ -101,6 +70,6 @@ private AwsResponseMetadata generateResponseMetadata(SdkHttpResponse response) {
10170

10271
@Override
10372
public boolean needsConnectionLeftOpen() {
104-
return needsConnectionLeftOpen;
73+
return delegate.needsConnectionLeftOpen();
10574
}
10675
}

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.protocols.xml.internal.unmarshall;
17+
18+
import java.io.IOException;
19+
import java.util.function.Function;
20+
import software.amazon.awssdk.annotations.SdkInternalApi;
21+
import software.amazon.awssdk.core.SdkPojo;
22+
import software.amazon.awssdk.core.SdkStandardLogger;
23+
import software.amazon.awssdk.core.http.HttpResponseHandler;
24+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
25+
import software.amazon.awssdk.http.SdkHttpFullResponse;
26+
import software.amazon.awssdk.utils.Logger;
27+
28+
/**
29+
* Response handler for REST-XML services (Cloudfront, Route53, and S3).
30+
*
31+
* @param <T> Indicates the type being unmarshalled by this response handler.
32+
*/
33+
@SdkInternalApi
34+
public final class XmlResponseHandler<T extends SdkPojo> implements HttpResponseHandler<T> {
35+
36+
private static final Logger log = Logger.loggerFor(XmlResponseHandler.class);
37+
38+
private final XmlProtocolUnmarshaller unmarshaller;
39+
private final Function<SdkHttpFullResponse, SdkPojo> pojoSupplier;
40+
private final boolean needsConnectionLeftOpen;
41+
42+
public XmlResponseHandler(XmlProtocolUnmarshaller unmarshaller,
43+
Function<SdkHttpFullResponse, SdkPojo> pojoSupplier,
44+
boolean needsConnectionLeftOpen) {
45+
this.unmarshaller = unmarshaller;
46+
this.pojoSupplier = pojoSupplier;
47+
this.needsConnectionLeftOpen = needsConnectionLeftOpen;
48+
}
49+
50+
@Override
51+
public T handle(SdkHttpFullResponse response, ExecutionAttributes executionAttributes) throws Exception {
52+
try {
53+
return unmarshallResponse(response);
54+
} finally {
55+
if (!needsConnectionLeftOpen) {
56+
closeStream(response);
57+
}
58+
}
59+
}
60+
61+
private void closeStream(SdkHttpFullResponse response) {
62+
response.content().ifPresent(i -> {
63+
try {
64+
i.close();
65+
} catch (IOException e) {
66+
log.warn(() -> "Error closing HTTP content.", e);
67+
}
68+
});
69+
}
70+
71+
@SuppressWarnings("unchecked")
72+
private T unmarshallResponse(SdkHttpFullResponse response) throws Exception {
73+
SdkStandardLogger.REQUEST_LOGGER.trace(() -> "Parsing service response XML.");
74+
T result = unmarshaller.unmarshall(pojoSupplier.apply(response), response);
75+
SdkStandardLogger.REQUEST_LOGGER.trace(() -> "Done parsing service response.");
76+
return result;
77+
}
78+
79+
@Override
80+
public boolean needsConnectionLeftOpen() {
81+
return needsConnectionLeftOpen;
82+
}
83+
}

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

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.protocols.xml.internal.unmarshall;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.awscore.util.AwsHeader.AWS_REQUEST_ID;
20+
21+
import java.util.Collections;
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.Optional;
25+
import org.junit.Test;
26+
import org.mockito.Mockito;
27+
import software.amazon.awssdk.awscore.AwsResponse;
28+
import software.amazon.awssdk.core.SdkField;
29+
import software.amazon.awssdk.core.SdkPojo;
30+
import software.amazon.awssdk.core.http.HttpResponseHandler;
31+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
32+
import software.amazon.awssdk.http.AbortableInputStream;
33+
import software.amazon.awssdk.http.SdkHttpFullResponse;
34+
import software.amazon.awssdk.utils.ImmutableMap;
35+
36+
public class AwsXmlResponseHandlerTest {
37+
38+
@Test
39+
public void handleResponse_awsResponse_shouldAddResponseMetadata() throws Exception {
40+
HttpResponseHandler<FakeResponse> delegate = Mockito.mock(HttpResponseHandler.class);
41+
AwsXmlResponseHandler<FakeResponse> responseHandler = new AwsXmlResponseHandler<>(delegate);
42+
43+
SdkHttpFullResponse response = new TestSdkHttpFullResponse();
44+
ExecutionAttributes executionAttributes = new ExecutionAttributes();
45+
FakeResponse fakeResponse = FakeResponse.builder().build();
46+
47+
Mockito.when(delegate.handle(response, executionAttributes)).thenReturn(fakeResponse);
48+
49+
assertThat(responseHandler.handle(response, executionAttributes)
50+
.responseMetadata().requestId()).isEqualTo("1234");
51+
}
52+
53+
@Test
54+
public void handleResponse_nonAwsResponse_shouldReturnDirectly() throws Exception {
55+
HttpResponseHandler<SdkPojo> delegate = Mockito.mock(HttpResponseHandler.class);
56+
AwsXmlResponseHandler<SdkPojo> responseHandler = new AwsXmlResponseHandler<>(delegate);
57+
58+
SdkHttpFullResponse response = new TestSdkHttpFullResponse();
59+
ExecutionAttributes executionAttributes = new ExecutionAttributes();
60+
FakeSdkPojo fakeResponse = new FakeSdkPojo();
61+
62+
Mockito.when(delegate.handle(response, executionAttributes)).thenReturn(fakeResponse);
63+
64+
assertThat(responseHandler.handle(response, executionAttributes)).isEqualTo(fakeResponse);
65+
}
66+
67+
private static final class FakeSdkPojo implements SdkPojo {
68+
69+
@Override
70+
public List<SdkField<?>> sdkFields() {
71+
return Collections.emptyList();
72+
}
73+
}
74+
75+
private static final class FakeResponse extends AwsResponse {
76+
private FakeResponse(Builder builder) {
77+
super(builder);
78+
}
79+
80+
@Override
81+
public Builder toBuilder() {
82+
return new Builder();
83+
}
84+
85+
@Override
86+
public List<SdkField<?>> sdkFields() {
87+
return Collections.emptyList();
88+
}
89+
90+
public static Builder builder() {
91+
return new Builder();
92+
}
93+
94+
private static final class Builder extends AwsResponse.BuilderImpl {
95+
96+
@Override
97+
public FakeResponse build() {
98+
return new FakeResponse(this);
99+
}
100+
}
101+
}
102+
103+
private static class TestSdkHttpFullResponse implements SdkHttpFullResponse {
104+
@Override
105+
public Builder toBuilder() {
106+
return null;
107+
}
108+
109+
@Override
110+
public Optional<AbortableInputStream> content() {
111+
return Optional.empty();
112+
}
113+
114+
@Override
115+
public Optional<String> statusText() {
116+
return Optional.empty();
117+
}
118+
119+
@Override
120+
public int statusCode() {
121+
return 0;
122+
}
123+
124+
@Override
125+
public Map<String, List<String>> headers() {
126+
return ImmutableMap.of(AWS_REQUEST_ID, Collections.singletonList("1234"));
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)