Add OpenAPI extension for auto-documenting Restlet applications' APIs#1484
Add OpenAPI extension for auto-documenting Restlet applications' APIs#1484
Conversation
jlouvel
left a comment
jlouvel
left a comment
There was a problem hiding this comment.
Thank you @anicolas for this significant contribution!
I've left a few comments along the way, also could you add the boilerplate copyright header, including your name as author (additional author if you reused significant code from the older Swagger extension)?
| import org.restlet.routing.Filter; | ||
| import org.restlet.routing.Router; | ||
|
|
||
| public class RestletOpenApiApplication extends Application { |
There was a problem hiding this comment.
I would suggest "OpenApiApplication" as it extends Restlet's Application, which makes it explicit enough
|
|
||
| private Paths paths = new Paths(); | ||
|
|
||
| private OpenAPI openAPI = new OpenAPI(); |
There was a problem hiding this comment.
Could we rename to "openApi" for consistency within RF?
| } | ||
| } | ||
| } else { | ||
| LOGGER.info("Route type ignored: {}", route.getClass()); |
There was a problem hiding this comment.
Could we access the Logger like other RF code, not directly via SLF4J?
There was a problem hiding this comment.
Oh, nice catch, I wasn't aware of this!
|
|
||
| import java.util.Optional; | ||
|
|
||
| class OpenApiAnnotationProcessor { |
There was a problem hiding this comment.
Could we make this class 'public', as well as other classes in the openapi package? That would be consistent with the RF codebase.
There was a problem hiding this comment.
Sure, I tried to minimize the public API but let's be consistent!
|
This looks great. Exciting progress!! My last comment would be to move all supporting classes from org.restlet.ext.openapi to org.restlet.ext.openapi.internal package, as done for other extension, to keep the top package level as clean as possible. If those classes can be directly used or exposed to the user, then it is fine to keep them that way. @thboileau Did you get a chance to review as well? |
|
|
||
| private Router router; | ||
|
|
||
| private OpenAPIConfiguration config; |
There was a problem hiding this comment.
should we define a default configuration (to prevent NPE in the processTypeToContent method)?
There was a problem hiding this comment.
Nice catch, thanks! I think it's better to immediately fail in read if we do not have it (same for router). I added two checks in the read method to fail early.
thboileau
left a comment
thboileau
left a comment
There was a problem hiding this comment.
it's fine for the next release: 2.7.0-m3
|
3 participants
The aim
Add OpenAPI extension for auto-documenting Restlet applications APIs.
The solution
Introduce a new
org.restlet.ext.openapiextension that leveragesswagger-coreandswagger-annotationsunder-the-hood.Reproduction
Check-list
DO NOT REVIEW