Fixes the non-working query parameter when it's an Enum

[IvanVas]

Fixes the non-working query parameter when it's a Enum.
Previously, it'd try to iterate through a string, thus creating parameters like &1=s&2=t&3=r...

✅ Validated on the Spec with Enum param.

Based on this fix.

@amakhrov
@davidgamero
@mkusaka
@joscha
@petejohansonxo
@macjohnny
@topce
@akehir

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package || exit
  ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
  ./bin/utils/export_docs_generators.sh || exit
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Reproducer Spec

openapi: 3.0.0
info:
  title: Widget API
  version: 1.0.0

paths:
  /api/widget_url:
    get:
      summary: Get  widget
      parameters:
        - name: some_id
          in: query
          description: The ID
          schema:
            $ref: '#/components/schemas/some_id'
        - name: widget_type
          in: query
          description: Descr
          required: true
          schema:
            $ref: '#/components/schemas/widget_type'
      responses:
        '200':
          description: Descr
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/url_response'

components:
  schemas:
    some_id:
      type: string
      format: uuid
      description: The unique identifie
      example: "7d943c51-e4ff-4e57-9558-08cab6b963c7"

    widget_type:
      type: string
      description: The widget type
      enum:
        - activate
        - set

    url_response:
      type: object
      required:
        - url
      properties:
        url:
          type: string
          format: uri
          description: The URL for the requested widget
          example: "https://widgets.example.com/id"

[joscha]

You might need to check in a new spec that uses this code path. It would be good to check two samples in. One with the existing code and how it behaves and one with the change. At first glance I am unsure this change is correct for all cases, I think it may depend on how the spec defines serialization of the query params?

[IvanVas]

@joscha

It would be good to check two samples in

How do I create the generator-specific samples? Can I add a Spec for typescript only?

At first glance I am unsure this change is correct for all cases, I think it may depend on how the spec defines serialization of the query params?

Not sure how to proceed.
It seems that the isExplode should've been false for that case, but I'm not familiar enough with the codebase to dig that deep.

[amakhrov]

It seems that the isExplode should've been false for that case

According to the spec, explode defaults to true for form style (which includes query params) - so this is correct.

What's causing the trouble is that referenced enum is not considered a primitive type (while inline enums are primitive, I believe). It happens here:

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

Lines 4343 to 4348 in 8ce332a

	if (languageSpecificPrimitives().contains(type)) {
	property.isPrimitiveType = true;
	} else {
	property.complexType = property.baseType;
	property.isModel = true;
	}

For referenced enums, the type variable is the referenced model name (WidgetType in your example spec), and not the actual type of the referenced model (string).

There is even a unit test that verifies this particular behavior:

openapi-generator/modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java

Line 4832 in 8916987

Assertions.assertFalse(referencedEnumSchemaProperty.isPrimitiveType);

I'm not sure if other generators are also buggy currently, or whether they somehow expect this behavior.

Perhaps @wing328 knows more (he added the abovementioned unit test)

One way to fix it specifically for the typescript generator could be to add a separate conditional block just for referenced enums. (The nesting of those IFs will suck tho)

  {{#isPrimitiveType}}
    requestContext.setQueryParam("{{baseName}}", ObjectSerializer.serialize({{paramName}}, "{{{dataType}}}", "{{dataFormat}}"));
  {{/isPrimitiveType}}
+ {{#isEnumRef}
+   requestContext.setQueryParam("{{baseName}}", ObjectSerializer.serialize({{paramName}}, "{{{dataType}}}",
+ {{^isEnumRef}
  {{^isPrimitiveType}}
  ...
  {{/isPrimitiveType}}
+ {{/isEnumRef}

Looks like typescript-fetch generator does smth like this already

[IvanVas]

@amakhrov,
Thank you for the deep analysis!

That issue might be directly relevant to the discussion.
So that seems to be a bug that was patched with the new tag, AFAIU.
typescript-fetch template you shared uses exactly that tag.
So that might be a way to go.
WDYT?

[amakhrov]

Yeah, it should work. Im still concerned that with this approach we'll likely need to make the same update in other TS generators - instead of having a solution that applies to them all.
But whatever. It's definitely safer this way :)

[IvanVas]

@amakhrov
I've updated accordingly.

It'd be great to add this case to some samples input.
No changes after generating all samples, so I assume the case isn't covered in any.
It's not uncommon, I'd say.
WDYT? Which one to use?

@wing328, do you have an opinion on adding to samples?

[amakhrov]

looks good to me!

ideally we also have some tests or samples for that. maybe just an issue-specific minimal sample to start with (like this one)?

[wing328]

@wing328, do you have an opinion on adding to samples?

perfectly fine with me to add another sample folder and include it in the CI for testing to ensure it's covered moving forward

[wing328]

We can use Echo API to test it; https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/echo_api.yaml#L212

PM me via Slack so that we can work together to add the test.

https://join.slack.com/t/openapi-generator/shared_invite/zt-2uoef5v0g-XGwo8~2oJ3EoziDSO1CmdQ