Bash skip empty query param fix

[ccouzens]

Skip empty params when generating URLs

I have a route with many optional query parameters.
I observed that when they weren't specified, they were still included in
the routes, but without a value or the `&` character to separate them.

Running:
```bash
bash bash_keycloak/client.sh -v --oauth2-bearer "$access_token" --host "$api_host" --dry-run realmUsersGet realm=demo_realm2 [email protected] username=foo
```

Would produce the route:
`http://localhost:8080/auth/admin/realms/demo_realm2/[email protected]=firstName=lastName=max=search=&username=foo`

After this change it produces the route:
`http://localhost:8080/auth/admin/realms/demo_realm2/[email protected]&username=foo`

Note, I haven't written much bash, so I'm not yet used to its
conventions.

PR checklist

• Read the contribution guidelines.

• Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change.
  Note that ./bin/bash-petstore.sh and ./bin/openapi3/bash-petstore.sh change the same files. I have committed with the output of ./bin/bash-petstore.sh.

• Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.

• Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
  @frol @bkryza @kenjones-cisco

Description of the PR

See above

[wing328]

@ccouzens I notice the following if condition in the code (line 455):

        if [[ "${collection_type}" == "" ]]; then
            local vcount=0
            for qvalue in "${parameter_values[@]}"; do
                if [[ $((vcount++)) -gt 0 ]]; then
                    parameter_value+="&"
                fi
                parameter_value+="${qparam}=${qvalue}"
            done

so what about using the following instead?

if [[ "${operation_parameters[$qparam]}" == "" ]]; then
...
done

[ccouzens]

Hi @wing328, thanks for reviewing :)

so what about using the following instead?

if [[ "${operation_parameters[$qparam]}" == "" ]]; then
...
done

I tried it. It has a different behaviour for when a blank parameter is passed in. For example invoking the client with username=

bash bash_keycloak/client.sh -v --oauth2-bearer "$access_token" --host "$api_host" realmUsersGet realm=demo_realm2 [email protected] username=

+isset creates the path /auth/admin/realms/demo_realm2/[email protected]=.

== "" creates the path /auth/admin/realms/demo_realm2/[email protected].

I think they're both wrong.
I think if the parameter is specified but blank, it should be creating the path /auth/admin/realms/demo_realm2/[email protected]&username=.

I'll see if I can update it to get the 3rd behaviour.

[ccouzens]

With the commit I've just pushed, it has the 3rd behaviour
(/demo_realm2/[email protected]&username=)

[wing328]

What about using nullable in the spec to determine the desired behavior?
If nullable is set to true, then

/demo_realm2/[email protected]&username=

otherwise (default):

/demo_realm2/[email protected]

[ccouzens]

Would nullable be the right thing to look for? Isn't an empty string different to null?

There is a field allowEmptyValue for query params.

Default value is false.

It defaults to not allowing empty parameters.

Use of this property is NOT RECOMMENDED, as it is likely to be removed in a later revision.

I think with that warning it doesn't make sense to implement the property. Presumably when (if) it gets removed query parameters will never be allowed to be empty. I'll make the change to drop empty parameters as you originally suggested.

[ccouzens]

I've rebased and put in that change https://github.com/OpenAPITools/openapi-generator/compare/9ce50fb68e0f9729cdb23e4a764a38577193c122..3dc421e8b6684bc699556e6b46c24002bc476689#diff-9dfff4ad1f2cbb0764f6c97b86551379

There's a lot of noise due to rebasing off of master

[wing328]

@ccouzens Looks like the rebase failed.

Can you ping me via https://Gitter.im (ID: wing328) when you've time so that I can help you out and get this PR merged into master soon?

[wing328]

@ccouzens thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553