brandur / json_schema

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

Dereference schema children for subschemas without a reference #110

Closed kytrinyx closed 5 years ago

kytrinyx commented 5 years ago

I made three separate commits in order to make this easier to review.

The first commit is just a pure re-arrangement, in order to make it easier to understand exactly what changed in the second commit.

The second commit adds a failing test for the oneOf scenario reported in #100. It fails with the following:

   1) Error:
JsonSchema::ReferenceExpander#test_0025_it handles oneOf with nested references to an external schema:
NoMethodError: undefined method `max_length' for nil:NilClass
    /Users/kytrinyx/code/json_schema/test/json_schema/reference_expander_test.rb:373:in `block (2 levels) in <top (required)>'

It also adds the fix.

Given that the edge case in #100 describes oneOf with a reference to an external schema, I also figured we should test it with a reference to a local schema. ✨It failed ✨.

The third commit adds a failing test for the "local schema" variant of oneOf, which gives the following error:

  1) Failure:
JsonSchema::ReferenceExpander#test_0026_it handles oneOf with nested references to a local schema [/Users/kytrinyx/code/json_schema/test/json_schema/reference_expander_test.rb:418]:
Expected: 3
  Actual: nil

It then fixes it.

Closes #100.

kytrinyx commented 5 years ago

Hey there, just checking in quickly. If there's anything I can do to help clarify any of the changes here, let me know. I know this part of the code is really tricky!

brandur commented 5 years ago

Sorry @kytrinyx! I was on vacation for a week and am still unwinding my inbox. Thanks a lot for the incredible effort in digging into this venerable bug!

A+++ commit hygiene / comments / code / tests — you're making this really easy for me.

brandur commented 5 years ago

Released as 0.20.4.