bugfix: throw when a subresources id is not found#1819
Conversation
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
from
878a35f to
5ab51da
Compare
| * @throws RuntimeException | ||
| * @throws ItemNotFoundException | ||
| */ | ||
| public function getSubresource(string $resourceClass, array $identifiers, array $context, string $operationName = null) |
There was a problem hiding this comment.
It's not the dataprovider's responsability to throw an ItemNotFoundException. To stay constistent with ItemDataProvider, please patch this inside the ReadListener instead.
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
2 times, most recently
from
2c725b4 to
e3d891b
Compare
No, security patches only on 2.0. Also, could you add a phpunit test |
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
from
e3d891b to
a5f3294
Compare
|
@soyuka Unit test added. |
| $data = $this->subresourceDataProvider->getSubresource($attributes['resource_class'], $identifiers, $attributes['subresource_context'], $attributes['subresource_operation_name']); | ||
|
|
||
| if (null === $data) { | ||
| throw new ItemNotFoundException(sprintf('Object of type "%s" with identifiers %s could not be found', $attributes['resource_class'], implode(',', $identifiers)), 400); |
There was a problem hiding this comment.
Why don't we use the NotFoundHttpException here?
There was a problem hiding this comment.
I think it's better too yeah.
| $listener = new ReadListener($collectionDataProvider->reveal(), $itemDataProvider->reveal(), $subresourceDataProvider->reveal()); | ||
| $listener->onKernelRequest($event->reveal()); | ||
|
|
||
| $this->assertSame($data, $request->attributes->get('data')); |
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
3 times, most recently
from
ff787d4 to
52fa4be
Compare
| $data = $this->subresourceDataProvider->getSubresource($attributes['resource_class'], $identifiers, $attributes['subresource_context'], $attributes['subresource_operation_name']); | ||
|
|
||
| if (null === $data) { | ||
| throw new NotFoundHttpException(sprintf('Object of type "%s" with identifiers %s could not be found', $attributes['resource_class'], implode(',', $identifiers))); |
There was a problem hiding this comment.
Can you change the message to keep consistency with this?
core/src/EventListener/ReadListener.php
Line 175 in d4aa84f
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
from
52fa4be to
2a8ef27
Compare
Simperfit
force-pushed
the
bug/throw-when-an-subresources-id-is-not-found
branch
from
2a8ef27 to
a65e09e
Compare
|
thanks @Simperfit |
|
When this update will be release? Thanks |
|
It should be in 2.2.6 and later |
|
The code is not on this release https://github.com/api-platform/core/releases/tag/v2.2.7 I download the zip file and the code is missing and missing from here too https://github.com/api-platform/core/blob/v2.2.7/src/EventListener/ReadListener.php |
|
The variable $data is not null when you try to access a subresource ex: post/id/comments I think that we have to update the line https://github.com/Simperfit/core/blob/a65e09e354df3f5f408234e0cf8eb1e4b3bbd18b/src/EventListener/ReadListener.php#L150 with this code |
5 participants
This could go maybe agasint 2.0 since it has been introduced in 2.0, WDYT ?