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

Default for collectionFormat is "multi" but should be "csv" #1893

Closed
michaelkourlas opened this issue Aug 24, 2016 · 7 comments
Closed

Default for collectionFormat is "multi" but should be "csv" #1893

michaelkourlas opened this issue Aug 24, 2016 · 7 comments

Comments

@michaelkourlas
Copy link

michaelkourlas commented Aug 24, 2016

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":

collectionFormat

string

[...]

Default value is 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.

@webron
Copy link
Contributor

webron commented Aug 24, 2016

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.

@michaelkourlas
Copy link
Author

@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?

@webron
Copy link
Contributor

webron commented Aug 24, 2016

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.

@michaelkourlas
Copy link
Author

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".

@michaelkourlas
Copy link
Author

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.

@webron
Copy link
Contributor

webron commented Aug 24, 2016

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 multi. Of course, there may be tests that would require fixing. PRs are welcome :)

@fehguy
Copy link
Contributor

fehguy commented Dec 24, 2016

This is configurable now in 1.5.11

@fehguy fehguy closed this as completed Dec 24, 2016
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

No branches or pull requests

3 participants