-
Notifications
You must be signed in to change notification settings - Fork 113
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 Java OpenAPI generation for REST API #2447
Fix Java OpenAPI generation for REST API #2447
Conversation
Signed-off-by: Long Nguyen <[email protected]>
…ditional_properties on state proofs Signed-off-by: Long Nguyen <[email protected]>
4cab69d
to
0671262
Compare
Codecov Report
@@ Coverage Diff @@
## main #2447 +/- ##
=========================================
Coverage 90.83% 90.83%
Complexity 2432 2432
=========================================
Files 420 420
Lines 11593 11593
Branches 1013 1013
=========================================
Hits 10530 10530
Misses 732 732
Partials 331 331 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Do you mind adding the execution of the openapi generator so that we don't break you guys in the future? Something like this to hedera-mirror-rest/pom.xml
:
<plugin>
<groupId>org.openapitools</groupId>
<artifactId>openapi-generator-maven-plugin</artifactId>
<version>${openapi-generator.version}</version>
<executions>
<execution>
<goals>
<goal>generate</goal>
</goals>
<configuration>
<inputSpec>${project.basedir}/api/v1/openapi.yml</inputSpec>
<generatorName>java</generatorName>
</configuration>
</execution>
</executions>
</plugin>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you all so much for the approvals! I'll be adding another commit with the actual generation execution @steven-sheehy mentioned. |
a635d12
Signed-off-by: Long Nguyen <[email protected]>
a635d12
to
489cb5a
Compare
Looks like the generation succeeded in REST API / test (v1) (pull_request)!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
* Fix 400 ref bug in OpenAPI spec * Add properly named response objects for major API calls and delete additional_properties on state proofs * Add Java generation of OpenAPI spec for validation Signed-off-by: Long Nguyen <[email protected]>
* Fix 400 ref bug in OpenAPI spec * Add properly named response objects for major API calls and delete additional_properties on state proofs * Add Java generation of OpenAPI spec for validation Signed-off-by: Long Nguyen <[email protected]>
Description:
This PR modifies the OpenAPI YAML spec file in order to support Java REST client generation when using OpenAPITools/openapi-generator.
Notes for reviewer:
400
errors was actually wrong: you were actually nesting a schema reference within an existing definition, so you ended up with the following.became
, which, of course, breaks the build when you do try to generate the client files. Doing this change resulted in much more consistent response code blocks (400s and 404s now share the same structure of pointing to a reference).
InlineResponseXXX
objects when generated, which is not good for naming. Screenshot included below. I moved these to their own reference names—this also keeps consistency with the overall structure of writing this spec file.additional_properties
from the spec for state proofs to get generation to work. According to the current alpha API for state proofs, there is no such property in the response, which might explain why it breaks generation.Checklist