-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Default for collectionFormat is "multi" but should be "csv" #1893
Comments
There's no contradiction. If by default jax-rs implementations treat these parameters as multi, then by default, we will describe those as multi and not csv. The spec says that if a collectionFormat is not declared, then it defaults to csv. |
@webron: Sorry, I don't follow. What do you mean when you say that "by default jax-rs implementations treat these parameters as multi"? Shouldn't all implementations be using "csv" by default (i.e. if no collectionFormat is provided) since that's what the spec says? |
The spec is implementation agnostic. It simply says - if there's no collectionFormat mentioned, it will default to csv. It's there for convenience so you don't have to specify it over and over again. However, when it comes to jax-rs, it doesn't use csv by default. It uses multi. That's implementation specific. So when the spec is generated, it's supposed to document the actual API, not what the defaults in the spec are set to be. So... it is set to multi, not csv. |
I'm sorry, but I'm still confused. My understanding is that swagger-core is an implementation of the Swagger specification, and so the models for parameters must behave in accordance with the specification, including any default values for properties defined by the specification. I don't understand what any of this has to do with JAX-RS. Let me explain why I opened this issue in the first place. I was using swagger-codegen to generate a Java API library from an API specification file. swagger-codegen uses swagger-parser to parse the API specification file. swagger-parser in turn uses swagger-models (part of swagger-core) to represent the specification using Java objects. The API specification that I am using defines a particular query parameter that looks something like this: "testParam": {
"name": "testParam",
"in": "query",
"type": "array",
"items": {
"type": "string"
}
} As you can see, the parameter is of type "array" but has no explicitly defined collectionFormat. The authors of the API specification relied on the default behaviour being "csv", as defined in the Swagger specification. However, when swagger-parser parsed this API specification, it returned a parameter object with a collectionFormat of "multi", which caused problems with the generated Java API. I believe this happened because swagger-parser relies on swagger-models to provide the default values for unspecified properties, and the QueryParameter class in swagger-models defines the default collectionFormat to be "multi", even though it is supposed to be "csv". |
Ah, I think I've figured out what's going on here. You're saying that swagger-core is used to generate an API specification from an existing JAX-RS web service, so that's why the "default" value for collectionFormat is just whatever JAX-RS does, in this case "multi". However, swagger-core is used by swagger-parser to go in the opposite direction - to parse an existing specification. In this case, the default value should be whatever the Swagger specification says, in this case "csv". Is swagger-core supposed to be used to parse Swagger specifications? If not, then this is a problem with swagger-parser; if so, then this is a problem with swagger-core. |
Okay, so yes, those details were important for the ticket. So the behavior of swagger-core for generating the spec is fine, and it has affected what you're describing in swagger-parser as a side effect. I would suggest opening a ticket on swagger-parser with the problem you stated, and linking to this issue from there. It's important for tracking. What needs to be changed is https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-models/src/main/java/io/swagger/models/parameters/QueryParameter.java#L11 and https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-models/src/main/java/io/swagger/models/parameters/FormParameter.java#L11, however the jax-rs parameter processor needs to be updated as well to set these by default to |
This is configurable now in 1.5.11 |
As part of issue #1160, it looks like the default value for "collectionFormat" for parameters was changed to "multi". However, according to the Swagger specification, the default is supposed to be "csv":
I realize there's been some discussion about changing the default to "multi" in version 3.0 of the spec, but swagger-core is currently supposed to be compatible with version 2.0, right? As it stands now, this breaks APIs that rely on the default behaviour provided for in the spec.
The text was updated successfully, but these errors were encountered: