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

additionalProperties is generated with a boolean in swagger2 #2170

Closed
ling1726 opened this issue May 15, 2019 · 16 comments · Fixed by RicoSuter/NJsonSchema#984
Closed

additionalProperties is generated with a boolean in swagger2 #2170

ling1726 opened this issue May 15, 2019 · 16 comments · Fixed by RicoSuter/NJsonSchema#984

Comments

@ling1726
Copy link

When using webapi2swagger with outputType set as swagger2 , the additionalProperties field is generated with a boolean set to false.

This is not correct for swagger2 since the boolean value was only introduced for this field in open api 3

@RicoSuter
Copy link
Owner

What't the alternative way to express this in Swagger 2?

@ling1726
Copy link
Author

In Swagger 2 additionalProperties cannot take a boolean value. Either it is present, or it takes a Schema Object as value

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#schema-object

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#schema-object

See the differences in spec in the above specifications (v2 and v3)

@RicoSuter
Copy link
Owner

So we should just disable generation of "additionalProperties": false because this is the default for Swagger 2? The default for OpenAPI 3/JSON Schema is true I think so there it is needed... correct?

@ling1726
Copy link
Author

I agree with disabling the generation of this field in swagger 2.

I think you can disable it for OpenApi 3 too, since if it's not present the spec assumes that field is true.

On a broader note, how is this property generated if it does take a schema object in NSwag ?

@RicoSuter
Copy link
Owner

It is generated as a property with an object, e.g. "additionalProperties": { ... }

Currently it is only set to false because the default is true if it is true and has no schema then it is omitted. The big question is what are the defaults of this property for Swagger2/OpenAPI3/JSON Schema?

@ling1726
Copy link
Author

The default is only true for OpenAPI3

Swagger2 has no notion of a boolean value for this property

@RicoSuter
Copy link
Owner

The question is the behavior...

@ling1726
Copy link
Author

ling1726 commented May 28, 2019

If you check here https://swagger.io/docs/specification/data-models/data-types/

you'll see that additionalProperties: true is equivalent to additionalProperties: {} which is the default behaviour. setting one of the two previous values, just makes this explicit

@RicoSuter
Copy link
Owner

you'll see that additionalProperties: true is equivalent to additionalProperties: {} which is the default behaviour

How can I disable additional properties when not setting the property defaults to additionalProperties: {}?

@RicoSuter
Copy link
Owner

@ling1726
Copy link
Author

According to that issue thread in swagger, that would should mean that we can assume additionalProperties as false if it isn't present. If the field is present then it should be a JSON Schema.

Only in the OAS v3 spec is additionalProperties: false actually required. As seen in this PR OAI/OpenAPI-Specification#1548

@RicoSuter
Copy link
Owner

Just nice, what a big mess these whole JSON Schema/Swagger 2/OpenAPI 3 specs are...

@ling1726
Copy link
Author

I agree 👍

My main issue here, is swagger v2 parsers/validators might fail at the generated swagger output, since they will not expect this field to have a boolean as a value

@RicoSuter
Copy link
Owner

Created a PR ... tests missing

@ling1726
Copy link
Author

Cool, I'll take a look at it in a bit, unless you want to go ahead and merge it yourself ;)

@RicoSuter
Copy link
Owner

Please check/review the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants