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

[BUG][GO] Query parameters that are not required should be generated as pointers in go-server #19169

Closed
6 tasks done
phil-bugdahn-hs opened this issue Jul 15, 2024 · 3 comments

Comments

@phil-bugdahn-hs
Copy link

phil-bugdahn-hs commented Jul 15, 2024

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

When using the go-server generator, query parameters with required: false are being generated as value types. Using the example spec below, this generates a method signature like the following:

type DefaultAPIServicer interface { 
	RootGet(context.Context, bool) (ImplResponse, error)
}

Since the default value of bool is false, it is not possible to tell whether the client passed false to the parameter or simply did not provide it at all. By using a pointer instead, the parameter could be nil when not passed by the client. The method signature should look like the following:

type DefaultAPIServicer interface { 
	RootGet(context.Context, *bool) (ImplResponse, error)
}
openapi-generator version

7.7.0

OpenAPI declaration file content or url
openapi: 3.0.0
security: []
info:
  description: >-
    This is a sample spec to demonstrate handling non-required boolean query parameters in the go-server generator
  version: 1.0.0
  title: Boolean sample
  license:
    name: Apache-2.0
    url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
paths:
  /:
    get:
      summary: Sample endpoint
      description: Demonstrate handling of non-required boolean query parameters
      parameters:
        - in: query
          name: someParam
          description: a test parameter
          schema:
            type: boolean
          required: false
      responses:
        '200':
          description: Demo
          content:
            application/json:
              schema:
                type: string
Generation Details

This bug can be reproduced using the go-server generator with no additional settings.

Steps to reproduce
  • Copy the above spec into a file and save at ./openapi/openapi.yaml
  • Run the generator with the command: java -jar openapi-generator-cli.jar generate -i ./openapi/openapi.yaml -o ./server-gen -g go-server
  • Observe the generated server code in the server-gen directory, eg. ./server-gen/go/api.go, note that the RootGet method signature takes a bool rather than a *bool parameter.
Related issues/PRs
Suggest a fix

#19149

Would appreciate the Go technical committee reviewing this issue and my suggested fix (@antihax, @grokify, @kemokemo, @jirikuncar, @ph4r5h4d, @lwj5)

@paulson-george-hs
Copy link

Hoping to get this merged in soon as its a fairly major bug with a trivial fix, thats already implemented in the generator for the other languages.

@lwj5
Copy link
Contributor

lwj5 commented Jul 16, 2024

I dont think it is a bug, have you tried?

openapi: 3.0.0
security: []
info:
  description: >-
    This is a sample spec to demonstrate handling non-required boolean query parameters in the go-server generator
  version: 1.0.0
  title: Boolean sample
  license:
    name: Apache-2.0
    url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
paths:
  /:
    get:
      summary: Sample endpoint
      description: Demonstrate handling of non-required boolean query parameters
      parameters:
        - in: query
          name: someParam
          description: a test parameter
          schema:
            type: boolean
            nullable: true
          required: false
      responses:
        '200':
          description: Demo
          content:
            application/json:
              schema:
                type: string

@phil-bugdahn-hs
Copy link
Author

@lwj5 Thanks for this sample, I had tried something similar but didn't realize the nullable property had to be under schema. This does indeed address the problem so I will close this issue/PR. Thanks for your time.

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

Successfully merging a pull request may close this issue.

3 participants