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

A $ref needs to be wrapped inside a schema which is not according to spec. #402

Closed
x12a1f opened this issue May 1, 2015 · 21 comments
Closed

Comments

@x12a1f
Copy link

x12a1f commented May 1, 2015

This is a followup to swagger-api/swagger-ui#1228

I have

 "Person" : {
         "type" : "object",
         "title" : "Information about a person",
         "properties" : {
            "externalId1" : {
               "title" : "first",
               "type" : "object",
               "$ref" : "#/definitions/map1"
            }

which does not work. The answer was to wrap it inside a "schema" and then it works:

"externalId1" : {
               "title" : "first",
               "type" : "object",
               "schema": {  // < -------------- This will make it work like you expect
                    "$ref" : "#/definitions/map1"
                } // ------------------- :)
            },

However, after looking into the spec of both swagger and JSON schema, there is no reference anywhere the "schema" wrapper is required.

As far as I can tell, my version without the "schema" wrapper is correct.

The example here https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#simple-model does not use the "schema" wrapper.

Am I missing something?

@x12a1f
Copy link
Author

x12a1f commented May 1, 2015

Also, when using https://github.com/apigee-127/swagger-tools to validate the swagger file, the validator also does not like having the "schema" wrapper present.

@ponelat
Copy link
Member

ponelat commented May 1, 2015

I think you're right... @webron knows the spec better than I (better than anyone?) but there seems to be an inconsistency of what works and what's in the spec.

@webron
Copy link
Contributor

webron commented May 3, 2015

@ralphvanetten - that's not the way to define it, but without seeing what #/definitions/map1, it would be difficult to suggest the proper solution.

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

It is the example from swagger-api/swagger-ui#1228 :

{
   "paths" : {
      "/person" : {
         "post" : {
            "parameters" : [
               {
                  "in" : "body",
                  "name" : "body",
                  "schema" : {
                     "$ref" : "Person"
                  }
               }
            ],
            "tags" : [
               "person"
            ],
            "summary" : "Create new person",
            "responses" : {
               "200" : {
                  "schema" : {
                     "$ref" : "Person"
                  },
                  "description" : "Success"
               }
            }
         }
      }
   },
   "swagger" : "2.0",
   "basePath" : "/person",
   "info" : {
      "title" : "Person API",
      "version" : "1",
      "description" : ""
   },
   "tags" : [
      {
         "name" : "person",
         "description" : "Persons"
      }
   ],
   "definitions" : {
      "map1" : {
         "type" : "object",
         "properties" : {
            "anId1" : {
               "title" : "Id",
               "type" : "string"
            }
         },
         "title" : "map1"
      },
      "map2" : {
         "type" : "object",
         "properties" : {
            "anId2" : {
               "title" : "Id",
               "type" : "string"
            }
         },
         "title" : "map2"
      },
       "Person" : {
         "type" : "object",
         "title" : "Information about a person",
         "properties" : {
            "externalId1" : {
               "title" : "first",
               "type" : "object",
               "$ref" : "#/definitions/map1"
            },
            "externalId2" : {
               "title" : "second",
                "type" : "array",
                "items": {
                    "$ref" : "#/definitions/map2"
                }
            }
         }
      }       
   }
}

The suggestion given in that issue is to wrap the $ref inside a schema but I can not find any reference in the swagger spec or the JSON schema spec this extra schema is needed.

@webron
Copy link
Contributor

webron commented May 4, 2015

Are you trying to define maps with map1 and map2? Because that's not the way to do it.

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

No, perhaps its a poor choice for those names, but map1 and map2 are supposed to be normal JSON objects with a couple of fields.

@webron
Copy link
Contributor

webron commented May 4, 2015

Okay, just wanted to make sure.

This how "Person" should be defined:

{
  "type": "object",
  "title": "Information about a person",
  "properties": {
    "externalId1": {
      "$ref": "#/definitions/map1"
    },
    "externalId2": {
      "title": "second",
      "type": "array",
      "items": {
        "$ref": "#/definitions/map2"
      }
    }
  }
}

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

So title and type are not allowed?

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

Ah, yes, I tried it and as soon as you add title it does not work anymore.

@webron
Copy link
Contributor

webron commented May 4, 2015

You add anything when there's $ref. This is a restriction of JSON Reference.

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

Ok, thanks for the reference.
However, I tried putting a description there and this works :

         "properties" : {
            "externalId1" : {
                "$ref" : "#/definitions/map1",
                "description": "hello"
            },

selection_109

If this is not allowed, how can you add a description which shows up in swagger-ui?

@webron
Copy link
Contributor

webron commented May 4, 2015

The parser may read it somehow, but most parsers would ignore it. The way to add the description would be in the referenced object.

@x12a1f
Copy link
Author

x12a1f commented May 4, 2015

Ok, I thought the description is used to describe the field (externalId1 and externalId2 in this case).
If the description must be on the referenced object, then you can not have a different description if you have multple references to the same object?

So something like:

"properties" : {
            "externalId1" : {
                "$ref" : "#/definitions/map1",
                "description": "This is the ID for the first thing."
            },
            "externalId3" : {
                "$ref" : "#/definitions/map1",
                "description": "This is the ID for the third thing."
            },

is not correct and not otherwise possible?

@webron
Copy link
Contributor

webron commented May 4, 2015

If you use $ref, that is not possible. As explained, this is a limitation of JSON Reference.

@x12a1f
Copy link
Author

x12a1f commented May 7, 2015

Isn't this a bit of a flaw in the (swagger) spec?

I have a fairly large object used by many other objects. Using $ref would be a nice solution to keep the swagger file small but if you do you can't have different descriptions in the object which uses the reference.

@webron
Copy link
Contributor

webron commented May 7, 2015

It's not a flaw in the Swagger spec. It's a limitation of JSON Reference which is used by JSON Schema which is used by Swagger. Not much we can do about it besides contest the JSON Reference spec, I wouldn't get my hopes up actually affecting it.

@x12a1f
Copy link
Author

x12a1f commented May 7, 2015

I looked at the JSON reference spec. again and according to the spec it is fine to have other properties besides $ref :

If a JSON value does not have these characteristics, then it SHOULD
NOT be interpreted as a JSON Reference.
...
Any members other than "$ref" in a JSON Reference object SHALL be
ignored.

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

So it seems the spec leaves room for adding a title and description property which are then shown by swagger-ui.

@webron
Copy link
Contributor

webron commented May 7, 2015

Nope. SHALL be ignored. I don't know of any JSON Schema/Reference parser that does not ignore those. It's not going to change.

@fehguy
Copy link
Contributor

fehguy commented May 8, 2015

OK while it would be very convenient that $ref and other fields could merge together, json schema simply doesn't support this. I'm closing this issue, and we can put other requests in the swagger-spec project.

@mutkur
Copy link

mutkur commented Jun 20, 2017

Going by the above discussion is it legal to have a $ref along with other fields 'type' and 'description' etc? I have

"BatchOrdersPromotions_3": {
			"type": "object",
			"properties": {
				"promotionId": {
					"description": "The unique id of the promotion.",
					"type": "string"
				},
				"promotionEffectivePeriod": {
					"type": "object",
					"$ref": "#/definitions/BatchOrdersPromotionEffectivePeriod_5"
				}
			}
		}

In the above snippet is it legal to have field "type":"object" with the property promotionEffectivePeriod since it is a $ref? If that is legal, is it required that the "promotionEffectivePeriod" have properties of itself since it is defined as "object" type. I could not find any references in the spec.

@webron
Copy link
Contributor

webron commented Jun 20, 2017

It's legal in terms of whether it's valid to write it that way or not. In terms of meaning - the "type": "object" is just going to be ignored.

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