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

Specification Document Splitting Clarification #463

Open
scottcurtis2605 opened this issue Jan 7, 2021 · 10 comments
Open

Specification Document Splitting Clarification #463

scottcurtis2605 opened this issue Jan 7, 2021 · 10 comments

Comments

@scottcurtis2605
Copy link
Contributor

scottcurtis2605 commented Jan 7, 2021

Currently, the Location and formats section of the MP OpenAPI specification reads as follows:

Vendors are required to fetch a single document named openapi with an extension of yml, yaml or json, inside the application module’s root META-INF folder. If there is more than one document found that matches one of these extensions the behavior of which file is chosen is undefined (i.e. each vendor may implement their own logic), which means that application developers should only place a single openapi document into that folder.

Here, the specification states that vendors are only required to retrieve a single document, which contrasts with the OpenAPI specification, which states the following:

An OpenAPI document MAY be made up of a single document or be divided into multiple, connected parts at the discretion of the user. In the latter case, $ref fields MUST be used in the specification to reference those parts as follows from the JSON Schema definitions.

From local tests, it would seem that splitting the OpenAPI document across multiple files, as permitted by the OpenAPI specification, is not supported within MP OpenAPI implementations. If this is the case, the MP OpenAPI specification is a little vague on whether splitting is permitted, as the MP specification states that vendors are only required to fetch a single document:

Vendors are required to fetch a single document named openapi with an extension of yml, yaml or json, inside the application module’s root META-INF folder.

This stems from an issue raised against Open Liberty #OpenLiberty/open-liberty#15428. This was further tested with Open Liberty implementation of 2.0 which consumes SmallRye, which displayed the same behaviour as described in the issue.

If splitting is not supported by MP OpenAPI, I believe we should update the specification to clarify and reflect this.

@EricWittmann
Copy link
Contributor

I wouldn't say that the MP OpenAPI specification doesn't support splitting, but it depends on exactly what use-case you're looking for. For example, if you want a way for the MP OpenAPI compatible platform to produce split output, perhaps with all of the schemas in one file and the paths/operations in another file, then you are correct that is not supported.

However, I don't see why you couldn't use existing annotations and also the static file support to insert $references to external definitions using absolute URLs.

I can imagine wanting to package applications by bundling domain java beans into a re-usable JAR library that also includes an openapi.yaml file with those same java beans defined as JSON Schemas. And then wanting to use such a library in an MP OpenAPI application and have the platform generate a $ref to an appropriate pre-defined JSON Schema rather than auto-generating new JSON Schemas from the Java code. I'm not sure if that's what you're looking for, but that would be super cool. It might be more of a SmallRye feature than a spec feature though...

In any case, can you expand on what you're specifically trying to do? The linked issue seems to describe a use-case where you want to include multiple static openapi YAML files in the deployment and be able to have references to them. Is that it?

@MikeEdgar
Copy link
Member

To add to @EricWittmann's comment, splitting should be supported by the spec using $refs and absolute URLs. The part that is not specifically supported is that the external files are available under the /openapi context (as seems to be the expectation in the linked OL issue). If the externally referenced file were available at the application's context path (for example), it should work - provided no other limitation imposed by OL's validation of the $refs.

@igbluz
Copy link

igbluz commented Jan 11, 2021

@EricWittmann: Your imagination is actually just this what I need, to have a re-usable JAR library with the openapi.yaml and the java beans defined as JSON Schemas. Yes I would like to include some openapi YAML files into a main openapi YAML file by some reference. Actually with OL this does not work. I also tried the hint from @MikeEdgar and using the following structure:
src
!- main
!-- webapp
!--- META-INF
!---- problem
!----- problem.yaml
!---- openapi.yaml

with the following code in the openapi.yaml

        '500':
          description: Internal server Error
          content:
            application/json:
              schema:
                $ref: 'problem/problem.yaml#/components/schemas/InternalProblem'

but this still gives me the following OL message:

[INFO] - Message: The "problem/problem.yaml#/components/schemas/InternalProblem" value is an invalid reference, Location: #/paths/1phonenumber1validator/get/responses/500/content/application~1json/schema

@msmiths
Copy link
Contributor

msmiths commented Jan 11, 2021

@MikeEdgar This is just validation code within OL that outputs informational data to the logs... it does not affect the model that is generated. So, if the SmallRye implementation correctly handles references, then the model generated by the mpOpenAPI-2.0 feature in OL should be correct. We would need to update the validation code in the mpOpenAPI-2.0 feature to reflect this in order to avoid generating misleading log entries.

@igbluz
Copy link

igbluz commented Jan 12, 2021

@scottcurtis2605 If I undestand correctly there will be no support for splitting the OpenAPI document across multiple yaml files on the MP Open API 2.0 level and therefore also no support for OL, correct? So I will have to use one openapi.yaml and to define all JSON Schema self contained in the same YAML. If so I will close my reported bug.

@MikeEdgar
Copy link
Member

If the message from OL is only a warning, I still think should work fine. The key is putting the separate file in an accessible location and using an absolute path in the ref.

@scottcurtis2605
Copy link
Contributor Author

scottcurtis2605 commented Jan 12, 2021

@igbluz Yes you are correct. As of the initial release for OL mpOpenAPI-2.0, we do not officially support document splitting; this may be implemented at a later date.

@msmiths
Copy link
Contributor

msmiths commented Jan 14, 2021

@MikeEdgar I am not convinced that we can claim that document splitting is supported as long as the user follows the caveats that you have outlined, i.e., the referenced files are put in an accessible location and absolute references are used. My understanding of what @igbluz is trying to achieve is that the referenced files should be packaged along with the implementation classes, i.e., resource class files and the YAML document contained in the same JAR file. This JAR is then packaged in the relevant WAR and the openapi.yaml includes references to schemas defined by the YAML document embedded in the JAR. I am not an expert on references, but I believe that the intention is that the runtime should be able to resolve the references without needing to make an HTTP request to a file that is really local.

I believe that it is worth putting more thought into this scenario in a future version of the MP OpenAPI specification and providing a clearer definition of what is and is not supported by MP OpenAPI.

@MikeEdgar
Copy link
Member

@msmiths - I absolutely agree with you. I think this comes down to defining support for split OpenAPI files and support for generating split files. My earlier comments were intended to help get it working in the current state environment.

Azquelt pushed a commit to Azquelt/microprofile-open-api that referenced this issue Mar 17, 2022
Make sure graphviz is available on build
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

No branches or pull requests

5 participants