Adding java chaincode for private-data sample #345
Conversation
sijocherian
force-pushed
the
asset-privatedata.javachaincode
branch
from
f06fed4
to
dc7aacf
Compare
| errorMessage = String.format("Empty input in Transient map: color"); | ||
| } | ||
| if (size <= 0) { | ||
| errorMessage = String.format("Empty input in Transient map: size"); |
There was a problem hiding this comment.
If empty it would be <= 0 ?
There was a problem hiding this comment.
size being int, will have default of 0. But I just realized that json.getInt() will throw Exception if key is not present.
For parity with chaicode-go, I will keep the error check, but will alter the json parsing to deal with empty values in transient map. Let me know if you think, otherwise.
| errorMessage = String.format("Empty input in Transient map: size"); | ||
| } | ||
| if (appraisedValue <= 0) { | ||
| errorMessage = String.format("Empty input in Transient map: appraisedValue"); |
There was a problem hiding this comment.
If empty it would be <= 0 ?
| System.out.printf("CreateAsset Put: collection %s, ID %s\n", ASSET_COLLECTION_NAME, assetID); | ||
| System.out.printf(" Put: collection %s, ID %s\n", ASSET_COLLECTION_NAME, new String(asset.serialize())); |
There was a problem hiding this comment.
Are you using some convention of indents?
There was a problem hiding this comment.
I was using an indent thinking it is easier to read, but it is probably not useful when reading chaincode logs. I will remove indent.
| ChaincodeStub stub = ctx.getStub(); | ||
|
|
||
| CompositeKey aggKey = stub.createCompositeKey(AGREEMENT_KEYPREFIX, assetID); | ||
| System.out.printf(" ReadTransferAgreement Put: collection %s, ID %s, Key %s\n", ASSET_COLLECTION_NAME, assetID, aggKey); |
There was a problem hiding this comment.
Get not Put.
Are you using some convention of indents?
| System.err.println(errorMessage); | ||
| throw new ChaincodeException(errorMessage, AssetTransferErrors.INCOMPLETE_INPUT.toString()); | ||
| } | ||
| if (assetPriv.getAppraisedValue() <= 0) { // appraisedValue field must be a positive integer |
There was a problem hiding this comment.
If appraised value wasn't passed, would it be 0?
There was a problem hiding this comment.
verified that if appraisedVal is not passed, it goes into catch block in ln 367
| import org.junit.jupiter.api.Nested; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public final class AssetTransferTest { |
There was a problem hiding this comment.
This test is good, although I don't think we necessarily need it for every sample.
I would rather see an integration test using the existing javascript application in ci/azure-pipelines.yml , that would ensure everything works end-to-end and that there is functional parity between the go chaincode and java chaincode. Not to mention it would be less work to re-use the existing application to drive the end-to-end test.
There was a problem hiding this comment.
Agree with JS-app & CI-pipeline to drive auto integration test.
I still think few good/complex chaincode unit test samples, goes a long way for chaincode developer experience.
Unit tests (especially) in chaincode saves a lot of dev time. This test shows some advanced ClientIdentity & Context mocking in java. If we have further discussion here on this and find this useful, I can extend the PrivData tutorial to describe the unit-test.
My personal experience was chaincode dev iteration takes longer than normal development, if you have to deploy to a network and test with some kind of app.
There was a problem hiding this comment.
Sounds good! I agree it would be good to mention the unit test approach in the https://hyperledger-fabric.readthedocs.io/en/latest/chaincode4ade.html tutorial as a baseline, and then mention any of the more advanced approaches in the other advanced tutorials.
Java chaincode that can be deployed via test-network, with shortname 'private' Unit test for core transactions Bugfix for QueryAsset txn function signature Incorporated PR feedback Signed-off-by: Sijo Cherian <sijo@ibm.com>
sijocherian
force-pushed
the
asset-privatedata.javachaincode
branch
from
dc7aacf
to
dfa4cf7
Compare
|
Incorporated PR so far. Ready for another look. |
|
This one is ready for review. |
-Reflects all transactions as in chaincode-go
-Tested with application JS