Skip to content

Response Examples Clashing With "allOf"? #459

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

Closed
Smolations opened this issue Jun 4, 2015 · 8 comments
Closed

Response Examples Clashing With "allOf"? #459

Smolations opened this issue Jun 4, 2015 · 8 comments
Assignees
Milestone

Comments

@Smolations
Copy link

Firstly, I apologize for the amount of info in this issue, but I couldn't get around it. 😄

It seems that the fairly new PR merge that added "allOf" support on schemas has broken some rendering when mixed with Response Object examples. Here are the pertinent global definitions on the swagger object:

"definitions": {
    "media": {
        "description": "Media model",
        "required": ["id"],
        "properties": {
            "id": {
                "description": "Media's id",
                "format": "int32",
                "type": "integer"
            },
            "name": {
                "description": "Media's name",
                "type": "string"
            },
            "description": {
                "description": "Media's description",
                "type": "string"
            },
            "caption": {
                "description": "Media's caption",
                "type": "string"
            },
            "user_id": {
                "description": "Media's user_id",
                "format": "int32",
                "type": "integer"
            },
            "created_at": {
                "description": "Media's created_at",
                "type": "string"
            },
            "updated_at": {
                "description": "Media's updated_at",
                "type": "string"
            },
            "owner_id": {
                "description": "Media's owner_id",
                "format": "int32",
                "type": "integer"
            },
            "owner_type": {
                "description": "Media's owner_type",
                "type": "string"
            },
            "src": {
                "description": "Media's src",
                "type": "string"
            }
        },
        "type": "object"
    },
    "new_model": {
        "required": ["prop4"],
        "properties": {
            "prop1": {
                "format": "int32",
                "type": "integer"
            },
            "prop2": {
                "items": {
                    "type": "string"
                },
                "type": "array"
            },
            "prop3": {
                "$ref": "#/definitions/media"
            },
            "prop4": {
                "properties": {
                    "prop4_1": {
                        "type": "boolean"
                    },
                    "prop4_2": {
                        "type": "string"
                    }
                },
                "type": "object"
            },
            "prop5": {
                "items": {
                    "properties": {
                        "prop5_1": {
                            "items": {
                                "type": "string"
                            },
                            "type": "array"
                        },
                        "prop5_2": {
                            "type": "string"
                        }
                    },
                    "type": "object"
                },
                "type": "array"
            },
            "prop6": {
                "allOf": [{
                    "$ref": "#/definitions/media"
                }, {
                    "properties": {
                        "prop6_1": {
                            "type": "string"
                        }
                    },
                    "type": "object"
                }]
            }
        },
        "type": "object"
    },
    "new_model2": {
        "allOf": [{
            "$ref": "#/definitions/media"
        }, {
            "properties": {
                "extra_media_prop1": {
                    "default": 10,
                    "format": "int32",
                    "type": "integer"
                }
            },
            "type": "object"
        }, {
            "properties": {
                "extra_media_prop2": {
                    "properties": {
                        "nilla": {
                            "type": "string"
                        },
                        "wafers": {
                            "type": "string"
                        }
                    },
                    "type": "object"
                }
            },
            "type": "object"
        }]
    }
}

It should be noted that new_model.prop6 uses "allOf", referencing the media definition, and that new_model2 uses "allOf" with the same reference, but specified on the root schema (not a property's schema defined in a parent schema).

And here are the responses for the path I am working with (I've been experimenting so pay no attention to the fact that a 204 response has an associated schema haha):

"responses": {
    "200": {
        "description": "standard response for successful HTTP requests",
        "schema": {
            "$ref": "#/definitions/media"
        },
        "examples": {
            "application/json": {
                "id": 0,
                "src": "blah.com/image.png"
            }
        }
    },
    "204": {
        "description": "request processed, no content returned",
        "schema": {
            "$ref": "#/definitions/new_model"
        }
    },
    "422": {
        "description": "request unable to be followed due to semantic errors",
        "schema": {
            "$ref": "#/definitions/new_model2"
        }
    }
}

The example renders correctly in the Response Class view in the UI. However, the example is also showing up--INSTEAD OF the media definition--for new_model.prop3 and new_model.prop6 (though, new_model.prop6.prop6_1 is absent). Interestingly, the correct media definition is getting rendered correctly for the 422 response, which uses new_model2. My suspicion is that "allOf" was implemented correctly when specified on a schema at the top level, but incorrectly in nested schemas. Removing the example from the response results in the correct media definition showing up everywhere as expected/defined.

Here are the rendered Model Schemas for reference:

// Response Class (Status 200)
{
  "id": 0,
  "src": "blah.com/image.png"
}

// Response Messages (204)
{
  "prop1": 0,
  "prop2": [
    "string"
  ],
  "prop3": {
    "id": 0,
    "src": "blah.com/image.png"
  },
  "prop4": {
    "prop4_1": true,
    "prop4_2": "string"
  },
  "prop5": [
    {
      "prop5_1": [
        "string"
      ],
      "prop5_2": "string"
    }
  ],
  "prop6": {
    "id": 0,
    "src": "blah.com/image.png"
  }
}

// Response Messages (422)
{
  "id": 0,
  "name": "string",
  "description": "string",
  "caption": "string",
  "user_id": 0,
  "created_at": "string",
  "updated_at": "string",
  "owner_id": 0,
  "owner_type": "string",
  "src": "string",
  "extra_media_prop2": {
    "nilla": "string",
    "wafers": "string"
  },
  "extra_media_prop1": 10
}
@Smolations
Copy link
Author

UPDATE: After removing the example attached to the response object, I was still experiencing this strange behavior. It appears that the first "allOf" in a schema--including a $ref--becomes the new reference for the model in the $ref, regardless of whether or not an example is present.

@fehguy
Copy link
Contributor

fehguy commented Jun 5, 2015

OK currently, the allOf support is limited to top-level models defined in the #/definitions section of the schema. We will need a lot more tests to support arbitrary locations of allOf for now, or we may cause trouble in cases where someone names a property allOf, for example.

@fehguy
Copy link
Contributor

fehguy commented Jun 5, 2015

For the record, here's the JSON after running your sample through the resolver:

{
  "definitions": {
    "media": {
      "description": "Media model",
      "required": [
        "id"
      ],
      "properties": {
        "id": {
          "description": "Media's id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "name": {
          "description": "Media's name",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "description": {
          "description": "Media's description",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "caption": {
          "description": "Media's caption",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "user_id": {
          "description": "Media's user_id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "created_at": {
          "description": "Media's created_at",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "updated_at": {
          "description": "Media's updated_at",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "owner_id": {
          "description": "Media's owner_id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "owner_type": {
          "description": "Media's owner_type",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "src": {
          "description": "Media's src",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "extra_media_prop2": {
          "properties": {
            "nilla": {
              "type": "string"
            },
            "wafers": {
              "type": "string"
            }
          },
          "type": "object",
          "x-resolved-from": "self"
        },
        "extra_media_prop1": {
          "default": 10,
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "self"
        }
      },
      "type": "object"
    },
    "new_model": {
      "required": [
        "prop4"
      ],
      "properties": {
        "prop1": {
          "format": "int32",
          "type": "integer"
        },
        "prop2": {
          "items": {
            "type": "string"
          },
          "type": "array"
        },
        "prop3": {
          "$ref": "#/definitions/media"
        },
        "prop4": {
          "properties": {
            "prop4_1": {
              "type": "boolean"
            },
            "prop4_2": {
              "type": "string"
            }
          },
          "type": "object"
        },
        "prop5": {
          "items": {
            "properties": {
              "prop5_1": {
                "items": {
                  "type": "string"
                },
                "type": "array"
              },
              "prop5_2": {
                "type": "string"
              }
            },
            "type": "object"
          },
          "type": "array"
        },
        "prop6": {
          "x-composed": true,
          "$ref": "#/definitions/media",
          "properties": {
            "prop6_1": {
              "type": "string",
              "x-resolved-from": "self"
            }
          },
          "type": "object"
        }
      },
      "type": "object"
    },
    "new_model2": {
      "x-composed": true,
      "x-resolved-from": [
        "#/definitions/media"
      ],
      "description": "Media model",
      "required": [
        "id"
      ],
      "properties": {
        "id": {
          "description": "Media's id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "name": {
          "description": "Media's name",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "description": {
          "description": "Media's description",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "caption": {
          "description": "Media's caption",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "user_id": {
          "description": "Media's user_id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "created_at": {
          "description": "Media's created_at",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "updated_at": {
          "description": "Media's updated_at",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "owner_id": {
          "description": "Media's owner_id",
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "#/definitions/media"
        },
        "owner_type": {
          "description": "Media's owner_type",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "src": {
          "description": "Media's src",
          "type": "string",
          "x-resolved-from": "#/definitions/media"
        },
        "extra_media_prop2": {
          "properties": {
            "nilla": {
              "type": "string"
            },
            "wafers": {
              "type": "string"
            }
          },
          "type": "object",
          "x-resolved-from": "self"
        },
        "extra_media_prop1": {
          "default": 10,
          "format": "int32",
          "type": "integer",
          "x-resolved-from": "self"
        }
      },
      "type": "object"
    }
  }
}

@Smolations
Copy link
Author

Alright, it's good to know the current constraints of the feature. However, I removed the definition of the nested "allOf" property so that only the root reference in new_model2 remained and the same problem is persisting. Now the reference to the core media model seems to be corrupted because the 200 response is now displaying the same thing as the 422 response from my earlier post.

@webron webron modified the milestones: v2.1, v2.1.1 Jun 8, 2015
@webron webron modified the milestones: v2.1.1, v2.1.2 Jun 24, 2015
@glen-84
Copy link

glen-84 commented Jul 22, 2015

OK currently, the allOf support is limited to top-level models defined in the #/definitions section of the schema.

I didn't know this, and it might explain why I could never get composition/polymorphism working correctly (at least not in the UI).

We have our definitions in separate files, so it would be useful if we could use composition in other places.

... or we may cause trouble in cases where someone names a property allOf, for example.

But that would be under the properties key, wouldn't it? allOf is at the same level as properties.

@zachlendon
Copy link

@fehguy When you mentioned running the example through the resolver, which resolver are you referring to. A ModelResolver? I'm experiencing a similar issue to this one and am curious...

@fehguy
Copy link
Contributor

fehguy commented Jul 28, 2015

Hi, the swagger-js has a class called Resolver which will resolve remote references as well as combine models for allOf support. Please share more details of what you're having trouble with? And a sample test case would be golden.

fehguy added a commit that referenced this issue Sep 6, 2015
added resolution of inline property `allOf` constructs per #459
@fehguy
Copy link
Contributor

fehguy commented Sep 6, 2015

fixed in 83667b8

@fehguy fehguy closed this as completed Sep 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants