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

Object composition of primitive types rebilly.com #98

Closed
sergey-tihon opened this issue Jul 9, 2016 · 7 comments
Closed

Object composition of primitive types rebilly.com #98

sergey-tihon opened this issue Jul 9, 2016 · 7 comments

Comments

@sergey-tihon
Copy link

Schema https://api.apis.guru/v2/specs/rebilly.com/2.1/swagger.json has very strange use of allOf object composition

  "definitions": {
    "ApiKey": {
      "description": "API secret Key.",
      "properties": {
        ...
        "createdTime": {
          "allOf": [
            {
              "$ref": "#/definitions/ServerTimestamp"
            }
          ],
          "description": "The API key created time"
        },
        ...
      },
      "type": "object"
    },

where ServerTimestamp is:

    "ServerTimestamp": {
      "description": "Read-only timestamp, automatically assigned on back-end.",
      "format": "date-time",
      "readOnly": true,
      "type": "string"
    },

Is it correct use of allOf ?

Swagger allows combining and extending model definitions using the allOf property of JSON Schema, in effect offering model composition. allOf takes in an array of object definitions that are validated independently but together compose a single object.

@IvanGoncharov
Copy link
Member

@sergey-tihon Yes, it corrects it's only possible way to override description.
You can rewrite it as:

createTime:
  allOf:
    - $ref: '#/definitions/ServerTimestamp'
    - description: The API key created time

But in this case, it's less obvious what takes priority.

@sergey-tihon
Copy link
Author

But in the json schema it is written like

createTime:
  allOf:
    - $ref: '#/definitions/ServerTimestamp'
  description: The API key created time

@sergey-tihon
Copy link
Author

Am I right that is not possible to write

createTime:
  $ref: '#/definitions/ServerTimestamp'
  description: The API key created time

and we have to use composition in this case and transform it into

createTime:
  allOf:
    - $ref: '#/definitions/ServerTimestamp'
  description: The API key created time

@IvanGoncharov
Copy link
Member

Disclaimer: I helped Rebilly to create their spec and suggest them to use this pattern.

Both forms are valid according to JSON Schema and Swagger.
But one that Rebilly use clearly show which description should be used.
It's related to documentation engine we developing for them: http://rebilly.github.io/ReDoc/

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Jul 9, 2016

Am I right that is not possible to write
and we have to use composition in this case and transform it into

@sergey-tihon Yes exactly.
From JSON ref spec:

Any members other than "$ref" in a JSON Reference object SHALL be
ignored.

https://tools.ietf.org/html/draft-pbryan-zyp-json-ref-03

But it's very common mistake to think that $ref some kind of merging, look at validation log for APIs.guru:
https://travis-ci.org/APIs-guru/api-models/builds/143078357
Every EXTRA_REFERENCE_PROPERTIES warning is such mistake.
In long run I plan to automatically fix such warnings using allOf workaround.

P.S. But we need something more appropriate like $merge :(

@sergey-tihon
Copy link
Author

My confusion was about members of allOf array.
I guess was that this array should contains objects or ref to objects (but not primitive types)
So I will need to combine properties from all objects from array to get result type.

But in this case you uses primitive type in allOf array and just modify it's metadata (description) without touching the type.

@IvanGoncharov
Copy link
Member

So I will need to combine properties from all objects from array to get result type.

It's a tricky part since JSON Schema not merging-friendly, it was never intended to be used for something other than JSON validation :(

But in this case you uses primitive type in allOf array and just modify it's metadata (description) without touching the type.

It could be anything not only metadata.

sergey-tihon added a commit to sergey-tihon/SwaggerProvider that referenced this issue Apr 13, 2017
sergey-tihon added a commit to fsprojects/SwaggerProvider that referenced this issue Apr 13, 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

2 participants