Skip to content

Added missing clientRequestToken to TransactWriteItemsEnhancedRequest #1891

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 2 commits into from
Jun 12, 2020
Merged

Added missing clientRequestToken to TransactWriteItemsEnhancedRequest #1891

merged 2 commits into from
Jun 12, 2020
+42 −0

Conversation

Quanzzzz
Copy link
Contributor

Added missing clientRequestToken to TransactWriteItemsEnhancedRequest class to align usage of TransactWriteItems in v1 client.

Description

Motivation and Context

This pr is to solve issue #1808

Testing

One unit test is added, testing if TransactWriteItemsRequest has the clientRequestToken passed by the enhanced request.

Screenshots (if appropriate)

Types of changes

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

License

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

@Quanzzzz Quanzzzz requested a review from millems June 11, 2020 23:27
@Quanzzzz Quanzzzz linked an issue Jun 11, 2020 that may be closed by this pull request
Missing client idempotency token in enhanced client #1808
Closed
@Quanzzzz Quanzzzz force-pushed the fix-missing-token branch from 2c55c2d to 57f0e2b Compare June 11, 2020 23:58
zoewangg
zoewangg reviewed Jun 12, 2020
View reviewed changes
Copy link
Contributor

@zoewangg zoewangg left a comment

Choose a reason for hiding this comment

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

Can we add a changelog entry?

...va/software/amazon/awssdk/enhanced/dynamodb/model/TransactWriteItemsEnhancedRequestTest.java
@@ -167,6 +174,17 @@ public void builder_maximal_builder_style() {
assertThat(builtObject.transactWriteItems().get(3).conditionCheck().key().get("id").s(), is(fakeItem.getId()));
}

@Test
public void setClientRequestTokenTest() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, the naming pattern for most of our unit tests is something like: given_when_then,

so maybe builder_passRequestToken_shouldWork?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to figure out a name in this pattern but I failed so I just used the current simple one lol. Will change it.

@codecov-commenter
Copy link

Codecov Report

Merging #1891 into master will decrease coverage by 0.04%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1891      +/-   ##
============================================
- Coverage     76.24%   76.20%   -0.05%     
  Complexity      187      187              
============================================
  Files          1076     1076              
  Lines         32568    32507      -61     
  Branches       2569     2546      -23     
============================================
- Hits          24833    24773      -60     
  Misses         6476     6476              
+ Partials       1259     1258       -1
Flag Coverage Δ Complexity Δ
#unittests 76.20% <77.77%> (-0.05%) 187.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
...amodb/model/TransactWriteItemsEnhancedRequest.java 81.25% <75.00%> (-1.68%) 0.00 <0.00> (ø)
...ternal/operations/TransactWriteItemsOperation.java 100.00% <100.00%> (ø) 0.00 <0.00> (ø)
.../awssdk/http/apache/internal/net/SdkSslSocket.java 36.36% <0.00%> (-22.73%) 0.00% <0.00%> (ø%)
...ttp/nio/netty/internal/nrs/HttpStreamsHandler.java 60.76% <0.00%> (-3.85%) 0.00% <0.00%> (ø%)
...ssdk/core/internal/async/FileAsyncRequestBody.java 84.76% <0.00%> (-2.86%) 0.00% <0.00%> (ø%)
...tials/WebIdentityTokenFileCredentialsProvider.java 35.55% <0.00%> (-2.23%) 0.00% <0.00%> (ø%)
...ery/internal/marshall/QueryMarshallerRegistry.java 77.77% <0.00%> (-2.23%) 0.00% <0.00%> (ø%)
...awssdk/codegen/poet/client/specs/ProtocolSpec.java 81.57% <0.00%> (-2.15%) 0.00% <0.00%> (ø%)
...o/netty/internal/utils/BetterFixedChannelPool.java 56.32% <0.00%> (-1.73%) 0.00% <0.00%> (ø%)
.../amazon/awssdk/http/apache/ProxyConfiguration.java 49.03% <0.00%> (-0.97%) 0.00% <0.00%> (ø%)
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9142b0b...16c6a4b. Read the comment docs.

@Quanzzzz Quanzzzz merged commit e868c53 into aws:master Jun 12, 2020
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@Quanzzzz Quanzzzz mentioned this pull request Jun 12, 2020
Merged
1 task
aws-sdk-java-automation added a commit that referenced this pull request Jan 12, 2022
@aws-sdk-java-automation
Merge pull request #1891 from aws/staging/8b839efa-873f-406b-a601-070…
8c438b0 
…677f6c45d

Pull request: release <- staging/8b839efa-873f-406b-a601-070677f6c45d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoewangg zoewangg zoewangg left review comments

@millems millems millems 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.

Missing client idempotency token in enhanced client
4 participants
@Quanzzzz @codecov-commenter @millems @zoewangg