Replace $ref field by reference to jsonReference object

[joelwurtz]

This allow to have more consistency, and also better tool integration (as it can know a particular path can have a jsonReference)

[webron]

Thanks for this, but I'm afraid this doesn't work out. As you've noticed, but schema and pathItem already have a $ref in it which you removed and integrated elsewhere in a different manner.

However, validation wise, the current state is the right one.

For schema $refs, it is not technically forbidden to have other properties with $ref. The only thing the JSON Reference spec says is that other properties SHALL be ignored, but it does not say they cannot be included. If someone wishes to add a description with a $ref even though it 'SHALL' be ignored, so be it.

For the Path Item Object, the situation is a tad more complicated. We have made a mistake in version 2.0 of the spec by allowing $ref to live alongside other properties in that object. Yes, it breaks the JSON Reference spec, but when we realized that it was too late to change. So technically, the Swagger/OpenAPI Spec 2.0 allow for that to exist, regardless of how JSON References parsers behave. It's an unfortunate mistake that would hopefully be resolved in the next version, but the JSON Schema for the spec must represent that option as valid.

[joelwurtz]

It's no really about having other properties in the json reference but more about having a different object. (and maybe if you want to respect the jsonReference, why not setting the additionalProperties to {} ? Which say IMO that it can have other properties but we don't care about that)

Here is my use case: i have a generator which takes a json schema and transform this into stupid class (pojo / popo / ...) and also handle the serialization / deserialization part.

By having the $ref inside the Schema, it's only a property of an existing object, so it's harder (not impossible) to determine if it's a reference or not.

However having the separation at an upper level and not in the schema will clearly differentiate a Schema object from a JsonReference object, which is better because you dont handle them the same way.

[webron]

While I get your use case (assuming you mean taking the Swagger JSON Schema and generating simple classes for that), however, the changes you propose here would make valid specs render as invalid.

[joelwurtz]

IMO the last change should be BC compatible with valid specs (about other properties). However it's also less restrictive on json reference.

[webron]

Again, the change to the Path Item Object is wrong and does not follow the spec.

Personally, I'm not comfortable with merging the PR. The part for the Schema Object is derived from the JSON Schema for JSON Schema itself. The only reason the jsonReference exists in the first place is to define it in places where only that's applicable. I find the change to be adding more complexity to the already-complicated schema to support a specific case rather than to enhance validation, which is the real purpose for the schema.

Until the first issue is fixed, I cannot merge the PR. However, even with it fixed, unless more people ask for this kind of explicit separation, I'm not sure it would be wise to merge it still. I'd like to keep the PR open and observe feedback from the community though. If there's demand for it, then by all means it's something to consider.

[jsdevel]

This is where tests would really help out.

[webron]

I honestly don't think there's room for the PR to be merged, so testing or no testing, it doesn't matter.

[jsdevel]

I didn't mean to suggest that this PR should go, but that tests would help signal backwards incompatible changes in the future.

[webron]

Tests always help ;) It would definitely make sense to include a more complete set of tests in general, but that would probably go in more in the next iteration of the spec.

[jsdevel]

Speaking of the next iteration, do you foresee it still being referred to as a swagger spec?

[webron]

No need to foresee, I believe there was an official announcement. It will be officially referred to as the "OpenAPI Specification" (hence the repo name).

[jsdevel]

sweet! (sorry to hijack the discussion ;) )

[webron]

No worries ;)