Standardize Parameter Models

[dilipkrish]

It seems odd that Serializable parameters be treated differently that Body Parameters. In the interest of consistency, I would think that regardless of the type of parameter it should always have a schema property.

"/api/pet/findByStatus": {
            "get": {
                ...
                "summary": "Finds Pets by status",
                "description": "Multiple status values can be provided with comma seperated strings",
                ...
                "parameters": [
                    {
                        "name": "status",
                        "in": "query",
                        "description": "Status values that need to be considered for filter",
                        "required": true,
                        "type": "string", //<-- NOTE: Seems like this should be standardized
                        "default": "available"
                    }
                ],

I would expect it to be rendered as shown below

"/api/pet/findByStatus": {
            "get": {
                ...
                "summary": "Finds Pets by status",
                "description": "Multiple status values can be provided with comma seperated strings",
                ...
                "parameters": [
                    {
                        "name": "status",
                        "in": "query",
                        "description": "Status values that need to be considered for filter",
                        "required": true,
                        "schema": {
                            "type": "string"
                        }, 
                        "default": "available"
                    }
                ],

[webron]

This was done intentionally to avoid allowing to define objects as non-body parameters. It would probably change for some types of form parameters but is unlikely to change for path/query/header parameters.

[dilipkrish]

I see, yet still it seems like an un-needed branching logic we have to deal with. Its seems pretty obvious that one cannot have objects in query/path/header and cookies. From a consumption-tooling perspective I'd prefer to not have different logic based on the type of parameter and rely on a schema property that will give information about the model.

[webron]

It's not really that obvious. In 1.2, you technically could have models as types for query and path parameters, which pretty much lead to this structure.

I don't disagree the divergence is inconvenient for tooling (and iirc, I wasn't too happy about it when we decided to do that in 2.0), but sometimes it's about finding the balance between helping the tools and helping people create proper specs.

I tend to think it's more important to protect from human-errors than make tooling-development easier. It's all in the numbers.

[MrTrick]

There is a valid concern about ensuring people don't define impossible things. However, within a technically valid swagger spec someone could already declare, e.g.:

• "produces":["image/jpg"] and a response that is JSON.
• A parameter "foo" that is "type":"integer", "maximum":3, "minimum":78

Supporting the 'schema' attr for all parameters allows better reuse of code and communication of intent. ("Ah, I see that the 'id' param must follow the same spec as the 'id' attribute of the response!")

Swagger validation could still check that the 'schema' doesn't try anything invalid, some of which can be performed by the 'schema.json' definition.
Comprehensive validation should be capable of finding all of those erroneous examples above.

[webron]

Even if we allowed referencing a 'schema object', to keep a similar structure, it would be a different limited 'schema object', so the structure of the parameter would be the same, but the validation will not. It'll complicate the spec, but it may be a necessary evil.

[MrTrick]

@webron I would argue that it could be a complete schema...
...It would just be implicitly "allOf"d with the swagger specification of what a parameter (query, path, etc) is allowed to be.

If the intersection of "valid swagger parameter" and "schema validation" is a NULL set, that parameter just won't be able to be used. (Friendly implementations, editors etc could detect and advise)

[webron]

Considering we don't allow model definitions in parameters other than body parameters, I don't see how that's going to work.

[MrTrick]

@webron I'm not advocating 'model' definitions in parameters, or object-type parameters. I'm advocating allowing 'schema', so that schemas that are defined elsewhere already can be reused. (as well as standardization benefits etc)

A simple example;
A: "Person" is a model, with a particular id;

"definitions": {
    ...
    "person_id": { 
        "type": "string", 
        "pattern": "^([A-Z]{3}\s?(\d{3}|\d{2}|d{1})\s?[A-Z])|([A-Z]\s?(\d{3}|\d{2}|\d{1})\s?[A-Z]{3})|(([A-HK-PRSVWY][A-HJ-PR-Y])\s?([0][2-9]|[1-9][0-9])\s?[A-HJ-PR-Z]{3})$" 
    },
    "Person": { 
        "type": "object", 
        "properties": { 
            "id": { "$ref": "/#definitions/person_id"}, 
            ... 
         }
    },
    ...
}

B: GET "/person/{id}" is a path returning a person object.
The parameter has to be a person id. If it's doesn't meet the above definition there's no way that the object exists, and no point feeding it to the application.
Currently the only way to define the id parameter is copy/paste;

{ 
    "name": "id",
    "in": "path", 
    "required": true,
    "type": "string",
    "pattern": "^([A-Z]{3}\s?(\d{3}|\d{2}|d{1})\s?[A-Z])|([A-Z]\s?(\d{3}|\d{2}|\d{1})\s?[A-Z]{3})|(([A-HK-PRSVWY][A-HJ-PR-Y])\s?([0][2-9]|[1-9][0-9])\s?[A-HJ-PR-Z]{3})$" 
},

which I'll argue is difficult to read, difficult to maintain (what if the person_id schema changes?), and not semantically helpful.
If the schema keyword was allowed on any param it could be defined instead as;

{ 
    "name": "id",
    "in": "path", 
    "required": true,
    "schema": { "$ref": "#/definitions/person_id" }
},

Your concern about developers defining invalid parameter types is justified, but it is possible for a library to catch that; the swagger schema checker can use something like "schema": { "properties": {"type": {"enum":[ ...a valid type... ]} } either directly or after dereferencing.

[jharmn]

Interesting. So to refine the concept, the following fields would be part of a parameter definition:

• name
• description
• in
• required
• default

All other fields could be referenced from a schema, as is the norm within models.

So to take your examples a bit further, you could end up with this:

swagger.json

"parameters": [
                    {
                        "$ref": "parameters.json#status" 
                    }
                ],

parameters.json

"status": {
  "name": "status",
  "in": "query",
  "description": "Status values that need to be considered for filter",
  "required": true,
  "default": "available",
  "schema": {
    "$ref": "model.json#id"
  }
}

model.json

{
  "type": "string",
  "pattern": "([A-Z]{5,8})"
}

From here, you could also expect to use model.json from a request/response/etc as well...making the ID definition completely DRY.

+1

[MrTrick]

@jasonh-n-austin yes that's correct. My example was kept very simple to avoid confusion, but one would probably use a reference for the parameter too.

I think for backwards-compatibility Swagger should continue to support those current validation fields, allow 'schema', and implementations should check values against both if both are defined.

[webron]

Maybe I came off wrong. I'm all for allowing reusability for primitive definitions. The only restrictions is that we should limit those to primitive definitions. This will be more challenging when it comes to the JSON schema of the spec, the spec document itself, and possibly some aspects of the tooling, but in this case, I don't think these factors should stop us from providing support for it.

[MrTrick]

Sounds good to me, at least as far as; A solution to the "reusability" issue should not try to completely change other aspects of the spec.

[webron]

Parent issue: #565.

[webron]

@dilipkrish - I believe this can be closed following #654. Please reopen if you feel otherwise.