Move request checksum interceptors to pipeline stages #4174
Conversation
.../sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java
Outdated
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Outdated
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Outdated
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Show resolved
Hide resolved
| } | ||
|
|
||
| return checksumSpecs != null | ||
| && checksumSpecs.headerName() != null |
There was a problem hiding this comment.
Do we need to check headerName? It seems we are not checking headerName in the existing code
There was a problem hiding this comment.
discussed offline, to leave in.
the asyncTrailerInterceptor had this check
| context.executionContext().interceptorContext().httpRequest(), | ||
| clientType, checksumSpecs, hasRequestBody, | ||
| context.executionContext().interceptorContext().requestBody() | ||
| .map(requestBody -> requestBody.contentStreamProvider() != null).orElse(false)); |
There was a problem hiding this comment.
I understand this is what current code uses, but can we at least create a variable for this context.executionContext().interceptorContext().requestBody() .map(requestBody -> requestBody.contentStreamProvider() != null).orElse(false)) because it's a bit hard to read.
There was a problem hiding this comment.
extracted to isContentStreaming variable
| * Stage to implement the "httpChecksum" and "httpChecksumRequired" C2J traits, and flexible checksums. | ||
| */ | ||
| @SdkInternalApi | ||
| public class HttpChecksumStage implements MutableRequestToRequestPipeline { |
There was a problem hiding this comment.
Can we add some tests for this class?
There was a problem hiding this comment.
added test for stage
|
Kudos, SonarCloud Quality Gate passed! |
* Move request checksum interceptors to pipeline stages * Refactor into one combined stage * Refactoring * Fix typo error * Refactoring * Fix typo * Refactoring * HttpChecksumStage test * Refactoring
Motivation and Context
Checksumming needs to happen after request compression, which is being implemented as a request pipeline stage. Refactoring and migrating the request checksum interceptors to a request pipeline stage.
Modifications
Testing
Integration tests ran successfully
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License