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 properties in resource view show description of object and not description of the property #41

Open
x12a1f opened this issue Feb 10, 2017 · 7 comments
Labels

Comments

@x12a1f
Copy link

x12a1f commented Feb 10, 2017

In the list of properties of a resource, the description of an object is taken from the referenced object and not from the property. This gets confusing if the same object is used multiple times as different properties.
I would expect to see the description of the property and not the description of the object:

selection_217

I tried to change it myself but my knowledge about Go is very poor.

The spec file I used:

{
  "x-navigateMethodsByName": true,
  "definitions": {
    "AccountList": {
      "type": "object",
      "title": "AccountList",
      "properties": {
        "first": {
          "type": "object",
          "description": "first property",
          "$ref": "#/definitions/MyObject"
        },
        "second": {
          "type": "object",
          "description": "second property",
          "$ref": "#/definitions/MyObject"
        }

      }
    },
    "MyObject": {
      "type": "object",
      "title": "My Object",
      "properties": {
        "second": {
          "description": "second",
          "type": "string"
        }
      }
    }
  },
  "info": {
    "title": "minimal"
  },
  "swagger": "2.0",
  "paths": {
    "/account": {
      "get": {
        "summary": "List accounts",
        "consumes": [
          "application/json"
        ],
        "produces": [
          "application/json"
        ],
        "responses": {
          "200": {
            "description": "Success",
            "schema": {
              "$ref": "#/definitions/AccountList"
            }
          }
        }
      }
    }
  }
}
@zxchris
Copy link
Collaborator

zxchris commented Feb 20, 2017

I've a test case for this somewhere too, thanks for the example. There has been quite a lot of work and discussion around the context of property descriptions, and I was sure we'd got this right (for example, we have lots of resources that contain addresses which are defined as a standard, reusable definition. Each address instance is different though, so the description of each address property is taken from the property and not the referenced definition.

I can confirm I can reproduce with your example. Will investigate, as we agree with you, that is the way it should be. There might be a subtle difference in our specifications. We will see...

@zxchris
Copy link
Collaborator

zxchris commented Feb 20, 2017

If you define the second property as follows (with the $ref within an items member), then you get the result you expect (for second).

        "second": {
          "type": "object",
          "description": "second property",
          "items" : {
               "$ref": "#/definitions/MyObject"
          }
        }

items appears only mandatory for arrays (and outside of in in body). To be honest, the specification isn't that explicit, and parsers generally seem to manage. We wonder if using an items member for no array references is strictly wrong. Either way, DapperDox is not working for specifications that do not have items and that is incorrect.

Fixing....

EDIT: Actually, we're convinced "items": { "$ref": .... } is wrong for all but array types.

@zxchris
Copy link
Collaborator

zxchris commented Feb 20, 2017

The problem seems to stem from go-openapi, the specification parser used by DapperDox. Without the "items":{} wrapper around "$ref": ... we do not get a Description returned by go-openapi.

I've opened an issue over there, so we'll see what they say. We've sent them quite a detailed report. go-openapi/spec#22

@zxchris
Copy link
Collaborator

zxchris commented Feb 20, 2017

We've got the answer. JSON schema states that other properties in an object with $ref need to be ignored. http://json-schema.org/latest/json-schema-core.html#rfc.section.7 So what go-openapi and thus DapperDox is doing is correct.

However, wrapping $ref in an items:{} block is also (technically) wrong, as items should only be used for arrays. Because DapperDox needs to support it for arrays, it also "works" for object.

The correct solution (advised by the go-openapi guys) is this:

{
  "first": {
    "allOf" : [
      {
        "type": "object",
        "description": "first property"
      },
      {
        "$ref": "#/definitions/MyObject"
      }
    ]
  }
}

Basically, merge the partial schema object, containing type and description with the referenced object, thus adding description to MyObject (type is mandatory, so must be there).

However, the current release of DapperDox doesn't process this correctly because it expects each element of the allOf array to be a complete schema object, each with properties. We just hadn't seen or expected this use case.

We're working on a more complete solution, so will leave this issue open.

@zxchris zxchris added the bug label Feb 20, 2017
@x12a1f
Copy link
Author

x12a1f commented Feb 21, 2017

Great, thanks!

@zxchris
Copy link
Collaborator

zxchris commented Apr 5, 2017

Still working on this. It's a little complicated!

@x12a1f
Copy link
Author

x12a1f commented May 10, 2017

I was testing some other schema and I found a solution with works:

{
  "first": {
    "type": "object",
    "description": "first property"
    "allOf" : [
      {
        "$ref": "#/definitions/MyObject"
      }
    ]
  }
}

I'm not sure if this is according to the spec but it is rendered as expected in dapperdox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants