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.

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

Change JKey methods to throw InvalidKeyException and add equals and hashCode #7143

Merged
merged 1 commit into from
Jun 16, 2023

Conversation

jsync-swirlds
Copy link
Member

@jsync-swirlds jsync-swirlds commented Jun 15, 2023

Issue #7133

  • Removed DecoderException from JKey and all other code that uses JKey
    • Almost all of the uses are in test code, surprisingly little non-test code uses JKey
  • Removed all other uses of DecoderException from the codebase

Issue #7137

  • Added equals and hashCode to JKey. * Note that this implementation is not efficient, but is a good bridge to the PBJ Key object that replaces JKey soon.
    • One remaining use is in platform-sdk in CommonUtils line 232.
      • A call to apache commons Hex.decodeHex is wrapped in a try/catch that catches the DecoderException

@jsync-swirlds jsync-swirlds requested review from a team, iwsimon and kimbor as code owners June 15, 2023 22:01
@jsync-swirlds jsync-swirlds requested review from a team and jasperpotts June 15, 2023 22:01
@jsync-swirlds jsync-swirlds force-pushed the 07133-D-remove-decoder-exception-jkey branch 2 times, most recently from 873d9f3 to 65bae61 Compare June 15, 2023 22:15
@jsync-swirlds jsync-swirlds requested a review from povolev15 as a code owner June 15, 2023 22:15
@jsync-swirlds jsync-swirlds self-assigned this Jun 15, 2023
@jsync-swirlds jsync-swirlds added this to the v0.40 milestone Jun 15, 2023
@jsync-swirlds jsync-swirlds force-pushed the 07133-D-remove-decoder-exception-jkey branch from 65bae61 to 9706289 Compare June 15, 2023 22:18
@github-actions
Copy link

github-actions bot commented Jun 15, 2023

Node: E2E Test Results

    1 files      1 suites   18m 14s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit b06c62e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jun 15, 2023

Node: Integration Test Results

266 tests   266 ✔️  27m 39s ⏱️
    4 suites      0 💤
    4 files        0

Results for commit b06c62e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jun 15, 2023

Node: Unit Test Results

    1 482 files      1 482 suites   17m 52s ⏱️
101 172 tests 101 164 ✔️ 8 💤 0
107 526 runs  107 518 ✔️ 8 💤 0

Results for commit b06c62e.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Patch coverage: 52.38% and project coverage change: +21.89 🎉

Comparison is base (5cc13e1) 68.40% compared to head (9706289) 90.30%.

Additional details and impacted files
@@              Coverage Diff               @@
##             develop    #7143       +/-   ##
==============================================
+ Coverage      68.40%   90.30%   +21.89%     
+ Complexity     23951    18767     -5184     
==============================================
  Files           2205     1469      -736     
  Lines         142537    54674    -87863     
  Branches        8364     5670     -2694     
==============================================
- Hits           97506    49372    -48134     
+ Misses         43322     4144    -39178     
+ Partials        1709     1158      -551     
Impacted Files Coverage Δ
.../app/workflows/handle/AdaptedMonoProcessLogic.java 85.36% <0.00%> (ø)
...ce/file/impl/codec/FileServiceStateTranslator.java 96.55% <ø> (ø)
...n/java/com/hedera/node/app/service/mono/Utils.java 9.52% <0.00%> (ø)
...hedera/node/app/service/mono/pbj/PbjConverter.java 1.43% <0.00%> (ø)
...contracts/precompile/codec/TokenCreateWrapper.java 98.26% <ø> (ø)
...ntracts/precompile/impl/TokenCreatePrecompile.java 97.13% <0.00%> (ø)
...ono/txns/consensus/TopicUpdateTransitionLogic.java 87.33% <0.00%> (+22.48%) ⬆️
...node/app/service/mono/legacy/core/jproto/JKey.java 67.39% <45.83%> (+14.42%) ⬆️
...app/service/mono/ledger/accounts/AliasManager.java 91.11% <50.00%> (ø)
...ation/consensus/txns/UpdateTopicResourceUsage.java 100.00% <100.00%> (+34.88%) ⬆️
... and 5 more

... and 962 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

 * Removed `DecoderException` from JKey and all other code that uses `JKey`
    * Almost all of the uses are in test code, surprisingly little non-test code uses JKey
 * Removed all other uses of `DecoderException` from the codebase
    * One remaining use is in platform-sdk in `CommonUtils` line 232.
        * A call to apache commons `Hex.decodeHex` is wrapped in a try/catch that catches the `DecoderException`

Issue 7137 `JKey` has no `equals` or `hashCode` implementation
 * Added `equals` and `hashCode` to `JKey`.
    * Note that this implementation is not efficient, but is a good bridge to the PBJ Key object that replaces JKey soon.

Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com>
@jsync-swirlds jsync-swirlds force-pushed the 07133-D-remove-decoder-exception-jkey branch from 9706289 to b06c62e Compare June 16, 2023 16:55
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

65.5% 65.5% Coverage
0.0% 0.0% Duplication

@jsync-swirlds jsync-swirlds merged commit e9a4f7d into develop Jun 16, 2023
@jsync-swirlds jsync-swirlds deleted the 07133-D-remove-decoder-exception-jkey branch June 16, 2023 18:53
imalygin pushed a commit that referenced this pull request Jun 20, 2023
… and `hashCode` (#7143)

Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

JKey has no equals() or hashCode() methods implemented change the JKey methods to throw InvalidKeyException
3 participants