interagent / prmd

JSON Schema tools and doc generation for HTTP APIs
MIT License
2.1k stars 172 forks source link

? Support oneOf for descriptions? #132

Open kretz opened 10 years ago

kretz commented 10 years ago

I have a situation where the same resource may be returned in two different forms. The forms have two attributes in common type, and details. Depending on type the content of details is different. An example:

{
  "id": 1,
  "type": "video",
  "details": {
    "framerate": 25
  }
}

and

{
  "id": 2
  "type": "image",
  "details": {
    "exposure": 1000
  }
}

I want to have a JSON Schema that can validate both the content of details and that it is matching the type. One way that I think would work is if I used "oneOf" in the properties. An attempt to do so is the following:

{
  "$schema": "http://json-schema.org/draft-04/hyper-schema",
  "title": "Media",
  "definitions": {
    "id": {
      "description": "unique identifier of media",
      "example": "01234567-89ab-cdef-0123-456789abcdef",
      "format": "uuid",
      "type": [
        "string"
      ]
    },
    "identity": {
      "$ref": "/schemata/media#/definitions/id"
    },
    "type": {
      "description": "type of media",
      "example": "image",
      "enum": ["image", "video"],
      "type": [
        "string"
      ]
    },
    "video": {
      "description": "video",
      "type": [
        "object"
      ],
      "properties": {
        "type": {
          "$ref": "/schemata/media#/definitions/type"
        },
        "details": {
          "type": ["object"],
          "properties": {
            "framerate": {
              "type": [
                "number"
              ],
              "example": "60"
            }
          }
        }
      }
    },
    "image": {
      "description": "image",
      "type": [
        "object"
      ],
      "properties": {
        "type": {
          "$ref": "/schemata/media#/definitions/type"
        },
        "details": {
          "type": ["object"],
          "properties": {
            "exposure": {
              "type": [
                "number"
              ],
              "example": "1000"
            }
          }
        }
      }
    }
  },
  "links": [
    {
      "description": "Info for media",
      "href": "/media/{(%2Fschemata%2Fmedia%23%2Fdefinitions%2Fidentity)}",
      "method": "GET",
      "rel": "self",
      "title": "Info"
    }
  ],
  "properties": {
    "oneOf": [
      { "$ref": "/schemata/media#/definitions/video"},
      { "$ref": "/schemata/media#/definitions/image"}
    ]
  },
  "type": [
    "object"
  ],
  "description": "Media description",
  "id": "schemata/media"
}

This, however, does not go through the parsing. The "oneOf" seems to be the problem.

Is there any way to achieve my goals with prmd?

Thanks, Martin

geemus commented 10 years ago

I think that looks legitimate, from the pure json-schema perspective anyway. I just don't think we encountered an example like that while working on prmd, so we may just have an oversight there. Another possibility would be to simply expose these as two different resources. Since they are largely different anyway (and it seems like it could easily be a lot more different attributes than just that). Even when things are stored in the same table internally, I think it can be easier for external users if they are separated. What do you think?

kretz commented 10 years ago

I think separate resources would be a good solution in some cases. Separation can make things more clear. Thanks for suggesting.

In my case I also want an endpoint that returns all media items regardless of type. And allows filtering on other attributes. I guess that could be yet another endpoint?

For now, I still want to stick with just one endpoint. I guess no support at the moment, then?

Martin On Jul 7, 2014 5:02 PM, "Wesley Beary" notifications@github.com wrote:

I think that looks legitimate, from the pure json-schema perspective anyway. I just don't think we encountered an example like that while working on prmd, so we may just have an oversight there. Another possibility would be to simply expose these as two different resources. Since they are largely different anyway (and it seems like it could easily be a lot more different attributes than just that). Even when things are stored in the same table internally, I think it can be easier for external users if they are separated. What do you think?

— Reply to this email directly or view it on GitHub https://github.com/interagent/prmd/issues/132#issuecomment-48190201.

geemus commented 10 years ago

Yeah. I can see the value of this and perhaps we should think more about how to support this (and provide docs that make sense to go along with it. But it isn't currently supported. We could discuss how you might make those changes, but I suspect I at least am not likely to be able to find time to make that kind of change in the near future.

kretz commented 10 years ago

I'd like to take a stab at supporting this. A very short intro on where it is best to do changes (to get me started) for this would be appreciated!

geemus commented 10 years ago

Sure. I think there are a couple threads to pull on here. Within prmd there is the question of how best to document this. I think a good place to start with that might be sketching out an example (perhaps from the schema above) of what good docs might look like. The other big part is validation, which is likely to be more in the json_schema gem than in here per se. For that I would defer to @brandur, who can probably provide much better guidance than I.

brandur commented 10 years ago

Wow, cool idea!

One thing to consider is that this section here:

  "properties": {
    "oneOf": [
      { "$ref": "/schemata/media#/definitions/video"},
      { "$ref": "/schemata/media#/definitions/image"}
    ]
  },

Is not actually valid JSON schema. Check the spec here:

The value of "properties" MUST be an object. Each value of this object MUST be an object, and each object MUST be a valid JSON Schema.

Essentially the oneOf keyword is designed to be used from within another schema somewhere. It's possible that you might want a third resource here that can be represented as either an image or a video.

geemus commented 10 years ago

@brandur ah, good catch. That does seem problematic with this approach.

indrat commented 10 years ago

So it seems that prmd does go part of the way with allowing "oneOf", "anyOf" etc since I can use prmd on the following partial adaption of Example 2 from json-schema.org.

{
  "$schema": "http://json-schema.org/draft-04/hyper-schema",
  "title": "FIXME - Entry",
  "definitions": {
    "created_at": {
      "description": "when entry was created",
      "example": "2012-01-01T12:00:00Z",
      "format": "date-time",
      "type": [
        "string"
      ]
    },
    "id": {
      "description": "unique identifier of entry",
      "example": "01234567-89ab-cdef-0123-456789abcdef",
      "format": "uuid",
      "type": [
        "string"
      ]
    },
    "identity": {
      "$ref": "/schemata/entry#/definitions/id"
    },
    "updated_at": {
      "description": "when entry was updated",
      "example": "2012-01-01T12:00:00Z",
      "format": "date-time",
      "type": [
        "string"
      ]
    },
    "diskDevice": {
        "properties": {
            "type": { "enum": [ "disk" ] },
            "device": {
                "type": "string",
                "pattern": "^/dev/[^/]+(/[^/]+)*$"
            }
        },
        "required": [ "type", "device" ],
        "type": "object",
        "additionalProperties": false
    },
    "diskUUID": {
        "properties": {
            "type": { "enum": [ "disk" ] },
            "label": {
                "type": "string",
                "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$"
            }
        },
        "required": [ "type", "label" ],
        "type": "object",
        "additionalProperties": false
    },
    "nfs": {
        "properties": {
            "type": { "enum": [ "nfs" ] },
            "remotePath": {
                "type": "string",
                "pattern": "^(/[^/]+)+$"
            },
            "server": {
                "type": "string",
                "anyOf": [
                    { "format": "host-name" },
                    { "format": "ipv4" },
                    { "format": "ipv6" }
                ]
            }
        },
        "required": [ "type", "server", "remotePath" ],
        "type": "object",
        "additionalProperties": false
    },
    "tmpfs": {
        "properties": {
            "type": { "enum": [ "tmpfs" ] },
            "sizeInMB": {
                "type": "integer",
                "minimum": 16,
                "maximum": 512
            }
        },
        "required": [ "type", "sizeInMB" ],
        "type": "object",
        "additionalProperties": false
    }
  },
  "description": "FIXME",
  "links": [
    {
      "description": "Create a new entry.",
      "href": "/entrys",
      "method": "POST",
      "rel": "create",
      "schema": {
        "properties": {
        },
        "type": [
          "object"
        ]
      },
      "title": "Create"
    },
    {
      "description": "Delete an existing entry.",
      "href": "/entrys/{(%2Fschemata%2Fentry%23%2Fdefinitions%2Fidentity)}",
      "method": "DELETE",
      "rel": "destroy",
      "title": "Delete"
    },
    {
      "description": "Info for existing entry.",
      "href": "/entrys/{(%2Fschemata%2Fentry%23%2Fdefinitions%2Fidentity)}",
      "method": "GET",
      "rel": "self",
      "title": "Info"
    },
    {
      "description": "List existing entrys.",
      "href": "/entrys",
      "method": "GET",
      "rel": "instances",
      "title": "List"
    },
    {
      "description": "Update an existing entry.",
      "href": "/entrys/{(%2Fschemata%2Fentry%23%2Fdefinitions%2Fidentity)}",
      "method": "PATCH",
      "rel": "update",
      "schema": {
        "properties": {
        },
        "type": [
          "object"
        ]
      },
      "title": "Update"
    }
  ],
  "properties": {
    "created_at": {
      "$ref": "/schemata/entry#/definitions/created_at"
    },
    "id": {
      "$ref": "/schemata/entry#/definitions/id"
    },
    "updated_at": {
      "$ref": "/schemata/entry#/definitions/updated_at"
    },
    "storage": {
        "type": "object",
        "oneOf": [
            { "$ref": "/schemata/entry#/definitions/diskDevice" },
            { "$ref": "/schemata/entry#/definitions/diskUUID" },
            { "$ref": "/schemata/entry#/definitions/nfs" },
            { "$ref": "/schemata/entry#/definitions/tmpfs" }
        ]
    }
  },
  "required": [ "storage" ],
  "id": "schemata/entry"
}

prmd will combine and verify that schema however when running prmd doc the resulting markdown has null for the value of storage example of the "oneOf":

{
  "created_at": "2012-01-01T12:00:00Z",
  "id": "01234567-89ab-cdef-0123-456789abcdef",
  "updated_at": "2012-01-01T12:00:00Z",
  "storage": null
}

And when "anyOf" is substituted for "oneOf" then prmd doc fails with the following:

$ prmd combine --meta meta.json schemata/ > schema.json
$ prmd verify schema.json
$ prmd doc schema.json > schema.md
(erubis:32:in `block in extract_attributes': undefined method `gsub!' for nil:NilClass (NoMethodError)
        from (erubis:6:in `each'
        from (erubis:6:in `extract_attributes'
        from (erubis:25:in `result'
        from /usr/local/lib/ruby/gems/2.1.0/gems/erubis-2.7.0/lib/erubis/evaluator.rb:65:in `eval'
        from /usr/local/lib/ruby/gems/2.1.0/gems/erubis-2.7.0/lib/erubis/evaluator.rb:65:in `result'
        from (erubis:7:in `block in result'
        from (erubis:4:in `each'
        from (erubis:4:in `map'
        from (erubis:4:in `result'
        from /usr/local/lib/ruby/gems/2.1.0/gems/erubis-2.7.0/lib/erubis/evaluator.rb:65:in `eval'
        from /usr/local/lib/ruby/gems/2.1.0/gems/erubis-2.7.0/lib/erubis/evaluator.rb:65:in `result'
        from /usr/local/lib/ruby/gems/2.1.0/gems/prmd-0.6.2/lib/prmd/template.rb:16:in `render'
        from /usr/local/lib/ruby/gems/2.1.0/gems/prmd-0.6.2/lib/prmd/commands/render.rb:17:in `render'
        from /usr/local/lib/ruby/gems/2.1.0/gems/prmd-0.6.2/bin/prmd:93:in `<top (required)>'
        from /usr/local/bin/prmd:23:in `load'
        from /usr/local/bin/prmd:23:in `<main>'

Any ideas on what needs to be done to get the docs to generate successfully?

geemus commented 10 years ago

I think we maybe have some similar examples in our stuff. Basically in cases like that in addition to one/any of, we explicitly provide an example at the same level (ie an example key nested an the same depth as the one/any of). Otherwise I wasn't sure how best to decide which of the possibilities you should choose from, though I guess you could just use the first one or something. Does that make sense and/or work for you? We can probably improve the error message at least either way, but wanted to continue the conversation a bit before we went too far down a possibly wrong way. Thanks!

indrat commented 10 years ago

I put an example attribute at the same level which resulted that example getting put into the markdown as you've suggested.

I also tried putting using a "oneOf": [ { … }, { … } ] as the example attribute. That put a {…} or {…} in the table but put that "oneOf" directly into the curl example which makes the curl example less useful. Having a way to have all the examples pulled up from the "$ref" objects and put into the table with one of the them put into the curl example, without the "oneOf" would be good.

geemus commented 10 years ago

Interesting. I guess we do something similar to that when the enum keyword appears. Maybe we could treat oneOf with examples in the same sort of way. I worry that this might be pretty complex for something that seems like it might be a bit of an edge case, but I'd be happy to discuss it further. In the meantime, I suspect you could specify BOTH enum and example at the top level. Example will be what shows up in curl, but the table will say something about it being one of the specific ones. This might be overly strict in terms of validation, but it should get the docs the way you want (I think). So we can maybe start from there and figure out if there is a better way that is more accurate in terms of validations if that is needed. Thanks!

indrat commented 10 years ago

As alluded to above when using this method the separate models being referenced are not actually included in the default/generated markdown docs. It would be great to have a Models section that enumerates all of the schemas/models from the various definitions sections. I think I saw something like that in https://github.com/interagent/prmd/pull/150, is there any chance of something like getting added to prmd?

Or is there a better way of structuring the schema such that they come out in the docs?

geemus commented 10 years ago

I think the intention with the current output is for the model to get its own info (title/description/attributes) prior to the list of links for the API. Is that the sort of thing you mean? Do you just want that to be separated and listed independently or are you after a more drastic change? Perhaps a short pseudo-code type example would help me know more specifically what you intended? Thanks!