OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.85k stars 6.59k forks source link

[BUG] [Spring] Default interfaces do not contain response mediatypes except application/json #11731

Open networkinss opened 2 years ago

networkinss commented 2 years ago

Bug Report Checklist

Sample openapi doc causing the issue. issue_missingmediatypes.txt

Description

The default interface does not contain the mediatypes (empty) except for mediatype application/json. I provided a yaml with mediatypes application/hal+json and text/plain to reproduce the issue. This leads to a http status 500 if the stub is executed and api called with Postman. Spring throws exception: org.springframework.util.InvalidMimeTypeException: Invalid mime type "": 'mimeType' must not be empty because the code (is) mediaType.isCompatibleWith(MediaType.valueOf(""))) contains an empty string instead of (should) mediaType.isCompatibleWith(MediaType.valueOf("application/hal+json")))

openapi-generator version

6.0.0

OpenAPI declaration file content or url

File uploaded as issue_missingmediatypes.txt, please rename to issue_missingmediatypes.yaml.

Generation Details

java -jar openapi-generator-cli-6.0.0.jar generate -i issue_missingmediatypes.yaml -g spring

Steps to reproduce

Generate stub for generator "spring" and execute "mvn spring-boot:run". Request API with Postman or curl --location --request GET 'http://localhost:8080/api/v3/books' \ --header 'Accept: application/hal+json'

Related issues/PRs

None.

Suggest a fix

I made a test method to check that and provided a fix for the mustache file: methodBody.mustache Line 9 and 11 were changed. I wrote a test here: https://github.com/networkinss/openapi-generator/blob/fix_issue_11731/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java

New test method is: doGenerateMediatypes

Fix is ready for PR, if there are no objections.

networkinss commented 2 years ago

My fix is not correct yet. If works if there is just one mediatype. But it fails for multiple media types, as could be seen for the failed sample generations. My mustace experience is probably unsufficient to make it correct now.

MelleD commented 2 years ago

Looks related to this issue https://github.com/OpenAPITools/openapi-generator/issues/8701 and https://github.com/OpenAPITools/openapi-generator/issues/8700

borsch commented 2 years ago

@networkinss @MelleD @tamershahin I'll take a look on all of this issues/requests

tamershahin commented 2 years ago

I think that in general mustache is not enough to render this correctly. in the data it has to render there is only one returnTypes, while all of them need to extracted.

I did some play around with the mustache file and the Kotlin-spring generator like this:

openapi

responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ComplexObject'
            text/plain:
              schema:
                type: string
                example: pong 

api.mustache

   class {{classname}}Controller({{#serviceInterface}}@Autowired(required = true) val service: {{classname}}Service{{/serviceInterface}}) {
    {{#operation}}
>>>> added bit >>>>        {{#produces}}
            {{#swagger2AnnotationLibrary}}
                @Operation(
                summary = "{{{summary}}}",
                operationId = "{{{operationId}}}",
                description = "{{{notes}}}",
                responses = [{{#responses}}
                    ApiResponse(responseCode = "{{{code}}}", description = "{{{message}}}"{{#baseType}}, content = [Content(schema = Schema(implementation = {{{baseType}}}::class))]{{/baseType}}){{^-last}},{{/-last}}{{/responses}} ]{{#hasAuthMethods}},
                security = [ {{#authMethods}}SecurityRequirement(name = "{{name}}"{{#isOAuth}}, scopes = [ {{#scopes}}"{{scope}}"{{^-last}}, {{/-last}}{{/scopes}} ]{{/isOAuth}}){{^-last}},{{/-last}}{{/authMethods}} ]{{/hasAuthMethods}}
                ){{/swagger2AnnotationLibrary}}{{#swagger1AnnotationLibrary}}
                @ApiOperation(
                value = "{{{summary}}}",
                nickname = "{{{operationId}}}",
                notes = "{{{notes}}}"{{#returnBaseType}},
                response = {{{.}}}::class{{/returnBaseType}}{{#returnContainer}},
                responseContainer = "{{{.}}}"{{/returnContainer}}{{#hasAuthMethods}},
                authorizations = [{{#authMethods}}Authorization(value = "{{name}}"{{#isOAuth}}, scopes = [{{#scopes}}AuthorizationScope(scope = "{{scope}}", description = "{{description}}"){{^-last}}, {{/-last}}{{/scopes}}]{{/isOAuth}}){{^-last}}, {{/-last}}{{/authMethods}}]{{/hasAuthMethods}})
                @ApiResponses(
                value = [{{#responses}}ApiResponse(code = {{{code}}}, message = "{{{message}}}"{{#baseType}}, response = {{{.}}}::class{{/baseType}}{{#containerType}}, responseContainer = "{{{.}}}"{{/containerType}}){{^-last}},{{/-last}}{{/responses}}]){{/swagger1AnnotationLibrary}}
            @RequestMapping(
            method = [RequestMethod.{{httpMethod}}],
            value = ["{{#lambda.escapeDoubleQuote}}{{path}}{{/lambda.escapeDoubleQuote}}"]{{#singleContentTypes}}{{#hasProduces}},
            produces = "{{{vendorExtensions.x-accepts}}}"{{/hasProduces}}{{#hasConsumes}},
            consumes = "{{{vendorExtensions.x-content-type}}}"{{/hasConsumes}}{{/singleContentTypes}}{{^singleContentTypes}}{{#hasProduces}},
            produces = ["{{{mediaType}}}"{{^-last}}, {{/-last}}]{{/hasProduces}}{{#hasConsumes}},
            consumes = [{{#consumes}}"{{{mediaType}}}"{{^-last}}, {{/-last}}{{/consumes}}]{{/hasConsumes}}{{/singleContentTypes}}
            )
            {{#reactive}}{{^isArray}}suspend {{/isArray}}{{/reactive}}fun 

>>>> added bit >>>> {{#lambda.camelcase}}{{operationId}}-{{mediaType}}{{/lambda.camelcase}}({{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{^-last}},{{/-last}}{{/allParams}}): ResponseEntity<{{>returnTypes}}> {{#returnTypes}}{{this}}{{/returnTypes}}{
            return {{>returnValue}}
            }
>>>> added bit >>>>       {{/produces}}
    {{/operation}}

returnValue.mustache

{{#serviceInterface}}ResponseEntity(

>>>> added bit >>>> service.{{operationId}}{{#lambda.camelcase}}{{mediaType}}{{/lambda.camelcase}}

({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}), {{#responses}}{{#-first}}HttpStatus.valueOf({{code}}){{/-first}}{{/responses}}){{/serviceInterface}}{{^serviceInterface}}ResponseEntity(HttpStatus.NOT_IMPLEMENTED){{/serviceInterface}}

service.mustache :

    interface {{classname}}Service {
    {{#operation}}
        {{#produces}}
            {{#reactive}}{{^isArray}}suspend {{/isArray}}{{/reactive}}fun 

>>>> added bit >>>> {{#lambda.camelcase}}{{operationId}}-{{mediaType}}{{/lambda.camelcase}}

({{#allParams}}{{paramName}}: {{^isBodyParam}}{{^isFile}}{{>optionalDataType}}{{/isFile}}{{#isFile}}org.springframework.web.multipart.MultipartFile{{^required}}?{{/required}}{{/isFile}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{>optionalDataType}}{{/reactive}}{{#reactive}}{{^isArray}}{{>optionalDataType}}{{/isArray}}{{#isArray}}Flow<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{^-last}}, {{/-last}}{{/allParams}}): {{>returnTypes}}
        {{/produces}}
    {{/operation}}
    }

the generate controller now has 2 methods, as the service interface. but only one returnType specified. e.g.:

interface MyApiService {
            fun getInfoByIdApplicationJson(Id: java.util.UUID): ComplexObject
            fun getInfoByIdTextPlain(Id: java.util.UUID): ComplexObject
..            
}
borsch commented 2 years ago

@tamershahin change seems to be correct but would be better to do this with additional configuration property so that users has a chance to disable this

tamershahin commented 2 years ago

@tamershahin change seems to be correct but would be better to do this with additional configuration property so that users has a chance to disable this

it's partially correct: I would expect that auto generated controller, interfaces, etc would report the right return object. e.g.:

interface MyApiService {
            fun getInfoByIdApplicationJson(Id: java.util.UUID): ComplexObject
>>>> this need to change >>>>            fun getInfoByIdTextPlain(Id: java.util.UUID): String
..            
}

if my requests has a header "Accept": "text/plain" the controller should work with getInfoByIdTextPlain but this implicitly relies on ContentNegotiation strategy that maybe is part of Spring config and has nothing to do with swagger controller/services/etc codegen..

maybe first we should try to replicate the spring boot setup we see more useful and possible to generate.. don't know

borsch commented 2 years ago

@networkinss I have fixed issue with custom content-type - https://github.com/OpenAPITools/openapi-generator/pull/12460

Regarding multiple response types - still in progress

s-jepsen commented 1 year ago

Any progress on this?