Add support for discriminator #32
Conversation
Discriminator option is supported for oneOf and anyOf. Only discriminator with mapping defined is supported now. Openapi docs for discriminator and mapping: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/
|
@ota42y hello, I still need to finish specs, but I wanted to get an early feedback from you, mainly on the parsing step. I'd probably need to do a custom parser, since the mapping schema is a bit weird (no Also not sure if I can call in the parser: discriminator.root.find_object(mapping[value[property_name]])is that lazily evaluated? Anyway, let me know what is acceptable for you and I'll finish the specs tomorrow. |
Ladas
force-pushed
the
add_discriminator_support
branch
from
6d58f3b
to
10e9070
Compare
Ladas
changed the title
| end | ||
|
|
||
| # TODO: this is likely O(n) for each validation, we need to find it once during parsing | ||
| resolved_schema = discriminator.root.find_object(mapping_target) |
There was a problem hiding this comment.
@ota42y so I am trying to move this to https://github.com/Ladas/openapi_parser/blob/c844e37dba4cbe605f9a3f8d565f84843acfe5bb/lib/openapi_parser/schemas/schema.rb#L115, to my own parser, but after spending several hours there I didn't figure out how it works. :-) Could you give me some hints? Is it even possible to do the lookup there?
If the discriminator.root.find_object(mapping_target) is or would be O(1), I don't really need to do that. :-)
There was a problem hiding this comment.
it looks like the find_object does O(n) lookup at worst, then caches the result, so this should be ok performance wise
There was a problem hiding this comment.
Yes, find_object method dose O(n) lookup at worst, but we cached result so I think there is no problem 👍
https://github.com/ota42y/openapi_parser/blob/master/lib/openapi_parser/concerns/findable.rb#L4
Ladas
changed the title
lib/openapi_parser/schemas/schema.rb
Outdated
| @@ -101,6 +102,18 @@ class Schema < Base | |||
| # @return [Hash{String => Schema}, nil] | |||
| openapi_attr_hash_object :properties, Schema, reference: true | |||
|
|
|||
| # @!attribute [r] discriminator | |||
| # @return [Hash{String => Schema}, nil] | |||
| openapi_attr_objects :discriminator, Schema | |||
There was a problem hiding this comment.
Sorry, we defines schemas to be the same attribute in OpenAPI 3 specification as much as possible.
So please create Discriminator class and move property_name and mapping to it.
There was a problem hiding this comment.
that make sense, Discriminator class added
| end | ||
|
|
||
| # TODO: this is likely O(n) for each validation, we need to find it once during parsing | ||
| resolved_schema = discriminator.root.find_object(mapping_target) |
There was a problem hiding this comment.
Yes, find_object method dose O(n) lookup at worst, but we cached result so I think there is no problem 👍
https://github.com/ota42y/openapi_parser/blob/master/lib/openapi_parser/concerns/findable.rb#L4
| "name" => "Mr. Cat", | ||
| "born_at" => "2019-05-16T11 =>37 =>02.160Z", | ||
| "description" => "Cat gentleman", | ||
| "nut_stock" => 10 # passing squirrel attribute here, but discriminator still picks cats and fails |
Ladas
force-pushed
the
add_discriminator_support
branch
2 times, most recently
from
c693ed4
to
bdb90ed
Compare
Ladas
force-pushed
the
add_discriminator_support
branch
from
bdb90ed
to
648ce67
Compare
|
@ota42y that you for the review, the data structure issue has been fixed. :-) |
|
This is a great feature!!! |
|
I just released 0.2.6 and which includes this feature 🙋♂ |
Discriminator option is supported for oneOf and anyOf. Only
discriminator with mapping defined is supported now.
Openapi docs for discriminator and mapping:
https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/
TODOs: