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

Extra JSON Reference properties ignored - reduces reusability of references #556

Closed
JimJafar opened this issue Feb 8, 2016 · 88 comments
Closed
Labels
re-use: ref/id resolution how $ref, operationId, or anything else is resolved

Comments

@JimJafar
Copy link

JimJafar commented Feb 8, 2016

Hi everyone.

I feel that the ignoring of extra properties when using JSON References goes against the goal of reusability.

Take this example from a fictional Swagger contract:

{
    addresses: [
        "homeAddress": {
            "description": "The premises at which electricity is provided.",
            "$ref": "#/definitions/Address"
        },
        "invoiceAddress": {
            "description": "The billing address - must be where the customer's credit card is registered.",
            "$ref": "#/definitions/Address"
        }
    ]
}

The "description" properties help to distinguish between the two uses of the "Address" Reference. Of course you could argue that better naming of the "homeAddress" and "invoiceAddress" properties could achieve this, but we are not always in control of property names in our data models and sometimes you need a more verbose description.

The Swagger editor (http://editor.swagger.io/#/) is an example of an application that could be improved by removing this limitation.

Is this something that has been raised previously? Please see here for a discussion on the Swagger Google Group: https://groups.google.com/forum/#!topic/swagger-swaggersocket/ewgimdO2cOI

@webron
Copy link
Member

webron commented Feb 8, 2016

Currently, we use JSON References as-is. While there's a huge advantage to that tooling-wise, it does hinder reusability in some cases, and the issue has been raised (unofficially) a few times. Overriding the description is one case, but there are also cases of wanting to override whether fields are required and so on. Not sure if we should change it, given the cost, but we should consider it.

@qcho
Copy link

qcho commented Feb 24, 2016

I think this is a very good Proposal.
In my example I want to set:

      parameters:
        - $ref: '#/parameters/fields'
          default: 'local_default'

@whitlockjc
Copy link
Member

I would be willing to update json-refs to have a flag that allowed you to handle this situation. Based on OpenAPI feature requests, I can see a few different things we'd need:

  • Merging keys, like the example above
  • Overriding/Replacing keys, possibly like the example below

The JSON Reference spec clearly says these should be ignored. But...there's nothing stopping json-refs from allowing extra functionality on top in an opt-in manner.

@whitlockjc
Copy link
Member

Realize, this has nothing specific to do with OpenAPI other than a number of the Node.js tools around OpenAPI use json-refs. A decision on either side does not have to reflect or impact the ability/decision of the other. I hope that is clear.

@fehguy
Copy link
Contributor

fehguy commented Feb 24, 2016

I would love this feature, but I strongly advise we don't do it. If we did, tooling support would be inconsistent because the spec says it's a no-no.

There is a construct in JSON schema draft 5 but I don't think we're considering updating to that (@webron?)

@whitlockjc
Copy link
Member

Good point. I was just pointing out that if the decision was to do this, I have no objection for my tooling. As for JSON Schema Draft 5, unless it alters the JSON References specification I don't see us being able to support this if we do things the JSON References way. I wonder if we should reach out to the JSON References authors?

@drewish
Copy link

drewish commented Feb 24, 2016

I'd also love to be able to override items from a $ref. I'd actually been using them before but now that the swagger editor is flagging them as warnings I'd been reconsidering it.

@whitlockjc
Copy link
Member

The warnings are accurate so don't attempt to merge/override properties in a referenced object as it won't work. I would love to see this addressed as well.

@webron
Copy link
Member

webron commented Feb 24, 2016

Parent meta issue: #579

@webron
Copy link
Member

webron commented Feb 24, 2016

I haven't checked draft 5, so I don't know if it has effects on JSON References as well (possibly with a new draft to that). I don't think we'll be moving to draft 5 as it's not official (yeah, an official draft).

Breaking away from specs is a challenge. Unlike some expansions to JSON Schema which are actually allowed by JSON Schema itself, we're suggesting here something that goes against the spec of JSON References.

That said, within the spec, the inability to add/override JSON References makes them unusable in some cases and forces people to not DRY or make incomplete documentation.

The real challenge here is not to say we allow it, but try to think of the edge cases that could help users abuse this feature in a way, and causing more problems for the tooling. I'd like to see this change happening, but we need to be careful here.

@fehguy
Copy link
Contributor

fehguy commented Feb 24, 2016

There is a merge construct which addresses this issue. But I think moving to draft 5 would be a big mistake right now.

@jmatcuk
Copy link

jmatcuk commented Mar 3, 2016

I have a team trying to use the swagger editor hosted on swaggers site to edit our API's but with the recent enhancement to the editor to use this update, our descriptions were showing as warnings. We have a very similar issue as @JimSangwine which I can post our actual example but for now we've just cloned an older tag of the swagger editor and are running it locally.

So the reason that I posted is to support this change but to ask for clarification, is @fehguy suggesting that we do not support this because the draft version of the JSON Schema does not support it and the swagger editor was upgraded to use the new draft version?

If i'm following correctly, it's a draft and a reason why this argument should be made to change the specification.

@IvanGoncharov
Copy link
Contributor

Actually it supported both in Swagger and Draft-4, you just need to use allOf keyword.
So instead of:

description: 'The premises at which electricity is provided.'
$ref": '#/definitions/Address'

You should write:

description: 'The premises at which electricity is provided.'
allOf:
  - $ref: '#/definitions/Address'

It standard usage of allOf nothing fancy here, and I think it fully solve this problem.

@tadhgpearson
Copy link

Thanks @JimSangwine - I also face this issue, indeed I opened this as a question on the JSON Schema Group
https://groups.google.com/forum/#!topic/json-schema/YS1dbWXk_4s

@IvanGoncharov - I really like this suggestion. It seems syntactically valid, but functionally unclear.
For example - if both elements happen to have a description, which one wins?
Furthermore, fields which include this type of reference are dropped from the output of the code generator, so for me, it's not an acceptable 'interim fix' :(

@ralfhandl
Copy link
Contributor

Found out accidentally today that Swagger UI supports and visualizes description within $ref object as in the initial example, whereas Swagger Editor issues a warning

@webron
Copy link
Member

webron commented Jul 21, 2016

Tackling PR: #741

@ghost
Copy link

ghost commented Jul 26, 2016

@IvanGoncharov Regarding the suggestion to use allOf for the purpose, I am not sure if this is the intended use of allOf as has been suggested. allOf intended use does not seem to suggest that it can be used for overriding property values or providing additional "description" property.

@tadhgpearson
Copy link

Maybe this can be best resolved by the proposed new $use extension of JSON Schema - see json-schema-org/json-schema-spec#98

@advance512
Copy link

allOf is pretty much unsupported in most tools.
e.g.
swagger-api/swagger-editor#476

@Arbuste
Copy link

Arbuste commented Aug 1, 2019

Hi, just adding one instance of reusaility need here.

My own personal need is to change the required fields of an object depending on which service it is requested in.

I tried to use the allOf trick but as I expected this is not well handled by the client generator. Anyway, I think this is not the purpose of the allOf element.

@RicoSuter
Copy link

I'd expect that this (OpenAPI 3)

"startLocation": {
    "description": "The location of the trip start.",
    "nullable": true,
    "oneOf": [
        {
            "$ref": "#/components/schemas/Location"
        }
    ]
},

or this (Swagger 2)

"startLocation": {
    "description": "The location of the trip start.",
    "allOf": [
        {
            "$ref": "#/components/schemas/Location"
        }
    ]
},

should be allowed, but eg. swagger-codegen does not allow this. Why is one of these samples not the current way to go? Or something similar to avoid $ref with additional properties?

@hkosova
Copy link
Contributor

hkosova commented Feb 24, 2020

In Schema Objects, $ref siblings will be supported in OAS 3.1 which will use the JSON Schema 2019-09. As mentioned in JSON Schema 2019-09 release notes, it now allows keywords alongside $ref.

However, this change does not affect $refs outside of Schema Objects, such as $refs in parameters or responses.

Related: #2099

@philsturgeon
Copy link
Contributor

As @hkosova mentions, siblings are now welcome in schema objects, and will likely be allowed in various other places around the OpenAPI spec.

jerstlouis added a commit to jerstlouis/ogcapi-tiles that referenced this issue Dec 24, 2021
…rsion only

- swagger-cli was validating the schemas, bundling and validating the bundle without any warning
- SwaggerUI was showing the API as invalid with these messages:
   "attribute components.schemas.tileSetMetadata.items is missing"
- A $ref fully replaces sibling properties and should always be used alone:
   "Any members other than "$ref" in a JSON Reference object SHALL be ignored."
   https://datatracker.ietf.org/doc/html/draft-pbryan-zyp-json-ref-03#section-3
- See also:
   https://stackoverflow.com/questions/33565090/json-schema-property-description-and-ref-usage
   OAI/OpenAPI-Specification#556
   OAI/OpenAPI-Specification#1514
   swagger-api/swagger-editor#1184
- NOTE: We may need to update the 2DTMS schemas as well with this in mind
jerstlouis added a commit to jerstlouis/ogcapi-tiles that referenced this issue Dec 24, 2021
- A $ref fully replaces sibling properties and should always be used alone:
   "Any members other than "$ref" in a JSON Reference object SHALL be ignored."
   https://datatracker.ietf.org/doc/html/draft-pbryan-zyp-json-ref-03#section-3
- See also:
   https://stackoverflow.com/questions/33565090/json-schema-property-description-and-ref-usage
   OAI/OpenAPI-Specification#556
   OAI/OpenAPI-Specification#1514
   swagger-api/swagger-editor#1184
- NOTE: We may need to update the 2DTMS schemas as well with these changes
jerstlouis added a commit to jerstlouis/ogcapi-tiles that referenced this issue Dec 24, 2021
- A $ref fully replaces sibling properties and should always be used alone:
   "Any members other than "$ref" in a JSON Reference object SHALL be ignored."
   https://datatracker.ietf.org/doc/html/draft-pbryan-zyp-json-ref-03#section-3
- See also:
   https://stackoverflow.com/questions/33565090/json-schema-property-description-and-ref-usage
   OAI/OpenAPI-Specification#556
   OAI/OpenAPI-Specification#1514
   swagger-api/swagger-editor#1184
- NOTE: We may need to update the 2DTMS schemas as well with these changes
@thejeff77
Copy link

I think that a good solution here would be to split the definition of description. you could split into field description and class description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re-use: ref/id resolution how $ref, operationId, or anything else is resolved
Projects
None yet
Development

No branches or pull requests