Skip to content

Conversation

@simon-pearson
Copy link

@simon-pearson simon-pearson commented Jan 22, 2021

…ault value for the explode property should be true. For all other styles the default value is false. Cover this in a unit test.
@simon-pearson simon-pearson changed the title When the style property on an OpenApiParameter is 'form' then the def… Query Parameters Serialization - Explode don't respect the default value Jan 22, 2021
@simon-pearson simon-pearson changed the title Query Parameters Serialization - Explode don't respect the default value Query Parameters Serialization - Explode doesn't respect the default value Jan 24, 2021
@darrelmiller darrelmiller merged commit 55317d3 into microsoft:vnext Mar 14, 2021
@safepage
Copy link

I understand this fix but the serialization code looks wrong:

writer.WriteProperty(OpenApiConstants.Explode, Explode, Style.HasValue && Style.Value == ParameterStyle.Form);

Maybe it should be:

writer.WriteProperty(OpenApiConstants.Explode, _explode);

Then the serialized model would reflect the actual values read in (as _explode is a nullable bool?).

Also, there is now an inconsistency as Style is also not set to a default value:
“Default values (based on value of in): for query - form; for path - simple; for header - simple; for cookie - form.”

@darrelmiller
Copy link
Member

@safepage The serialization code will write out any value that was read because that is what the Explode property does by default (unless it is the default value). Only if Explode has not be set, i.e. _explode is null will the default value be used. If the Explode property returns a "default" then it will match with the default passed as the third parameter to WriteProperty and no Explode value will be written to the output. This does mean that if a input document redundantly provides the default value, then it will be missing in the output.

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.

4 participants