Skip to content

Conversation

@mapitman
Copy link
Contributor

When a parameter has the style set to "form" and explode set to false,
the v3 serializer will not write out the value for explode. Swagger UI
then defaults the explode value to true, since it is not specified and
that is the correct default value when the style is set to "form". The
OpenAPI spec for this is right around here:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#fixed-fields-10

I'll be glad to make any changes required to get his accepted.

When a parameter has the style set to "form" and explode set to false,
the v3 serializer will not write out the value for explode. Swagger UI
then defaults the explode value to true, since it is not specified and
that is the correct default value when the style is set to "form". The
OpenAPI spec for this is right around here:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#fixed-fields-10
@msftclas
Copy link

msftclas commented Apr 27, 2019

CLA assistant check
All CLA requirements met.

@darrelmiller
Copy link
Member

darrelmiller commented Apr 27, 2019

Nice catch. It is this part of the spec that we were not doing properly:

When style is form, the default value is true. For all other styles, the default value is false.

Your changes look good to me. If you can sign the CLA, we can get these changes merged in.

@darrelmiller darrelmiller added this to the 1.1.4 milestone Apr 27, 2019
@mapitman
Copy link
Contributor Author

mapitman commented Apr 29, 2019

There is similar logic in the OpenApiHeader class. Would it make sense to implement the same change there?

https://github.com/Microsoft/OpenAPI.NET/blob/5742db6b5f0f8bd9990fbf43a7e03a8b0b6f674a/src/Microsoft.OpenApi/Models/OpenApiHeader.cs#L131

@mapitman
Copy link
Contributor Author

Any chance of merging this sometime soon?

@darrelmiller
Copy link
Member

@mapitman I will look into this next week. Sorry for the delay

@jgreywolf
Copy link

I am interested in this fix as well. Any update?

@darrelmiller
Copy link
Member

I need to get a certificate updated before I can redeploy. Working on it.

@boekabart
Copy link

Appreciate you're busy and doing this for the community, @darrelmiller , but any update?

@darrelmiller
Copy link
Member

@boekabart My apologies. I have the new cert. Now I need to actually deploy it. I will try and do this later today.

@darrelmiller darrelmiller merged commit 4f329d1 into microsoft:master Aug 22, 2019
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.

5 participants