Skip to content

Commit

Permalink
fix(symfony): exception_status bad merge (#4981)
Browse files Browse the repository at this point in the history
Co-authored-by: ArnoudThibaut <thibaut.arnoud@gmail.com>
  • Loading branch information
soyuka and ArnoudThibaut committed Sep 19, 2022
1 parent 530ef37 commit 56875b3
Show file tree
Hide file tree
Showing 11 changed files with 259 additions and 19 deletions.
33 changes: 33 additions & 0 deletions features/main/exception_to_status.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Feature: Using exception_to_status config
As an API developer
I can customize the status code returned if the application throws an exception

@createSchema
@!mongodb
Scenario: Configure status code via the operation exceptionToStatus to map custom NotFound error to 404
When I add "Content-Type" header equal to "application/ld+json"
And I send a "GET" request to "/dummy_exception_to_statuses/123"
Then the response status code should be 404
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"

@!mongodb
Scenario: Configure status code via the resource exceptionToStatus to map custom NotFound error to 400
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/dummy_exception_to_statuses/123" with body:
"""
{
"name": "black"
}
"""
Then the response status code should be 400
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"

@!mongodb
Scenario: Configure status code via the config file to map FilterValidationException to 400
When I add "Content-Type" header equal to "application/ld+json"
And I send a "GET" request to "/dummy_exception_to_statuses"
Then the response status code should be 400
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use ApiPlatform\State\ProviderInterface;
use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaRestrictionMetadataInterface;
use ApiPlatform\Symfony\Validator\ValidationGroupsGeneratorInterface;
use Doctrine\Persistence\ManagerRegistry;
use phpDocumentor\Reflection\DocBlockFactoryInterface;
use Ramsey\Uuid\Uuid;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
Expand Down Expand Up @@ -496,6 +497,11 @@ private function registerDoctrineOrmConfiguration(ContainerBuilder $container, a
return;
}

// For older versions of doctrine bridge this allows autoconfiguration for filters
if (!$container->has(ManagerRegistry::class)) {
$container->setAlias(ManagerRegistry::class, 'doctrine');
}

$container->registerForAutoconfiguration(QueryItemExtensionInterface::class)
->addTag('api_platform.doctrine.orm.query_extension.item');
$container->registerForAutoconfiguration(DoctrineQueryCollectionExtensionInterface::class)
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/Resources/config/api.xml
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,9 @@
<argument type="service" id="api_platform.symfony.iri_converter.skolem" />
</service>
<service id="ApiPlatform\Api\IriConverterInterface" alias="api_platform.symfony.iri_converter" />

<service id="api_platform.error_listener" class="ApiPlatform\Symfony\EventListener\ErrorListener" parent="exception_listener">
<argument key="$controller">api_platform.action.exception</argument>
</service>
</services>
</container>
4 changes: 1 addition & 3 deletions src/Symfony/Bundle/Resources/config/symfony/events.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@
</service>

<service id="api_platform.listener.exception" class="ApiPlatform\Symfony\EventListener\ExceptionListener">
<argument>api_platform.action.exception</argument>
<argument type="service" id="logger" on-invalid="null" />
<argument>false</argument>
<argument type="service" id="api_platform.error_listener" on-invalid="null" />

<tag name="kernel.event_listener" event="kernel.exception" method="onKernelException" priority="-96" />
<tag name="monolog.logger" channel="request" />
Expand Down
42 changes: 42 additions & 0 deletions src/Symfony/EventListener/ErrorListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Symfony\EventListener;

use ApiPlatform\Action\ExceptionAction;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\EventListener\ErrorListener as SymfonyErrorListener;

/**
* This error listener extends the Symfony one in order to add
* the `_api_operation` attribute when the request is duplicated.
* It will later be used to retrieve the exceptionToStatus from the operation ({@see ExceptionAction}).
*/
final class ErrorListener extends SymfonyErrorListener
{
protected function duplicateRequest(\Throwable $exception, Request $request): Request
{
$dup = parent::duplicateRequest($exception, $request);

if ($request->attributes->has('_api_operation')) {
$dup->attributes->set('_api_operation', $request->attributes->get('_api_operation'));
}

// TODO: remove legacy layer in 3.0
if ($request->attributes->has('_api_exception_to_status')) {
$dup->attributes->set('_api_exception_to_status', $request->attributes->get('_api_exception_to_status'));
}

return $dup;
}
}
6 changes: 1 addition & 5 deletions src/Symfony/EventListener/ExceptionListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
namespace ApiPlatform\Symfony\EventListener;

use ApiPlatform\Util\RequestAttributesExtractor;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\EventListener\ErrorListener;

Expand All @@ -26,11 +25,8 @@
*/
final class ExceptionListener
{
private readonly ErrorListener $errorListener;

public function __construct(string|object|array|null $controller, LoggerInterface $logger = null, bool $debug = false)
public function __construct(private readonly ErrorListener $errorListener)
{
$this->errorListener = new ErrorListener($controller, $logger, $debug);
}

public function onKernelException(ExceptionEvent $event): void
Expand Down
111 changes: 111 additions & 0 deletions tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;

use ApiPlatform\Metadata\ApiFilter;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use ApiPlatform\Metadata\Put;
use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException;
use ApiPlatform\Tests\Fixtures\TestBundle\Filter\RequiredFilter;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

#[ApiResource(
exceptionToStatus: [NotFoundHttpException::class => 400],
operations: [
new Get(exceptionToStatus: [NotFoundException::class => 404]),
new Put(),
new GetCollection(),
]
)]
#[ApiFilter(RequiredFilter::class)]
#[ORM\Entity]
class DummyExceptionToStatus
{
/**
* @var int|null The id
*/
#[ORM\Column(type: 'integer')]
#[ORM\Id]
#[ORM\GeneratedValue(strategy: 'AUTO')]
private ?int $id = null;

/**
* @var string|null The dummy name
*/
#[ORM\Column(nullable: true)]
private ?string $name = null;

/**
* @var string|null The dummy title
*/
#[ORM\Column(nullable: true)]
private ?string $title = null;

/**
* @var string The dummy code
*/
#[ORM\Column]
private string $code;

public function getId(): ?int
{
return $this->id;
}

public function setId(int $id): self
{
$this->id = $id;

return $this;
}

public function getName(): ?string
{
return $this->name;
}

public function setName(?string $name): self
{
$this->name = $name;

return $this;
}

public function getTitle(): ?string
{
return $this->title;
}

public function setTitle(?string $title): self
{
$this->title = $title;

return $this;
}

public function getCode(): ?string
{
return $this->code;
}

public function setCode(string $code): self
{
$this->code = $code;

return $this;
}
}
22 changes: 22 additions & 0 deletions tests/Fixtures/TestBundle/Exception/NotFoundException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Tests\Fixtures\TestBundle\Exception;

final class NotFoundException extends \Exception
{
public function __construct(string $message = '', int $code = 0, ?\Throwable $previous = null)
{
parent::__construct($message, $code, $previous);
}
}
26 changes: 26 additions & 0 deletions tests/Fixtures/TestBundle/State/DummyExceptionToStatusProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Tests\Fixtures\TestBundle\State;

use ApiPlatform\Metadata\Operation;
use ApiPlatform\State\ProviderInterface;
use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException;

class DummyExceptionToStatusProvider implements ProviderInterface
{
public function provide(Operation $operation, array $uriVariables = [], array $context = []): iterable|object|null
{
throw new NotFoundException();
}
}
1 change: 1 addition & 0 deletions tests/Fixtures/app/config/config_common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -409,3 +409,4 @@ services:
arguments: [ '@doctrine' ]
tags:
- { name: 'api_platform.state_processor' }

23 changes: 12 additions & 11 deletions tests/Symfony/EventListener/ExceptionListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@

use ApiPlatform\Symfony\EventListener\ExceptionListener;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\EventListener\ErrorListener;
use Symfony\Component\HttpKernel\HttpKernelInterface;

/**
Expand All @@ -35,13 +34,13 @@ class ExceptionListenerTest extends TestCase
public function testOnKernelException(Request $request): void
{
$kernel = $this->prophesize(HttpKernelInterface::class);
$kernel->handle(Argument::type(Request::class), HttpKernelInterface::SUB_REQUEST, false)->willReturn(new Response())->shouldBeCalled();

$listener = new ExceptionListener('foo:bar', null, false);
$event = new ExceptionEvent($kernel->reveal(), $request, \defined(HttpKernelInterface::class.'::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : HttpKernelInterface::MASTER_REQUEST, new \Exception());
$listener->onKernelException($event);
$event = new ExceptionEvent($kernel->reveal(), $request, HttpKernelInterface::MAIN_REQUEST, new \Exception());

$this->assertInstanceOf(Response::class, $event->getResponse());
$errorListener = $this->prophesize(ErrorListener::class);
$errorListener->onKernelException($event)->shouldBeCalled();
$listener = new ExceptionListener($errorListener->reveal());
$listener->onKernelException($event);
}

public function getRequest(): array
Expand All @@ -54,8 +53,9 @@ public function getRequest(): array

public function testDoNothingWhenNotAnApiCall(): void
{
$listener = new ExceptionListener('foo:bar', null, false);
$event = new ExceptionEvent($this->prophesize(HttpKernelInterface::class)->reveal(), new Request(), \defined(HttpKernelInterface::class.'::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : HttpKernelInterface::MASTER_REQUEST, new \Exception());
$errorListener = $this->prophesize(ErrorListener::class);
$listener = new ExceptionListener($errorListener->reveal());
$event = new ExceptionEvent($this->prophesize(HttpKernelInterface::class)->reveal(), new Request(), HttpKernelInterface::MAIN_REQUEST, new \Exception());
$listener->onKernelException($event);

$this->assertNull($event->getResponse());
Expand All @@ -66,8 +66,9 @@ public function testDoNothingWhenHtmlRequested(): void
$request = new Request([], [], ['_api_respond' => true]);
$request->setRequestFormat('html');

$listener = new ExceptionListener('foo:bar', null, false);
$event = new ExceptionEvent($this->prophesize(HttpKernelInterface::class)->reveal(), $request, \defined(HttpKernelInterface::class.'::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : HttpKernelInterface::MASTER_REQUEST, new \Exception());
$errorListener = $this->prophesize(ErrorListener::class);
$listener = new ExceptionListener($errorListener->reveal());
$event = new ExceptionEvent($this->prophesize(HttpKernelInterface::class)->reveal(), $request, HttpKernelInterface::MAIN_REQUEST, new \Exception());
$listener->onKernelException($event);

$this->assertNull($event->getResponse());
Expand Down

0 comments on commit 56875b3

Please sign in to comment.