Javalin: OpenApi Plugin Improvements

Created on 28 Aug 2019  路  18Comments  路  Source: tipsy/javalin

Javalin creates a basic documentation for all paths added to the Pathmatcher. If I enable cors for all origins, a basic documentation is added

"/*": {
      "options": {
        "summary": "Options with wildcard",
        "operationId": "optionsWithWildcard"
      }
    }

Now I want to document or drop this section from documentation, how can I do that?
Also this is not valid OpenApi 3.0 because the response section is missing (see OpenApi Spec. Operation Object

QUESTION

Most helpful comment

Ok, i try to finish this in <2 weeks

All 18 comments

@maxemann96 If @TobiasWalle didn't built it in, maybe there should be an option to exclude paths?

There is already a way to hide paths (ignore = true in Annotations or ignore() method in DSL handler). But this cannot be used by paths added by plugins (like the CorsPlugin) or other "externally" added paths.

I think it would be better that I can myself add documentation to "externally" added paths, one way would be to make handlerMetaInfoList in OpenApiHandler public or something like that.

Second the default documentation is not standard conform, a responses object is required. A problem from simply adding this in openBuilderDsl is that OpenApi expects at least 1 response in responses object which "SHOULD" be a successfull response.

There is already a way to hide paths (ignore = true in Annotations or ignore() method in DSL handler). But this cannot be used by paths added by plugins (like the CorsPlugin) or other "externally" added paths.
I think it would be better that I can myself add documentation to "externally" added paths, one way would be to make handlerMetaInfoList in OpenApiHandler public or something like that.

Yeah, the other plugins can't know about the OpenAPI plugin, so having an option to modify the handlerMetaInfoList sounds like an okay solution. I don't mind making the list public, please feel free to create a PR.

Second the default documentation is not standard conform, a responses object is required. A problem from simply adding this in openBuilderDsl is that OpenApi expects at least 1 response in responses object which "SHOULD" be a successfull response.

I don't understand this paragraph (my OpenAPI knowledge is lacking). This sounds more like a general problem than something related to CORS ?

I don't understand this paragraph (my OpenAPI knowledge is lacking). This sounds more like a general problem than something related to CORS ?

Yes, this is a separate problem not regarded to the cors plugin. OpenApi wants to have a responses object for each path added, containing at least one response which should be a positive response (200 <= response code <= 299).

Yeah, the other plugins can't know about the OpenAPI plugin, so having an option to modify the handlerMetaInfoList sounds like an okay solution. I don't mind making the list public, please feel free to create a PR.

I will create this in the next days, also checking in the cors plugin, if the openapi plugin is present and automatically add the correct documentation.

@TobiasWalle Do you have a better idea regarding to handlerMetaInfoList?

@maxemann96 I already thought about this problem but I didn't have time to implement it.

My preferred solution would be if you could exclude paths from the documentation via the Plugin configuration. For example:

        val openApiOptions = OpenApiOptions(Info().version("1.0").description("My Application"))
                .path("/swagger-docs")
                .ignoreDocumentation("/*")
                .ignoreDocumentation("/*", HttpMethod.GET)
                .ignoreDocumentationWithPathPrefix("/my-ignored-paths")
                .ignoreDocumentationWithPathPrefix("/my-ignored-paths", HttpMethod.POST)

Then we could filter out all the documentation for the matching paths without tinkering with internal objects.

If we want to override or set some documentation we could do it the same way (But the method is mandatory):

        val openApiOptions = OpenApiOptions(Info().version("1.0").description("My Application"))
                .path("/swagger-docs")
                .setDocumentation("/*", HttpMethod.GET, document())

What do you think about this?

I intentionally didn't include any checks if the swagger definition ist correct, because that would be out of scope of this plugin. If we want to check the generated file, we should use an official library that does that. As the we use the original OpenAPI class, this shouldn't be much of a problem.

But the default should be correct, maybe we can add a default 200 Response if the user didn't define anything.

@TobiasWalle I created a work in progress pullrequest for this, pls checkout the checklist.

I also think that a check is out of scope. Maybe we can check the generated json with the Specification Schema and log a warning?

We can add a default 200 with the hint (in generated json and log) that the documentation is missing (or a 418 indicating that the api creator forgot something :)

Excluding a path list and overriding some documentation sounds good.

@maxemann96 I checked out your list and updated it a little bit. All Features should be tested.

  • [ ] Add OpenApi documentation to the CORS Plugin (I don't think disabling is necessary)
  • [ ] Add default 200 response, if no response is defined
  • [ ] Add ignoreDocumentation and ignoreDocumentationWithPathPrefix Api
  • [ ] Add overrideDocumentation Api
  • [ ] Warn about invalid swagger schemas

It would be great if you would a separate PR for each point on the list. This makes reviewing it much easier.

Since i cannot edit your comment (so i cannot "tick" the feature list), heres a copy:

  • [x] Ignore OpenApi documentation in CORS Plugin #732
  • [x] Add default 200 response, if no response is defined #734
  • [x] Add ignoreDocumentation and ignoreDocumentationWithPathPrefix Api #733
  • [x] Add overrideDocumentation Api #747
  • [x] Warn about invalid openapi schemas #736

@tipsy When do you plan to release the next version? Then i will try to finish this issue due the date :)

I haven't set a date yet. Would be nice if it could be out in a couple of weeks?

Ok, i try to finish this in <2 weeks

@maxemann96 can you rename this issue to reflect the remaining work?

@maxemann96 is this done now?

Yes, all things done

Perfect, great work!

Hi, I'm new to Javalin but i have a requirement to implement OpenApi without registring with javalin and produce swagger documentation. How can i implement this please help asap.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

valtterip picture valtterip  路  5Comments

gane5h picture gane5h  路  3Comments

vikascn picture vikascn  路  4Comments

accron-1 picture accron-1  路  4Comments

ShikaSD picture ShikaSD  路  5Comments