Openapi-generator: [BUG][Spring] generated api interface doesn't have @RequestParam annotation

[t2y]

Bug Report Checklist

• [x] Have you provided a full/minimal spec to reproduce the issue?
• [x] Have you validated the input using an OpenAPI validator (example)?
• [x] What's the version of OpenAPI Generator used?
• [x] Have you search for related issues/PRs?
• [x] What's the actual output vs expected output?
• [ ] [Optional] Bounty to sponsor the fix (example)

Description

I updated openapi-generator from 4.0.1 to 4.0.2. Then, I found generated api interface from same spec is different between 4.0.1 and 4.0.2.

For some reason, @RequestParam is not generated for api parameters. Here is an example.

• 4.0.1

@ApiParam(value = "", allowableValues = "json, csv, tsv", defaultValue = "json")
@Valid
@RequestParam(value = "format", required = false, defaultValue = "json")
Format format

• 4.0.2

@ApiParam(value = "", allowableValues = "json, csv, tsv", defaultValue = "json")
@Valid
Format format

openapi-generator version

4.0.2

OpenAPI declaration file content or url

openapi: 3.0.2
info:
  title: info
  description: info
  version: 0.1.0

paths:
  /example/api:
    get:
      summary: summary
      description: description
      parameters:
      - $ref: '#/components/parameters/requiredQueryParam'
      - $ref: '#/components/parameters/formatParam'
      responses:
        200:
          description: response
          content:
            application/json:
              schema:
                type: string

components:
  parameters:
    requiredQueryParam:
      description: set query
      in: query
      name: query
      required: true
      schema:
        type: string
    formatParam:
      description: set format
      in: query
      name: format
      required: false
      schema:
        $ref: '#/components/schemas/format'

  schemas:
    format:
      default: json
      description: response format
      enum:
      - json
      - csv
      type: string

Command line used for generation

$ mvn clean package
$ java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i ./simple-example-spec/openapi.spec -g spring -o output

Steps to reproduce

Generate api code for spring with 4.0.1 and 4.0.2. Then compare the api parameters of api interface.

Related issues/PRs

I couldn't find any similar issues.

Suggest a fix

[t2y]

I found some parameters have @RequestParam and some parameters don't.

[macjohnny]

@t2y can you give an example spec and point to the parameters that generate @RequestParam and those that dont?

[t2y]

@macjohnny Now I made a test environment to run openapi-generator cli for git bisect. I will investigate which revision affects this issue in this weekend.

Currently, my production spec is complicated, so I will try to create simple example spec if possible.

[t2y]

I confirmed 4.0.3 still has this issue.

[t2y]

I did git bisect. This revision is the first bad commit.

• https://github.com/OpenAPITools/openapi-generator/commit/146c1fb25530699661211bcdd92b76491902b4b7
• https://github.com/OpenAPITools/openapi-generator/issues/2655

[t2y]

@macjohnny I updated openapi.yaml in above description to be able to reproduce this issue.

In my environment, generated api interface has diff between 0d701b7ce9e21c9405cac5cf17facd48dba542fc and 146c1fb25530699661211bcdd92b76491902b4b7 as below. The format parameter doesn't have @RequestParam annotation. Could you confirm?

$ diff -u s3-good/src/main/java/org/openapitools/api/ExampleApi.java s3-bad/src/main/java/org/openapitools/api/ExampleApi.java
--- s3-good/src/main/java/org/openapitools/api/ExampleApi.java  2019-07-13 14:46:47.000000000 +0900
+++ s3-bad/src/main/java/org/openapitools/api/ExampleApi.java   2019-07-13 14:49:11.000000000 +0900
@@ -26,7 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
[email protected](value = "org.openapitools.codegen.languages.SpringCodegen", date = "2019-07-13T14:46:46.993+09:00[Asia/Tokyo]")
[email protected](value = "org.openapitools.codegen.languages.SpringCodegen", date = "2019-07-13T14:49:11.556+09:00[Asia/Tokyo]")

 @Validated
 @Api(value = "example", description = "the example API")
@@ -42,7 +42,7 @@
     @RequestMapping(value = "/example/api",
         produces = { "application/json" },
         method = RequestMethod.GET)
-    default ResponseEntity<String> exampleApiGet(@NotNull @ApiParam(value = "set query", required = true) @Valid @RequestParam(value = "query", required = true) String query,@ApiParam(value = "set format", allowableValues = "json, csv", defaultValue = "json") @Valid @RequestParam(value = "format", required = false, defaultValue="json") Format format) {
+    default ResponseEntity<String> exampleApiGet(@NotNull @ApiParam(value = "set query", required = true) @Valid @RequestParam(value = "query", required = true) String query,@ApiParam(value = "set format", allowableValues = "json, csv", defaultValue = "json") @Valid Format format) {
         return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);

     }

[macjohnny]

@t2y thanks for the analysis
a workaround for your problem could be to inline the parameter, i.e.

components:
    formatParam:
      description: set format
      in: query
      name: format
      required: false
      default: json
      description: response format
      enum:
        - json
        - csv
      type: string

I guess either components.schemas should not be used for query params or
https://github.com/OpenAPITools/openapi-generator/blob/146c1fb25530699661211bcdd92b76491902b4b7/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java#L2213
should be extended to also check that the type is object
@wing328 @jimschubert @jmini what is your opinion?

[t2y]

remind, @wing328 @jimschubert @jmini what do you think about above solution?

[jimschubert]

@t2y thanks for pinging. I'm very far behind on emails and didn't see the last mention in the thread.

It looks like property.isModel assignment could add an && !ModelUtils.isEnum(p)? This function doesn't exist, but may be necessary for this case

[manderson23]

I also see missing @RequestParam with date-time format.

- name: start
  in: query
  schema:
     type: string
     format: date-time
  required: true
  description: The start time.

If I delete format to have the parameter a String then @RequestParam is present.

[t2y]

@macjohnny @jimschubert This bug is critical for me, and block upgrading version. Do you have a plan to fix?

[macjohnny]

@t2y would you like to submit a PR with a fix?

[t2y]

@macjohnny I don't know the background of this issue. But, I can try to fix and create a PR, so I will do it next week if no one will fix.

[macjohnny]

This would be very appreciated

[peyerroger]

Blocking for me as well. A fix would be highly appreciated.

[arrigod]

This is needed by me as well. A fix would be very appreciated.

[jorgerod]

This is needed by me as well. A fix would be highly appreciated.

[macjohnny]

@arrigod @peyerroger would you like to file a PR to fix this?

[peyerroger]

@macjohnny I'll try...

[t2y]

@peyerroger Sorry, I haven't started yet. Please!

[melissapalmer]

I have the same issue for date

        - name: dateParam
          in: query
          required: false
          schema:
            type: string
            format: datedate

@wing328 @jimschubert @jmini @macjohnny is the fix not just to remove the {{^isModel}} from the /JavaSpring/queryParams.mustache template? As suggested by @macjohnny at: https://github.com/OpenAPITools/openapi-generator/issues/2053#issuecomment-495561796

Test example at: https://github.com/melissapalmer/openapi-generator-cli showing working params and not working with template override .

[peyerroger]

@melissapalmer That was my first idea as well, but it would reintroduce #2655 this issue here.

Instead of ^isModel we could use #isPrimitiveType. I wrote a couple of test cases to ensure it's properly working and it does.

What I don't really understand is why this change was introduced 146c1fb? Was the only reason the fix for #2655 or is there more behind this commit? Could we remove the following line? Or did this fix something else?
property.isModel = ModelUtils.isModel(p);
in DefaultCodegen.java

@macjohnny Could you give me some advice on this?

[jimschubert]

@peyerroger correct, that commit was intended to fix #2655. I think there are two options.... first, we could add to the condition as I suggested previously, something like:

property.isModel = ModelUtils.isModel(p) &&
   !ModelUtils.isEnum(p) && // new method
   !ModelUtils.isPrimitive(p); //to be extra safe

Or second, we could revert that commit and reopen the bug.

[jimschubert]

I've opened #3855 with a proposed fix. Would someone from this thread be able to test it out and provide feedback?

[peyerroger]

Hi guys,

I created a PR as well ( #3857 ). Here's what I did:

• I reverted this 146c1fb, but kept the test to ensure that there's no regression on issue #2655
• I used isPrimitiveType instead of ^isModel in the mustache template. This seems to be a much simpler solution and a lot less invasive as the solution introduced in 146c1fb
• I added a couple of tests to ensure enums, localdates and so on are working properly

Cheers,
Roger

[t2y]

@jimschubert #3855 was merged into 4.1.3, so this issue was fixed I think.

[nbrugger-tgm]

Why is the issue not closed?
Is the bug still there or isn't it tested?