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

[C# Client] Boolean query parameters are now lowercase #1711

Merged
merged 3 commits into from
Nov 2, 2018

Conversation

LangdalP
Copy link
Contributor

@LangdalP LangdalP commented Nov 2, 2018

The ConvertToString method of generated C# clients serializes boolean values to True and False, which means that boolean values in query parameters are either True or False.

The OpenAPI 3.0 specification does not specify how boolean values are to be serialized when they are query parameters. However, when using a nodejs server generated by https://editor.swagger.io/, requests with queryParam=True and queryParam=False fail the built in validation in the server. The validation code is taken from swagger-tools, and can be seen here: https://github.com/apigee-127/swagger-tools/blob/master/lib/validators.js#L484 . I have tested that lower-casing the boolean values makes the requests pass this validation.

From this, and from doing some googling, it would seem that people around the internet are of the opinion that boolean query parameters should either be serialized as "true" and "false", or alternatively "1" and "0".

I propose that one should use "true" and "false".

Edit: Sorry about the useless second commit. It would seem that the GitHub code editor automatically adds a newline at the end of the file. I can create a new PR if this is problematic.

The `ConvertToString` method serializes boolean values to `True` and `False`.

The OpenAPI 3.0 specification does not specify how boolean values are to be serialized when they are query parameters. However, when using a nodejs server generated by https://editor.swagger.io/, requests with `queryParam=True` and `queryParam=False` fail the built in validation in the server. The validation code is taken from `swagger-tools`, and can be seen here: https://github.com/apigee-127/swagger-tools/blob/master/lib/validators.js#L484

From this, and from doing some googling, it would seem that people around the internet are of the opinion that boolean query parameters should either be serialized as "true" and "false", or alternatively "1" and "0".

I propose that one should use "true" and "false".
@LangdalP LangdalP changed the title Boolean query parameters are now lowercase C# API Client: Boolean query parameters are now lowercase Nov 2, 2018
@LangdalP LangdalP changed the title C# API Client: Boolean query parameters are now lowercase [C# Client] Boolean query parameters are now lowercase Nov 2, 2018
@RicoSuter
Copy link
Owner

Thanks for the pr. We should use ToLowerInvariant()

@LangdalP
Copy link
Contributor Author

LangdalP commented Nov 2, 2018

Good catch, ToLowerInvariant is better.

@RicoSuter RicoSuter merged commit ae78aba into RicoSuter:master Nov 2, 2018
@LangdalP LangdalP deleted the patch-1 branch November 2, 2018 16:58
@RicoSuter RicoSuter mentioned this pull request Nov 15, 2018
5 tasks
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