OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
28.91k stars 9.07k forks source link

Proposal: Querystring in Path Specification #164

Closed howlowck closed 8 months ago

howlowck commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

chrisdostert commented 9 years ago

@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 commented 9 years ago

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.

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

chrisdostert commented 9 years ago

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 commented 9 years ago

+1

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

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 commented 9 years ago

@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 commented 9 years ago

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 commented 8 years ago

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 commented 8 years ago

+1

tebs1200 commented 8 years ago

+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 commented 8 years ago

+1

webron commented 8 years ago

Parent: #574.

davidnewcomb commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

iambingoNJU commented 6 years ago

@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 commented 3 years ago

Is there any progress on this?

darrelmiller commented 3 years ago

No. It is one of the topics under consideration for the next version https://github.com/OAI/OpenAPI-Specification/issues/2572

hyeonisism commented 2 years ago

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 commented 8 months ago

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.