Add oneOf schema support #103
Conversation
| @@ -0,0 +1,16 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
I don't think this file is needed, is it?
| @@ -235,6 +234,17 @@ protected function generateMapStatementForFreeFormObject(Field $root, Variable $ | |||
| return $this->builder->return($schemaInit); | |||
| } | |||
|
|
|||
| protected function hasComposite(array $fields): bool | |||
| $ifStmt = $this->builder->expr( | ||
| } | ||
|
|
||
| if ($root->getDiscriminator()) { |
There was a problem hiding this comment.
can you please add in tests example when there's oneOf but no discriminator? because discriminator is optional so it should work even without it...
| ); | ||
|
|
||
| $assignMethodName = $this->builder->expr( | ||
| $this->builder->assign($this->builder->var('methodName'), $this->builder->concat($this->builder->val('set'), $this->builder->funcCall('ucfirst', [$payloadDiscriminator]))) |
There was a problem hiding this comment.
please use multiple lines for better readability, here and below
| $field->getName(), | ||
| $payloadVariable, | ||
| ] | ||
| if (!$root->hasOneOf()) { |
There was a problem hiding this comment.
same comment I made on the other PR, I think this code is modelling the anyOf, not the oneOf (the difference being oneOf must match only 1 schema)
There was a problem hiding this comment.
we could add support for both anyOf and oneOf here IMO, and just do the extra check/constraint about the matching 1,N schemas, wdyt?
oleg-docler
force-pushed
the
one-of-with-discriminator
branch
from
d4a9a77 to
2af3b3e
Compare
| $schema->setMachine($this->machineMapper->toSchema($payload)); | ||
| } catch (UnexpectedResponseBodyException $exception) { | ||
| } | ||
| if (empty($schema->toArray())) { |
There was a problem hiding this comment.
what if we add a $matches = 0 before all the tries, and inside the try after the $schema->set... we $matches ++
we know when generating this Mapper if it's oneOf or anyOf, so we can already add the code when it's anyOf, we check if ($matches === 0) throw UnexpectedResponseBodyException, when it's oneOf we check if $matches !== 1 throw UnexpectedResponseBodyException
There was a problem hiding this comment.
and then we can remove this if (empty(toarray()
| - type: number | ||
| - type: string | ||
| - $ref: '#/components/schemas/EmbeddedObject' | ||
| - $ref: '#/components/schemas/EmbeddedObject' |
There was a problem hiding this comment.
looks a bit weird anyOf X and X... should be X and Y
src/Input/Factory/FieldFactory.php
Outdated
| } | ||
|
|
||
| /** @var Reference $anyOfSchema */ | ||
| foreach ($schema->anyOf as $anyOfSchema) { |
There was a problem hiding this comment.
maybe to avoid duplicates we could have just 1 block
if (isset($schema->oneOf) || isset($schema->anyOf)) {
...
then in the foreach ($schema->oneOf ?? $schema->anyOf or smth like that
OR use some private method if you prefer, sending the $schema->oneOf or $schema->anyOf, typehinting with Reference wdyt?
| ), | ||
| ]; | ||
| if ($root->hasOneOf()) { | ||
| $tryStatements[] = $this->builder->return($schemaVar); |
There was a problem hiding this comment.
I don't think this return is doing much, my suggestions with that $matches would already discard the need for this return when it's oneOf
| '/SchemaMapper/GetExampleResponseBodyMapperWithoutDiscriminator.php', | ||
| '/SchemaMapper/OneOfResponseBodyMapperWithoutDiscriminator.php', | ||
| self::BASE_NAMESPACE . SchemaMapperGenerator::NAMESPACE_SUBPATH . '\\GetExampleResponseBodyMapper', | ||
| ConfigurationBuilder::fake()->build(), |
There was a problem hiding this comment.
can you please extend the all these generator tests you added with the php 8.0 version too?
|
please add test for anyOf + discriminator also |
No description provided.