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

feat: Added support of compareTo #158

Merged

imalygin merged 10 commits into main from 103-add-support-of-comparable

Jan 12, 2024

Member

imalygin commented Dec 22, 2023 •

edited

Loading

PR highlights:

by default all fields of a class are used in compareTo method.
pbj.comparable option in a comment can be used to specify limited set of fields to be used in compareTo.

// <<<pbj.comparable = "int32Number, text, subObject" >>>

there was a small modification in Protobuf grammar definition to support it
Bytes and BytesValue fields are compared using byte-by-byte comparison
repeatable fields are not supported

Misc:

refactored ModelGenerator: extracted code-generating code into several independent methods
upgraded Gradle version to 8.5 to support Java 21
added dependency to Mockito framework in pbj-compiler

Related issue(s):

Fixes #103


          103 Added support of compareTo

31da340

Misc:
- refactored ModelGenerator: extracted code-generating code into several independent methods
- upgraded Gradle version to 8.5 to support Java 21
- added dependency to Mockito framework in pbj-compiler

Signed-off-by: Ivan Malygin <[email protected]>

github-actions bot commented Dec 22, 2023 •

edited

Loading

JUnit Test Report

    47 files ±0     47 suites ±0 3m 29s ⏱️ +5s
1 111 tests +6 1 110 ✔️ +6   1 💤 ±0 0 ❌ ±0
6 966 runs +6 6 948 ✔️ +6 18 💤 ±0 0 ❌ ±0

Results for commit 1d597cb. ± Comparison against base commit baabed4.

♻️ This comment has been updated with latest results.

Contributor

codacy-production bot commented Dec 22, 2023 •

edited

Loading

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +19.21% (target: -1.00%)	✅ 9.25%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`f4367bd`)	6430	1624	25.26%
Head commit (`1d597cb`)	4214 (-2216)	1874 (+250)	44.47% (+19.21%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#158)	346	32	9.25%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation}

github-actions bot commented Dec 22, 2023 •

edited

Loading

Integration Test Report

    232 files +  16     232 suites +16 6m 11s ⏱️ -42s
87 138 tests +144 87 138 ✔️ +144 0 💤 ±0 0 ❌ ±0
87 366 runs +144 87 366 ✔️ +144 0 💤 ±0 0 ❌ ±0

Results for commit 1d597cb. ± Comparison against base commit baabed4.

♻️ This comment has been updated with latest results.

anthony-swirldslabs requested changes

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java Outdated Show resolved Hide resolved

jsync-swirlds reviewed

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

artemananiev reviewed

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

artemananiev reviewed

View reviewed changes

pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/OneOf.java Outdated Show resolved Hide resolved

artemananiev reviewed

View reviewed changes

pbj-integration-tests/src/main/proto/comparable.proto Show resolved Hide resolved

imalygin force-pushed the 103-add-support-of-comparable branch from 69dd3f9 to 04af982 Compare

January 11, 2024 19:39

jsync-swirlds reviewed

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

imalygin force-pushed the 103-add-support-of-comparable branch from 04af982 to a28cced Compare

January 11, 2024 20:00


          103 Addressed PR comments:

cb37aaa

- now fields are considered comparable only if they are explicitly mentioned in `pbj.comparable` option
- preserving order of the comparable fields
- added proper support for OneOf fields
- added support for `Bytes` field
- fixed formatting
- added tests for non-compilable cases

Signed-off-by: Ivan Malygin <[email protected]>

imalygin force-pushed the 103-add-support-of-comparable branch from a28cced to cb37aaa Compare

January 11, 2024 20:25


          103 Removed usage of incorrect annotation

a797ad3

Signed-off-by: Ivan Malygin <[email protected]>

imalygin force-pushed the 103-add-support-of-comparable branch from b9e6d00 to a797ad3 Compare

January 11, 2024 20:35

jsync-swirlds reviewed

View reviewed changes

Member

jsync-swirlds left a comment

Impressive effort so far, but there are a few minor issues, and some items that need to be fixed.

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java Outdated Show resolved Hide resolved

pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/OneOf.java Outdated Show resolved Hide resolved

pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/OneOf.java Outdated Show resolved Hide resolved

pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java Outdated Show resolved Hide resolved

anthony-swirldslabs reviewed

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java Outdated Show resolved Hide resolved

imalygin and others added 4 commits

January 11, 2024 18:29


          Update pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/im…

21afb50

…pl/Common.java

Co-authored-by: Joseph Sinclair <[email protected]>
Signed-off-by: Ivan Malygin <[email protected]>


          Addressed review comments

95b6379

Signed-off-by: Ivan Malygin <[email protected]>


          Introduced ComparableOneOf that is used if oneOf field mentioned in…

c9c5f73

… `pbj.comparable` option.

Signed-off-by: Ivan Malygin <[email protected]>


          Addressed review comments

6fb378b

Signed-off-by: Ivan Malygin <[email protected]>

imalygin force-pushed the 103-add-support-of-comparable branch from f10c738 to 6fb378b Compare

January 12, 2024 02:46

jsync-swirlds reviewed

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java Outdated Show resolved Hide resolved

imalygin added 2 commits

January 12, 2024 12:12


          Added missing import

01f626f

Fixed regex for Comparable record
Added test for primitive wrappers and unsigned values
Fixed compareTo for BytesValue

Signed-off-by: Ivan Malygin <[email protected]>


          Addressed PR comment - fixed handling of an object with repeated fiel…

aa29694

…d not mentioned in `pbj.comparable`

Signed-off-by: Ivan Malygin <[email protected]>

jsync-swirlds reviewed

View reviewed changes

pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java Outdated Show resolved Hide resolved


          Addressed PR comment - Bytes.compareTo usese compareUnsigned now

1d597cb

Signed-off-by: Ivan Malygin <[email protected]>

jsync-swirlds approved these changes

View reviewed changes

Member

jsync-swirlds left a comment

Excellent work, Ivan.

anthony-swirldslabs approved these changes

View reviewed changes

pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/LookupHelper.java Show resolved Hide resolved

imalygin merged commit 1394b82 into main

9 of 10 checks passed

imalygin deleted the 103-add-support-of-comparable branch

January 12, 2024 20:57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

rbair23 rbair23 left review comments

netopyr netopyr left review comments

artemananiev artemananiev left review comments

jsync-swirlds jsync-swirlds approved these changes

anthony-swirldslabs anthony-swirldslabs approved these changes

Labels

None yet