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

'items' inside schema objects should be strictly optional. #404

Closed
IvanGoncharov opened this issue Jul 13, 2015 · 13 comments
Closed

'items' inside schema objects should be strictly optional. #404

IvanGoncharov opened this issue Jul 13, 2015 · 13 comments

Comments

@IvanGoncharov
Copy link
Contributor

It's continuation of discussion from apigee-127/swagger-tools/issues/245.

Here it said:

Unless stated otherwise, the property definitions follow the JSON Schema specification as referenced here.

And I don't see any requirements for items inside Schema object.
But JSON Schema treat items as optional field.

@IvanGoncharov
Copy link
Contributor Author

JSON Schema is not the source of truth but the spec is.

I totally agree, Swagger spec just reference Draft 4, so main document is indeed Swagger spec.
And it could be that I misunderstand Swagger spec.
So can you please pinpoint me to a place in spec, that implicitly or explicitly require 'items' to be present?

@webron
Copy link
Member

webron commented Jul 13, 2015

If it's unclear, I'll clarify it in the spec itself. At the moment, I will not introduce this restriction to the JSON Schema for the reasons mentioned in the original issue.
You can try submitting a PR for such a modification, and I'll review it, but can't make any promises, it depends on the level of complexity it'll introduce.

@whitlockjc
Copy link
Member

I agree with @IvanGoncharov on this one. JSON Schema Draft v4 has a number of optional properties that when not specified, default to {} which basically means allow anything here. For example, additionalProperties defaults to {} when not provided. In my opinion, making items be optional and treated as {} would make this consistent with other parts of JSON Schema that Swagger explicitly does support.

@webron
Copy link
Member

webron commented Jul 13, 2015

Since the issue is talking about the current version of the spec, this will not change as the original intent was to make it mandatory (and non-empty, and limited to one type and so on). We can certainly discuss it for a future version (in the related issues), and I'm not saying it won't be supported but rather that we need to be smart about it. We can't say 'let's support it' without considering the implications of it, and there are implications.

@whitlockjc
Copy link
Member

Yes, anything I'm agreeing with would be suggested for future releases. :) It doesn't help that the spec requires items but the JSON Schema does not enforce it. We can fix that in the future as well.

@webron
Copy link
Member

webron commented Jul 13, 2015

I'm definitely for discussing it, just like we discussed it in the work group for 2.0. We need to be smart though, and not say we support it because JSON Schema supports it. Draft 5 could support wormholes, doesn't mean we automatically need to support it too ;) The more complicated a standard becomes (be it JSON Schema or Swagger), the harder it would be for tools to catch up with them and provide 100% compliance (HTML, XML, SGML, sounds familiar?). We need to be careful and not just say "let's support X because someone defined it that way!". Again, not saying things shouldn't be supported, just let's think about the implications as a whole before jumping.

@whitlockjc
Copy link
Member

I mean the JSON Schema for Swagger 2.0 documents does not enforce it. ;) I am not suggesting the approach that just because JSON Schema supports it, Swagger should.

@webron
Copy link
Member

webron commented Jul 13, 2015

Unfortunately, I can't get around to introducing it into the JSON Schema right now (time restrictions, nothing else). If @IvanGoncharov manages to add it and it doesn't create too much bloat, I'd be happy to merge it and make it part of the schema we provide.

@webron
Copy link
Member

webron commented Jul 21, 2016

Tackling PR: #741

@mrpotes
Copy link

mrpotes commented Oct 26, 2016

Slightly related - I've just run into this limitation of v2 of the spec via the swagger-ui library. Some of our endpoints have an operation to fetch the JSON Schema for their payload, and I want to expose those in our OpenAPI descriptor. Naturally, I assumed the best way to do that would be to specify the response schema as:

{
  "$ref": "http://json-schema.org/draft-04/schema#"
}

However, when this is included, swagger-ui throws errors because at some point it assumes that referenced schemas must also conform to the swagger restriction of items being mandatory, and JSON Schema draft-04 specification doesn't include it when anything is permitted. While I understand how this has come about, it is a headache.

@fehguy
Copy link
Contributor

fehguy commented Oct 26, 2016

This isn't going to be changed in v3. If you don't know what's inside your array, then that's a problem for consumer.

But it sounds like you do. The endpoint returns the json schema, and you have a choice.

  • Return a string which contains that URL. Assuming your endpoints want consumers to read the schema (I don't know that they will), then this should be enough
  • Return the schema itself, but set the response inner type to "object". It's the truth--you're returning a JSON object, and the consumers hopefully know what to do with it. Swagger-ui, in this case, will treat it like any other payload.

@mrpotes
Copy link

mrpotes commented Oct 26, 2016

Sure, I understand that - that's why I'm commenting here, and not raising an issue in swagger-ui. It's no big deal, I can work around it. However, I thought it might help as an example in the context of this issue with understanding that this restriction makes it impossible to reference an externally managed schema that conforms to the JSON schema standard.

We have standards to facilitate interoperation in our industry (and given that the OpenAPI standard is a great example of this, I'm sure you understand) - but by not supporting the standard upon which you are largely building, it restricts the usefulness of yours.

@webron
Copy link
Member

webron commented Feb 22, 2017

We've decided to make items mandatory in 3.0 if type is array, but the wording in the spec has been updated to better reflect that.

@webron webron closed this as completed Feb 22, 2017
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

5 participants