OAI / OpenAPI-Specification

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

Support for Either #57

Closed ikitommi closed 7 years ago

ikitommi commented 10 years ago

hi.

Would like to be able present a type of Either[Dog,Cat] (or in clojure (either Dog, Cat, Moose) in the Spec. Quite common construct in the FP land. Would be like a enum, but with values of any type.

Currently such a thing could be presented as a complex type with optional fields for all the value => at least the swagger-ui would be able to generate sample models (with all possible alternatives in place) for it. Not perfect, but ~works. Requires also special coercion in the server side to map back into Either.

http://www.scala-lang.org/api/2.11.0/index.html#scala.util.Either

https://github.com/Prismatic/schema/blob/master/test/cljx/schema/core_test.cljx#L194-L203

ikitommi commented 10 years ago

... in JSON Schema this would be implemented most likely with oneOf like:

{
  "oneOf": [
    { "$ref": "Cat" },
    { "$ref": "Dog" }
  ]
}
webron commented 10 years ago

Okay, in Swagger 2.0 we explicitly decided not to support this. There are various issues with supporting it ranging from how deterministic such an API is to can most common development languages support this feature easily.

I'm closing this issue for now, but marking it as a proposal for the next version of the spec, so that we may revisit this decision.

ahultgren commented 10 years ago

I wonder, if I have a resource which sends a response as an array that contains both say articles and ads, how would i define such a response in swagger? Combining all properties in one response object is really ugly and kinda makes validation based on the spec useless. I think oneOf would be a good solution for this case, unless I've missed another way of doing it?

webron commented 10 years ago

We thrive to describe deterministic APIs. Sending a single array of both articles and ads makes it that much more difficult to process, especially for strongly-typed languages, so we don't support it. If we can find an elegant way to do it, we'll be more than happy to add support for it.

jphastings commented 9 years ago

@webron Which languages would be most important to support? I mentioned on IRC the use case of GeoJSON, which I think is an important structure to be able to validate:

{
  "type": "Point",
  "coordinates": [-3.0107731, 16.7733758]
}
{
  "type": "LineString",
  "coordinates": [[-3.0107731, 16.7733758], [-101.8314831,35.2090451]]
}
davidsklar commented 9 years ago

I am new to Swagger and have already encountered several areas where this would be useful. With respect to the deterministic-API goal, what are popular-enough strongly-typed languages that would have great difficulty processing, e.g. a heterogenous collection? (As long as the possible types of the collection were explicitly enumerated and were defined (in swagger and the other language) in an appropriate way to indicate a relationship (parent-class/child-class, etc) that played nice with the type system.

stevage commented 9 years ago

I think I understand the reasons for discouraging people from designing APIs where different types of objects can appear in the same place. (The term "non-deterministic API" doesn't seem quite right to me though.)

But anyway, it's a bit of a problem for documenting existing APIs (or GeoJSON, above). I'm working on one at the moment where a certain response contains an array of "stops" and "lines", where each is basically:

{
  "type": "stop", # or "line"
  "result": {
     # other properties depend on type
  }
}

Obviously it's possible to implement this in Swagger but it's pretty hacky and requires duplication and having to explain in words what's going on.

webron commented 9 years ago

This is not about API design only, this is also related to the ability to provide support for it in the various tools we offer around Swagger. Since there is a workaround for some cases, it may be good enough for now. That said. it does not mean it will not be considered for the next version, we just need to do it wisely.

bgrant0607 commented 9 years ago

What is the workaround for unions, discriminated or otherwise?

The Kubernetes API has a number of such cases. We currently use an object with all optional fields, where only one of the fields is expected. For example, PersistentVolumeSpec may contain details specific to a particular storage backend (swagger 1.2, but I believe it would be similar in 2.0): https://github.com/kubernetes/kubernetes/blob/master/api/swagger-spec/v1.json#L11994

fehguy commented 9 years ago

I believe with the spec as-is, you would document the common shared parameters in a base schema and use a discriminator field to represent acceptable concrete types. For the next version of the spec, agreed this needs attention.

Fresa commented 9 years ago

I also agree this is needed to fully support polymorphism.

erikvanzijst commented 9 years ago

Since there is a workaround for some cases, it may be good enough for now.

I've just run into this also, evaluating Swagger 2.0 for our existing API, but I'm not sure what the suggested workaround is?

webron commented 9 years ago

@erikvanzijst - the workaround is related to when you have a common property between the options (such as type) which can be used as the discriminator. You can find more information about the discriminator in the spec itself.

https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#schemaDiscriminator https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#composition-and-inheritance-polymorphism

erikvanzijst commented 9 years ago

Right. I'll have a look at that.

witoldsz commented 9 years ago

@webron

We thrive to describe deterministic APIs. Sending a single array of both articles and ads makes it that much more difficult to process, especially for strongly-typed languages, so we don't support it.

This is the most ridiculous explanation I could possibly imagine about not supporting oneOf :flushed:

IvanGoncharov commented 9 years ago

@witoldsz @webron I think this discussion shifted towards incompatibility with JSON Schema topic. So it became duplicate of #333. Can we move discussion there and close this issue?

webron commented 9 years ago

@witoldsz - sometimes the wording I use is not the best, but even if you disagree there are probably better ways to express it. If you'd like to discuss it further, I'm all for it.

@IvanGoncharov - while they are certainly related, I think it should remain open. When we start actual work on the next version we'll do a consolidation of issues (there are a few that apply).

gmathieu commented 9 years ago

The value of the chosen property has to be the friendly name given to the model under the definitions property

@webron, if I understand this properly, all the discriminator values must be declared inside definitions at the root. Does anyone have a workaround for conflicting discriminator values illustrated in the following?

---
definitions:
  resource1:
    discriminator: type
    required:
      - type
    properties:
      type:
        type: string
        enum:
          - custom

  resource2:
    discriminator: type
    required:
      - type
    properties:
      type:
        type: string
        enum:
          - custom

  # resource1.type=custom
  custom: {}

  # resource2.type=custom
  custom: {}
webron commented 9 years ago

@gmathieu - not sure what you're trying to do there, but this may be a bit more of a support issue. Mind providing more details in the google group?

gmathieu commented 9 years ago

Thanks for the quick response!

Anyone who's interested in the follow question: https://groups.google.com/forum/#!topic/swagger-swaggersocket/snqVqH_cCRY

mewalig commented 8 years ago

A common use case I run into is very simple: a parameter must be one string out of several predefined values (e.g. the parameter "pet" can be only be "dog" or "cat"). This seems ridiculously simple to add Swagger support for, if confined to that case (let's call it 'string-only-oneOf').

I understand that supporting a more generic case is harder, and that it may be redundant, if you eventually implement that generalized support, to have first supported only the simple (string-only-oneOf) case. But in making the decision not to support any such cases, in order to come up with (or devote resources to) the more generalized solution, what seems to be missing from this discussion is any consideration of whether those other cases are worth the delay in solving the simple (string-only-oneOf) case.

Just how often do these other cases arise relative to the simple case? At some point, if the simple case is very common, and the other cases are very uncommon, isn't it worth it to bite the idealistic bullet and solve the simple case separately from the generalized case?

Or even more simply put: at some point, if there is enough demand for the simple case, then, regardless of how much that demand is relative to the other cases, if there is a simple solution to that simple case, won't it be worth implementing?

At the least it might be useful, if it hasn't already been done, to take some sort of gauge (an informal survey, perhaps) of what the demand might be just of that one simple case.

abstratt commented 8 years ago

@mewalig Doesn't the existing enum support do exactly what you need?

mewalig commented 8 years ago

@abstratt: Man I wish I had seen that earlier. Thanks! (am I supposed to delete my original comment or leave it there?)

webron commented 8 years ago

@mewalig - I tend to think it's better to keep things intact. Even posts like that can help us understand what may be wrong, if not in implementation than in documentation.

@abstratt - thanks for providing the answer.

ghost commented 8 years ago

A different opportunity around parameters:-

I am looking for oneOf for defining parameters with a type of in: header (not that is header specific necessarily). We support either a HTTP Basic Authorization Header or a token based Authentication-Token header. I cannot set them both to required: true as it is one or the other not both, nor can I set them both to required: false which would indicate the user need not pass either.

So in this case it is more ambiguous to the client not to have it. That I understand the implementation complexities for the tooling though so it is a rock and a hard place. I appreciate not many people need this type of functionality but when you do the option of putting the information in the description fields or setting both parameters to true and living with getting both headers means neither are my favourite options.

webron commented 8 years ago

@JockMacMad - specifically for such headers, we have the security section, which supports describing an either/or security scheme so your use case can be described now. And as an added bonus, you don't have to repeat the definition for each operation and define it globally if it indeed applies to all operations.

ghost commented 8 years ago

@webron Indeed I have such a section defined for HTTP Basic and OpenToken. It looks look I need to pour over the spec again I think I missed something... and I don't want to dig myself deeper into the hole I just started :)

webron commented 8 years ago

Not at all. This is why we're here - to help make sense of things. Check out the securityDefinitions under the Swagger Object where you can define your schemes. Anything that you define under one scheme would have an AND between them. Then look into security under the Swagger Object or security under the Operation Object to define what's required globally or at the operation level respectively. That value is an array of schemes as defined in the Security Definitions, and between these you'll have an OR.

For your case, you'll create two schemes, and refer to both in the array.

ghost commented 8 years ago

@webron Okay I got it figured out.Thanks for that I had read it about 20 times already but still managed to not quite grasp it :)

I had the security definition but not got the security working on the operation. Shame the token support means I have to set the type: oauth2 as we use OpenToken but I can live with that, there is always a compromise to be had somewhere, and the scopes definition is a bonus for sure. The only downside is none of what I set shows in the HTML in the Editor i.e. no Auth types supported, the necessary headers for the call, nor the scopes (the scopes I can concede is not for the client but for the guy implementing the server side working from the definition it would have been a boon to see it in the HTML).

Again thanks for taking the time to reply.

webron commented 8 years ago

Parent: #579.

arthurdm commented 8 years ago

Hi. Is this the correct issue for tracking the inclusion of anyOff and oneOff into the first version of the OpenAPI Initiative spec? I have found many links back in here, but as the title says "Support for Either", I think it's worth the clarification.

The journey of trying to find a single issue that dealt with anyOff / oneOff took me to many different github issues and google group conversations, and I often encountered the response that this was "something to be considered for the next version".

I am encountering more and more users of RAML that went there simply because they could model their APIs using any part of JSON Schema. Granted, JSON Schema is not perfect and draft 4 seems a bit unattended, but it's still a spec many RESTful endpoints used before Swagger came along. So telling them to use Swagger is an uphill battle, specially when they are using the parts of the JSON Schema spec that are missing. If they can't model their current endpoints, why would they use Swagger? Nice tooling won't make up for an endpoint definition that doesn't properly describe or restrict the model. This is not meant to sound overcritical, it's just meant to report back what we're seeing in the field with customers.

My suggestion to the Open API Initiative members is to share this progression more openly, so that the whole community can help solve this. What are the specific blockers for code gen that anyOf/oneOff are causing? Which languages? Which swagger OS projects would work, and which would be broken as-is? Can language-specific limitations be placed on certain tool generators?

There's a lot of Swagger users out there that can help. If this information is already openly shared, a link in here would be great.

fehguy commented 8 years ago

Hi Arthur, this is a fine place for your comment. It is not currently supported and not clear if it will be. Just because one can model in another format doesn't make a good case for adding it to the OAS on it's own, but we are seriously considering it. There are many problems with doing so, including proposed changes in JSON Schema draft 5, tooling considerations, etc. Supporting this may give end users enough rope to certainly hang themselves, so it's being considered carefully. Please just track this one, we're addressing the inclusion (or not) very soon.

webron commented 8 years ago

@arthurdm thanks for the comment. Some of the issues are discussed in #333 (in... length).

As @fehguy mentioned above, just because another tool supports it, doesn't mean we should too. Our goal is not necessarily to attend all use cases (and the existing variety of options help).

We may end up saying we support these and and force the tools to try and handle the issues - I'm not sure we'll be doing our users a better service there though as some cases would require a lot of work to be supported (if they can be at all). That would lead to a different type of frustration - what's the point to be able to describe something if you can't actually use it? This is not just about the Swagger tooling (which are external to the OAI), but also true to any other tool developed out there. If we make it complicated to support, we may end up with less tools in the echo system. Do we tell users there are limitations? Do we tell tool developers they need to work harder? Just need to find the right balance.

pdo400 commented 8 years ago

anyOf is fundamental to a declarative schema. It's the or to allOf's and.

But I guess I care more about properly describing my API than having mediocre tooling around an improperly described API...

(And, sure, add more special cases like security rather than a general mechanism... THAT will make things easier to support... ??!?)

fehguy commented 8 years ago

huh?

witoldsz commented 8 years ago

+1 I also cannot understand the priorities here. What do I need "tools" for if the spec is unable to describe the API of my endpoints. I am also not sure why some guys here are telling the other how bad their API must be, knowing nothing but the "oneOf" requirement about the case.

Regards, Witold Szczerba 6 kwi 2016 00:59 "Patrick Whitesell" notifications@github.com napisał(a):

anyOf is fundamental to a declarative schema.

But I guess I care more about properly describing my API than having mediocre tooling around an improperly described API...

(And, sure, add more special cases like security rather than a general mechanism... THAT will make things easier to support... ??!?)

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/OAI/OpenAPI-Specification/issues/57#issuecomment-206025712

natebrunette commented 8 years ago

A common case is expanding entities. For example, using the Stripe Api, it may return a string, int, or null by default, but if you ask for the entire entity, it will return an object instead.

Granted, I believe I could specify each type it could be and copy every property of the entity, but in doing so, I lose a lot of reusability, which is one of the reasons I chose to use the spec.

In my opinion, it's odd to let the spec be defined by what's possible in a subset of languages or tools. If the problem is with statically typed languages, that sounds like a problem for implementations in those languages to solve. If the problem is with code generation tools, that sounds like a problem for those projects to solve.

lijo-jacob commented 8 years ago

Limiting the API Spec just because the tooling around it cannot support it does not make sense.

Here is a use case from the retail/search domain:- A keyword search API could return oneOf 1) Product Search Results 2) A keyword search redirect (Searching for special list of keywords could be redirected to special pages)

Keyword search API:

Endpoint: GET /search

Parameters: type=keyword q=makeup

Response could be any one of:-

1) Search Results

{
  "totalProducts": 77,
  "products": [
    ......
  ],
  "refinements": [
    .......
  ],
  "categories": [
    .......
  ],
  "keyword": "RAV4"
}

2) Search Redirects

{
  "searchRedirectTarget": {
    "targetScreen": "product",
    "targetValue": "P123456;skuId=246810",
    "targetUrl": "/my-toyota-rav4"
  },
  "keyword": "RAV4"
}
MatMoore commented 8 years ago

If OpenAPI wants to be agnostic about design-first vs code-first, then the specification has to be able to adequately describe real world APIs.

I don't think any decisions about inclusion in the specification should be determined by how easy or hard it is for a tool to implement. That is a problem that can be solved elsewhere, and if tool authors don't want to deal with it, then let them implement workarounds. By omitting something like this from the spec entirely you just force the problem on the users.

webron commented 8 years ago

Umm, allowing users to describe anything but have no tooling support doesn't change the problem being forced on the user - it just moves the problem from the design phase to the implementation or use phases. This is about keeping an ecosystem that works and plays well together - and yes, that may impose a limitation on design. Reiterating what's been said all along - supporting the entire spectrum of API designs is not the goal, but we're definitely looking to expand the percentage with each release, in a sustainable manner. Yes, some real world APIs will not be described by the OAS.

This is an old thread, and I don't remember all the details about it, and my opinions change over time too. Sometimes the problem is not with the 'what' we want to support but rather 'how'. This is definitely an issue we're going to try and tackle. We have yet to start dealing with this request.

blindsteal commented 8 years ago

I just stumbled upon this issue and can barely believe that this hasn't been resolved after 2 (!) years. I am currently working on a 'real world' project implementing an API, and having to use type: any instead of the proper types with 'anyOf' is a serious limitation. I can just guess why this issue hasn't gotten as much attention as it deserves: it is not common with statically typed languages where handling optional fields causes great pain (think Javas message format classes for deserialization, Go is similar I believe). Users of those languages seem to be the main audience of Swagger/OAI tooling). As of today however there is plenty of languages (Node, Ruby, Python, browser-based JS come to mind) that have absolutely no problem with that kind of API design. It is often even preferable to an increased number of endpoints or a 'wide-table'-like schema with a lot of optional attributes (see for example the search example mentioned). So +1 from me, don't let us hipster language users down ;)

jookshub commented 8 years ago

Hi guys,

after reading a lot, about this issue, I just want to throw in my vote for this to be supported soon.

If I understand all correctly it is currently not possible to state that an array must only consist of objects of a matching some given types?

My issue is simple, except in openAPI this seems not to be possible: have an array of articles that can have article-object that are either of type news or slides.

I want to use it for a teaser carousel, hence it is not possible to create something like this:

  articles:
    type: array
    items:
      $ref: "#/definitions/article"

article:
    type: object
    required:
      - id
      - headline
    properties:
      id:
        type: number
        format: int64
      headline:
        type: string
      content:
          anyOf:
            - $ref: "#/definitions/news"
            - $ref: "#/definitions/video"

Instead there is this discriminator Solution:

article:
    type: object
    discriminator: articleType
    required:
      - id
      - headline
      - articleType
    properties:
      id:
        type: number
        format: int64
      headline:
        type: string
      articleType:
        type: string
        default: news
        enum:
          - news
          - video          

However, this solutions leads to false warnings, if you do not use news or video as a reference somewhere else (is already a bug-ticket somewhere).

Further, I don't get how this discriminator-Field helps? At least in the Editor, it is not even mentioned nor does anything seem to be different. And I can't see the sub-type-details through the article, only the type.

Hope anyOf will be supported soon, this feels like a gap.

fehguy commented 8 years ago

@blindsteal the fact that it hasn't been addressed isn't due to sheer laziness. It's because it hasn't made sense for a variety of reasons. You can read up on the history, etc., but we don't add support for things that we don't think belong in the spec. Not saying it won't ever happen, but if we just did everything that people asked for without thinking about the impact, we'd end up with a useless ball of mud.

IvanGoncharov commented 8 years ago

@fehguy @jookshub I would suggest to find some middle ground. I think discriminator was a move in the right direction, but we need to provide an improved version of it. So I would love to hear your opinion on #707.

blindsteal commented 8 years ago

hi @fehguy ! I didn't mean to imply it was because of laziness, sorry 😁 I was just surprised because the history of this thread actually read like 4 out of 5 people were agreeing with the endeavour. discriminator solves a lot of my problems though, and probably the rest will vanish if #707 gets resolved :)

fehguy commented 8 years ago

@blindsteal consider joining the convo on #707 with some example use cases that discriminator may help with

webron commented 8 years ago

Tackling PR: #741

fehguy commented 8 years ago

allOf, oneOf, not will be supported in 3.0 spec. Approval by @OAI/tdc in comments, implementation details will be in this PR:

https://github.com/OAI/OpenAPI-Specification/pull/741

webron commented 7 years ago

Changes are in, closing as done!

luispabon commented 6 years ago

It's a shame this never made it to swagger 2. Makes it hacky to describe JSONAPI interfaces where lists can be of mixed types (for instance, includes).