brandur/json_schema

json_schema >= 0.14.0 errors while reference expanding against a schema which has reference to a link description object

okitan opened this issue · 7 comments

This is a brief sample code.

json = <<_JSON_
{
    "id": "http://example.com/poc.json",
    "$schema": "http://json-schema.org/draft-04/hyper-schema#",
    "definitions": {
        "getSelfEndpoint": { "$ref": "#/links/0" }
    },
    "properties": {
        "id":   { "type": "string" }
    },
    "links": [
        {
            "rel": "self",
            "method": "GET",
            "href": "/{id}"
        }
    ]
}
_JSON_

schema = JsonSchema::Parser.new.parse!(JSON.parse(json))
schema.expand_references #=> error!

It causes lib/json_schema/reference_expander.rb:156:inresolve_pointer': undefined method reference' for #<JsonSchema::Schema::Link:0x007f932b169390> (NoMethodError)

That is because the schema refers to LDO but JsonSchema::Schema::Link does not support reference and fails.

Ugh, my fault. Thanks for the report!

@okitan I released a fix for the problem in 0.14.2. Sorry about the trouble and let me know if you notice anything else.

Hm, something is still not quite here ... the test suite passed, but there seems to be another problem. I ended up yanking 0.14.2 and would recommend holding off on an upgrade for now.

Alright, 0.14.3 is out instead.

@brandur

I appreciate for your quick fix.
Actually it works, but I'm afraid I have some concern about it.

JsonSchema::Schema::Link comes to be a subclass of JsonSchema::Schema.
Basically, LDO is not a schema (no refs to "#" in https://github.com/json-schema-org/json-schema-spec/blob/master/archive/draft-04/hyper-schema.json#L116-L156). and subclass of JsonSchema::Schema seems weired to me.

Becoming a subclass, for example, JsonSchema::Schema::Link#additional_properties becomes to return true which is a default value of JsonSchema.
It is not per spec, and It may cause conflict when we made our own expansion to LDO.

I think the functionalities about attr_copyable should be as module, and both JsonSchema::Schema and JsonSchema::Schema::Link should include it.
There are no need of inheritance.

JsonSchema::Schema::Link comes to be a subclass of JsonSchema::Schema.
Basically, LDO is not a schema (no refs to "#" in https://github.com/json-schema-org/json-schema-spec/blob/master/archive/draft-04/hyper-schema.json#L116-L156). and subclass of JsonSchema::Schema seems weired to me.

Yeah, I agree that it's not ideal as well. The only reason I did it is that I got myself a little stuck in the way that reference expansions are handled. They work by copying a target "into" the current reference (also a Schema object, probably incorrectly), and so the source reference must have all the properties of the target in order for the expansion to work.

I may be able to work around that by keeping references as separately objects and have them "morph" into either a schema and link as they're materialized by dynamically inheriting all their properties. I'll think about it some more.

I think the functionalities about attr_copyable should be as module, and both JsonSchema::Schema and JsonSchema::Schema::Link should include it.
There are no need of inheritance.

Well, one nice thing that fell out of the refactor is that attr_copyable and friends did move into a separately module that can be mixed in (JsonSchema::Schema::Attributes). As described above though, the inheritance scheme is unfortunately still necessary for the time being.

I spent a few hours on this refactor, but have put it aside for the time being because the complexity was starting to spin out of control. I may still try to do it, but I'm not sure about the timeline.