Skip to content
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

Remove transactionMemo from TransactionSupplier and expirationTime from TokenClient #2423

Merged
merged 24 commits into from
Aug 19, 2021
Merged

Remove transactionMemo from TransactionSupplier and expirationTime from TokenClient #2423

merged 24 commits into from
Aug 19, 2021

Conversation

ijungmann
Copy link
Contributor

@ijungmann ijungmann commented Aug 18, 2021 •
edited by steven-sheehy
Loading

Description:

  • Remove the line setting transactionMemo in each TransactionSupplier, as it is already being set later in the TransactionPublisher.
  • Remove the line setting expirationTime in the TokenClient, as we already set the autoRenewPeriod for the token, and setting expirationTime on a token makes autoRenewPeriod null in the SDK.

Notes for reviewer:
Ran the tokenbase Acceptance tests to confirm the TokenClient still works, all passed.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Ian Jungmann added 22 commits August 9, 2021 09:40
Remover datagenerator and add first test
70746cc 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Add a bunch of tests and tweak attributes
a6682f7 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Add schedule tests and fixeds
592c222 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Remove datagenerator
da5d7de 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Merge branch 'main' into remove_data_generator
bf3bfb5
Test fixes
a988170 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Add back Utility tests
a0e3f8e 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Move Utility into a package and fix tests
4d7fcf4 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Fix headers
d00ba1e 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Fix headers
f3745ca 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Remove newlines between static final fields
82c0eeb 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Switch account tests to chaining
f0a9913 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Chain the rest of the tests
ae121a0 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Do field-by-field checks
962a04b 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Merge branch 'main' into remove_data_generator
892ea7c
Add null checks
a588955 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Reorder imports
fb27723 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Code smells
cc06f59 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Reorder asserts
f40f75b 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Remove transaction memo checks
c0675d6 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Clean up tests
91c311e 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Remove transaction memo and expiration time sets
c1fd069 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
@ijungmann ijungmann added P3 test Test infrastructure, automated tests required, etc monitor Area: Monitoring and dashboard labels Aug 18, 2021
@ijungmann ijungmann added this to the Mirror 0.39.0 milestone Aug 18, 2021
@ijungmann ijungmann self-assigned this Aug 18, 2021
@codecov
Copy link

codecov bot commented Aug 18, 2021 •
edited
Loading

Codecov Report

Merging #2423 (c99e600) into main (dc8570c) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head c99e600 differs from pull request most recent head bf8711a. Consider uploading reports for the commit bf8711a to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2423      +/-   ##
============================================
- Coverage     90.74%   90.70%   -0.04%     
  Complexity     2434     2434              
============================================
  Files           420      420              
  Lines         11633    11603      -30     
  Branches       1014     1014              
============================================
- Hits          10556    10525      -31     
- Misses          745      747       +2     
+ Partials        332      331       -1
Impacted Files Coverage Δ
...tion/account/AccountDeleteTransactionSupplier.java 60.00% <ø> (-3.64%) ⬇️
...ion/account/CryptoTransferTransactionSupplier.java 71.79% <ø> (-1.38%) ⬇️
...tion/account/AccountCreateTransactionSupplier.java 65.00% <100.00%> (-3.19%) ⬇️
...tion/account/AccountUpdateTransactionSupplier.java 75.00% <100.00%> (-2.28%) ⬇️
...ensus/ConsensusCreateTopicTransactionSupplier.java 76.47% <100.00%> (-2.48%) ⬇️
...ensus/ConsensusDeleteTopicTransactionSupplier.java 57.14% <100.00%> (-5.36%) ⬇️
...ensus/ConsensusUpdateTopicTransactionSupplier.java 77.27% <100.00%> (-1.90%) ⬇️
...on/schedule/ScheduleCreateTransactionSupplier.java 73.84% <100.00%> (-0.79%) ⬇️
...on/schedule/ScheduleDeleteTransactionSupplier.java 57.14% <100.00%> (-5.36%) ⬇️
...tion/schedule/ScheduleSignTransactionSupplier.java 57.14% <100.00%> (-5.36%) ⬇️
... and 13 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 dc8570c...bf8711a. Read the comment docs.

Ian Jungmann added 2 commits August 18, 2021 15:28
Merge with main
1b7f376 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
Cleanup
bf8711a 
Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 18, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ijungmann ijungmann marked this pull request as ready for review August 18, 2021 21:56
@ijungmann ijungmann requested a review from a team August 18, 2021 21:56
Nana-EC
Nana-EC approved these changes Aug 18, 2021
View reviewed changes
Copy link
Contributor

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LGTM

@ijungmann ijungmann merged commit 31e882d into main Aug 19, 2021
@ijungmann ijungmann deleted the post_datagenerator_remove_pr branch August 19, 2021 16:44
steven-sheehy added a commit that referenced this pull request Aug 23, 2021
@steven-sheehy @Nana-EC @xin-hedera
Merge main to release/0.39 (#2441)
95140be 
* Remove data generator (#2387)

* Remove the data generator module and all references to it.
* Move TransactionSuppliers and anything else needed by the monitor into the monitor module
* Add unit tests for the TransactionSuppliers

Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>

* Fix startDate logic on empty DB (#2421)

* Fix startDate logic on empty DB

The current MirrorDateRangePropertiesProcessor.newDateRangeFilter() currently doesn't honor the startDate set on an empty db

- Update newDateRangeFilter() logic to handle case when there's no lastFileInstant
- Add a test case to verify scenario

Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>

* Remove transactionMemo from TransactionSupplier and expirationTime from TokenClient (#2423)

* Remove setting transactionMemo in TransactionSuppliers
* Remove setting expirationTime in TokenClient

Signed-off-by: Ian Jungmann <ian.jungmann@hedera.com>

* Fix rest service startup crash (#2425)

- replace the one-time verifyDbConnection with on-demand transaction result & type loading

Signed-off-by: Xin Li <xin.li@hedera.com>

* Add missing token account association migration (#2424)

- add a db migration script to add the missing token account associations

Signed-off-by: Xin Li <xin.li@hedera.com>

Co-authored-by: Ian Jungmann <ian.jungmann@hedera.com>
Co-authored-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steven-sheehy steven-sheehy steven-sheehy approved these changes

@Nana-EC Nana-EC Nana-EC approved these changes

@xin-hedera xin-hedera xin-hedera approved these changes

Assignees

@ijungmann ijungmann

Labels
monitor Area: Monitoring and dashboard P3 test Test infrastructure, automated tests required, etc
Projects
None yet
Milestone
Mirror 0.39.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ijungmann @steven-sheehy @Nana-EC @xin-hedera