Skip to content

[Go][Server] Optional Query Parameters Support - Option 2 #17044

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

Closed

Conversation

icubbon
Copy link
Contributor

@icubbon icubbon commented Nov 12, 2023

If query parameters are not required and don't have a default, make them a pointer so that the server code can determine if they were provided or omitted.

A design decision was made to treat query params that were declared but not provided a value to be nil. e.g. GET api/foo?bar= would result in the bar variable being nil if it were not required, didn't have a default value, and was a numeric type.

This is a different option to #16980

I did not create a go-server-deprecated implementation for this PR. I want to wait before doing that work if this option is preferred.

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 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (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.1.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.

@lwj5 @wing328

@lwj5
Copy link
Contributor

lwj5 commented Nov 13, 2023

This is out of spec, you may want to create your own fork if you require such a design. api/foo?bar= does not mean nil, it means "".

Here's a good answer for the meaning in openapi https://stackoverflow.com/questions/45575493/what-does-required-in-openapi-really-mean

Query params follow similar logic.

@lwj5
Copy link
Contributor

lwj5 commented Nov 13, 2023

With regard to the empty value, I've found the following resources:

OAI/OpenAPI-Specification#1573
OAI/OpenAPI-Specification#3217

if you need to use empty values for your param, you can follow the case in OAI/OpenAPI-Specification#3217. However, the current go-server generator does not deal with polymorphism.

@icubbon icubbon closed this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants