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

Added optional path parameter support #210

Closed
wants to merge 1 commit into from
Closed

Added optional path parameter support #210

wants to merge 1 commit into from

Conversation

cyyuen
Copy link

@cyyuen cyyuen commented May 20, 2013

{
"apis": [
{
"path": "/foo(/{optional})",
"operations": [
"parameters": [
{
"dataType": "string",
"name": "optional",
"required": false,
"paramType": "path"
}
]
]
}
]
}

If the optional parameter is inputted, say the value is val, the
request path will become /foo/val.
Otherwise the path will become /foo

If one of the path parameter is set to be optional, all of the path
parameter with the same must be optional.
The following path:
/foo(/{optional})/{optional}
is invalid. invalid path exception will be throw

{
    "apis": [
        {
            "path": "/foo(/{optional})",
            "operations": [
                "parameters": [
                    {
                        "dataType": "string",
                        "name": "optional",
                        "required": false,
                        "paramType": "path"
                    }
                ]
            ]
        }
    ]
}

If the optional parameter is inputted, say the value is val, the
request path will become /foo/val.
Otherwise the path will become /foo

If one of the path parameter is set to be optional, all of the path
parameter with the same must be optional.
The following path:
    /foo(/{optional})/{optional}
is invalid. invalid path exception will be throw
@cyyuen
Copy link
Author

cyyuen commented May 22, 2013

@fehguy . How do you think about it?

I think the optional path parameter should be supported since the RESTful framework, say Slim supports the optional path parameter.

@fehguy
Copy link
Contributor

fehguy commented May 22, 2013

Hey @ytsTony what happens in this scenario?

You have these routes:

/foo/{bar}/find
/foo/{bar}

where bar is optional. User invokes /foo/find--which route do you follow? What should the user expect as a return?

@cyyuen
Copy link
Author

cyyuen commented May 22, 2013

These kind of routes would cause confusion, but I think the mechanism that handling such routes is the responsibility of the server side application. They might either forbid these case occur or have other way to deal with.

As swagger, sending such request should be supported since the user knows the exact path.

@cyyuen
Copy link
Author

cyyuen commented May 23, 2013

Hey @fehguy . I think the following scenario will cause the sample confusion as the one you point out with the optional path.

You have these routes:

/foo/{bar}/find
/foo/find/{bar}

User invokes /foo/find/find . It will also make the confusion.

@aruizca
Copy link

aruizca commented Mar 24, 2014

+1 I have this URL with an optional path param and the UI still thinks is required:

/api/user/list/{searchParam : (\w+)?}

@fehguy
Copy link
Contributor

fehguy commented Mar 24, 2014

Take a look here for details on the spec--optional path params are not supported.

https://github.com/wordnik/swagger-spec

@fehguy fehguy closed this Mar 24, 2014
@aruizca
Copy link

aruizca commented Mar 25, 2014

Well, maybe the official JSR does not include it, but that was clearly a mistake, because as you can see they are needed and there are workarounds and even someone has take the trouble to give you an elegant solution. Any other argument is just stubbornness :-)
I bet there are optional parameters in the new JSR 339: JAX-RS 2.0 spec

@fehguy
Copy link
Contributor

fehguy commented Mar 25, 2014

Hi, I'm not trying to be stubborn but rather follow the swagger-1.2 spec which is quite widely distributed--design mistake or not. I STRONGLY encourage you to submit issues to the spec for improvements as that's the way to move the ball forward.

@aruizca
Copy link

aruizca commented Mar 25, 2014

Fair enough! I will submit an issue then :-) 👍

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

Successfully merging this pull request may close these issues.

3 participants