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.

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

Fix JSON schema output for custom contracts #1427

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

nguyer
Copy link
Contributor

@nguyer nguyer commented Nov 20, 2023

Previously we were generating an incorrect JSON Schema for the response body on custom contract endpoints. This PR fixes this so that:

  • /inovke endpoints always return the correct schema for a FireFly Operation (which is the actual payload they return)
  • /query endpoints now return the correct schema based on the return value of the smart contract itself
    • If outputs are unnamed, the swagger generator now names them output, output1, output2, etc. like ETH/EVMConnect name them

@nguyer nguyer requested a review from a team as a code owner November 20, 2023 19:27
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (951e655) 99.99% compared to head (4d87450) 99.99%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1427   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files         321      321           
  Lines       23068    23088   +20     
=======================================
+ Hits        23066    23086   +20     
  Misses          1        1           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

},
// // TODO change to operation schema
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand what's left for this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I did it and forgot to remove the comment :) I will remove it

@@ -193,6 +192,40 @@ func contractJSONSchema(ctx context.Context, params *fftypes.FFIParams, hasLocat
return openapi3.NewSchemaRef("", s), nil
}

/**
* Parse the FFI and build a corresponding JSON Schema to describe the response body for "invoke" or "query".
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this response body is only for query outputs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Good catch. Misleading comments are the worst kind of comments.

@nguyer nguyer merged commit 50d2acd into hyperledger:main Nov 29, 2023
14 checks passed
@nguyer nguyer deleted the ffi2swagger-output branch November 29, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants