Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Open
networkinss opened this issue Feb 26, 2022 · 8 comments

Comments

@networkinss
Copy link
Contributor

networkinss commented Feb 26, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

MelleD commented Feb 28, 2022

Looks related to this issue
#8701
and
#8700

@borsch
Copy link
Member

borsch commented May 23, 2022

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

@tamershahin
Copy link

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
Copy link
Member

borsch commented May 24, 2022

@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
Copy link

@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
Copy link
Member

borsch commented May 25, 2022

@networkinss I have fixed issue with custom content-type - #12460

Regarding multiple response types - still in progress

@s-jepsen
Copy link
Contributor

Any progress on this?

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

Successfully merging a pull request may close this issue.

5 participants