Skip to content

Structural Refactoring of AwsChunkedEncodingInputStream to abstract AwsSignedChunkedEncodingInputStream #2780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 24, 2021
Merged

Structural Refactoring of AwsChunkedEncodingInputStream to abstract AwsSignedChunkedEncodingInputStream #2780

merged 1 commit into from
Oct 24, 2021

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Oct 20, 2021 •
edited
Loading

Motivation and Context

  • AwsSignedChunkedEncodingInputStream performed signing as well as chunked encoding.
  • The chunked Encoding can be abstracted out to a core packages , such that we can use for Unsigned Input streams too.

Description

  • Renamed existing AwsChunkedEncodingInputStream to AwsSignedChunkedEncodingInputStream and this only does signing related functions.
  • Created abstract class AwsChunkedEncodingInputStream which only does Chunked encoding.
  • Created Builder methods instead of constructors and made constructors as privare.

Testing

  • Junits

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@dagnir
Copy link
Contributor

dagnir commented Oct 21, 2021

Can you run the integ tests?

dagnir
dagnir reviewed Oct 21, 2021
View reviewed changes
...ore/src/main/java/software/amazon/awssdk/core/internal/io/AwsChunkedEncodingInputStream.java Outdated
protected abstract byte[] createFinalChunk(byte[] finalChunk);

/**
* Creates chunk for the given byte. The chucks could be appended with Signatures or any additional bytes by Concrete classes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

byte -> buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@joviegas
Copy link
Contributor Author

Can you run the integ tests?

Done

dagnir
dagnir approved these changes Oct 22, 2021
View reviewed changes
Copy link
Contributor

@dagnir dagnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 2 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

61.7% 61.7% Coverage
5.5% 5.5% Duplication

@joviegas joviegas merged commit df41964 into aws:master Oct 24, 2021
joviegas pushed a commit that referenced this pull request Oct 24, 2021
Resolving merge conflicts as a part of PR #2780 Deleteing AwsChunkedE…
62fbecc 
…ncodingConfig from auth package
@joviegas joviegas mentioned this pull request Oct 24, 2021
Merged
14 tasks
joviegas added a commit that referenced this pull request Oct 25, 2021
@joviegas
Resolving merge conflicts as a part of PR #2780 Deleteing AwsChunkedE…
52f17cb 
…ncodingConfig from auth package (#2793)
@joviegas joviegas mentioned this pull request Feb 17, 2022
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagnir dagnir dagnir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@joviegas @dagnir