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

[SPRING] Bug generating GET-Requests because of empty consumes attributes #7734

Closed
srsp opened this issue Feb 27, 2018 · 4 comments
Closed

Comments

@srsp
Copy link
Contributor

srsp commented Feb 27, 2018

Description

I am trying to generate a Java Spring API from yaml using mvn and the swagger-codegen-maven-plugin. More specifically I am trying to model a GET Request. The generated code looks like this:

    @RequestMapping(value = "/applications/{stage}",
        produces = "application/json",
        consumes = "",
        method = RequestMethod.GET)

Please note the consumes = "", which will prevent Springs Application Context from starting (using Spring Boot 2.0.0.RC2) and will produce a stacktrace

Swagger-codegen version

Affected: swagger-codegen 2.3.1
This is a regression from 2.2.3, where it works.

SpringBoot 2.0.0.RC2

Swagger declaration file content or url

YAML to reproduce

Command line used for generation

I am building with maven, so the swagger config looks like this:

              <inputSpec>${project.build.directory}/swagger-model/swagger.yaml</inputSpec>
              <language>spring</language>
              <apiPackage>com.github.srsp.rest.api</apiPackage>
              <modelPackage>com.github.srsp.rest.model</modelPackage>
              <modelNamePrefix></modelNamePrefix>
              <modelNameSuffix>Dto</modelNameSuffix>
              <configOptions>
                <interfaceOnly>true</interfaceOnly>
                <singleContentTypes>true</singleContentTypes>
                <basePackage>com.github.srsp.rest</basePackage>
                <configPackage>com.github.srsp.rest</configPackage>
                <swaggerDocketConfig>true</swaggerDocketConfig>
                <dateLibrary>java8</dateLibrary>
                <useOptional>true</useOptional>
                <bigDecimalAsString>true</bigDecimalAsString>
                <useBeanValidation>true</useBeanValidation>
Steps to reproduce

Build with mvn, try to start the application with mvn spring-boot:run. Application startup will fail.

Suggest a fix/enhancement

For HTTP GET there should not be a consume attribute present.

@jmini
Copy link
Contributor

jmini commented Mar 1, 2018

Would it work if the generated code would look like this?

    @RequestMapping(value = "/applications/{stage}",
        produces = "application/json",
        method = RequestMethod.GET)

@srsp
Copy link
Contributor Author

srsp commented Mar 1, 2018

Yes, that code would work.

I did a bit of digging yesterday. I think the following behaviour would be nice:

    get:
      produces:
        - application/json

leads to

    @RequestMapping(value = "/applications/{stage}",
        produces = "application/json",
        method = RequestMethod.GET)

If I leave out the produces in yaml as well, I would get

    @RequestMapping(value = "/applications/{stage}",
        method = RequestMethod.GET)

or

    @RequestMapping(value = "/applications/{stage}",
        produces = {},
        method = RequestMethod.GET)

The reasoning behind this: If you look at the @RequestMappings default for produces and consumes, then you'll notice it's just String[] consumes() default {};, so an empty String array. And this means, that Spring won't even look at these headers, when it tries to match the incoming request to a method.

So basically, if I don't define it in my yaml, I want the Spring default. Is that reasonable?

@jmini
Copy link
Contributor

jmini commented Mar 1, 2018

So basically, if I don't define it in my yaml, I want the Spring default. Is that reasonable?

I can not tell (I do not know the Spring framework and I am also not a swagger expert).


But I can tell you how you can test it locally:

I assume you are using Swagger v2 (master branch of this repository).

The codegen class corresponding to spring is io.swagger.codegen.languages.SpringCodegen.
The templateDir used by this codegen class is: JavaSpring/

I think the position to change is arround Line 97 in api.mustache.

Current implementation:

    @RequestMapping(value = "{{{path}}}",{{#singleContentTypes}}
        produces = "{{{vendorExtensions.x-accepts}}}",
        consumes = "{{{vendorExtensions.x-contentType}}}",{{/singleContentTypes}}{{^singleContentTypes}}{{#hasProduces}}
        produces = { {{#produces}}"{{{mediaType}}}"{{#hasMore}}, {{/hasMore}}{{/produces}} }, {{/hasProduces}}{{#hasConsumes}}
        consumes = { {{#consumes}}"{{{mediaType}}}"{{#hasMore}}, {{/hasMore}}{{/consumes}} },{{/hasConsumes}}{{/singleContentTypes}}
method = RequestMethod.{{httpMethod}})
  • The case "if not singleContentTypes" seems to handle what you have described => the corresponding template-code is what is between {{^singleContentTypes}} ... {{/singleContentTypes}}. There produces = ... is only added if hasProduces is true (because of the check {{#hasProduces}}..{{/hasProduces}}check) and the same forconsumes = ...with thehasConsumes` check.
  • I guess that you are in the other case: singleContentTypes is true => he corresponding template-code is what is between {{#singleContentTypes}} ... {{/singleContentTypes}}. You should verify that this is the case. (1)
  • If this is the case, you can see if adding {{#hasProduces}}..{{/hasProduces}} and {{#hasConsumes}}..{{/hasProduces}} works in the singleContentTypes case. (2)
  • If this is the case, then the solution might be something like (3)

(1) to perform such checks, I would add following line before @RequestMapping and then regenerate the code:

// === singleContentTypes {{#singleContentTypes}} IS TRUE {{/singleContentTypes}}
// === singleContentTypes {{^singleContentTypes}} IS FALSE {{/singleContentTypes}}

(2) check it like this:

// === hasProduces {{#hasProduces}} IS TRUE {{/hasProduces}}
// === hasConsumes {{#hasConsumes}} IS TRUE {{/hasConsumes}}

(3)

    @RequestMapping(value = "{{{path}}}",{{#singleContentTypes}}{{#hasProduces}}
        produces = "{{{vendorExtensions.x-accepts}}}", {{/hasProduces}}{{#hasConsumes}}
        consumes = "{{{vendorExtensions.x-contentType}}}",{{/hasConsumes}}{{/singleContentTypes}}{{^singleContentTypes}}{{#hasProduces}}
        produces = { {{#produces}}"{{{mediaType}}}"{{#hasMore}}, {{/hasMore}}{{/produces}} }, {{/hasProduces}}{{#hasConsumes}}
        consumes = { {{#consumes}}"{{{mediaType}}}"{{#hasMore}}, {{/hasMore}}{{/consumes}} },{{/hasConsumes}}{{/singleContentTypes}}
method = RequestMethod.{{httpMethod}})

If this works, you should propose a pull request.

Please test also the other case with multiple content types


If something is not clear enough, please continue the discussion.

@srsp
Copy link
Contributor Author

srsp commented Mar 2, 2018

Thank you, @jmini. I think your suggestion is spot on. I am still testing, but it looks very promising.

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

3 participants