brandur / json_schema

A JSON Schema V4 and Hyperschema V4 parser and validator.
MIT License
230 stars 45 forks source link

Do not update DocumentStore with subschema #55

Closed okitan closed 8 years ago

okitan commented 8 years ago

First, sorry for not adding any additional tests and my poor implementations(just workaround).

When I fought with my really complex json schema, I found the expansion of it wired. It returned incorrect reference.

That is because document store is updated by subschema: https://github.com/brandur/json_schema/blob/acd1ad328a375e0761122b9f3e5fd5e0b2b013c8/lib/json_schema/reference_expander.rb#L264

okitan commented 8 years ago

@brandur

What do you think about this PR. It seems difficult to add extra tests about it, but existing tests are all green.

brandur commented 8 years ago

@okitan Hey! My apologies for not getting to this sooner :/

Looks good to me! Would you mind if we refactored the code a little bit to look more like this? (I think it's a little easier to grok.)


    def add_reference(schema)
      uri = URI.parse(schema.uri)

      # In case we've already added a schema for the same reference, don't
      # re-add it unless the new schema's pointer path is shorter than the one
      # we've already stored.
      stored_schema = lookup_reference(uri)
      if stored_schema && stored_schema.pointer.length < schema.pointer.length
        return
      end

      if uri.absolute?
        @store.add_schema(schema)
      else
        @local_store.add_schema(schema)
      end
    end
okitan commented 8 years ago

@brandur

The codes and comment look easy to understand. Thanks.

brandur commented 8 years ago

@okitan Cool! I'll pull this in and make the tweaks. Thanks!

brandur commented 8 years ago

Released as 0.12.2.