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

Investigate multiple schema case (different mime types) in ModelUtils #144

Closed
jmini opened this issue May 24, 2018 · 31 comments
Closed

Investigate multiple schema case (different mime types) in ModelUtils #144

jmini opened this issue May 24, 2018 · 31 comments

Comments

@jmini
Copy link
Member

jmini commented May 24, 2018

In ModelUtils those two methods return a single Schema for a RequestBody or a ApiResponse:

  • ModelUtils.getSchemaFromRequestBody(RequestBody)
  • ModelUtils.getSchemaFromResponse(ApiResponse)

This feels wrong because when multiple Mime types are defined (like application/json or application/xml) different Schemas can be returned.

With #74 an additional warning log was added:

[main] WARN org.openapitools.codegen.utils.ModelUtils - Multiple schemas found, returning only the first one

We should investigate on the impacts.

When multiple mime types are defined we might need to compare if the defined schema are different or not.

@jimschubert jimschubert added Announcement Project/release related announcements General: Discussion General: Suggestion and removed Announcement Project/release related announcements labels Jun 10, 2018
@jimschubert
Copy link
Member

@jmini I've tagged this as general discussion/suggestion. We don't have a tag for infrastructural code that affects all generators, and it wasn't clear to me if you're documenting a thought or looking for additional inputs (I agree that in OAS 3.0, you can end up with different models but it's not as easy in OAS 2.0).

@JanMosigItemis
Copy link

Hi there, I recently encountered these two warnings and find them rather annoying. That is: Although my input files are perfectly valid (from what I can tell), my build log contains these warnings, indicating that there is something wrong where afaik there isn't.

Is it possible to suppress those warnings or get rid of them by any other means?

@alizeynalli90
Copy link

Hi I am also getting this warning when i generate Apis from my yaml file. The thing is I am having text/csv or application/json contents for the same endpoint in order to avoid duplicate calls. It works perfectly fine for different response forms. But I find the warning annoying. So suppressing the warnings would be good. Any ideas?

@UnleashSpirit
Copy link
Contributor

Hi, i'm facing the same issue but in request.
Although for response it may not be a problem, in request I need the two (or more) possibilities to be generated (typescript-angular)
Here I got only one and can't use the second option. I can uderstand that it may be a conception problem from service but legacy doesn't offer many options some times.

@ngarg-panw
Copy link

Hi, I am also facing the same issue. Here is my swagger:
requestBody: content: application/vnd.api+json: schema: title: convertRequest type: object required: [data] properties: data: type: object properties: templateType: type: string templateVersion: type: string application/octet-stream: schema: title: templateFile type: object properties: templateFile: type: string format: binary application/zip: schema: type: string format: binary

I am getting the following response:
> Task :openApiGenerate systemProperties will be renamed to globalProperties in version 5.0 Multiple schemas found in the OAS 'content' section, returning only the first one (application/vnd.api+json) Multiple schemas found in the OAS 'content' section, returning only the first one (application/vnd.api+json) Multiple schemas found in the OAS 'content' section, returning only the first one (application/vnd.api+json) Multiple MediaTypes found, using only the first one

Any help is appreciated. Thanks!

@typhoon2k
Copy link
Contributor

typhoon2k commented Sep 3, 2020

@jmini , @jimschubert, IMO the easiest approach here would be to build CodegenOperation for each combination of possible request body content types schemas and response content types schemas in operation. This will allow to keep existing templates without modifications.

Methods mentioned above will need to have additional contentType parameter.
Additionally to support this approach generation of operation IDs will need to be enhanced (to avoid duplicate operation/method/function names in generated code).

@jimschubert
Copy link
Member

@typhoon2k would you be able to provide an example of what you mean?

The issues that I see are:

  • Generators that expect a single response or request body would have to be updated to support multiple. This includes built-in templates, but also affects user customizable templates.
  • Many built-in generators don't support content negotiation, and because Mustache templates are logic-less, there's a lot of code that would unintentionally iterate over multiple request bodies or responses. For example:
    image
    At best, those generated outputs would result in erroneous code or broken compilation. At worst, this could introduce attack vectors (DoS, buffer overflow/underrun) in business critical applications where such code doesn't result in compile-time errors.
  • The last part of your comment assumes we can generate a new operation per request/response body. That's not the case because some languages and frameworks don't support method overloading. Many specifications can define the same schema for request/response but define the transport format (xml, json, etc), and some frameworks such as aspnetcore handle those additional responses via annotation and IoC (see example).

@typhoon2k
Copy link
Contributor

typhoon2k commented Sep 4, 2020

@jimschubert , let's use the following example:

paths:
  /person:
    post:
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/SuperManA"
          application/xml:
            schema:
              $ref: "#/components/schemas/SuperManA"
          application/avro:
            schema:
              $ref: "#/components/schemas/SuperManB"
      operationId: someOperation
      responses:
        '201':
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/SuperManA"
            application/xml:
              schema:
                $ref: "#/components/schemas/SuperManB"
            application/avro:
              schema:
                $ref: "#/components/schemas/SuperManB"

Taking into account current situation (that many generators expect only one schema in request body and response) my idea was that we can generate 4 CodegenOperations:

  1. someOperation1:
  • path: /person
  • consumes: has "application/json" and "application/xml"
  • produces: "application/json"
  • bodyParam: SuperManA
  • responses: SuperManA
  1. someOperation2:
  • path: /person
  • consumes: "application/json" and "application/xml"
  • produces: "application/xml" and "application/avro"
  • bodyParam: SuperManA
  • responses: SuperManB
  1. someOperation3:
  • path: /person
  • consumes: "application/avro"
  • produces: "application/json"
  • bodyParam: SuperManB
  • responses: SuperManA
  1. someOperation4:
  • path: /person
  • consumes: "application/avro"
  • produces: "application/xml" and "application/avro"
  • bodyParam: SuperManB
  • responses: SuperManB

I.e. expectation to have single request body and response is not broken.

On your example with JSONifying response - I feel that this example works only for specific use cases, where only one response is defined (or maybe generator is reducing list of responses to only one 2xx response). Also it would be useful to include isJson/isXml/isBinary/etc into CodegenResponse to make sure that correct conversion is used.

On your last item - unfortunately I don't have enough knowledge on those languages/frameworks, but if framework permits unique method/function names (generated using unique operationId) with duplicate paths & different content types, then that should work. If duplicate paths are not permitted, then this approach won't work.

I consider proposed approach more as a workaround to reduce amount of necessary changes.
Proper solution would be to introduce additional map in CodegenParameter/CodegenResponse to hold schemas (per content type). In transition period all existing (deprecated) schema-related getters in these 2 classes can return properties of first schema in underlying map.

@jimschubert
Copy link
Member

@typhoon2k I'm assuming your example expects 1:1 matching with request and response content types? Again, that's not really something we can assume across the board due to content negotiation. For instance, a request can pass xml and accept json, while another expects xml+xml. We'd have to generate each permutation of these in some generators.

What do you think about if we were to move the logic which grabs the first element of each of these lists into DefaultCodegen somewhere, and allow generators to pass all request/responses to templates? It would also allow generators that require it to generate transient operations with the additional parameters.

That way we can phase this support rather than tackling as all-or-nothing. I think if we go that route, we could add a generator feature enum or two, like MultipleContentTypesRequest and MultipleContentTypesResponse. This would also allow people to search for this support on our site.

@typhoon2k
Copy link
Contributor

@jimschubert

I'm assuming your example expects 1:1 matching with request and response content types?
No, any schema, with any content type in request and any content type in response. Permutations only based on request schema and response schema.

I was thinking about replacing all CodegenProperty related fields with Map<String, CodegenProperty> in CodegenParameter and CodegenResponse (i.e. map from content type to schema). As I said for backwards compatibility getters of replaced fields can return values of the same fields from first CodegenProperty in map.
I will take a look on this (also on your proposal) during next week.

@JanMosigItemis
Copy link

Thanks for working on the issue guys 👍

@typhoon2k
Copy link
Contributor

@jimschubert , I've reviewed dependencies that we have on CodegenParameter and CodegenResponse fields and have a feeling that it will be quite challenging to implement my proposal.
I agree with you that introducing new fields in CodegenOperation that would return new MultipleContentTypesRequest and MultipleContentTypesResponse and filling them in DefaultCodegen would be a bit easier and would guarantee that nothing is broken.

@cshamrick
Copy link

@typhoon2k What's the status of this change? I'm currently developing an API that is versioned via the Accept header, which if documented correctly in our oas definition, would ideally produce separate operations (e.g. someOperationV1, someOperationV2) as you described in your proposal above. Has there been any movement toward this type of setup?

@YassinHajaj
Copy link

Hi, any updates on this change?

@tomaszwozniak8
Copy link

I'm also curious about updates on this.

@alexrashed
Copy link
Contributor

In my opinion, content negotiation is a very basic concept of HTTP / REST.
We are also facing similar problems and would love to see a solution for that.

@nikosmoum
Copy link

Any updates on this? Returning different schema based on accept header is common imo. We are currently forced to workaround this, which is not ideal, so it would be great if this was implemented.

@gbormann
Copy link

We're suffering from the same issue.

What's the status of this issue after 2.5years?

@exejutable
Copy link

Any status of this?

@chutzemischt
Copy link

chutzemischt commented Aug 3, 2021

Are there any plans to implement this in the near future? IMO this is a bug. Why can't the OpenAPI Generator handle multiple content types? Other generators handle this correctly.

I think having different content types in the requestBody is a common use case, e.g.:

/api/item:
    post:
      operationId: addItem
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Item'
          multipart/form-data:
            schema:
              type: object
              properties:
                item:
                  $ref: '#/components/schemas/Item'
                file:
                  $ref: '#/components/schemas/InputStream'

@blaubaer
Copy link

I have also problems to find a way to deal with that except breaking the REST principle by having multiple methods for multiple response content types.

I'm also open for alternatives.

@jamesfoster
Copy link

Multiple schemas found, returning only the first one

In my case, at least, the client I want to generate only needs to support a single content type. However, "the first one" is not the one I want. Is there a way to specify which one to use?

generate [...] --preferred-content-type application/json

Thanks for your assistance.

@artemptushkin
Copy link

Please start support content negotiation, it's one of the basic REST concepts. Should be on the high demand, imo

@MelleD
Copy link
Contributor

MelleD commented Dec 8, 2021

Is there any news here? What about this PR (uh 2019?). Doesn't it solve the problem?

#3991

Or this one?
#10973

@spacether
Copy link
Contributor

spacether commented Dec 20, 2021

Yes #10973 solves this problem; I am using it in this PR: #8325 to allow users to send request bodies with different body content types supported.
One can see it working here:
https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python-experimental/petstore_api/api/pet_api_endpoints/add_pet.py#L78

pet_parameter = api_client.RequestBody(
    content={
        'application/json': api_client.MediaType(schema=SchemaForRequestBodyApplicationJson),
        'application/xml': api_client.MediaType(schema=SchemaForRequestBodyApplicationXml),
    },
    required=True,
)

@artemptushkin
Copy link

@spacether Just to extend your answer based on what I see. For Java, it's expected to be fixed with #6708 in 5.3.1

@MelleD
Copy link
Contributor

MelleD commented Dec 21, 2021

@artemptushkin There is an extension for Java and / or Spring? I haven't found a PR yet?

@artemptushkin
Copy link

@MelleD I refer to this comment of @spacether #6708 (comment) so, I assume only so far. But I'd expect content-negotiation problem will be closed with #6708

@tamershahin
Copy link

any news about this?

@banool
Copy link

banool commented Jul 15, 2022

I've tried every typescript generator here but they all don't work for the multiple schema case: https://openapi-generator.tech/docs/generators/.

I'm running this command:

openapi-generator generate -g typescript-fetch -i ../../../api/doc/openapi_v2.yaml -o src/generated/ --enable-post-process-file

I looked at the help message and other docs and can't find any relevant flags to enable this.

Given the issue is closed I would expect this to work, am I missing something?

@spacether
Copy link
Contributor

spacether commented Jul 15, 2022

The investigation is done. One generator has implemented multiple content types. The information is available for any other generators to use.

This is an investigation ticket not an implementation ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests