Skip to content

Commit 09cde79

Browse files
authored
Merge pull request #1662 from aws/millem/maybe-fix-lemmy-i-hope
Fixed issue where handler.prepare() was being invoked after credential resolution.
2 parents 16b293b + 353de46 commit 09cde79

File tree

3 files changed

+44
-42
lines changed

3 files changed

+44
-42
lines changed

core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,9 @@ private static <InputT extends SdkRequest, OutputT extends SdkResponse> Executio
176176
ExecutionAttributes clientOverrideExecutionAttributes,
177177
ExecutionAttributes requestOverrideExecutionAttributes) {
178178

179-
if (clientOverrideExecutionAttributes != null) {
180-
executionAttributes = clientOverrideExecutionAttributes.merge(executionAttributes);
181-
}
182179

183-
if (requestOverrideExecutionAttributes != null) {
184-
executionAttributes = requestOverrideExecutionAttributes.merge(executionAttributes);
185-
}
180+
executionAttributes.putAbsentAttributes(requestOverrideExecutionAttributes);
181+
executionAttributes.putAbsentAttributes(clientOverrideExecutionAttributes);
186182

187183
return executionAttributes;
188184
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/ExecutionAttributes.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ public ExecutionAttributes merge(ExecutionAttributes lowerPrecedenceExecutionAtt
7676
return new ExecutionAttributes(copiedAttributes);
7777
}
7878

79+
/**
80+
* Add the provided attributes to this attribute, if the provided attribute does not exist.
81+
*/
82+
public void putAbsentAttributes(ExecutionAttributes lowerPrecedenceExecutionAttributes) {
83+
if (lowerPrecedenceExecutionAttributes != null) {
84+
lowerPrecedenceExecutionAttributes.getAttributes().forEach(attributes::putIfAbsent);
85+
}
86+
}
87+
7988
/**
8089
* Set the provided attribute in this collection of attributes if it does not already exist in the collection.
8190
*/

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseAsyncClientHandler.java

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import software.amazon.awssdk.core.http.Crc32Validation;
3535
import software.amazon.awssdk.core.http.ExecutionContext;
3636
import software.amazon.awssdk.core.http.HttpResponseHandler;
37+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
3738
import software.amazon.awssdk.core.interceptor.InterceptorContext;
3839
import software.amazon.awssdk.core.internal.InternalCoreExecutionAttribute;
3940
import software.amazon.awssdk.core.internal.http.AmazonAsyncHttpClient;
@@ -88,51 +89,47 @@ public <InputT extends SdkRequest, OutputT extends SdkResponse, ReturnT> Complet
8889
AsyncResponseTransformer<OutputT, ReturnT> asyncResponseTransformer) {
8990

9091
return measureApiCallSuccess(executionParams, () -> {
91-
// Running beforeExecution interceptors and modifyRequest interceptors.
92-
ExecutionContext context = invokeInterceptorsAndCreateExecutionContext(executionParams);
92+
if (executionParams.getCombinedResponseHandler() != null) {
93+
// There is no support for catching errors in a body for streaming responses. Our codegen must never
94+
// attempt to do this.
95+
throw new IllegalArgumentException("A streaming 'asyncResponseTransformer' may not be used when a "
96+
+ "'combinedResponseHandler' has been specified in a "
97+
+ "ClientExecutionParams object.");
98+
}
9399

94-
TransformingAsyncResponseHandler<Response<ReturnT>> combinedResponseHandler =
95-
createStreamingCombinedResponseHandler(executionParams, asyncResponseTransformer, context);
100+
ExecutionAttributes executionAttributes = executionParams.executionAttributes();
101+
executionAttributes.putAttribute(InternalCoreExecutionAttribute.EXECUTION_ATTEMPT, 1);
96102

97-
return doExecute(executionParams, context, combinedResponseHandler);
98-
});
99-
}
103+
AsyncStreamingResponseHandler<OutputT, ReturnT> asyncStreamingResponseHandler =
104+
new AsyncStreamingResponseHandler<>(asyncResponseTransformer);
100105

101-
private <InputT extends SdkRequest, OutputT extends SdkResponse, ReturnT> TransformingAsyncResponseHandler<Response<ReturnT>>
102-
createStreamingCombinedResponseHandler(ClientExecutionParams<InputT, OutputT> executionParams,
103-
AsyncResponseTransformer<OutputT, ReturnT> asyncResponseTransformer,
104-
ExecutionContext context) {
105-
if (executionParams.getCombinedResponseHandler() != null) {
106-
// There is no support for catching errors in a body for streaming responses. Our codegen must never
107-
// attempt to do this.
108-
throw new IllegalArgumentException("A streaming 'asyncResponseTransformer' may not be used when a "
109-
+ "'combinedResponseHandler' has been specified in a "
110-
+ "ClientExecutionParams object.");
111-
}
106+
// For streaming requests, prepare() should be called as early as possible to avoid NPE in client
107+
// See https://github.com/aws/aws-sdk-java-v2/issues/1268. We do this with a wrapper that caches the prepare
108+
// result until the execution attempt number changes. This guarantees that prepare is only called once per
109+
// execution.
110+
TransformingAsyncResponseHandler<ReturnT> wrappedAsyncStreamingResponseHandler =
111+
IdempotentAsyncResponseHandler.create(
112+
asyncStreamingResponseHandler,
113+
() -> executionAttributes.getAttribute(InternalCoreExecutionAttribute.EXECUTION_ATTEMPT),
114+
Integer::equals);
115+
wrappedAsyncStreamingResponseHandler.prepare();
112116

113-
AsyncStreamingResponseHandler<OutputT, ReturnT> asyncStreamingResponseHandler =
114-
new AsyncStreamingResponseHandler<>(asyncResponseTransformer);
117+
// Running beforeExecution interceptors and modifyRequest interceptors.
118+
ExecutionContext context = invokeInterceptorsAndCreateExecutionContext(executionParams);
115119

116-
// For streaming requests, prepare() should be called as early as possible to avoid NPE in client
117-
// See https://github.com/aws/aws-sdk-java-v2/issues/1268. We do this with a wrapper that caches the prepare
118-
// result until the execution attempt number changes. This guarantees that prepare is only called once per
119-
// execution.
120-
TransformingAsyncResponseHandler<ReturnT> wrappedAsyncStreamingResponseHandler =
121-
IdempotentAsyncResponseHandler.create(
122-
asyncStreamingResponseHandler,
123-
() -> context.executionAttributes().getAttribute(InternalCoreExecutionAttribute.EXECUTION_ATTEMPT),
124-
Integer::equals);
125-
wrappedAsyncStreamingResponseHandler.prepare();
120+
HttpResponseHandler<OutputT> decoratedResponseHandlers =
121+
decorateResponseHandlers(executionParams.getResponseHandler(), context);
126122

127-
HttpResponseHandler<OutputT> decoratedResponseHandlers =
128-
decorateResponseHandlers(executionParams.getResponseHandler(), context);
123+
asyncStreamingResponseHandler.responseHandler(decoratedResponseHandlers);
129124

130-
asyncStreamingResponseHandler.responseHandler(decoratedResponseHandlers);
125+
TransformingAsyncResponseHandler<? extends SdkException> errorHandler =
126+
resolveErrorResponseHandler(executionParams.getErrorResponseHandler(), context, crc32Validator);
131127

132-
TransformingAsyncResponseHandler<? extends SdkException> errorHandler =
133-
resolveErrorResponseHandler(executionParams.getErrorResponseHandler(), context, crc32Validator);
128+
TransformingAsyncResponseHandler<Response<ReturnT>> combinedResponseHandler =
129+
new CombinedResponseAsyncHttpResponseHandler<>(wrappedAsyncStreamingResponseHandler, errorHandler);
134130

135-
return new CombinedResponseAsyncHttpResponseHandler<>(wrappedAsyncStreamingResponseHandler, errorHandler);
131+
return doExecute(executionParams, context, combinedResponseHandler);
132+
});
136133
}
137134

138135
private <InputT extends SdkRequest, OutputT extends SdkResponse> TransformingAsyncResponseHandler<Response<OutputT>>

0 commit comments

Comments
 (0)