Hal jsonschema#4357
Conversation
| { | ||
| $this->schemaFactory = $schemaFactory; | ||
|
|
||
| if ($schemaFactory instanceof BaseSchemaFactory) { |
There was a problem hiding this comment.
I've moved the check now to the public method addDistinctFormat from the hydra schema factory.
| */ | ||
| public function buildSchema(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?string $operationType = null, ?string $operationName = null, ?Schema $schema = null, ?array $serializerContext = null, bool $forceCollection = false): Schema; | ||
|
|
||
| public function addDistinctFormat(string $format): void; |
There was a problem hiding this comment.
Adding a method to an existing interface is a BC break, you cannot do it.
There was a problem hiding this comment.
The problem here is that we decorating the hydra schema factory with the hal schemafactory. So the schemafactory property in the hal schema factory will never be a "BaseSchemaFactory".
We've got HalSchemaFacotry contains the HydraSchemaFactory contains the BaseSchemaFactory. Since you only could add distinct formats in the baseSchemaFactory, it is impossible for the HalSchemaFactory to add the 'hal' as a distinct format.
Do you've got any tips to add the hal distinct format without extending the schemafactory interface?
There was a problem hiding this comment.
You can keep the public method in the HydraSchemaFactory but without adding it to the interface too.
There was a problem hiding this comment.
Good point, I did work now with the method_exists function.
| $definitions = $resultSchema->getDefinitions(); | ||
| $rootDefinitionKey = $resultSchema->getRootDefinitionKey(); | ||
|
|
||
| $this->assertEquals(str_replace('\\', '.', Dummy::class).'.jsonhal', $rootDefinitionKey); |
There was a problem hiding this comment.
This line duplicates a previous test.
There was a problem hiding this comment.
I removed the duplicate assert also from the hydra test.
|
Hello, |
|
@alanpoulain fixed phpstan errors and ready for review. Thx in advance! |
| } | ||
|
|
||
| return $schema; | ||
| } |
There was a problem hiding this comment.
Shouldn't you add a public addDistinctFormat method here too then?
In case this factory would be decorated too in the future?
There was a problem hiding this comment.
Good point, will add it.
|
really good feature if you could rebase, thanks! |
arnedesmedt
force-pushed
the
hal-jsonschema
branch
from
dea2e37 to
3bd9736
Compare
|
Did the rebase. |
|
@alanpoulain can this be merged? Or do you expect something else to merge it? |
alanpoulain
force-pushed
the
hal-jsonschema
branch
2 times, most recently
from
0d62a50 to
7053bfb
Compare
alanpoulain
force-pushed
the
hal-jsonschema
branch
from
7053bfb to
3709adf
Compare
|
Thank you @arnedesmedt and @coudenysj! |
|
🥳 |
…m#4357) Co-authored-by: Jachim Coudenys <jachim.coudenys@combellgroup.com> Co-authored-by: Alan Poulain <contact@alanpoulain.eu>
4 participants
Hi, this is a replacing MR for #4353
My colleague @coudenysj is now on holiday, so I've written the tests for his MR.
I've also needed to extend the SchemaFactoryInterface, because the decoration pattern is using the following order: Hal -> Hydra -> BaseSchemaFactory, we can't use the addDistinctFormat method only on the BaseSchemaFactory.