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

Proposal: Querystring in Path Specification #164

Closed
howlowck opened this issue Oct 17, 2014 · 30 comments
Closed

Proposal: Querystring in Path Specification #164

howlowck opened this issue Oct 17, 2014 · 30 comments
Labels
Moved to Moonwalk Issues that can be closed or migrated as being addressed in Moonwalk

Comments

@howlowck
Copy link

Issue

In some APIs, different query string values specifies the operation done on the resource, thus, result in dramatically different response types and/or parameter requirements. In the case of Foursquare Venues Search API, depending on the intent query string value, the requirement specs of other query string vary.
Such distinction deserves a separate path entity, to allow different descriptions, parameter requirements, responses, etc.

Current State

Currently due to Swagger Spec, on swagger-ui, the generated url for testing is incorrect if a query string is included in the path.

"paths": {
    "/bluelights?query=nearby": {

becomes:
.../bluelights?query=nearby?more=...

Proposal

Allow Query Strings in the path key

The example above is allowed. so for example, depending on the path key, "/bluelights?query=nearby" and "/bluelights" are considered separate path entities.

@mohsen1
Copy link
Contributor

mohsen1 commented Oct 20, 2014

This is the same issue with Content-Type operation overloading we are discussing at #146
The idea is your endpoint/operation behaves differently based on one or many parameters. How should we address this?

@chrisdostert
Copy link

Another scenario to consider why this should be added:
lets say i have a few distinct search operations on a photos resource:

Get: /photos?mentions=Jim "retrieves photos where jim is mentioned"
Get: /photos?hashtags=selfie "retrieves photos where selfie is hashtagged"

I want my interface to be narrow per SOLID coding principles and don't have a use case that requires searching on both of these attributes simultaneously so it would just be confusing to merge them into one operation.

Without the swagger spec supporting parameter based operation discrimination I can't describe both of these operations separately.

@earth2marsh
Copy link
Member

@chrisdostert If I understand your example correctly, the functionality is the same in both cases (return a list of photos), but the query string is serving as a filter on the responses. Is what you're hoping to do a primarily a documentation concern (i.e. list them as separate methods in Swagger-UI), or is it something more?

Swagger's point of view is that resource paths plus verb are the signature, and that the response object is deterministic from that signature. So far, the project has resisted allowing query parameters to factor into the operation signature, and it is a pretty fundamental to the Swager point of view on API design.

Perhaps @webron or @fehguy have more insight.

@chrisdostert
Copy link

@earth2marsh yes they are distinct methods. They perform different operations and should be individually documented.

@chrisdostert
Copy link

@webron and @fehguy , welcoming discussion regarding why an API containing the operations above either conflicts with the "Swagger point of view on API design" or, if it provides a valid justification to enhance Swagger to consider query parameters in it's definition of an operation signature

@fehguy
Copy link
Contributor

fehguy commented Mar 16, 2015

We did spend time on this subject and decided it wasn't the right direction. That can change for future versions, but here are some of the reasons.

  • Making an operation deterministic while considering query params can get very complicated. What if QPs are optional? The selection criteria gets very troublesome
  • Do we consider values in selecting the operation path? Does /foo/bar?count=1 get treated differently from /foo/bar?count={count}?
  • As a consumer of the service, how much "logic" is required to interpret how different query param values affect the response type? You could for instance have ?type=dog or ?type=order and return different response models, because this constitutes a different operation path. Is that intuitive for the consumer, or does it place more burden on them?

The conclusion was that this was a can of worms and it'd make the api harder to work with. There are other ways to represent the same logic without relying on query parameters that may be more easily understood. The rest of the toolchain would become very, very complicated to make consistent and reliable.

I hope that helps at least with the past decision making.

@mohsen1
Copy link
Contributor

mohsen1 commented Mar 16, 2015

An operation can return different type of responses (with the same response code) in many APIs. The reason why an operation is returning certain response can be a lot of things. It can be because a query param is determining it (your example), it can be content negotiation (Content-Type header), it can be internationalization or localization. Current version of Swagger does not support operation overloading (having different response for an operation with the same response code) but that doesn't mean we can't have it in the next next version of Swagger. That's why this issue has Swagger.Next tag.

Swagger should not describe logic of your API. In your API query param is determining the response shape but in other APIs it can be different. My proposal for this is to describe all possible responses without binding each response to a query param or a header param or anything else. That logic can be described in description field of each response.

Here is my proposal for operator overloading:

swagger: '2.1'
info:
  version: 1.0.0
  title: Overloading example
paths:
  /:
    get:
      responses:
        200:
          oneOf:
            - description: Returns a string
              schema:
                type: string
            - description: Returns a number
              schema:
                type: integer

@chrisdostert
Copy link

@fehguy the concerns raised by your examples are squelched if you only consider the presence of query parameters and not their values (as operation discriminators). Thoughts?

@fehguy
Copy link
Contributor

fehguy commented Mar 16, 2015

side question. isn't that intent more clearly represented in a well-designed path as opposed to query params?

@chrisdostert
Copy link

I mean you get into personal preferences with that question right?
a path based implementation of the examples might be something like:
Get: /users/jim/photos-mentioned-in "retrieves photos where jim is mentioned"
Get: /hashtags/selfie/photos-labeled-with "retrieves photos where selfie is hashtagged"

obviously there are alternative path and query parameter based implementations out there from what i've given in my examples (that ultimately expose the same operations) but I think you'd end up with nearly a fair split with regards to people preferring path based over parameter based and I'd certainly say it would be case by case with respect to the particular operation being exposed.

@zdila
Copy link

zdila commented Jun 4, 2015

+1

In our case we need it for POST to execute 4 various security actions:

  • /v1/security/credentials?action=changePassword
  • /v1/security/credentials?action=requestPasswordReset
  • /v1/security/credentials?action=resetPassword
  • /v1/security/credentials?action=remindUsername

Every action is implemented as different method with different POST body.

We don't want to distinguish actions by path (eg. /v1/security/credentials/changePassword) because paths should be nouns, not verbs.

@olensmar
Copy link

olensmar commented Jun 4, 2015

@zdila in this specific case you could perhaps model your api as follows:

PUT /v1/security/credentials/password -> changes the password (body contains new password)
DELETE /v1/security/credentials/password -> resets the password
POST /v1/security/credentials/password/reset -> requests a password reset (body could contain a message)
POST /v1/security/credentials/username/reminder -> creates a reminder (body could contain a message)

if you agree that a "reset" could be a noun (as in "I'd like to request a password reset") - then the (somewhat lofty) requirement "paths should be nouns, not verbs." is at least fulfilled :-)

/Ole

@SamG1000
Copy link

I have a similar issue but in my case my response type is the same:

For example, let's there there is a User service that allows you to create, remove, or search for users:

GET /user/{userid}
PUT /user/{userid}
POST /user/

And search API:

GET /user?username={username}
GET /user?firstName={first}&lastName={last}
GET /user?phone={phone}
GET /user?address1={add1}&city={city}&state={state}

Theoretically they all can be defined as a single Swagger operation

GET /user?username={username}&firstName={first}&lastName={last}&phone={phone}&address1={add1}&city={city}&state={state}

But how to you document which fields belong to which operation? Making them all optional is miss-information. Also client code should really have 4 separate methods.

Hypothetically, you could add:

GET /user/searchByUsername?username
GET /user/searchByName?firstName={first}&lastName={last}

But this would go against REST-ful principle of a verbs in the resource URI - "/user/searchByUsername" is not a resource.

You could also go all out and treat search as a resource:

POST /search/username?username="username"
201 Created 
/search/search12345

GET /search/search12345
{
   username : "username",
   firstName : "first",
   lastName : "last",
   ...
}

The first option is preferred and property describes the API. Any recommendations?

@Pangamma
Copy link

Pangamma commented Nov 3, 2015

I'd love to see this get implemented. It looks like @SamG1000 has already said what I was about to say. QueryString params are useful for searches, and they keeps the code clean. Documentation is also better when you're able to split it up by the parameter set.

/REST/Customers/
/REST/Customers/{ID}
/REST/Customers?phone=xxxx
/REST/Customers?email=xxx
/REST/Customers?firstName=xxx
/REST/Customers?lastName=xxx

@olensmar please consider adding this ability. We'd like to start using this at my current company, but to do that we'd need to be able to use the QueryString paths so we wouldn't have to change all of our routes across the project.

@rajeshkamal5050
Copy link

+1

@tebs1200
Copy link

tebs1200 commented Jan 25, 2016

+1

There's another useful scenario where the structure of a response can change substantially based on a query parameter - expansion of nested collections.

To reduce the number of API queries a client needs to make, it's often valuable to provide the option to include all instances of a related collection in a single response. Say you have an /api/articles/{id} endpoint and a related /api/articles/{id}/comments endpoint. You can provide the article and it's comments in a single query and response with something like /api/articles/{id}?expand=comments

@yaronf
Copy link

yaronf commented Mar 27, 2016

+1

@webron
Copy link
Member

webron commented Mar 27, 2016

Parent: #574.

@davidnewcomb
Copy link

davidnewcomb commented Sep 14, 2016

I came here looking for help with deciding how to deal with different versions of objects returned from a RESTful API which I think is a subset of the wider problem of returning different responses from the same resource.
The majority of the used cases given above seem to be trying to overcome really poor RESTful API design models. The best example of this is chrisdostert (although he is one of many): "I have 2 completely different operations on the same resource that return the same (or different) things but I have chosen to put them under the same RESTful resource name". While the programming language allows him to do this, it is up to the spec writers to promote good design. This was the best self-defeating argument for better API design. Try /photos/mention/{name} or /photos/hashtags/{tags}. This adheres to RESTful principles, uses nouns, fits in with the current swagger spec and produces a nice clean (cachable?) interface. Making an interface narrow doesn't mean reusing the same name for everything!
While users might have a preference for path-based or query-based parameters, the REST interface definitely has a preference for path-based parameters. So speaking with my hard hat on "get with the program"! Swagger documents RESTful interfaces so the more RESTful you are the more swagger will work for you. The more query-based you are the more cases there will be (rightly-so) to stop you getting what you want from swagger.
Even @howlowck includes a case where "query" should be included in the path as it is a parameter that changes the resultant object. In his case he has actually made a trade off between writing less documentation over writing a poorer interface. If there are many different search types with different search results he should document them individually (as he knows what they are in advance) and make them part of the RESTful interface (or get Foursquare Venues Search API) to be more RESTful.
I think allowing query parameters to be included in swagger definitions is a terrible idea and if you need to do it then it shows that your interface needs refactoring to be more RESTful.
In my case with versioning, I don't think my returned objects are going to be wildly different but if they are there is a case for creating a new resource type.
So while documentation shouldn't dictate program design it should promote best practice.

@darrelmiller
Copy link
Member

@davidnewcomb

REST interface definitely has a preference for path-based parameters

Can you point to specifications that substantiate this claim? My understanding is this a misconception based on text in RFC 2396 that was corrected in RFC 3986 back in 2005. A resource is identified by a URI and query parameters are an integral part of that identifier.

Calling other people's designs out because they don't conform to your perspective of what is RESTful is not helpful. If you see issues in designs that violate RESTful constraints then you are welcome to explain the negative system effects. However, recognize that every choice is a trade off and there may be valid reasons for choosing to violate a constraint.

OpenAPI is a specification to help describe HTTP APIs, it uses the term REST loosely because that is the term people have come to expect when building HTTP APIs. Actual REST based systems do not use design time descriptions of resources, as that would inhibit evolvability, so debating REST constraints here is not useful.

@davidnewcomb
Copy link

After doing a lot of extra reading, I must now eat humble pie. I have been labouring under out-of-date knowledge. It also turns out all my knowledge about internet caching was also out-of-date too as that was the (previous) primary reason for being against this proposal.

Now armed with the most up-to-date information, I'm still apprehensive about this proposal. While I realise there are many influences on the RESTful design an organisation might take, it looks like there must be a matrix of query parameters, headers, paths and whatever else in order to describe what combinations will have what return types.
Some of the users above talk about receiving an object with restricted data depending on who you are logged in as, which will present significant challenges for the code-generation side of swagger.
The nice thing about the current implementation is that I can see what type of object, I will get when I make a call.

@darrelmiller
Copy link
Member

@davidnewcomb I agree that using only the path to identify the resource is a nice simplification that is sufficient for most cases. However, a significant number of people come to OpenAPI to try and document an existing API. We have to balance encouraging good practices with being compatible with the wide range of ways HTTP APIs are implemented.

@iambingoNJU
Copy link

I just want to know if this feature has been added to the latest version of the spec and if it is implemeted in swagger editor. Could anybody help?

@chrisdostert
Copy link

sorry for late response but @davidnewcomb wait.. I said no such thing AFAICT; not sure where that quote back in 2016 is coming from

@darrelmiller
Copy link
Member

@iambingoNJU No. Version 3.0 of the spec still only uses the path portion of the resource identifier to identify operations.

@iambingoNJU
Copy link

@darrelmiller Thx! And sorry for my late reply. And how do you fix this kind of problem? Maybe I think I should change the service endpoint.

@djebos
Copy link

djebos commented Aug 20, 2021

Is there any progress on this?

@darrelmiller
Copy link
Member

No. It is one of the topics under consideration for the next version #2572

@hyeonisism
Copy link

Sometimes there are the same path, but there are different operations with query param.
It looks like @SamG1000 has already said what I was about to say.
Also, I don't think the definition of paths is wrong.

Open API spec is designed to understand and interact with consumers.
However, this problem has led to misunderstand HTTP API.

So each path which has a different query param should be defined to understand.
If not, people who have similar problems have to redesign the HTTP API for open api spec or consumer should misunderstand through documents.

I hope this problem will be solved in the next version.

@handrews
Copy link
Member

This general concept of this proposal has been accepted into Moonwalk under the "operation signatures" principle. 🎉

Since a change of this magnitude can't go in the 3.x release line, I'm closing this issue in favor of the Moonwalk discussions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Moved to Moonwalk Issues that can be closed or migrated as being addressed in Moonwalk
Projects
None yet
Development

No branches or pull requests