Extending queries by modifing TxPaginationMeta #1092

Pawlak00 · 2021-06-18T13:47:22Z

Description of the Change

This is prove of concept for extending queries with time interval (from,to). Protobufs were modified and some basic logic was added. Validation of passed strings is not provided yet. Modified queries are getAccountTransactions and GetAccountAssetTransactions
Protobufs were modified, then neccesary getters were added.
Passed parameters are attached to sql query in executeTransactionsQuery, by applyer lamba function,
created separately in each function

Benefits

What benefits will be realized by the code change?
Allowing to query for Transactions in given time interval and beetwen two given block heights

Possible Drawbacks

What are the possible side-effects or negative impacts of the code change?
Usage of many if's in GetAccountTransactions/GetAccountAssetTransactions in postgres_specific_query_executor.cpp

Usage Examples or Tests [optional]

By rebuilding python library with protbuffs from code you can use it in Python library

Alternate Designs [optional]

Usage of different types for passing data as timestamp was considered, but I decided to use string, as it is not needed to cast from int to string

baziorek · 2021-06-18T13:52:11Z

There is reply from Iroha Core Team:

It looks like we have to change string format to protobuf datetime format.

.gitignore

CMakeLists.txt

docs/source/requirements.txt

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp

shared_model/backend/protobuf/queries/impl/proto_tx_pagination_meta.cpp

shared_model/backend/protobuf/queries/proto_get_account_asset_transactions.hpp

shared_model/validators/validators_common.cpp

baziorek

Global changes to correct:

Please don't change whitecharacters of lines which You don't change
Please don't remove/add newlines at the end of existing files
Please don't add/remove newlines between lines which You don't change

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp

shared_model/validators/validators_common.cpp

test/module/shared_model/query_mocks.hpp

.gitignore

baziorek

There are still changes in whitespaces/newline removing in files with not changed code. Please correct its (remove Your changes from the files). Also it would be good not to change parts of files not around changed code.

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp

shared_model/backend/protobuf/queries/impl/proto_get_account_transactions.cpp

shared_model/backend/protobuf/queries/proto_get_account_transactions.hpp

shared_model/builders/protobuf/builder_templates/query_template.hpp

shared_model/interfaces/queries/get_account_transactions.hpp

shared_model/validators/validators_common.hpp

baziorek

Code is almost ready. Just few small corrections and adding the test I've mentioned in the review.
The next thing is to update documentation. Then please let me know.
If the next version would be good we can ask Iroha-Core-Team to take a look into the code.

shared_model/builders/protobuf/builder_templates/query_template.hpp

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp

kuvaldini · 2021-07-09T16:21:25Z

Please
git rebase support/1.2.x --signof

baziorek

Small changes, but rather IMO it looks good.
Now correct suggestions from iroha core team. Then change from Draft into normal PR. Then we can ask Iroha Core Team on chat for review.

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp

docs/source/develop/api/queries.rst

baziorek

Code looks really good now. So I'm satisfied with the code, now it is time to satisfy Iroha Team:).

test/module/shared_model/validators/protobuf/proto_query_validator_test.cpp

kuvaldini · 2021-07-22T07:19:37Z

Did you check this PR does not break existing API?

kamilsa · 2021-07-22T08:42:15Z

@Pawlak00 can you please rebase your commits on the latest support/1.2.x?

kamilsa · 2021-07-22T10:56:42Z

Did you check this PR does not break existing API?

@kuvaldini I think it does not. Fields added queries.proto are optional and order is preserved

shared_model/validators/validators_common.cpp

baziorek · 2021-07-22T17:12:30Z

Something happend during rebasing from:
main branch into support/1.2.x

kamilsa · 2021-07-23T11:58:24Z

Something happend during rebasing from:
main branch into support/1.2.x

@Pawlak00 I think the easiest thing to remove redundant commits is just squash everything into a single commit. Otherwise there are no major issues

shared_model/validators/protobuf/proto_query_validator.cpp

kuvaldini · 2021-07-28T07:15:34Z

Please make sure you include only your own changes to this PR.

These commits must be removed from PR.

The best approach is to squash all in one commit. I suggest

cd iroha
git checkout origin/support/1.2.x -b new-tx-pagination-meta
git checkout Pawlak00/main -- .
git add .....................
git commit -m"[client-api] GOOD short with exhaustive description commit message"

This need to be carefully validated.

kuvaldini · 2021-07-28T07:21:00Z

@Pawlak00 ^^

If need help, You can reach me for help in telegram for instant messaging https://t.me/kuvaldini

cc @baziorek

Pawlak00 · 2021-07-28T15:25:01Z

I messed up with one test, so it is not ready to merge now, hope i will figure it out today

kuvaldini · 2021-07-28T15:32:23Z

Good luck. It is very good test failed before merge, not after

…

On July 28, 2021 at 6:25 PM Piotr Pawłowski ***@***.*** > wrote: I messed up with one test, so it is not ready to merge now, hope i will figure it out today — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub #1092 (comment) , or unsubscribe https://github.com/notifications/unsubscribe-auth/ALJH3F673PPGPGD3MDWD6ZDT2AONPANCNFSM465WYISA .

Pawlak00 · 2021-07-29T14:03:59Z

Issue is solved, so it is ready to proceed @kuvaldini @baziorek

docs/source/develop/api/queries.rst

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp

…tAccountAssetsTransactions Signed-off-by: Piotr Pawlowski <[email protected]> validation improved, tests added Signed-off-by: Piotr Pawlowski <[email protected]> validation fixed Signed-off-by: Piotr Pawlowski <[email protected]> time validation fixed Signed-off-by: Piotr Pawlowski <[email protected]> const introduced Signed-off-by: Piotr Pawlowski <[email protected]> space fixed Signed-off-by: Piotr Pawlowski <[email protected]>

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]>

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]> Signed-off-by: Piotr Pawłowski <[email protected]>

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]>

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]> Signed-off-by: Piotr Pawłowski <[email protected]>

…tAccountAssetsTransactions (hyperledger-iroha#1092) Signed-off-by: Piotr Pawlowski <[email protected]> Signed-off-by: Piotr Pawłowski <[email protected]> Signed-off-by: G.Bazior <[email protected]>

baziorek self-requested a review June 18, 2021 13:49

baziorek requested changes Jun 29, 2021

View reviewed changes

Pawlak00 marked this pull request as draft June 29, 2021 16:05

baziorek requested changes Jun 30, 2021

View reviewed changes

baziorek requested changes Jul 2, 2021

View reviewed changes

baziorek requested changes Jul 9, 2021

View reviewed changes

shared_model/builders/protobuf/builder_templates/query_template.hpp Outdated Show resolved Hide resolved

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp Outdated Show resolved Hide resolved

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp Show resolved Hide resolved

baziorek approved these changes Jul 9, 2021

View reviewed changes

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp Outdated Show resolved Hide resolved

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp Outdated Show resolved Hide resolved

test/module/irohad/ametsuchi/postgres_query_executor_test.cpp Show resolved Hide resolved

Pawlak00 marked this pull request as ready for review July 16, 2021 09:26

kamilsa requested review from iceseer and kuvaldini July 16, 2021 11:26

kuvaldini suggested changes Jul 16, 2021

View reviewed changes

docs/source/develop/api/queries.rst Outdated Show resolved Hide resolved

kuvaldini reviewed Jul 19, 2021

View reviewed changes

docs/source/develop/api/queries.rst Outdated Show resolved Hide resolved

baziorek approved these changes Jul 20, 2021

View reviewed changes

test/module/shared_model/validators/protobuf/proto_query_validator_test.cpp Outdated Show resolved Hide resolved

Pawlak00 requested review from kuvaldini July 20, 2021 11:59

This was referenced Jul 22, 2021

extending TxPaginationMeta hyperledger-iroha/iroha-javascript#63

Merged

Extending TxPaginationMeta in queries hyperledger-iroha/iroha-python#77

Merged

kamilsa reviewed Jul 22, 2021

View reviewed changes

shared_model/validators/validators_common.cpp Outdated Show resolved Hide resolved

shared_model/validators/validators_common.cpp Outdated Show resolved Hide resolved

iceseer suggested changes Jul 27, 2021

View reviewed changes

shared_model/validators/protobuf/proto_query_validator.cpp Outdated Show resolved Hide resolved

shared_model/validators/protobuf/proto_query_validator.cpp Show resolved Hide resolved

Pawlak00 requested a review from iceseer July 27, 2021 13:39

Pawlak00 mentioned this pull request Jul 30, 2021

Extending get pending transactions #1300

Merged

kuvaldini approved these changes Jul 30, 2021

View reviewed changes

docs/source/develop/api/queries.rst Show resolved Hide resolved

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp Outdated Show resolved Hide resolved

irohad/ametsuchi/impl/postgres_specific_query_executor.cpp Outdated Show resolved Hide resolved

iceseer approved these changes Jul 30, 2021

View reviewed changes

kuvaldini approved these changes Jul 30, 2021

View reviewed changes

kamilsa merged commit 09d8d18 into hyperledger-iroha:support/1.2.x Jul 30, 2021

kuvaldini pushed a commit that referenced this pull request Jul 30, 2021

[client-api] extending TxPaginationMeta in GetAccountTransactions, Ge…

3e29d93

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]>

kuvaldini mentioned this pull request Jul 30, 2021

v1.3: support/1.2.x with fixed DCO to main #1302

Closed

Pawlak00 mentioned this pull request Aug 6, 2021

Extending tx pagination meta hyperledger-iroha/iroha-java#83

Merged

kuvaldini pushed a commit that referenced this pull request Aug 31, 2021

[client-api] extending TxPaginationMeta in GetAccountTransactions, Ge…

23bdd07

…tAccountAssetsTransactions (#1092) Signed-off-by: Piotr Pawlowski <[email protected]>

baziorek added 1.x HL internship 2021 labels Sep 11, 2021

nxsaken added the iroha1 The legacy version of Iroha. label Apr 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extending queries by modifing TxPaginationMeta #1092

Extending queries by modifing TxPaginationMeta #1092

Pawlak00 commented Jun 18, 2021 •

edited

Loading

baziorek commented Jun 18, 2021

baziorek left a comment

baziorek left a comment

baziorek left a comment

kuvaldini commented Jul 9, 2021

baziorek left a comment •

edited

Loading

baziorek left a comment

kuvaldini commented Jul 22, 2021

kamilsa commented Jul 22, 2021

kamilsa commented Jul 22, 2021

baziorek commented Jul 22, 2021

kamilsa commented Jul 23, 2021

kuvaldini commented Jul 28, 2021 •

edited

Loading

kuvaldini commented Jul 28, 2021

Pawlak00 commented Jul 28, 2021

kuvaldini commented Jul 28, 2021 via email

Pawlak00 commented Jul 29, 2021

Extending queries by modifing TxPaginationMeta #1092

Extending queries by modifing TxPaginationMeta #1092

Conversation

Pawlak00 commented Jun 18, 2021 • edited Loading

Description of the Change

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

baziorek commented Jun 18, 2021

baziorek left a comment

Choose a reason for hiding this comment

baziorek left a comment

Choose a reason for hiding this comment

baziorek left a comment

Choose a reason for hiding this comment

kuvaldini commented Jul 9, 2021

baziorek left a comment • edited Loading

Choose a reason for hiding this comment

baziorek left a comment

Choose a reason for hiding this comment

kuvaldini commented Jul 22, 2021

kamilsa commented Jul 22, 2021

kamilsa commented Jul 22, 2021

baziorek commented Jul 22, 2021

kamilsa commented Jul 23, 2021

kuvaldini commented Jul 28, 2021 • edited Loading

kuvaldini commented Jul 28, 2021

Pawlak00 commented Jul 28, 2021

kuvaldini commented Jul 28, 2021 via email

Pawlak00 commented Jul 29, 2021

Pawlak00 commented Jun 18, 2021 •

edited

Loading

baziorek left a comment •

edited

Loading

kuvaldini commented Jul 28, 2021 •

edited

Loading