json-schema_builder icon indicating copy to clipboard operation
json-schema_builder copied to clipboard

9 endless recursion fix

Open Haniyya opened this issue 6 years ago • 6 comments

Fixes #9

This fixes the endless recursion happening on extending objects with null: true.

The problem was that, when extending, objects get reinitialized. In the extract_type they were then further extended using any_of(null) which caused an infinite loop.

Another problem was that adding children to nullable entities could lead to the childrens properties getting merged not into the any_of structure, but the root structure, which was not desired. I added a test and and a build_any_of call to Object#initialize_children.

I've moved the any_of(null) call to initialization (the normal one) so that this does not happen.

I also added a specification of what I think the resulting schema should look like and had to do horrible hacks to get there. I aim to clean those up shortly, thus the.

Haniyya avatar Mar 05 '19 09:03 Haniyya

@Haniyya Thanks a lot for your work here!

This fixes the problem with the endless loop, but introduced a different problem as it seems that the properties of an object with null: true (or probably simply an anyOf?) are duplicated outside the anyOf.

The following code generates a wrong schema:

def schema
  object.tap do |base_obj|
    foo_obj = base_obj.object(:foo, required: true)
    bar_obj = foo_obj.object(:bar, null: true, required: true)
    bar_obj.string :baz
  end
end
{
  "type": "object",
  "properties": {
    "foo": {
      "type": "object",
      "properties": {
        "bar": {
          "anyOf": [
            {
              "type": "object",
              "properties": {
                "baz": {
                  "type": "string"
                }
              }
            },
            {
              "type": "null"
            }
          ],
          "properties": {
            "baz": {
              "type": "string"
            }
          }
        }
      },
      "required": [
        "bar"
      ]
    }
  },
  "required": [
    "foo"
  ]
}

Writing the same schema using nesting instead of extending, it looks correct:

def expected_schema
  object do
    object :foo, required: true do
      object :bar, null: true, required: true do
        string :baz
      end
    end
  end
end
{
  "type": "object",
  "required": [
    "foo"
  ],
  "properties": {
    "foo": {
      "type": "object",
      "required": [
        "bar"
      ],
      "properties": {
        "bar": {
          "anyOf": [
            {
              "type": "object",
              "properties": {
                "baz": {
                  "type": "string"
                }
              }
            },
            {
              "type": "null"
            }
          ]
        }
      }
    }
  }
}

stex avatar Mar 05 '19 11:03 stex

@Stex Should be fixed. Issue was that Object#initialize_children was called after the any_of construction was done, so the properties were added to the main object.

Haniyya avatar Mar 06 '19 09:03 Haniyya

@parrish Could you take a look at the changes here and possibly release a new gem version if you have time? That would help a lot :)

stex avatar Mar 07 '19 14:03 stex

@Haniyya I found another problem with this PR: When using nested "nullable" anyOfs, the generated schema contains a type as well as the anyOf. This causes the schema validation to fail as type: 'string' is more important than the anyOf:

def schema
  object.tap do |base_obj|
    obj = base_obj.object :nullable_object, null: true
    obj.string :nullable_string, null: true
  end
end
{
  "type": "object",
  "properties": {
    "nullable_object": {
      "anyOf": [
        {
          "type": "object",
          "properties": {
            "nullable_string": {
              "type": "string",
              "anyOf": [
                {
                  "type": "string"
                },
                {
                  "type": "null"
                }
              ]
            }
          }
        },
        {
          "type": "null"
        }
      ]
    }
  }
}

def expected_schema
  object do
    object :nullable_object, null: true do
      string :nullable_string, null: true
    end
  end
end
{
  "type": "object",
  "properties": {
    "nullable_object": {
      "anyOf": [
        {
          "type": "object",
          "properties": {
            "nullable_string": {
              "anyOf": [
                {
                  "type": "string"
                },
                {
                  "type": "null"
                }
              ]
            }
          }
        },
        {
          "type": "null"
        }
      ]
    }
  }
}

stex avatar Mar 07 '19 15:03 stex

@Stex Added a horrible fix. I have a better solution in mind but that would require quite a bit of refactoring work:

  1. Make new entities out of any/all/one_of attributes
  2. Have Nullable be a specialization of any_of that proxies all calls from the DSL module to it's initial object
  3. ???
  4. Profit.

So I basically could do this nicely, but It would take a lot of rework. For now this should work.

Haniyya avatar Apr 01 '19 08:04 Haniyya

Accidentally closed this. Not used to github.

Haniyya avatar Apr 01 '19 08:04 Haniyya