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

Add support for repeated topics in contract logs REST API #3673

Merged
merged 5 commits into from May 3, 2022
Merged

Add support for repeated topics in contract logs REST API #3673

merged 5 commits into from May 3, 2022

Conversation

ar-conmit
Copy link
Contributor

Signed-off-by: Anton Rusev anton.rusev.code@gmail.com

Description:

Related issue(s):

Fixes #3505

Notes for reviewer:

Checklist

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

@ar-conmit
feat: add support for repeated topics
b39905c 
Signed-off-by: Anton Rusev <anton.rusev.code@gmail.com>
@codecov
Copy link

codecov bot commented Apr 29, 2022 •
edited

Codecov Report

Merging #3673 (1c8b85c) into main (03a216f) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #3673      +/-   ##
============================================
- Coverage     92.78%   92.75%   -0.03%     
- Complexity     2579     2613      +34     
============================================
  Files           483      491       +8     
  Lines         14822    14944     +122     
  Branches       1515     1525      +10     
============================================
+ Hits          13752    13862     +110     
- Misses          752      762      +10     
- Partials        318      320       +2
Impacted Files Coverage Δ
.../importer/parser/record/entity/EntityListener.java 60.00% <0.00%> (-31.67%) ⬇️
...a/com/hedera/mirror/monitor/MonitorProperties.java 58.33% <0.00%> (-8.34%) ⬇️
...sus/ConsensusSubmitMessageTransactionSupplier.java 78.57% <0.00%> (-7.15%) ⬇️
.../java/com/hedera/mirror/monitor/HederaNetwork.java 94.73% <0.00%> (-5.27%) ⬇️
...a/mirror/monitor/publish/TransactionPublisher.java 89.51% <0.00%> (-1.40%) ⬇️
...on/schedule/ScheduleCreateTransactionSupplier.java 81.81% <0.00%> (-1.26%) ⬇️
...importer/reader/record/RecordFileReaderImplV5.java 81.92% <0.00%> (-0.79%) ⬇️
...a/com/hedera/mirror/importer/MirrorProperties.java 92.30% <0.00%> (-0.29%) ⬇️
...ter/config/MirrorDateRangePropertiesProcessor.java 84.70% <0.00%> (-0.18%) ⬇️
...or/monitor/expression/ExpressionConverterImpl.java 94.66% <0.00%> (-0.14%) ⬇️
... and 23 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 03a216f...1c8b85c. Read the comment docs.

@ar-conmit
test: remove obsolete snapshot
923b1c3 
Signed-off-by: Anton Rusev <anton.rusev.code@gmail.com>
natanasow
natanasow previously approved these changes Apr 29, 2022
View reviewed changes
Copy link
Contributor

@natanasow natanasow left a comment

Choose a reason for hiding this comment

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

LGTM

@steven-sheehy steven-sheehy requested a review from a team April 29, 2022 15:21
@steven-sheehy steven-sheehy added this to the 0.56.0 milestone Apr 29, 2022
@steven-sheehy steven-sheehy added enhancement Type: New feature P2 rest Area: REST API labels Apr 29, 2022
@steven-sheehy steven-sheehy changed the title feat: add support for repeated topics Add support for repeated topics Apr 29, 2022
@steven-sheehy steven-sheehy changed the title Add support for repeated topics Add support for repeated topics in contract logs REST API Apr 29, 2022
Nana-EC
Nana-EC requested changes Apr 30, 2022
View reviewed changes
Copy link
Collaborator

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

Simplification suggestion.
Also need to update openapi.yml to highlight repetition support. Noting that repetitions within topics are OR and across remain AND

hedera-mirror-rest/controllers/contractController.js Outdated Show resolved Hide resolved
@steven-sheehy steven-sheehy modified the milestones: 0.56.0, 0.57.0 May 2, 2022
@steven-sheehy steven-sheehy added the json-rpc Related to JSON-RPC Bridge work label May 2, 2022
@edwin-greene edwin-greene self-requested a review May 2, 2022 19:42
edwin-greene
edwin-greene previously approved these changes May 2, 2022
View reviewed changes
Copy link
Contributor

@edwin-greene edwin-greene left a comment

Choose a reason for hiding this comment

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

Looks good, just a few things to change from the other review.

@ar-conmit
docs: fix LogTopicQueryParam schema to accept array of strings
81bd4b6 
Signed-off-by: Anton Rusev <anton.rusev.code@gmail.com>
@ar-conmit
refactor: simplify repeated code block
92f03b2 
Signed-off-by: Anton Rusev <anton.rusev.code@gmail.com>
@ar-conmit ar-conmit dismissed stale reviews from edwin-greene and natanasow via 92f03b2 May 3, 2022 07:42
@ar-conmit
test: fix integration db tests
1c8b85c 
Signed-off-by: Anton Rusev <anton.rusev.code@gmail.com>
steven-sheehy
steven-sheehy approved these changes May 3, 2022
View reviewed changes
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

LGTM

hedera-mirror-rest/__tests__/controllers/contractController.test.js
{
key: constants.filterKeys.TOPIC3,
operator: utils.opsMap.eq,
value: '0000150',
Copy link
Member

Choose a reason for hiding this comment

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

nit: Redundant to say topic3=150 OR topic3=150

xin-hedera
xin-hedera approved these changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@xin-hedera xin-hedera left a comment

Choose a reason for hiding this comment

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

LGTM

Nana-EC
Nana-EC approved these changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@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

@steven-sheehy steven-sheehy removed this from the 0.57.0 milestone May 3, 2022
@steven-sheehy steven-sheehy added this to the 0.56.0 milestone May 3, 2022
@steven-sheehy steven-sheehy merged commit e00a6a4 into hashgraph:main May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Daniel-K-Ivanov Daniel-K-Ivanov Daniel-K-Ivanov requested changes

@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

@natanasow natanasow natanasow left review comments

@edwin-greene edwin-greene Awaiting requested review from edwin-greene

Assignees

@ar-conmit ar-conmit

Labels
enhancement Type: New feature json-rpc Related to JSON-RPC Bridge work P2 rest Area: REST API
Projects
None yet
Milestone
0.56.0
Development

Successfully merging this pull request may close these issues.

Support repeated topics for contract logs API
7 participants
@ar-conmit @Daniel-K-Ivanov @steven-sheehy @natanasow @Nana-EC @xin-hedera @edwin-greene