feat(odap): odap-hermes plugin first implementation by jscode017 · Pull Request #1275 · hyperledger-cacti/cacti

jscode017 · 2021-08-27T04:30:24Z

The first pull request for odap

jscode017 · 2021-08-27T04:30:39Z

codecov-commenter · 2021-08-27T06:27:33Z

Codecov Report

Merging #1275 (4f579ea) into main (4b075f7) will increase coverage by 0.24%.
The diff coverage is 71.93%.

@@            Coverage Diff             @@
##             main    #1275      +/-   ##
==========================================
+ Coverage   69.78%   70.03%   +0.24%     
==========================================
  Files         336      368      +32     
  Lines       12626    14108    +1482     
  Branches     1513     1701     +188     
==========================================
+ Hits         8811     9880    +1069     
- Misses       2992     3299     +307     
- Partials      823      929     +106

Impacted Files	Coverage Δ
...enerated/openapi/typescript-axios/configuration.ts	`9.09% <9.09%> (ø)`
...cript/generated/openapi/typescript-axios/common.ts	`42.85% <42.85%> (ø)`
...escript/gateway/common/transfer-complete-helper.ts	`53.12% <53.12%> (ø)`
...escript/generated/openapi/typescript-axios/base.ts	`58.33% <58.33%> (ø)`
...rc/main/typescript/gateway/common/commit-helper.ts	`58.40% <58.40%> (ø)`
.../typescript/gateway/common/lock-evidence-helper.ts	`60.83% <60.83%> (ø)`
...hermes/src/main/typescript/gateway/odap-gateway.ts	`69.36% <69.36%> (ø)`
...escript/gateway/common/initiate-transfer-helper.ts	`80.70% <80.70%> (ø)`
...ugin-odap-hermes/src/main/typescript/public-api.js	`83.33% <83.33%> (ø)`
...pescript/generated/openapi/typescript-axios/api.ts	`86.06% <86.06%> (ø)`
... and 33 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 4b075f7...4f579ea. Read the comment docs.

petermetz

@jscode017 Please remove all package-lock.json files, we no longer need them since the migration to yarn.

packages/cactus-odap-hermes/src/main/json/openapi.json

...odap-hermes/src/test/typescript/integration/odap/odap-api-call-with-ledger-connector.test.ts

packages/cactus-odap-hermes/README.md

petermetz

@jscode017 Sorry for the slow response, I left an initial batch of comments in the code but likely there'll be more once I took a second, more detailed pass.

What I would also like to ask you is to consider submitting multiple pull requests, one for the besu connector changes that you added, another one for the fabric connector changes and then finally the odap plugin changes which would be a third PR that you can mark as dependent on the previous two so that the merging order is guaranteed to be the correct one.

package.json

...-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/src/assetTransfer.ts

petermetz · 2021-09-01T00:26:31Z

@jscode017 If you need help with conflict resolution just let me know!

RafaelAPB · 2021-09-02T15:38:56Z

@petermetz thanks for your detailed review. The variables can be camelCase. The standard does not enforce it at this level.

@jscode017 let me know when you will be able to make the requested changes on the docs!

petermetz · 2021-09-02T16:20:24Z

@petermetz thanks for your detailed review. The variables can be camelCase. The standard does not enforce it at this level.

@jscode017 let me know when you will be able to make the requested changes on the docs!

@RafaelAPB No worries!
@jscode017 If you need help with conflict resolution just let me know, happy to help! Either here or via the usual channels including the daily pair programming sessions.

jscode017 · 2021-09-03T04:16:11Z

@petermetz thanks for your detailed review. The variables can be camelCase. The standard does not enforce it at this level.

@jscode017 let me know when you will be able to make the requested changes on the docs!

@RafaelAPB
I would first try to resolve some code reviews and rebase. Then starts with redrawing the flow chart of odap(I would want to add it in the doc). Then I would do the docs :).

jscode017 · 2021-09-03T10:28:11Z

@petermetz @RafaelAPB Its weird, the situation remains the same. I would fail at one machine but pass other twos

jscode017 · 2021-09-06T11:40:09Z

@RafaelAPB I made a small change to readme
Are there some specific things or directions you would want me to do?

RafaelAPB · 2021-09-09T14:50:01Z

Looks good to me! @jscode017 @petermetz

petermetz

@jscode017 Are there two duplicated LockAsset.json files or the two separate ones are different for a specific reason?

.cspell.json

...lugin-ledger-connector-fabric/src/main/typescript/common/get-transaction-receipt-by-tx-id.ts

petermetz · 2021-09-14T20:21:53Z

@petermetz @RafaelAPB Its weird, the situation remains the same. I would fail at one machine but pass other twos

@jscode017 The CI is in a bit of a pickle right now, we are trying to stabilize it as we speak, please hang tight with regards that.

RafaelAPB · 2021-09-19T20:03:59Z

@jscode017 Jason, i've noticed some things: https://github.com/jscode017/cactus/blob/fabric-receipt/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/common/get-transaction-receipt-by-tx-id.ts#L12 --> this function should be called getTransactionReceiptByTxID because it is generic. On the other hand, https://github.com/jscode017/cactus/blob/fabric-receipt/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts#L1078 this function should be called getTransactionReceiptForLockContractByTxID because it is meant specifically for the functionality of ODAP.
Also, it should be generic enough because there might be different types of receipts.

@petermetz does this makes sense?

jscode017 · 2021-09-20T13:39:34Z

@jscode017 Jason, i've noticed some things: https://github.com/jscode017/cactus/blob/fabric-receipt/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/common/get-transaction-receipt-by-tx-id.ts#L12 --> this function should be called getTransactionReceiptByTxID because it is generic. On the other hand, https://github.com/jscode017/cactus/blob/fabric-receipt/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts#L1078 this function should be called getTransactionReceiptForLockContractByTxID because it is meant specifically for the functionality of ODAP.
Also, it should be generic enough because there might be different types of receipts.

@petermetz does this makes sense?

@RafaelAPB , I am thinking that is that a way that the function getTransactionReceiptForLockContractByTxID(options)https://github.com/jscode017/cactus/blob/08e7001826d1218827aba3590dc2a3ca29f8bacd/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts#L1087
could be parameterized?
e.g we call a interface here, and pass in the implementation function when we construct fabric connector

petermetz · 2021-09-24T18:28:04Z

@petermetz does this makes sense?

@RafaelAPB Yup, makes sense.

e.g we call a interface here, and pass in the implementation function when we construct fabric connector

@jscode017 When you say "pass in the implementation function" do you literally mean passing in a variable that contains a reference to a function?

RafaelAPB · 2021-09-27T01:37:05Z

@jscode017 Do you need any help completing the PR?

jscode017 · 2021-09-27T01:45:30Z

@jscode017 Do you need any help completing the PR?

@RafaelAPB , I think I would first need to complete the dependency: #1302
Could you take a look at my fabric-transact-receipt?
I may need to formalize the receipt as peter suggested.
#1302 (comment)

RafaelAPB · 2021-09-27T20:18:53Z

@jscode017 Do you need any help completing the PR?

@RafaelAPB , I think I would first need to complete the dependency: #1302
Could you take a look at my fabric-transact-receipt?
I may need to formalize the receipt as peter suggested.
#1302 (comment)

Done

odap_log

...dap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/VERSION

...odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator-ignore

packages/cactus-plugin-odap-hermes/package.json

packages/cactus-plugin-odap-hermes/src/main/typescript/public-api.d.ts

...-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/src/assetTransfer.ts

petermetz

@jscode017 I pushed a commit with a refreshed yarn lock file that will hopefully resolve the issue that the CI was having

...odap-hermes/src/test/typescript/fabric-contracts/lock-asset/chaincode-typescript/tslint.json

...connector-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/tslint.json

...-connector-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/.gitignore

petermetz

@jscode017

The CI is now failing with the issue of the build generating files that are not under version control, see details I took from the logs below.
You can probably solve it by running the configure script and then committing the generated files to version control. Let me know if you need more help and I can take a deeper look!

2021-11-01T22:49:35.3855857Z 
2021-11-01T22:49:35.3857051Z Build at: 2021-11-01T22:49:35.099Z - Hash: 01acc4f688a0221c5146 - Time: 31553ms
2021-11-01T22:49:35.4044380Z Done in 390.01s.
2021-11-01T22:49:35.4187019Z ++ date +%s
2021-11-01T22:49:35.4208057Z + ENDED_AT=1635806975
2021-11-01T22:49:35.4208523Z + runtime=9205
2021-11-01T22:49:35.4211957Z ++ date +%FT%T%z
2021-11-01T22:49:35.4227641Z + echo '2021-11-01T22:49:35+0000 [CI] SUCCESS - runtime=9205 seconds.'
2021-11-01T22:49:35.4228730Z 2021-11-01T22:49:35+0000 [CI] SUCCESS - runtime=9205 seconds.
2021-11-01T22:49:35.4229328Z + checkWorkTreeStatus
2021-11-01T22:49:35.4230051Z + git update-index -q --refresh
2021-11-01T22:49:36.3688502Z ++ git diff-index --name-only HEAD --
2021-11-01T22:49:36.3763978Z + new_changed_files=packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES
2021-11-01T22:49:36.3766163Z + '[' '' '!=' packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES ']'
2021-11-01T22:49:36.3767644Z + echo 'Changes in the git index have been detected!'
2021-11-01T22:49:36.3768296Z Changes in the git index have been detected!
2021-11-01T22:49:36.3768759Z + git diff
2021-11-01T22:49:36.3831080Z diff --git a/packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES b/packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES
2021-11-01T22:49:36.3832832Z + exit 1
2021-11-01T22:49:36.3833242Z index a80cd4f..53250c0 100644
2021-11-01T22:49:36.3834783Z --- a/packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES
2021-11-01T22:49:36.3836925Z +++ b/packages/cactus-plugin-odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/FILES
2021-11-01T22:49:36.3838101Z @@ -1,8 +1,5 @@
2021-11-01T22:49:36.3838727Z -.gitignore
2021-11-01T22:49:36.3839201Z -.npmignore
2021-11-01T22:49:36.3839527Z  api.ts
2021-11-01T22:49:36.3839833Z  base.ts
2021-11-01T22:49:36.3840140Z  common.ts
2021-11-01T22:49:36.3840515Z  configuration.ts
2021-11-01T22:49:36.3841013Z -git_push.sh
2021-11-01T22:49:36.3841679Z  index.ts
2021-11-01T22:49:36.3974255Z 2021-11-01T22:49:36+0000 [CI] FAILURE - runtime=9206 seconds.
2021-11-01T22:49:36.4403370Z ##[error]Process completed with exit code 1.

petermetz

@jscode017 1.0.0-rc.2 just went out so you'll also need to update everything in your package from 1.0.0-rc.1 to 1.0.0-rc.2 accordingly.

jscode017 · 2021-11-03T04:55:04Z

@jscode017 1.0.0-rc.2 just went out so you'll also need to update everything in your package from 1.0.0-rc.1 to 1.0.0-rc.2 accordingly.

sure, I have updated it

petermetz · 2021-11-03T22:11:44Z

@jscode017 1.0.0-rc.2 just went out so you'll also need to update everything in your package from 1.0.0-rc.1 to 1.0.0-rc.2 accordingly.

sure, I have updated it

@jscode017 Thank you! Ping me with the "re-request review" button once all the other stuff is addressed as well (or if you have questions about any of those other comments I made)

jscode017 · 2021-11-05T11:53:51Z

@jscode017 1.0.0-rc.2 just went out so you'll also need to update everything in your package from 1.0.0-rc.1 to 1.0.0-rc.2 accordingly.

sure, I have updated it

@jscode017 Thank you! Ping me with the "re-request review" button once all the other stuff is addressed as well (or if you have questions about any of those other comments I made)

@petermetz , Sure. I believe have pushed the re-request review button, thanks.

Signed-off-by: jsjs026 <jasonhack518@gmail.com>

RafaelAPB · 2021-11-07T10:49:23Z

@CatarinaPedreira Almost ready! :)

petermetz

@jscode017 LGTM, thank you for fixing the CI and updating the versions!

jonathan-m-hamilton

LGTM

jscode017 force-pushed the odap-pr branch 2 times, most recently from 5b72ba0 to 689bc5e Compare August 31, 2021 11:49

jscode017 marked this pull request as ready for review August 31, 2021 14:01

jscode017 requested review from jonathan-m-hamilton and petermetz as code owners August 31, 2021 14:01

petermetz requested changes Sep 1, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

...-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/src/assetTransfer.ts Show resolved Hide resolved

jscode017 force-pushed the odap-pr branch from 9cd3b48 to 3f77a71 Compare September 3, 2021 06:15

jscode017 force-pushed the odap-pr branch from e5c846a to 6589c10 Compare September 6, 2021 11:39

jscode017 force-pushed the odap-pr branch from 3b8e34d to a562030 Compare September 8, 2021 05:41

petermetz requested changes Sep 13, 2021

View reviewed changes

.cspell.json Outdated Show resolved Hide resolved

petermetz requested changes Sep 13, 2021

View reviewed changes

...lugin-ledger-connector-fabric/src/main/typescript/common/get-transaction-receipt-by-tx-id.ts Show resolved Hide resolved

github-actions bot added the dependent label Sep 14, 2021

jscode017 changed the title ~~feat(odap): odap first pr~~ feat(odap): odap-hermes plugin first implementation Oct 5, 2021

jscode017 force-pushed the odap-pr branch 3 times, most recently from 92a06ac to 80f3b59 Compare October 16, 2021 05:43

petermetz requested changes Oct 19, 2021

View reviewed changes

odap_log Outdated Show resolved Hide resolved

jscode017 force-pushed the odap-pr branch from 80f3b59 to 768a860 Compare October 19, 2021 16:10

petermetz requested changes Oct 19, 2021

View reviewed changes

...dap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator/VERSION Outdated Show resolved Hide resolved

...odap-hermes/src/main/typescript/generated/openapi/typescript-axios/.openapi-generator-ignore Show resolved Hide resolved

petermetz requested changes Oct 21, 2021

View reviewed changes

packages/cactus-plugin-odap-hermes/package.json Show resolved Hide resolved

jscode017 force-pushed the odap-pr branch 2 times, most recently from 32c59c1 to a7fac07 Compare October 29, 2021 13:50

jscode017 requested a review from petermetz October 30, 2021 12:30

petermetz reviewed Nov 1, 2021

View reviewed changes

packages/cactus-plugin-odap-hermes/src/main/typescript/public-api.d.ts Outdated Show resolved Hide resolved

...-fabric/src/test/typescript/fixtures/go/lock-asset/chaincode-typescript/src/assetTransfer.ts Show resolved Hide resolved

petermetz requested changes Nov 1, 2021

View reviewed changes

jscode017 force-pushed the odap-pr branch from 0ae46f4 to ae00741 Compare November 2, 2021 00:18

petermetz requested changes Nov 2, 2021

View reviewed changes

jscode017 force-pushed the odap-pr branch from ae00741 to 2680ba0 Compare November 3, 2021 03:53

jscode017 requested a review from petermetz November 3, 2021 10:54

jscode017 force-pushed the odap-pr branch from ca9643b to b2cd951 Compare November 3, 2021 10:56

feat(odap): first implemenation for odap plugin and endpoints

0e5418d

Signed-off-by: jsjs026 <jasonhack518@gmail.com>

jscode017 force-pushed the odap-pr branch from b2cd951 to 0e5418d Compare November 5, 2021 11:55

petermetz approved these changes Nov 8, 2021

View reviewed changes

Merge branch 'main' into odap-pr

4f579ea

jonathan-m-hamilton approved these changes Nov 9, 2021

View reviewed changes

petermetz merged commit 51bf753 into hyperledger-cacti:main Nov 9, 2021

AndreAugusto11 mentioned this pull request Mar 9, 2022

refactor(plugin-odap-gateway): refactor implementation including more tests #1902

Merged

RafaelAPB mentioned this pull request Mar 14, 2022

Gateway-based interoperability: ODAP support #517

Closed

Comments

Conversation

jscode017 commented Aug 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jscode017 commented Aug 27, 2021

Uh oh!

codecov-commenter commented Aug 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

petermetz commented Sep 1, 2021

Uh oh!

RafaelAPB commented Sep 2, 2021

Uh oh!

petermetz commented Sep 2, 2021

Uh oh!

jscode017 commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jscode017 commented Sep 3, 2021

Uh oh!

jscode017 commented Sep 6, 2021

Uh oh!

RafaelAPB commented Sep 9, 2021

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

petermetz commented Sep 14, 2021

Uh oh!

RafaelAPB commented Sep 19, 2021

Uh oh!

jscode017 commented Sep 20, 2021

Uh oh!

petermetz commented Sep 24, 2021

Uh oh!

RafaelAPB commented Sep 27, 2021

Uh oh!

jscode017 commented Sep 27, 2021

Uh oh!

RafaelAPB commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

Uh oh!

petermetz left a comment

Choose a reason for hiding this comment

jscode017 commented Aug 27, 2021 •

edited

Loading

codecov-commenter commented Aug 27, 2021 •

edited

Loading

jscode017 commented Sep 3, 2021 •

edited

Loading

RafaelAPB commented Sep 27, 2021 •

edited

Loading