9 endless recursion fix #10
Conversation
true. Added that object to the specifications.
avoid endless recursion. Also added a horrible, disgusting hack to extend the initial object option (option as in possible anyOf value) by newly added attributes. This does look disguting and is probably unstable for a lot of cases.
lib/json/schema_builder/entity.rb
Outdated
| everything_else = schema.data.reject { |k, v| k == "anyOf" } | ||
| return unless everything_else.present? | ||
| schema.data.select! { |k, v| k == "anyOf" } | ||
| if initial = schema.data['anyOf'].find { |opt| opt.as_json.try(:[], 'type') == 'object' } |
There was a problem hiding this comment.
I assume that, when some is extending this, they intend to extend the object, not the null, so I extract the initial definition and extend it here. This is really ugly right now though and probably won't work in some cases. Happy for any comments here.
|
@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 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"
}
]
}
}
}
}
} |
…ptions there are merged into the any_of for nullables.
|
@stex Should be fixed. Issue was that |
|
@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 :) |
|
@Haniyya I found another problem with this PR: When using nested "nullable" 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 Added a horrible fix. I have a better solution in mind but that would require quite a bit of refactoring work:
So I basically could do this nicely, but It would take a lot of rework. For now this should work. |
|
Accidentally closed this. Not used to github. |
Fixes #9
This fixes the endless recursion happening on extending objects with
null: true.The problem was that, when extending, objects get
reinitialized. In theextract_typethey were then further extended usingany_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_ofcall toObject#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.