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

add "null value support" in spec? #229

Closed
fehguy opened this issue Dec 20, 2014 · 117 comments
Closed

add "null value support" in spec? #229

fehguy opened this issue Dec 20, 2014 · 117 comments

Comments

@fehguy
Copy link
Contributor

fehguy commented Dec 20, 2014

Per swagger-api/swagger-core#459 there are cases where passing null is important. For example:

[ null ]

will be quite different than []. See if this can be an option

@phil-scott-78
Copy link

Love to see this. Especially helpful for us with some of the datetime requirements we have. Right now I'm resorting to "x-isnullable" : true

@jphastings
Copy link

I'd like to see this too. I'm trying to define a PATCH endpoint which merges incoming fields of a JSON object over the existing one.

Accepting a document with age: null is the way I intend to specify that the value should be overwritten, rather than left as it currently is (which is what would happen when omitting the age key from the request).

(This usecase appears to be similar to chimmelb's referenced above. I could use JSONpatch, but it is overcomplex and requires oneOf to specify correctly )

@z0r1k
Copy link

z0r1k commented Feb 19, 2015

Having null as type would be handy.
Otherwise in Response Class (Status 200) Model Schema you get

{
  "payload": {},
  "status": "ok"
}

instead of

{
  "payload": null,
  "status": "ok"
}

@webron
Copy link
Member

webron commented Feb 19, 2015

@z0r1k - null normally makes sense in requests but less so in responses (though I can think of use cases it's useful for responses as well).

In your example, wouldn't it make more sense to just have the following if there's no payload?

{
  "status": "ok"
}

To me, null has the meaning of "this property existed before, and I want to nullify its value". If there's no value, or the value needs not change (for requests, for example), it just shouldn't be sent.

@tuukkamustonen
Copy link

I'm in similar situation with jphastings (and chimmelb). Allowing null values is required for PATCH as I don't want to hack around that using empty strings or other magic variables. Support for null would be needed.

@bpicolo
Copy link

bpicolo commented Apr 13, 2015

+1. Nullability is a sort of awkward thing to not have. It's definitely a distinct case from required. One is KEY must be present. One is VALUE of key must be present.

@freewilll
Copy link

+1. We're using the good suggestion by @enkafan too.

@jharmn
Copy link
Contributor

jharmn commented Jun 10, 2015

+1 ran into this today, trying to validate responses in tests against Swagger schema.

A suggestion for a non-breaking change: nullable: true. This would avoid the problem of API developers being silly and implementing polymorphic fields (although I could see a Postel's Law argument on the request side).

@webron
Copy link
Member

webron commented Jun 10, 2015

If you're suggesting x-nullable cough then sure, go ahead and use it for now :)

@wking
Copy link

wking commented Jun 25, 2015

On Sat, Dec 20, 2014 at 03:22:16PM -0800, Tony Tam wrote:

Per swagger-api/swagger-core#459 there are
cases where passing null is important.

I bumped into this for enums (swagger-api/swagger-js#507) and was sent
here. Arguing for nulls in responses:

I restrict the information that gets returned to a user based on their
authorization. So using my pet.species example from
swagger-api/swagger-js#507, returning a response without pet.species
means “I won't tell you” and returning a response with a null
pet.species as null means “I don't know”.

Is the resistance to nulls in enums “this is too much work to
implement” (i.e. “patches welcome”) or “we disagree with JSON Spec's
choices” (i.e. “patches will be refused”).

@webron
Copy link
Member

webron commented Jun 25, 2015

The 'resistance' is that 2.0 is finalized and won't change. There's no resistance to include it in a newer version of the spec (just need to make sure that we agree on the details).

@wking
Copy link

wking commented Jun 25, 2015

On Thu, Jun 25, 2015 at 12:51:54PM -0700, Ron wrote:

The 'resistance' is that 2.0 is finalized and won't change. There's
no resistance to include it in a newer version of the spec (just
need to make sure that we agree on the details).

The swagger 2.0 spec just defers to JSON Schema (latest!) for the enum
type [1,2] and it's listed as a “taken directly” property 3. So it
seems like swagger-js is just diverging from the spec, and could be
fixed in a 2.x.y patch relases. Users who currently don't use 'null'
in their specs won't be broken by us bringing swagger-js in line with
the spec in this regard. Or am I misunderstanding?

@webron
Copy link
Member

webron commented Jun 25, 2015

null is not a valid type as it is not listed in the spec. You can't use an invalid type as a type for enum. There's no full compliance with JSON Schema, and while the spec defers to JSON Schema in some sections, it also imposes limitations on others. If you feel there needs to be extra clarification on null not being usable anywhere, I'll be happy to clarify it in the spec itself.

@wking
Copy link

wking commented Jun 25, 2015

On Thu, Jun 25, 2015 at 01:08:54PM -0700, Ron wrote:

null is not a valid type as it is not
listed
in the spec. You can't use an invalid type as a type for enum.

Ah, that makes sense.

If you feel there needs to be extra clarification on null not
being usable anywhere, I'll be happy to clarify it in the spec
itself.

The current spec does allow it in x-* fields, but yeah, I think more
clarity in the spec would help (so we don't have to refer folks here).

So that clears me up on the 2.0 issue. How do I go about working
towards adding a null datatype for 3.0? Just open a swagger-spec
issue? Is this that issue? Should I PR swagger-js?

@webron
Copy link
Member

webron commented Jun 25, 2015

This is the issue.

Nothing to PR on swagger-js, it won't change we start an official process for the next iteration of the spec (and I don't know when that will be).

@wking
Copy link

wking commented Jun 25, 2015

On Thu, Jun 25, 2015 at 01:19:11PM -0700, Ron wrote:

Nothing to PR on swagger-js, it won't change we start an official
process for the next iteration of the spec (and I don't know when
that will be).

No hurry, I'll just patch my copy locally until we get to it.

@webron
Copy link
Member

webron commented Jun 25, 2015

Sounds good! Hopefully in the (not too far) future will have easier support for extensions (via vendor extensions) to the UI and that would make your patching easier, but can't make any promises as to when.

wking added a commit to wking/swagger-js that referenced this issue Jun 25, 2015
Avoid 'too much recursion' errors with properties like:

  "species": {
    "type": "string",
    "enum": [
      null,
      "cat",
      "dog"
    ]
  }

The JSON Schema spec says for enums [1]:

> Elements in the array MAY be of any type, including null.

But they aren't supported by Swagger 2.0, because null isn't one of
Swagger 2's types [2,3].  This commit gives a more obvious hint about
that.  It would be nice to have clarification in the spec so we could
link to tidier docs than the GitHub issue too [2].

It would be nice if I could point to line numbers (or the chain of
keys?) to find the broken entry in the parsed spec, but it doesn't
seem like that information is available in resolveAllOf and I'm not
clear enough on the larger picture here to know where that would plug
in.

[1]: http://json-schema.org/latest/json-schema-validation.html#anchor77
[2]: OAI/OpenAPI-Specification#229 (comment)
[3]: https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#data-types
@jagg81
Copy link

jagg81 commented Jul 18, 2015

I run into this same issue today. Specifically to my case, this makes Swagger harder to add support for in legacy systems, where breaking the API contract is not ideal or requires great effort.

+1 for supporting null values in future versions

@johndaviddunlap
Copy link

I started getting this error after using the spec converter to upgrade my working 1.2 spec to 2.0. I hate null as much as the next guy but, this is the real world, and we can't just pretend that it doesn't exist. It does and, as such, it needs to be represented. What I've always liked about swagger was that it allowed me to describe my API without trying to force me to restructure it and this seems, at least to me, to be a major and very unsettling deviation from that role in my stack.

For the time being, I appear to have been given no choice but to continue using my 1.2 spec until this issue is addressed.

@webron
Copy link
Member

webron commented Aug 11, 2015

Well, technically 1.2 doesn't allow it either. Just because the tools may be forgiving about it, doesn't mean it's supported.

@bpicolo
Copy link

bpicolo commented Aug 11, 2015

Something to note:
swagger-api/swagger-core#459 was closed in favor of this but this issue doesn't cover the whole issue that #459 does. When/if this is considered we definitely should look at it in terms of expanding to all array-of-types

@GiladShoham
Copy link

Hi,
i read all you wrote here, but i still don't fully understand.
i have situation like this:
i have some permission tree which every node is a role object.
every role has a field name parent which point to his role parent. but of course the root doesn't have a parent so he has null value for this field.
i have APIs to add and retrieve roles.
of course the user can't add new or other root, so he must have parent field for the new role.
but when he retrieve roles, he can get the root which has null.
currently i can't use the same scheme for post and get because for get the parent is not required / can be null but for post it's required.
(The same problem will be for post and patch because patch can update only specific fields like only the role name, so i need to make everything not required for patch)
is there any way i can use the same scheme for all? or at lease for post and get?

Thx.

@philsturgeon
Copy link
Contributor

philsturgeon commented Jul 7, 2017

We're not opening this up for discussion again at this point. The decision was made for 3.0 and it won't change. We might revisit it in the future.

Hey, it's the future! What's up April people. How are things looking today?

@webron I appreciate your strong stance, and your wish to avoid another bikeshed. It seems like there has been a lot of conversation around this, and I'd be interested to see the archives. For the casual observer however, it looks like this whole "type should be a string" thing is a cruel and unnecessary deviation from JSON Schema.

I've put off using Swagger for a very long time due to the whole "it's kinda JSON Schema, but an old version, and we added some bits, and ignored some bits" situation. I think Open API 3.0 would be an absolutely amazing time to resolve a bunch of these known deviations, even if it involves a bit of extra work right now.

Do you have a link to a convincing argument for why type: ['string', 'null'] is going to be ignored, or is it just "meh dont want to" (which I also understand. #opensource).

(And yeah I know we can nullable: true but that's not my concern.)

@cbornet
Copy link

cbornet commented Jul 7, 2017

@philsturgeon swagger is used to generate code including in strongly typed languages so it' not possible to support multi type fields.

@natebrunette
Copy link

@cbornet I've always felt like that should up to the implementors. There's no reason a generator for a dynamic language couldn't allow multi types, but a strongly typed language generator could restrict it.

@darrelmiller
Copy link
Member

@natebrunette This is a slippery slope to allowing tool vendors to optionally implement anything at which point, the value of the standard is severely reduced.

@cipresso
Copy link

cipresso commented Jul 7, 2017

The particular case of ["string", "null"] should be OK for a strongly-typed language. The type could be tested for null or a generator could create a type called StringOrNull with an isNull() method.

@handrews
Copy link
Member

handrews commented Jul 7, 2017

@philsturgeon swagger is used to generate code including in strongly typed languages so it' not possible to support multi type fields.

Are strongly typed languages the only use case for swagger? Dynamically typed languages are common in web programming, to put it mildly. Crippling them for the sake of strong static typing is an odd choice. If an API does not want to support clients in strongly typed languages that's a valid decision.

@mattdeboard
Copy link

mattdeboard commented Jul 7, 2017

I mean Option/Maybe/Optional etc. are real things so I don't get the static/strong typing argument here.

edit: anyway it seems this matter is still at its same status from a few months ago. Just sayin.

@philsturgeon
Copy link
Contributor

Yeah, @darrelmiller, I don't think this is a conversation about "Yolo everyone can just do whatever they want.", I think @natebrunette was more suggesting that if a strongly typed language does not support union types, there are ways to handle JSON Schema's multiple types in that language that dont blow the whole thing up.

Value objects are a thing.

@darrelmiller
Copy link
Member

@philsturgeon This whole conversation is much bigger than just type being an array. JSON Schema is much more suited to validating instance documents than it is at describing the shape of things. There are so many things you can do with JSON Schema that make projecting types from it challenging.

I'd rather move to using a far more constrained version of JSON schema that allows users to unambiguously describe the shape of payloads than to enable the full expressiveness of JSON Schema for validation.

However, I'm not sure that the subset of JSON Schema for modelling should be defined within the scope of the OAI and instead think it should be done under the umbrella of the JSON Schema work. With all the V3 work going on I haven't had time to bring this to the JSON Schema folks yet.

@cipresso
Copy link

cipresso commented Jul 7, 2017

This discussion is causing flashbacks to the challenges with supporting xsd:any, xsd:anyType in the XML glory days. I'm not advocating for reintroducing the horrors of xsd:any or xsd:anyType, but maybe the equivalent of xsi:nil.

@darrelmiller
Copy link
Member

@cipresso And the new nullable property in V3 does that https://github.com/OAI/OpenAPI-Specification/blob/OpenAPI.next/versions/3.0.md#fixed-fields-20 Admittedly not in the same way JSON Schema normally does it, but that's because we didn't want to re-open this can of worms :-)

@handrews
Copy link
Member

handrews commented Jul 7, 2017

@darrelmiller

However, I'm not sure that the subset of JSON Schema for modelling should be defined within the scope of the OAI and instead think it should be done under the umbrella of the JSON Schema work. With all the V3 work going on I haven't had time to bring this to the JSON Schema folks yet.

It is actually one of the three proposed new vocabularies, and the only one currently lacking a champion :-) We haven't really started any of these projects b/c all of the main contributors had Life Stuff(tm) going on for the last few months, but I do expect things to start moving on at least UI and Documentation soon (I see the Documentation vocabulary as focusing on the challenges of dynamic hypermedia as opposed to OpenAPI's focus on statically describable APIs).

https://github.com/json-schema-org/json-schema-vocabularies

@cbornet
Copy link

cbornet commented Jul 7, 2017

Also I'll add (again) that the swagger field "format" would be incompatible with an array of types.

@niquola
Copy link

niquola commented Jul 8, 2017

+1 for subset of json schema for structure/shape definition.

While working with FHIR standard, which also mix structure definition and validation, i came up to conclusion - that separation of this concerns is a good thing (as well as tricky;)

From other side you also could provide a different place in swagger doc for validation by unconstrained json schema.

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

No branches or pull requests