From 7611b29ecc52bb43fc1fbdd060c64e40b6bf3781 Mon Sep 17 00:00:00 2001 From: mahmood bazdar Date: Wed, 11 Sep 2019 14:49:18 +0430 Subject: [PATCH 1/3] Adding GraphQL custom error format support --- CHANGELOG.md | 1 + features/graphql/mutation.feature | 7 +- .../Symfony/Bundle/ApiPlatformBundle.php | 2 + .../ApiPlatformExtension.php | 3 + .../GraphQlExceptionFormatterPass.php | 47 +++++++++ .../Bundle/Resources/config/graphql.xml | 16 ++++ src/GraphQl/Action/EntrypointAction.php | 8 +- .../Exception/ExceptionFormatterCallback.php | 56 +++++++++++ .../ExceptionFormatterCallbackInterface.php | 29 ++++++ .../Exception/ExceptionFormatterFactory.php | 50 ++++++++++ .../ExceptionFormatterFactoryInterface.php | 24 +++++ .../Exception/ExceptionFormatterInterface.php | 39 ++++++++ .../ValidationExceptionFormatter.php | 71 ++++++++++++++ src/GraphQl/Resolver/Stage/ValidateStage.php | 12 +-- .../Symfony/Bundle/ApiPlatformBundleTest.php | 2 + .../ApiPlatformExtensionTest.php | 15 +++ .../GraphQlExceptionFormatterPassTest.php | 95 +++++++++++++++++++ .../ValidationExceptionFormatter.php | 71 ++++++++++++++ tests/Fixtures/app/config/config_common.yml | 5 + tests/GraphQl/Action/EntrypointActionTest.php | 6 +- .../ExceptionFormatterCallbackTest.php | 93 ++++++++++++++++++ .../ExceptionFormatterFactoryTest.php | 43 +++++++++ .../ValidationExceptionFormatterTest.php | 62 ++++++++++++ .../Resolver/Stage/ValidateStageTest.php | 3 +- 24 files changed, 743 insertions(+), 17 deletions(-) create mode 100644 src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php create mode 100644 src/GraphQl/Exception/ExceptionFormatterCallback.php create mode 100644 src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php create mode 100644 src/GraphQl/Exception/ExceptionFormatterFactory.php create mode 100644 src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php create mode 100644 src/GraphQl/Exception/ExceptionFormatterInterface.php create mode 100644 src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php create mode 100644 tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php create mode 100644 tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php create mode 100644 tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php create mode 100644 tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php create mode 100644 tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 98ee0ff0e1e..3dec65ca5f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ * Test: Add assertions for testing response against JSON Schema from API resource * GraphQL: Add support for multipart request so user can create custom file upload mutations (#3041) * GraphQL: Add support for name converter (#2765) +* GraphQL: Allow to format GraphQL errors based on exceptions. ## 2.5.0 beta 1 diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index ac8a8f36520..8b334e453c8 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -674,7 +674,12 @@ Feature: GraphQL mutation support Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].message" should be equal to "name: This value should not be blank." + And the JSON node "errors[0].status" should be equal to "400" + And the JSON node "errors[0].message" should be equal to "Validation Exception" + And the JSON node "errors[0].violations" should exist + And the JSON node "errors[0].violations[0].path" should be equal to "name" + And the JSON node "errors[0].violations[0].message" should be equal to "This value should not be blank." + Scenario: Execute a custom mutation Given there are 1 dummyCustomMutation objects diff --git a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php index a80a8a40a48..e4bc570a80a 100644 --- a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php +++ b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php @@ -17,6 +17,7 @@ use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\FilterPass; +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlMutationResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlQueryResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlTypePass; @@ -48,6 +49,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new GraphQlTypePass()); $container->addCompilerPass(new GraphQlQueryResolverPass()); $container->addCompilerPass(new GraphQlMutationResolverPass()); + $container->addCompilerPass(new GraphQlExceptionFormatterPass()); $container->addCompilerPass(new MetadataAwareNameConverterPass()); } } diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index fadf912ccc4..50182167640 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -28,6 +28,7 @@ use ApiPlatform\Core\DataProvider\ItemDataProviderInterface; use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface; use ApiPlatform\Core\DataTransformer\DataTransformerInterface; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; use ApiPlatform\Core\GraphQl\Resolver\MutationResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryCollectionResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryItemResolverInterface; @@ -426,6 +427,8 @@ private function registerGraphQlConfiguration(ContainerBuilder $container, array ->addTag('api_platform.graphql.mutation_resolver'); $container->registerForAutoconfiguration(GraphQlTypeInterface::class) ->addTag('api_platform.graphql.type'); + $container->registerForAutoconfiguration(ExceptionFormatterInterface::class) + ->addTag('api_platform.graphql.exception_formatter'); } private function registerLegacyBundlesConfiguration(ContainerBuilder $container, array $config, XmlFileLoader $loader): void diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php new file mode 100644 index 00000000000..cc4d569141c --- /dev/null +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php @@ -0,0 +1,47 @@ + + * + * 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\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; + +/** + * Injects GraphQL Exception formatters. + * + * @internal + * + * @author Mahmood Bazdar + */ +class GraphQlExceptionFormatterPass implements CompilerPassInterface +{ + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + if (!$container->getParameter('api_platform.graphql.enabled')) { + return; + } + + $formatters = []; + foreach ($container->findTaggedServiceIds('api_platform.graphql.exception_formatter', true) as $serviceId => $tags) { + foreach ($tags as $tag) { + $formatters[$tag['id'] ?? $serviceId] = new Reference($serviceId); + } + } + $container->getDefinition('api_platform.graphql.exception_formatter_locator')->addArgument($formatters); + $container->getDefinition('api_platform.graphql.exception_formatter_factory')->addArgument(array_keys($formatters)); + } +} diff --git a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml index 07617218bf5..84ac0e1aa83 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml @@ -159,7 +159,22 @@ + + + + + + + + + + + + + + + @@ -167,6 +182,7 @@ + %kernel.debug% %api_platform.graphql.graphiql.enabled% %api_platform.graphql.graphql_playground.enabled% diff --git a/src/GraphQl/Action/EntrypointAction.php b/src/GraphQl/Action/EntrypointAction.php index 18b00be2e72..f1a72dbb7cc 100644 --- a/src/GraphQl/Action/EntrypointAction.php +++ b/src/GraphQl/Action/EntrypointAction.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\GraphQl\Action; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallbackInterface; use ApiPlatform\Core\GraphQl\ExecutorInterface; use ApiPlatform\Core\GraphQl\Type\SchemaBuilderInterface; use GraphQL\Error\Debug; @@ -39,8 +40,9 @@ final class EntrypointAction private $graphiqlEnabled; private $graphQlPlaygroundEnabled; private $defaultIde; + private $exceptionFormatterCallback; - public function __construct(SchemaBuilderInterface $schemaBuilder, ExecutorInterface $executor, GraphiQlAction $graphiQlAction, GraphQlPlaygroundAction $graphQlPlaygroundAction, bool $debug = false, bool $graphiqlEnabled = false, bool $graphQlPlaygroundEnabled = false, $defaultIde = false) + public function __construct(SchemaBuilderInterface $schemaBuilder, ExecutorInterface $executor, GraphiQlAction $graphiQlAction, GraphQlPlaygroundAction $graphQlPlaygroundAction, ExceptionFormatterCallbackInterface $exceptionFormatterCallback, bool $debug = false, bool $graphiqlEnabled = false, bool $graphQlPlaygroundEnabled = false, $defaultIde = false) { $this->schemaBuilder = $schemaBuilder; $this->executor = $executor; @@ -50,6 +52,7 @@ public function __construct(SchemaBuilderInterface $schemaBuilder, ExecutorInter $this->graphiqlEnabled = $graphiqlEnabled; $this->graphQlPlaygroundEnabled = $graphQlPlaygroundEnabled; $this->defaultIde = $defaultIde; + $this->exceptionFormatterCallback = $exceptionFormatterCallback; } public function __invoke(Request $request): Response @@ -70,7 +73,8 @@ public function __invoke(Request $request): Response throw new BadRequestHttpException('GraphQL query is not valid.'); } - $executionResult = $this->executor->executeQuery($this->schemaBuilder->getSchema(), $query, null, null, $variables, $operation); + $executionResult = $this->executor->executeQuery($this->schemaBuilder->getSchema(), $query, null, null, $variables, $operation) + ->setErrorFormatter($this->exceptionFormatterCallback); } catch (BadRequestHttpException $e) { $exception = new UserError($e->getMessage(), 0, $e); diff --git a/src/GraphQl/Exception/ExceptionFormatterCallback.php b/src/GraphQl/Exception/ExceptionFormatterCallback.php new file mode 100644 index 00000000000..2a187e38be5 --- /dev/null +++ b/src/GraphQl/Exception/ExceptionFormatterCallback.php @@ -0,0 +1,56 @@ + + * + * 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\Core\GraphQl\Exception; + +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; + +/** + * @expremintal + * + * @author Mahmood Bazdar + */ +class ExceptionFormatterCallback implements ExceptionFormatterCallbackInterface +{ + private $exceptionFormatterFactory; + + public function __construct(ExceptionFormatterFactoryInterface $exceptionFormatterFactory) + { + $this->exceptionFormatterFactory = $exceptionFormatterFactory; + } + + /** + * {@inheritdoc} + */ + public function __invoke(Error $error): array + { + $formatters = $this->exceptionFormatterFactory->getExceptionFormatters(); + usort($formatters, function (ExceptionFormatterInterface $a, ExceptionFormatterInterface $b) { + if ($a->getPriority() == $b->getPriority()) { + return 0; + } + + return ($a->getPriority() > $b->getPriority()) ? -1 : 1; + }); + /** @var ExceptionFormatterInterface $exceptionFormatter */ + foreach ($formatters as $exceptionFormatter) { + if (null !== $error->getPrevious() && $exceptionFormatter->supports($error->getPrevious())) { + return $exceptionFormatter->format($error); + } + } + + // falling back to default GraphQL error formatter + return FormattedError::createFromException($error); + } +} diff --git a/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php b/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php new file mode 100644 index 00000000000..838f335a05a --- /dev/null +++ b/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php @@ -0,0 +1,29 @@ + + * + * 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\Core\GraphQl\Exception; + +use GraphQL\Error\Error; + +/** + * @expremintal + * + * @author Mahmood Bazdar + */ +interface ExceptionFormatterCallbackInterface +{ + /** + * Callback function will be used for formatting GraphQL errors. + */ + public function __invoke(Error $error): array; +} diff --git a/src/GraphQl/Exception/ExceptionFormatterFactory.php b/src/GraphQl/Exception/ExceptionFormatterFactory.php new file mode 100644 index 00000000000..dbb084c535d --- /dev/null +++ b/src/GraphQl/Exception/ExceptionFormatterFactory.php @@ -0,0 +1,50 @@ + + * + * 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\Core\GraphQl\Exception; + +use ApiPlatform\Core\GraphQl\Type\Definition\TypeInterface; +use Psr\Container\ContainerInterface; + +/** + * Get the registered services corresponding to GraphQL exception formatters. + * + * @author Mahmood Bazdar + */ +class ExceptionFormatterFactory implements ExceptionFormatterFactoryInterface +{ + private $exceptionFormatterLocator; + private $exceptionFormatterIds; + + /** + * @param string[] $exceptionFormatterIds + */ + public function __construct(ContainerInterface $exceptionFormatterLocator, array $exceptionFormatterIds) + { + $this->exceptionFormatterLocator = $exceptionFormatterLocator; + $this->exceptionFormatterIds = $exceptionFormatterIds; + } + + public function getExceptionFormatters(): array + { + $exceptionFormatters = []; + + foreach ($this->exceptionFormatterIds as $exceptionFormatterId) { + /** @var TypeInterface $exceptionFormatter */ + $exceptionFormatter = $this->exceptionFormatterLocator->get($exceptionFormatterId); + $exceptionFormatters[$exceptionFormatterId] = $exceptionFormatter; + } + + return $exceptionFormatters; + } +} diff --git a/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php b/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php new file mode 100644 index 00000000000..4fe279238d6 --- /dev/null +++ b/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php @@ -0,0 +1,24 @@ + + * + * 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\Core\GraphQl\Exception; + +/** + * Get Exception formatters. + * + * @author Mahmood Bazdar + */ +interface ExceptionFormatterFactoryInterface +{ + public function getExceptionFormatters(): array; +} diff --git a/src/GraphQl/Exception/ExceptionFormatterInterface.php b/src/GraphQl/Exception/ExceptionFormatterInterface.php new file mode 100644 index 00000000000..6f80bdb3407 --- /dev/null +++ b/src/GraphQl/Exception/ExceptionFormatterInterface.php @@ -0,0 +1,39 @@ + + * + * 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\Core\GraphQl\Exception; + +use GraphQL\Error\Error; + +/** + * @expremintal + * + * @author Mahmood Bazdar + */ +interface ExceptionFormatterInterface +{ + /** + * Formats the exception and returns the formatted array. + */ + public function format(Error $error): array; + + /** + * Check the exception, return true if you can format the exception. + */ + public function supports(\Throwable $exception): bool; + + /** + * Priority of your formatter in container. Higher number will be called sooner. + */ + public function getPriority(): int; +} diff --git a/src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php b/src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php new file mode 100644 index 00000000000..846a05a887f --- /dev/null +++ b/src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php @@ -0,0 +1,71 @@ + + * + * 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\Core\GraphQl\Exception\Formatter; + +use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Validator\ConstraintViolation; + +/** + * Formats Validation exception. + * + * @author Mahmood Bazdar + */ +class ValidationExceptionFormatter implements ExceptionFormatterInterface +{ + /** + * {@inheritdoc} + */ + public function format(Error $error): array + { + /** + * @var ValidationException + */ + $validationException = $error->getPrevious(); + $error = FormattedError::createFromException($error); + $error['message'] = $validationException->getMessage(); + $error['status'] = Response::HTTP_BAD_REQUEST; + $error['extensions']['category'] = Error::CATEGORY_GRAPHQL; + $error['violations'] = []; + + /** @var ConstraintViolation $violation */ + foreach ($validationException->getConstraintViolationList() as $violation) { + $error['violations'][] = [ + 'path' => $violation->getPropertyPath(), + 'message' => $violation->getMessage(), + ]; + } + + return $error; + } + + /** + * {@inheritdoc} + */ + public function supports(\Throwable $exception): bool + { + return $exception instanceof ValidationException; + } + + /** + * {@inheritdoc} + */ + public function getPriority(): int + { + return -100; + } +} diff --git a/src/GraphQl/Resolver/Stage/ValidateStage.php b/src/GraphQl/Resolver/Stage/ValidateStage.php index 14d98e51ae0..3c46aea7746 100644 --- a/src/GraphQl/Resolver/Stage/ValidateStage.php +++ b/src/GraphQl/Resolver/Stage/ValidateStage.php @@ -14,10 +14,7 @@ namespace ApiPlatform\Core\GraphQl\Resolver\Stage; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; -use ApiPlatform\Core\Validator\Exception\ValidationException; use ApiPlatform\Core\Validator\ValidatorInterface; -use GraphQL\Error\Error; -use GraphQL\Type\Definition\ResolveInfo; /** * Validate stage of GraphQL resolvers. @@ -48,13 +45,6 @@ public function __invoke($object, string $resourceClass, string $operationName, } $validationGroups = $resourceMetadata->getGraphqlAttribute($operationName, 'validation_groups', null, true); - try { - $this->validator->validate($object, ['groups' => $validationGroups]); - } catch (ValidationException $e) { - /** @var ResolveInfo $info */ - $info = $context['info']; - - throw Error::createLocatedError($e->getMessage(), $info->fieldNodes, $info->path); - } + $this->validator->validate($object, ['groups' => $validationGroups]); } } diff --git a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php index e186bbb60bd..2ee09ae6486 100644 --- a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php +++ b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php @@ -18,6 +18,7 @@ use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\FilterPass; +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlMutationResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlQueryResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlTypePass; @@ -42,6 +43,7 @@ public function testBuild() $containerProphecy->addCompilerPass(Argument::type(GraphQlTypePass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(GraphQlQueryResolverPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(GraphQlMutationResolverPass::class))->shouldBeCalled(); + $containerProphecy->addCompilerPass(Argument::type(GraphQlExceptionFormatterPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(MetadataAwareNameConverterPass::class))->shouldBeCalled(); $bundle = new ApiPlatformBundle(); diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 40aff578ba1..01e030cc9d0 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -63,6 +63,7 @@ use ApiPlatform\Core\DataTransformer\DataTransformerInterface; use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; use ApiPlatform\Core\GraphQl\Resolver\MutationResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryCollectionResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryItemResolverInterface; @@ -367,6 +368,10 @@ public function testDisableGraphQl() $containerBuilderProphecy->setDefinition('api_platform.graphql.type_converter', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.query_resolver_locator', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.mutation_resolver_locator', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.validation_exception_formatter', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_callback', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_locator', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_factory', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.command.export_command', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setParameter('api_platform.graphql.enabled', true)->shouldNotBeCalled(); $containerBuilderProphecy->setParameter('api_platform.graphql.enabled', false)->shouldBeCalled(); @@ -386,6 +391,8 @@ public function testDisableGraphQl() $this->childDefinitionProphecy->addTag('api_platform.graphql.query_resolver')->shouldNotBeCalled(); $containerBuilderProphecy->registerForAutoconfiguration(MutationResolverInterface::class)->shouldNotBeCalled(); $this->childDefinitionProphecy->addTag('api_platform.graphql.mutation_resolver')->shouldNotBeCalled(); + $containerBuilderProphecy->registerForAutoconfiguration(ExceptionFormatterInterface::class)->shouldNotBeCalled(); + $this->childDefinitionProphecy->addTag('api_platform.graphql.exception_formatter')->shouldNotBeCalled(); $containerBuilderProphecy->setAlias(GraphQlSerializerContextBuilderInterface::class, 'api_platform.graphql.serializer.context_builder')->shouldNotBeCalled(); $containerBuilder = $containerBuilderProphecy->reveal(); @@ -1057,6 +1064,10 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); $this->childDefinitionProphecy->addTag('api_platform.graphql.type')->shouldBeCalledTimes(1); + $containerBuilderProphecy->registerForAutoconfiguration(ExceptionFormatterInterface::class) + ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); + $this->childDefinitionProphecy->addTag('api_platform.graphql.exception_formatter')->shouldBeCalledTimes(1); + $containerBuilderProphecy->registerForAutoconfiguration(QueryItemResolverInterface::class) ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); $containerBuilderProphecy->registerForAutoconfiguration(QueryCollectionResolverInterface::class) @@ -1183,6 +1194,10 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo 'api_platform.graphql.resolver.stage.write', 'api_platform.graphql.resolver.stage.validate', 'api_platform.graphql.resolver.resource_field', + 'api_platform.graphql.exception_formatter_callback', + 'api_platform.graphql.validation_exception_formatter', + 'api_platform.graphql.exception_formatter_locator', + 'api_platform.graphql.exception_formatter_factory', 'api_platform.graphql.iterable_type', 'api_platform.graphql.upload_type', 'api_platform.graphql.type_locator', diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php new file mode 100644 index 00000000000..5121309df55 --- /dev/null +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php @@ -0,0 +1,95 @@ + + * + * 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\Core\Tests\Bridge\Symfony\Bundle\DependencyInjection\Compiler; + +use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; +use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; + +/** + * @author Mahmood Bazdar + */ +class GraphQlExceptionFormatterPassTest extends TestCase +{ + public function testProcess() + { + $filterPass = new GraphQlExceptionFormatterPass(); + + $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); + + $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::that(function (array $arg) { + return !isset($arg['foo']) && isset($arg['my_id']) && $arg['my_id'] instanceof Reference; + }))->shouldBeCalled(); + + $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterFactoryDefinitionProphecy->addArgument(['my_id'])->shouldBeCalled(); + + $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); + $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(true)->shouldBeCalled(); + $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['id' => 'my_id']]])->shouldBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldBeCalled(); + + $filterPass->process($containerBuilderProphecy->reveal()); + } + + public function testIdNotExist() + { + $filterPass = new GraphQlExceptionFormatterPass(); + + $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); + + $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::that(function (array $arg) { + return !isset($arg['foo']) && isset($arg['bar']) && $arg['bar'] instanceof Reference; + }))->shouldBeCalled(); + + $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterFactoryDefinitionProphecy->addArgument(['bar'])->shouldBeCalled(); + + $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); + $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(true)->shouldBeCalled(); + $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['hi' => 'hello']]])->shouldBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldBeCalled(); + + $filterPass->process($containerBuilderProphecy->reveal()); + } + + public function testDisabled() + { + $filterPass = new GraphQlExceptionFormatterPass(); + + $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); + + $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::any())->shouldNotBeCalled(); + + $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); + $exceptionFormatterFactoryDefinitionProphecy->addArgument(['my_id'])->shouldNotBeCalled(); + + $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); + $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(false)->shouldBeCalled(); + $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['id' => 'my_id']]])->shouldNotBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldNotBeCalled(); + $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldNotBeCalled(); + + $filterPass->process($containerBuilderProphecy->reveal()); + } +} diff --git a/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php b/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php new file mode 100644 index 00000000000..45c92d7b9a3 --- /dev/null +++ b/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php @@ -0,0 +1,71 @@ + + * + * 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\Core\Tests\Fixtures\TestBundle\GraphQl\ExceptionFormatter; + +use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Validator\ConstraintViolation; + +/** + * Formats Validation exception. + * + * @author Mahmood Bazdar + */ +class ValidationExceptionFormatter implements ExceptionFormatterInterface +{ + /** + * {@inheritdoc} + */ + public function format(Error $error): array + { + /** + * @var ValidationException + */ + $validationException = $error->getPrevious(); + $error = FormattedError::createFromException($error); + $error['message'] = 'Validation Exception'; + $error['status'] = Response::HTTP_BAD_REQUEST; + $error['extensions']['category'] = Error::CATEGORY_GRAPHQL; + $error['violations'] = []; + + /** @var ConstraintViolation $violation */ + foreach ($validationException->getConstraintViolationList() as $violation) { + $error['violations'][] = [ + 'path' => $violation->getPropertyPath(), + 'message' => $violation->getMessage(), + ]; + } + + return $error; + } + + /** + * {@inheritdoc} + */ + public function supports(\Throwable $exception): bool + { + return $exception instanceof ValidationException; + } + + /** + * {@inheritdoc} + */ + public function getPriority(): int + { + return 1; + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index e4c861cbb4a..603b2c9b733 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -321,3 +321,8 @@ services: decorates: 'api_platform.graphql.type_converter' arguments: ['@app.graphql.type_converter.inner'] public: false + + app.graphql.validation_exception_formatter: + class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\GraphQl\ExceptionFormatter\ValidationExceptionFormatter' + tags: + - { name: 'api_platform.graphql.exception_formatter' } \ No newline at end of file diff --git a/tests/GraphQl/Action/EntrypointActionTest.php b/tests/GraphQl/Action/EntrypointActionTest.php index 74ffd74aad0..c15aec9d7c1 100644 --- a/tests/GraphQl/Action/EntrypointActionTest.php +++ b/tests/GraphQl/Action/EntrypointActionTest.php @@ -16,6 +16,7 @@ use ApiPlatform\Core\GraphQl\Action\EntrypointAction; use ApiPlatform\Core\GraphQl\Action\GraphiQlAction; use ApiPlatform\Core\GraphQl\Action\GraphQlPlaygroundAction; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallbackInterface; use ApiPlatform\Core\GraphQl\ExecutorInterface; use ApiPlatform\Core\GraphQl\Type\SchemaBuilderInterface; use GraphQL\Executor\ExecutionResult; @@ -228,8 +229,11 @@ private function getEntrypointAction(array $variables = ['graphqlVariable']): En $schemaBuilderProphecy = $this->prophesize(SchemaBuilderInterface::class); $schemaBuilderProphecy->getSchema()->willReturn($schema->reveal()); + $exceptionFormatterCallback = $this->prophesize(ExceptionFormatterCallbackInterface::class)->reveal(); + $executionResultProphecy = $this->prophesize(ExecutionResult::class); $executionResultProphecy->toArray(false)->willReturn(['GraphQL']); + $executionResultProphecy->setErrorFormatter($exceptionFormatterCallback)->willReturn($executionResultProphecy); $executorProphecy = $this->prophesize(ExecutorInterface::class); $executorProphecy->executeQuery(Argument::is($schema->reveal()), 'graphqlQuery', null, null, $variables, 'graphqlOperationName')->willReturn($executionResultProphecy->reveal()); @@ -239,6 +243,6 @@ private function getEntrypointAction(array $variables = ['graphqlVariable']): En $graphiQlAction = new GraphiQlAction($twigProphecy->reveal(), $routerProphecy->reveal(), true); $graphQlPlaygroundAction = new GraphQlPlaygroundAction($twigProphecy->reveal(), $routerProphecy->reveal(), true); - return new EntrypointAction($schemaBuilderProphecy->reveal(), $executorProphecy->reveal(), $graphiQlAction, $graphQlPlaygroundAction, false, true, true, 'graphiql'); + return new EntrypointAction($schemaBuilderProphecy->reveal(), $executorProphecy->reveal(), $graphiQlAction, $graphQlPlaygroundAction, $exceptionFormatterCallback, false, true, true, 'graphiql'); } } diff --git a/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php b/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php new file mode 100644 index 00000000000..49ea27f7763 --- /dev/null +++ b/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php @@ -0,0 +1,93 @@ + + * + * 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\Core\Tests\GraphQl\Exception; + +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallback; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterFactory; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; + +/** + * @author Mahmood Bazdar + */ +class ExceptionFormatterCallbackTest extends TestCase +{ + /** + * @dataProvider formatExceptionProvider + */ + public function testFormatException(bool $supports, array $expect) + { + $errorProphecy = $this->prophesize(\Exception::class)->reveal(); + $error = new Error('Something', null, null, null, null, $errorProphecy); + + $exceptionFormatterProphecy = $this->prophesize(ExceptionFormatterInterface::class); + $exceptionFormatterProphecy->format($error)->willReturn(['error' => 'test']); + $exceptionFormatterProphecy->supports($error->getPrevious())->willReturn($supports); + $exceptionFormatterProphecy->getPriority()->willReturn(1); + $exceptionFormatter = $exceptionFormatterProphecy->reveal(); + + $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); + $exceptionFormatterLocatorProphecy->get('formatter')->willReturn($exceptionFormatter)->shouldBeCalled(); + $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['formatter']); + + $exceptionFormatterCallback = new ExceptionFormatterCallback($exceptionFormattersFactory); + $this->assertEquals($expect, $exceptionFormatterCallback($error)); + } + + public function formatExceptionProvider(): array + { + return [ + 'format supported exception' => [true, ['error' => 'test']], + 'falling back to default formatter for unsupported exceptions' => [ + false, + [ + 'message' => 'Internal server error', + 'extensions' => [ + 'category' => 'internal', + ], + ], + ], + ]; + } + + public function testFormatterPriority() + { + $errorProphecy = $this->prophesize(\Exception::class)->reveal(); + $error = new Error('Something', null, null, null, null, $errorProphecy); + + $exceptionFormatter1Prophecy = $this->prophesize(ExceptionFormatterInterface::class); + $exceptionFormatter1Prophecy->format($error)->willReturn(['error' => 1]); + $exceptionFormatter1Prophecy->supports($error->getPrevious())->willReturn(true); + $exceptionFormatter1Prophecy->getPriority()->willReturn(1); + $exceptionFormatter1 = $exceptionFormatter1Prophecy->reveal(); + + $exceptionFormatter2Prophecy = $this->prophesize(ExceptionFormatterInterface::class); + $exceptionFormatter2Prophecy->format($error)->willReturn(['error' => 2]); + $exceptionFormatter2Prophecy->supports($error->getPrevious())->willReturn(true); + $exceptionFormatter2Prophecy->getPriority()->willReturn(2); + $exceptionFormatter2 = $exceptionFormatter2Prophecy->reveal(); + + $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); + $exceptionFormatterLocatorProphecy->get('formatter1')->willReturn($exceptionFormatter1)->shouldBeCalled(); + $exceptionFormatterLocatorProphecy->get('formatter2')->willReturn($exceptionFormatter2)->shouldBeCalled(); + + $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['formatter1', 'formatter2']); + + $exceptionFormatterCallback = new ExceptionFormatterCallback($exceptionFormattersFactory); + + $this->assertEquals(['error' => 2], $exceptionFormatterCallback($error)); + } +} diff --git a/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php b/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php new file mode 100644 index 00000000000..8d576d89567 --- /dev/null +++ b/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php @@ -0,0 +1,43 @@ + + * + * 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\Core\Tests\GraphQl\Exception; + +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterFactory; +use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; + +/** + * @author Mahmood Bazdar + */ +class ExceptionFormatterFactoryTest extends TestCase +{ + public function testGetExceptionFormatters() + { + $exceptionFormatterProphecy = $this->prophesize(ExceptionFormatterInterface::class); + $exceptionFormatterProphecy->supports()->willReturn(true); + $exceptionFormatterProphecy->getPriority()->willReturn(1); + $exceptionFormatter = $exceptionFormatterProphecy->reveal(); + + $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); + $exceptionFormatterLocatorProphecy->get('foo')->willReturn($exceptionFormatter)->shouldBeCalled(); + + $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['foo']); + + $formatters = $exceptionFormattersFactory->getExceptionFormatters(); + $this->assertArrayHasKey('foo', $formatters); + $this->assertInstanceOf(ExceptionFormatterInterface::class, $formatters['foo']); + $this->assertEquals(['foo' => $exceptionFormatter], $formatters); + } +} diff --git a/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php b/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php new file mode 100644 index 00000000000..aef54bdafcc --- /dev/null +++ b/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php @@ -0,0 +1,62 @@ + + * + * 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\Core\Tests\GraphQl\Exception\Formatter; + +use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\GraphQl\Exception\Formatter\ValidationExceptionFormatter; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationList; + +/** + * @author Mahmood Bazdar + */ +class ValidationExceptionFormatterTest extends TestCase +{ + public function testFormat() + { + $exception = new ValidationException(new ConstraintViolationList([ + new ConstraintViolation('message 1', '', [], '', 'Field 1', 'invalid'), + new ConstraintViolation('message 2', '', [], '', 'Field 2', 'invalid'), + ])); + $error = new Error('test message', null, null, null, null, $exception); + $formatter = new ValidationExceptionFormatter(); + + $formattedError = $formatter->format($error); + $this->assertArrayHasKey('violations', $formattedError); + $this->assertEquals(400, $formattedError['status']); + $this->assertEquals([ + [ + 'path' => 'Field 1', + 'message' => 'message 1', + ], + [ + 'path' => 'Field 2', + 'message' => 'message 2', + ], + ], $formattedError['violations']); + $this->assertEquals(Error::CATEGORY_GRAPHQL, $formattedError['extensions']['category']); + } + + public function testSupports() + { + $exception = new ValidationException(new ConstraintViolationList([ + new ConstraintViolation('message 1', '', [], '', 'Field 1', 'invalid'), + ])); + $formatter = new ValidationExceptionFormatter(); + + $this->assertTrue($formatter->supports($exception)); + } +} diff --git a/tests/GraphQl/Resolver/Stage/ValidateStageTest.php b/tests/GraphQl/Resolver/Stage/ValidateStageTest.php index 7b064b75ef3..904bcd50213 100644 --- a/tests/GraphQl/Resolver/Stage/ValidateStageTest.php +++ b/tests/GraphQl/Resolver/Stage/ValidateStageTest.php @@ -18,7 +18,6 @@ use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Validator\Exception\ValidationException; use ApiPlatform\Core\Validator\ValidatorInterface; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -92,7 +91,7 @@ public function testApplyNotValidated(): void $object = new \stdClass(); $this->validatorProphecy->validate($object, ['groups' => $validationGroups])->shouldBeCalled()->willThrow(new ValidationException()); - $this->expectException(Error::class); + $this->expectException(ValidationException::class); ($this->validateStage)($object, $resourceClass, $operationName, $context); } From e34a42ca3bad4df027ba37f590fa692e8c46e844 Mon Sep 17 00:00:00 2001 From: Alan Poulain Date: Mon, 14 Oct 2019 20:12:29 +0200 Subject: [PATCH 2/3] Use error normalizers --- CHANGELOG.md | 2 +- features/graphql/authorization.feature | 12 +++ features/graphql/introspection.feature | 4 +- features/graphql/mutation.feature | 3 +- .../Symfony/Bundle/ApiPlatformBundle.php | 2 - .../ApiPlatformExtension.php | 3 - .../GraphQlExceptionFormatterPass.php | 47 --------- .../Bundle/Resources/config/graphql.xml | 33 +++---- src/GraphQl/Action/EntrypointAction.php | 48 ++++------ .../Action/GraphQlPlaygroundAction.php | 2 + src/GraphQl/Action/GraphiQlAction.php | 2 + .../Exception/ExceptionFormatterCallback.php | 56 ----------- .../ExceptionFormatterCallbackInterface.php | 29 ------ .../Exception/ExceptionFormatterFactory.php | 50 ---------- .../ExceptionFormatterFactoryInterface.php | 24 ----- .../Exception/ExceptionFormatterInterface.php | 39 -------- .../Factory/ItemMutationResolverFactory.php | 3 +- .../Resolver/Factory/ItemResolverFactory.php | 13 ++- src/GraphQl/Resolver/Stage/ReadStage.php | 13 ++- .../Stage/SecurityPostDenormalizeStage.php | 7 +- src/GraphQl/Resolver/Stage/SecurityStage.php | 7 +- src/GraphQl/Resolver/Stage/SerializeStage.php | 17 ++-- .../Serializer/Exception/ErrorNormalizer.php | 44 +++++++++ .../Exception/HttpExceptionNormalizer.php | 52 ++++++++++ .../Exception/RuntimeExceptionNormalizer.php | 49 ++++++++++ .../ValidationExceptionNormalizer.php} | 35 +++---- src/GraphQl/Serializer/ItemNormalizer.php | 2 +- src/GraphQl/Serializer/ObjectNormalizer.php | 2 +- src/GraphQl/Type/TypeBuilder.php | 4 +- .../Symfony/Bundle/ApiPlatformBundleTest.php | 2 - .../ApiPlatformExtensionTest.php | 23 ++--- .../GraphQlExceptionFormatterPassTest.php | 95 ------------------- .../ValidationExceptionFormatter.php | 71 -------------- tests/Fixtures/app/config/config_common.yml | 5 - tests/GraphQl/Action/EntrypointActionTest.php | 55 ++++++----- .../ExceptionFormatterCallbackTest.php | 93 ------------------ .../ExceptionFormatterFactoryTest.php | 43 --------- .../ValidationExceptionFormatterTest.php | 62 ------------ .../ItemMutationResolverFactoryTest.php | 3 +- .../Factory/ItemResolverFactoryTest.php | 7 +- .../GraphQl/Resolver/Stage/ReadStageTest.php | 6 +- .../SecurityPostDenormalizeStageTest.php | 4 +- .../Resolver/Stage/SecurityStageTest.php | 4 +- .../Resolver/Stage/SerializeStageTest.php | 9 +- .../Exception/ErrorNormalizerTest.php | 53 +++++++++++ .../Exception/HttpExceptionNormalizerTest.php | 70 ++++++++++++++ .../RuntimeExceptionNormalizerTest.php | 55 +++++++++++ .../ValidationExceptionNormalizerTest.php | 73 ++++++++++++++ 48 files changed, 548 insertions(+), 789 deletions(-) delete mode 100644 src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php delete mode 100644 src/GraphQl/Exception/ExceptionFormatterCallback.php delete mode 100644 src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php delete mode 100644 src/GraphQl/Exception/ExceptionFormatterFactory.php delete mode 100644 src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php delete mode 100644 src/GraphQl/Exception/ExceptionFormatterInterface.php create mode 100644 src/GraphQl/Serializer/Exception/ErrorNormalizer.php create mode 100644 src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php create mode 100644 src/GraphQl/Serializer/Exception/RuntimeExceptionNormalizer.php rename src/GraphQl/{Exception/Formatter/ValidationExceptionFormatter.php => Serializer/Exception/ValidationExceptionNormalizer.php} (60%) delete mode 100644 tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php delete mode 100644 tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php delete mode 100644 tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php delete mode 100644 tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php delete mode 100644 tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php create mode 100644 tests/GraphQl/Serializer/Exception/ErrorNormalizerTest.php create mode 100644 tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php create mode 100644 tests/GraphQl/Serializer/Exception/RuntimeExceptionNormalizerTest.php create mode 100644 tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dec65ca5f2..78f8c6c86bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 2.6.x-dev * MongoDB: Possibility to add execute options (aggregate command fields) for a resource, like `allowDiskUse` (#3144) +* GraphQL: Allow to format GraphQL errors based on exceptions (#3063) ## 2.5.1 @@ -43,7 +44,6 @@ * Test: Add assertions for testing response against JSON Schema from API resource * GraphQL: Add support for multipart request so user can create custom file upload mutations (#3041) * GraphQL: Add support for name converter (#2765) -* GraphQL: Allow to format GraphQL errors based on exceptions. ## 2.5.0 beta 1 diff --git a/features/graphql/authorization.feature b/features/graphql/authorization.feature index 7be5fb24499..4bce4a2bb25 100644 --- a/features/graphql/authorization.feature +++ b/features/graphql/authorization.feature @@ -18,6 +18,8 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." Scenario: An anonymous user tries to retrieve a secured collection @@ -38,6 +40,8 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." Scenario: An admin can retrieve a secured collection @@ -79,6 +83,8 @@ Feature: Authorization checking And the response should be in JSON And the header "Content-Type" should be equal to "application/json" And the JSON node "data.securedDummies" should be null + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." Scenario: An anonymous user tries to create a resource they are not allowed to @@ -96,6 +102,8 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Only admins can create a secured dummy." @createSchema @@ -151,6 +159,8 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." Scenario: A user can retrieve an item they owns @@ -186,6 +196,8 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." Scenario: A user can update an item they owns and transfer it diff --git a/features/graphql/introspection.feature b/features/graphql/introspection.feature index 0c093266985..4bffad9492b 100644 --- a/features/graphql/introspection.feature +++ b/features/graphql/introspection.feature @@ -3,9 +3,11 @@ Feature: GraphQL introspection support @createSchema Scenario: Execute an empty GraphQL query When I send a "GET" request to "/graphql" - Then the response status code should be 400 + Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].status" should be equal to 400 + And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "GraphQL query is not valid." Scenario: Introspect the GraphQL schema diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index 8b334e453c8..9a5b54d7b4a 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -675,12 +675,11 @@ Feature: GraphQL mutation support And the response should be in JSON And the header "Content-Type" should be equal to "application/json" And the JSON node "errors[0].status" should be equal to "400" - And the JSON node "errors[0].message" should be equal to "Validation Exception" + And the JSON node "errors[0].message" should be equal to "name: This value should not be blank." And the JSON node "errors[0].violations" should exist And the JSON node "errors[0].violations[0].path" should be equal to "name" And the JSON node "errors[0].violations[0].message" should be equal to "This value should not be blank." - Scenario: Execute a custom mutation Given there are 1 dummyCustomMutation objects When I send the following GraphQL request: diff --git a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php index e4bc570a80a..a80a8a40a48 100644 --- a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php +++ b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php @@ -17,7 +17,6 @@ use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\FilterPass; -use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlMutationResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlQueryResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlTypePass; @@ -49,7 +48,6 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new GraphQlTypePass()); $container->addCompilerPass(new GraphQlQueryResolverPass()); $container->addCompilerPass(new GraphQlMutationResolverPass()); - $container->addCompilerPass(new GraphQlExceptionFormatterPass()); $container->addCompilerPass(new MetadataAwareNameConverterPass()); } } diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 50182167640..fadf912ccc4 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -28,7 +28,6 @@ use ApiPlatform\Core\DataProvider\ItemDataProviderInterface; use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface; use ApiPlatform\Core\DataTransformer\DataTransformerInterface; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; use ApiPlatform\Core\GraphQl\Resolver\MutationResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryCollectionResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryItemResolverInterface; @@ -427,8 +426,6 @@ private function registerGraphQlConfiguration(ContainerBuilder $container, array ->addTag('api_platform.graphql.mutation_resolver'); $container->registerForAutoconfiguration(GraphQlTypeInterface::class) ->addTag('api_platform.graphql.type'); - $container->registerForAutoconfiguration(ExceptionFormatterInterface::class) - ->addTag('api_platform.graphql.exception_formatter'); } private function registerLegacyBundlesConfiguration(ContainerBuilder $container, array $config, XmlFileLoader $loader): void diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php deleted file mode 100644 index cc4d569141c..00000000000 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPass.php +++ /dev/null @@ -1,47 +0,0 @@ - - * - * 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\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler; - -use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Reference; - -/** - * Injects GraphQL Exception formatters. - * - * @internal - * - * @author Mahmood Bazdar - */ -class GraphQlExceptionFormatterPass implements CompilerPassInterface -{ - /** - * {@inheritdoc} - */ - public function process(ContainerBuilder $container) - { - if (!$container->getParameter('api_platform.graphql.enabled')) { - return; - } - - $formatters = []; - foreach ($container->findTaggedServiceIds('api_platform.graphql.exception_formatter', true) as $serviceId => $tags) { - foreach ($tags as $tag) { - $formatters[$tag['id'] ?? $serviceId] = new Reference($serviceId); - } - } - $container->getDefinition('api_platform.graphql.exception_formatter_locator')->addArgument($formatters); - $container->getDefinition('api_platform.graphql.exception_formatter_factory')->addArgument(array_keys($formatters)); - } -} diff --git a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml index 84ac0e1aa83..117b1ab8f87 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml @@ -159,22 +159,7 @@ - - - - - - - - - - - - - - - @@ -182,7 +167,7 @@ - + %kernel.debug% %api_platform.graphql.graphiql.enabled% %api_platform.graphql.graphql_playground.enabled% @@ -233,6 +218,22 @@ + + + + + + + + + + + + + + + + diff --git a/src/GraphQl/Action/EntrypointAction.php b/src/GraphQl/Action/EntrypointAction.php index f1a72dbb7cc..70785191e6a 100644 --- a/src/GraphQl/Action/EntrypointAction.php +++ b/src/GraphQl/Action/EntrypointAction.php @@ -13,21 +13,22 @@ namespace ApiPlatform\Core\GraphQl\Action; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallbackInterface; use ApiPlatform\Core\GraphQl\ExecutorInterface; use ApiPlatform\Core\GraphQl\Type\SchemaBuilderInterface; use GraphQL\Error\Debug; use GraphQL\Error\Error; -use GraphQL\Error\UserError; use GraphQL\Executor\ExecutionResult; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\Serializer\Normalizer\NormalizerInterface; /** * GraphQL API entrypoint. * + * @experimental + * * @author Alan Poulain */ final class EntrypointAction @@ -36,51 +37,49 @@ final class EntrypointAction private $executor; private $graphiQlAction; private $graphQlPlaygroundAction; + private $normalizer; private $debug; private $graphiqlEnabled; private $graphQlPlaygroundEnabled; private $defaultIde; - private $exceptionFormatterCallback; - public function __construct(SchemaBuilderInterface $schemaBuilder, ExecutorInterface $executor, GraphiQlAction $graphiQlAction, GraphQlPlaygroundAction $graphQlPlaygroundAction, ExceptionFormatterCallbackInterface $exceptionFormatterCallback, bool $debug = false, bool $graphiqlEnabled = false, bool $graphQlPlaygroundEnabled = false, $defaultIde = false) + public function __construct(SchemaBuilderInterface $schemaBuilder, ExecutorInterface $executor, GraphiQlAction $graphiQlAction, GraphQlPlaygroundAction $graphQlPlaygroundAction, NormalizerInterface $normalizer, bool $debug = false, bool $graphiqlEnabled = false, bool $graphQlPlaygroundEnabled = false, $defaultIde = false) { $this->schemaBuilder = $schemaBuilder; $this->executor = $executor; $this->graphiQlAction = $graphiQlAction; $this->graphQlPlaygroundAction = $graphQlPlaygroundAction; + $this->normalizer = $normalizer; $this->debug = $debug ? Debug::INCLUDE_DEBUG_MESSAGE | Debug::INCLUDE_TRACE : false; $this->graphiqlEnabled = $graphiqlEnabled; $this->graphQlPlaygroundEnabled = $graphQlPlaygroundEnabled; $this->defaultIde = $defaultIde; - $this->exceptionFormatterCallback = $exceptionFormatterCallback; } public function __invoke(Request $request): Response { - if ($request->isMethod('GET') && 'html' === $request->getRequestFormat()) { - if ('graphiql' === $this->defaultIde && $this->graphiqlEnabled) { - return ($this->graphiQlAction)($request); - } + try { + if ($request->isMethod('GET') && 'html' === $request->getRequestFormat()) { + if ('graphiql' === $this->defaultIde && $this->graphiqlEnabled) { + return ($this->graphiQlAction)($request); + } - if ('graphql-playground' === $this->defaultIde && $this->graphQlPlaygroundEnabled) { - return ($this->graphQlPlaygroundAction)($request); + if ('graphql-playground' === $this->defaultIde && $this->graphQlPlaygroundEnabled) { + return ($this->graphQlPlaygroundAction)($request); + } } - } - try { [$query, $operation, $variables] = $this->parseRequest($request); if (null === $query) { throw new BadRequestHttpException('GraphQL query is not valid.'); } - $executionResult = $this->executor->executeQuery($this->schemaBuilder->getSchema(), $query, null, null, $variables, $operation) - ->setErrorFormatter($this->exceptionFormatterCallback); - } catch (BadRequestHttpException $e) { - $exception = new UserError($e->getMessage(), 0, $e); - - return $this->buildExceptionResponse($exception, Response::HTTP_BAD_REQUEST); - } catch (\Exception $e) { - return $this->buildExceptionResponse($e, Response::HTTP_OK); + $executionResult = $this->executor + ->executeQuery($this->schemaBuilder->getSchema(), $query, null, null, $variables, $operation) + ->setErrorFormatter([$this->normalizer, 'normalize']); + } catch (\Exception $exception) { + $executionResult = (new ExecutionResult(null, [new Error($exception->getMessage(), null, null, null, null, $exception)])) + ->setErrorFormatter([$this->normalizer, 'normalize']); } return new JsonResponse($executionResult->toArray($this->debug)); @@ -211,11 +210,4 @@ private function decodeVariables(string $variables): array return $variables; } - - private function buildExceptionResponse(\Exception $e, int $statusCode): JsonResponse - { - $executionResult = new ExecutionResult(null, [new Error($e->getMessage(), null, null, null, null, $e)]); - - return new JsonResponse($executionResult->toArray($this->debug), $statusCode); - } } diff --git a/src/GraphQl/Action/GraphQlPlaygroundAction.php b/src/GraphQl/Action/GraphQlPlaygroundAction.php index 295064451f6..614b33ab49f 100644 --- a/src/GraphQl/Action/GraphQlPlaygroundAction.php +++ b/src/GraphQl/Action/GraphQlPlaygroundAction.php @@ -22,6 +22,8 @@ /** * GraphQL Playground entrypoint. * + * @experimental + * * @author Alan Poulain */ final class GraphQlPlaygroundAction diff --git a/src/GraphQl/Action/GraphiQlAction.php b/src/GraphQl/Action/GraphiQlAction.php index 13c24532fce..1a749cbba25 100644 --- a/src/GraphQl/Action/GraphiQlAction.php +++ b/src/GraphQl/Action/GraphiQlAction.php @@ -22,6 +22,8 @@ /** * GraphiQL entrypoint. * + * @experimental + * * @author Alan Poulain */ final class GraphiQlAction diff --git a/src/GraphQl/Exception/ExceptionFormatterCallback.php b/src/GraphQl/Exception/ExceptionFormatterCallback.php deleted file mode 100644 index 2a187e38be5..00000000000 --- a/src/GraphQl/Exception/ExceptionFormatterCallback.php +++ /dev/null @@ -1,56 +0,0 @@ - - * - * 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\Core\GraphQl\Exception; - -use GraphQL\Error\Error; -use GraphQL\Error\FormattedError; - -/** - * @expremintal - * - * @author Mahmood Bazdar - */ -class ExceptionFormatterCallback implements ExceptionFormatterCallbackInterface -{ - private $exceptionFormatterFactory; - - public function __construct(ExceptionFormatterFactoryInterface $exceptionFormatterFactory) - { - $this->exceptionFormatterFactory = $exceptionFormatterFactory; - } - - /** - * {@inheritdoc} - */ - public function __invoke(Error $error): array - { - $formatters = $this->exceptionFormatterFactory->getExceptionFormatters(); - usort($formatters, function (ExceptionFormatterInterface $a, ExceptionFormatterInterface $b) { - if ($a->getPriority() == $b->getPriority()) { - return 0; - } - - return ($a->getPriority() > $b->getPriority()) ? -1 : 1; - }); - /** @var ExceptionFormatterInterface $exceptionFormatter */ - foreach ($formatters as $exceptionFormatter) { - if (null !== $error->getPrevious() && $exceptionFormatter->supports($error->getPrevious())) { - return $exceptionFormatter->format($error); - } - } - - // falling back to default GraphQL error formatter - return FormattedError::createFromException($error); - } -} diff --git a/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php b/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php deleted file mode 100644 index 838f335a05a..00000000000 --- a/src/GraphQl/Exception/ExceptionFormatterCallbackInterface.php +++ /dev/null @@ -1,29 +0,0 @@ - - * - * 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\Core\GraphQl\Exception; - -use GraphQL\Error\Error; - -/** - * @expremintal - * - * @author Mahmood Bazdar - */ -interface ExceptionFormatterCallbackInterface -{ - /** - * Callback function will be used for formatting GraphQL errors. - */ - public function __invoke(Error $error): array; -} diff --git a/src/GraphQl/Exception/ExceptionFormatterFactory.php b/src/GraphQl/Exception/ExceptionFormatterFactory.php deleted file mode 100644 index dbb084c535d..00000000000 --- a/src/GraphQl/Exception/ExceptionFormatterFactory.php +++ /dev/null @@ -1,50 +0,0 @@ - - * - * 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\Core\GraphQl\Exception; - -use ApiPlatform\Core\GraphQl\Type\Definition\TypeInterface; -use Psr\Container\ContainerInterface; - -/** - * Get the registered services corresponding to GraphQL exception formatters. - * - * @author Mahmood Bazdar - */ -class ExceptionFormatterFactory implements ExceptionFormatterFactoryInterface -{ - private $exceptionFormatterLocator; - private $exceptionFormatterIds; - - /** - * @param string[] $exceptionFormatterIds - */ - public function __construct(ContainerInterface $exceptionFormatterLocator, array $exceptionFormatterIds) - { - $this->exceptionFormatterLocator = $exceptionFormatterLocator; - $this->exceptionFormatterIds = $exceptionFormatterIds; - } - - public function getExceptionFormatters(): array - { - $exceptionFormatters = []; - - foreach ($this->exceptionFormatterIds as $exceptionFormatterId) { - /** @var TypeInterface $exceptionFormatter */ - $exceptionFormatter = $this->exceptionFormatterLocator->get($exceptionFormatterId); - $exceptionFormatters[$exceptionFormatterId] = $exceptionFormatter; - } - - return $exceptionFormatters; - } -} diff --git a/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php b/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php deleted file mode 100644 index 4fe279238d6..00000000000 --- a/src/GraphQl/Exception/ExceptionFormatterFactoryInterface.php +++ /dev/null @@ -1,24 +0,0 @@ - - * - * 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\Core\GraphQl\Exception; - -/** - * Get Exception formatters. - * - * @author Mahmood Bazdar - */ -interface ExceptionFormatterFactoryInterface -{ - public function getExceptionFormatters(): array; -} diff --git a/src/GraphQl/Exception/ExceptionFormatterInterface.php b/src/GraphQl/Exception/ExceptionFormatterInterface.php deleted file mode 100644 index 6f80bdb3407..00000000000 --- a/src/GraphQl/Exception/ExceptionFormatterInterface.php +++ /dev/null @@ -1,39 +0,0 @@ - - * - * 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\Core\GraphQl\Exception; - -use GraphQL\Error\Error; - -/** - * @expremintal - * - * @author Mahmood Bazdar - */ -interface ExceptionFormatterInterface -{ - /** - * Formats the exception and returns the formatted array. - */ - public function format(Error $error): array; - - /** - * Check the exception, return true if you can format the exception. - */ - public function supports(\Throwable $exception): bool; - - /** - * Priority of your formatter in container. Higher number will be called sooner. - */ - public function getPriority(): int; -} diff --git a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php index fe5948b990a..5214bfa7232 100644 --- a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php +++ b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php @@ -24,7 +24,6 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Util\ClassInfoTrait; use ApiPlatform\Core\Util\CloneTrait; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use Psr\Container\ContainerInterface; @@ -106,7 +105,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul $mutationResolver = $this->mutationResolverLocator->get($mutationResolverId); $item = $mutationResolver($item, $resolverContext); if (null !== $item && $resourceClass !== $itemClass = $this->getObjectClass($item)) { - throw Error::createLocatedError(sprintf('Custom mutation resolver "%s" has to return an item of class %s but returned an item of class %s.', $mutationResolverId, $resourceMetadata->getShortName(), (new \ReflectionClass($itemClass))->getShortName()), $info->fieldNodes, $info->path); + throw new \LogicException(sprintf('Custom mutation resolver "%s" has to return an item of class %s but returned an item of class %s.', $mutationResolverId, $resourceMetadata->getShortName(), (new \ReflectionClass($itemClass))->getShortName())); } } diff --git a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php index ed00054e537..d7ab7387666 100644 --- a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php +++ b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php @@ -21,7 +21,6 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Util\ClassInfoTrait; use ApiPlatform\Core\Util\CloneTrait; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use Psr\Container\ContainerInterface; @@ -72,7 +71,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul throw new \LogicException('Item from read stage should be a nullable object.'); } - $resourceClass = $this->getResourceClass($item, $resourceClass, $info); + $resourceClass = $this->getResourceClass($item, $resourceClass); $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass); $queryResolverId = $resourceMetadata->getGraphqlAttribute($operationName, 'item_query'); @@ -80,7 +79,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul /** @var QueryItemResolverInterface $queryResolver */ $queryResolver = $this->queryResolverLocator->get($queryResolverId); $item = $queryResolver($item, $resolverContext); - $resourceClass = $this->getResourceClass($item, $resourceClass, $info, sprintf('Custom query resolver "%s"', $queryResolverId).' has to return an item of class %s but returned an item of class %s.'); + $resourceClass = $this->getResourceClass($item, $resourceClass, sprintf('Custom query resolver "%s"', $queryResolverId).' has to return an item of class %s but returned an item of class %s.'); } ($this->securityStage)($resourceClass, $operationName, $resolverContext + [ @@ -102,13 +101,13 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul /** * @param object|null $item * - * @throws Error + * @throws \UnexpectedValueException */ - private function getResourceClass($item, ?string $resourceClass, ResolveInfo $info, string $errorMessage = 'Resolver only handles items of class %s but retrieved item is of class %s.'): string + private function getResourceClass($item, ?string $resourceClass, string $errorMessage = 'Resolver only handles items of class %s but retrieved item is of class %s.'): string { if (null === $item) { if (null === $resourceClass) { - throw Error::createLocatedError('Resource class cannot be determined.', $info->fieldNodes, $info->path); + throw new \UnexpectedValueException('Resource class cannot be determined.'); } return $resourceClass; @@ -121,7 +120,7 @@ private function getResourceClass($item, ?string $resourceClass, ResolveInfo $in } if ($resourceClass !== $itemClass) { - throw Error::createLocatedError(sprintf($errorMessage, (new \ReflectionClass($resourceClass))->getShortName(), (new \ReflectionClass($itemClass))->getShortName()), $info->fieldNodes, $info->path); + throw new \UnexpectedValueException(sprintf($errorMessage, (new \ReflectionClass($resourceClass))->getShortName(), (new \ReflectionClass($itemClass))->getShortName())); } return $resourceClass; diff --git a/src/GraphQl/Resolver/Stage/ReadStage.php b/src/GraphQl/Resolver/Stage/ReadStage.php index bfe4bde7ac1..3f16806e16b 100644 --- a/src/GraphQl/Resolver/Stage/ReadStage.php +++ b/src/GraphQl/Resolver/Stage/ReadStage.php @@ -21,8 +21,8 @@ use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Util\ClassInfoTrait; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** * Read stage of GraphQL resolvers. @@ -63,9 +63,6 @@ public function __invoke(?string $resourceClass, ?string $rootClass, string $ope } $args = $context['args']; - /** @var ResolveInfo $info */ - $info = $context['info']; - $normalizationContext = $this->serializerContextBuilder->create($resourceClass, $operationName, $context, true); if (!$context['is_collection']) { @@ -74,11 +71,11 @@ public function __invoke(?string $resourceClass, ?string $rootClass, string $ope if ($identifier && $context['is_mutation']) { if (null === $item) { - throw Error::createLocatedError(sprintf('Item "%s" not found.', $args['input']['id']), $info->fieldNodes, $info->path); + throw new NotFoundHttpException(sprintf('Item "%s" not found.', $args['input']['id'])); } if ($resourceClass !== $this->getObjectClass($item)) { - throw Error::createLocatedError(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceMetadata->getShortName()), $info->fieldNodes, $info->path); + throw new \UnexpectedValueException(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceMetadata->getShortName())); } } @@ -92,11 +89,13 @@ public function __invoke(?string $resourceClass, ?string $rootClass, string $ope $normalizationContext['filters'] = $this->getNormalizedFilters($args); $source = $context['source']; + /** @var ResolveInfo $info */ + $info = $context['info']; if (isset($source[$rootProperty = $info->fieldName], $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY])) { $rootResolvedFields = $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY]; $subresourceCollection = $this->getSubresource($rootClass, $rootResolvedFields, $rootProperty, $resourceClass, $normalizationContext, $operationName); if (!is_iterable($subresourceCollection)) { - throw new \UnexpectedValueException('Expected subresource collection to be iterable'); + throw new \UnexpectedValueException('Expected subresource collection to be iterable.'); } return $subresourceCollection; diff --git a/src/GraphQl/Resolver/Stage/SecurityPostDenormalizeStage.php b/src/GraphQl/Resolver/Stage/SecurityPostDenormalizeStage.php index d6ab052d085..23c7b4cd86d 100644 --- a/src/GraphQl/Resolver/Stage/SecurityPostDenormalizeStage.php +++ b/src/GraphQl/Resolver/Stage/SecurityPostDenormalizeStage.php @@ -15,8 +15,7 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Security\ResourceAccessCheckerInterface; -use GraphQL\Error\Error; -use GraphQL\Type\Definition\ResolveInfo; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Security post denormalize stage of GraphQL resolvers. @@ -61,8 +60,6 @@ public function __invoke(string $resourceClass, string $operationName, array $co return; } - /** @var ResolveInfo $info */ - $info = $context['info']; - throw Error::createLocatedError($resourceMetadata->getGraphqlAttribute($operationName, 'security_post_denormalize_message', 'Access Denied.'), $info->fieldNodes, $info->path); + throw new AccessDeniedHttpException($resourceMetadata->getGraphqlAttribute($operationName, 'security_post_denormalize_message', 'Access Denied.')); } } diff --git a/src/GraphQl/Resolver/Stage/SecurityStage.php b/src/GraphQl/Resolver/Stage/SecurityStage.php index b3afa035618..6297c6eba4d 100644 --- a/src/GraphQl/Resolver/Stage/SecurityStage.php +++ b/src/GraphQl/Resolver/Stage/SecurityStage.php @@ -15,8 +15,7 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Security\ResourceAccessCheckerInterface; -use GraphQL\Error\Error; -use GraphQL\Type\Definition\ResolveInfo; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Security stage of GraphQL resolvers. @@ -53,8 +52,6 @@ public function __invoke(string $resourceClass, string $operationName, array $co return; } - /** @var ResolveInfo $info */ - $info = $context['info']; - throw Error::createLocatedError($resourceMetadata->getGraphqlAttribute($operationName, 'security_message', 'Access Denied.'), $info->fieldNodes, $info->path); + throw new AccessDeniedHttpException($resourceMetadata->getGraphqlAttribute($operationName, 'security_message', 'Access Denied.')); } } diff --git a/src/GraphQl/Resolver/Stage/SerializeStage.php b/src/GraphQl/Resolver/Stage/SerializeStage.php index 8c1d6feff07..18e72b51a8e 100644 --- a/src/GraphQl/Resolver/Stage/SerializeStage.php +++ b/src/GraphQl/Resolver/Stage/SerializeStage.php @@ -18,8 +18,6 @@ use ApiPlatform\Core\GraphQl\Serializer\ItemNormalizer; use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; -use GraphQL\Error\Error; -use GraphQL\Type\Definition\ResolveInfo; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; /** @@ -72,8 +70,6 @@ public function __invoke($itemOrCollection, string $resourceClass, string $opera $normalizationContext = $this->serializerContextBuilder->create($resourceClass, $operationName, $context, true); $args = $context['args']; - /** @var ResolveInfo $info */ - $info = $context['info']; $data = null; if (!$isCollection) { @@ -96,7 +92,7 @@ public function __invoke($itemOrCollection, string $resourceClass, string $opera } if (null !== $data && !\is_array($data)) { - throw Error::createLocatedError('Expected serialized data to be a nullable array.', $info->fieldNodes, $info->path); + throw new \UnexpectedValueException('Expected serialized data to be a nullable array.'); } if ($isMutation) { @@ -109,16 +105,15 @@ public function __invoke($itemOrCollection, string $resourceClass, string $opera } /** - * @throws Error + * @throws \LogicException + * @throws \UnexpectedValueException */ private function serializePaginatedCollection(iterable $collection, array $normalizationContext, array $context): array { $args = $context['args']; - /** @var ResolveInfo $info */ - $info = $context['info']; if (!($collection instanceof PaginatorInterface)) { - throw Error::createLocatedError(sprintf('Collection returned by the collection data provider must implement %s', PaginatorInterface::class), $info->fieldNodes, $info->path); + throw new \LogicException(sprintf('Collection returned by the collection data provider must implement %s.', PaginatorInterface::class)); } $offset = 0; @@ -127,14 +122,14 @@ private function serializePaginatedCollection(iterable $collection, array $norma if (isset($args['after'])) { $after = base64_decode($args['after'], true); if (false === $after) { - throw Error::createLocatedError(sprintf('Cursor %s is invalid', $args['after']), $info->fieldNodes, $info->path); + throw new \UnexpectedValueException(sprintf('Cursor %s is invalid.', $args['after'])); } $offset = 1 + (int) $after; } if (isset($args['before'])) { $before = base64_decode($args['before'], true); if (false === $before) { - throw Error::createLocatedError(sprintf('Cursor %s is invalid', $args['before']), $info->fieldNodes, $info->path); + throw new \UnexpectedValueException(sprintf('Cursor %s is invalid.', $args['before'])); } $offset = (int) $before - $nbPageItems; } diff --git a/src/GraphQl/Serializer/Exception/ErrorNormalizer.php b/src/GraphQl/Serializer/Exception/ErrorNormalizer.php new file mode 100644 index 00000000000..05b214f1c58 --- /dev/null +++ b/src/GraphQl/Serializer/Exception/ErrorNormalizer.php @@ -0,0 +1,44 @@ + + * + * 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\Core\GraphQl\Serializer\Exception; + +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; +use Symfony\Component\Serializer\Normalizer\NormalizerInterface; + +/** + * Normalize GraphQL error (fallback). + * + * @experimental + * + * @author Alan Poulain + */ +final class ErrorNormalizer implements NormalizerInterface +{ + /** + * {@inheritdoc} + */ + public function normalize($object, $format = null, array $context = []): array + { + return FormattedError::createFromException($object); + } + + /** + * {@inheritdoc} + */ + public function supportsNormalization($data, $format = null): bool + { + return $data instanceof Error; + } +} diff --git a/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php b/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php new file mode 100644 index 00000000000..c40927bf38c --- /dev/null +++ b/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php @@ -0,0 +1,52 @@ + + * + * 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\Core\GraphQl\Serializer\Exception; + +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\Serializer\Normalizer\NormalizerInterface; + +/** + * Normalize HTTP exceptions. + * + * @experimental + * + * @author Alan Poulain + */ +final class HttpExceptionNormalizer implements NormalizerInterface +{ + /** + * {@inheritdoc} + */ + public function normalize($object, $format = null, array $context = []): array + { + /** @var HttpException */ + $httpException = $object->getPrevious(); + $error = FormattedError::createFromException($object); + $error['message'] = $httpException->getMessage(); + $error['status'] = $statusCode = $httpException->getStatusCode(); + $error['extensions']['category'] = $statusCode < 500 ? 'user' : Error::CATEGORY_INTERNAL; + + return $error; + } + + /** + * {@inheritdoc} + */ + public function supportsNormalization($data, $format = null): bool + { + return $data instanceof Error && $data->getPrevious() instanceof HttpException; + } +} diff --git a/src/GraphQl/Serializer/Exception/RuntimeExceptionNormalizer.php b/src/GraphQl/Serializer/Exception/RuntimeExceptionNormalizer.php new file mode 100644 index 00000000000..61bc85fe7e8 --- /dev/null +++ b/src/GraphQl/Serializer/Exception/RuntimeExceptionNormalizer.php @@ -0,0 +1,49 @@ + + * + * 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\Core\GraphQl\Serializer\Exception; + +use GraphQL\Error\Error; +use GraphQL\Error\FormattedError; +use Symfony\Component\Serializer\Normalizer\NormalizerInterface; + +/** + * Normalize runtime exceptions to have the right message in production mode. + * + * @experimental + * + * @author Alan Poulain + */ +final class RuntimeExceptionNormalizer implements NormalizerInterface +{ + /** + * {@inheritdoc} + */ + public function normalize($object, $format = null, array $context = []): array + { + /** @var \RuntimeException */ + $runtimeException = $object->getPrevious(); + $error = FormattedError::createFromException($object); + $error['message'] = $runtimeException->getMessage(); + + return $error; + } + + /** + * {@inheritdoc} + */ + public function supportsNormalization($data, $format = null): bool + { + return $data instanceof Error && $data->getPrevious() instanceof \RuntimeException; + } +} diff --git a/src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php similarity index 60% rename from src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php rename to src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php index 846a05a887f..a281036d713 100644 --- a/src/GraphQl/Exception/Formatter/ValidationExceptionFormatter.php +++ b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php @@ -11,35 +11,36 @@ declare(strict_types=1); -namespace ApiPlatform\Core\GraphQl\Exception\Formatter; +namespace ApiPlatform\Core\GraphQl\Serializer\Exception; use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; use GraphQL\Error\Error; use GraphQL\Error\FormattedError; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Serializer\Normalizer\NormalizerInterface; use Symfony\Component\Validator\ConstraintViolation; /** - * Formats Validation exception. + * Normalize validation exceptions. + * + * @experimental * * @author Mahmood Bazdar + * @author Alan Poulain */ -class ValidationExceptionFormatter implements ExceptionFormatterInterface +final class ValidationExceptionNormalizer implements NormalizerInterface { /** * {@inheritdoc} */ - public function format(Error $error): array + public function normalize($object, $format = null, array $context = []): array { - /** - * @var ValidationException - */ - $validationException = $error->getPrevious(); - $error = FormattedError::createFromException($error); + /** @var ValidationException */ + $validationException = $object->getPrevious(); + $error = FormattedError::createFromException($object); $error['message'] = $validationException->getMessage(); $error['status'] = Response::HTTP_BAD_REQUEST; - $error['extensions']['category'] = Error::CATEGORY_GRAPHQL; + $error['extensions']['category'] = 'user'; $error['violations'] = []; /** @var ConstraintViolation $violation */ @@ -56,16 +57,8 @@ public function format(Error $error): array /** * {@inheritdoc} */ - public function supports(\Throwable $exception): bool - { - return $exception instanceof ValidationException; - } - - /** - * {@inheritdoc} - */ - public function getPriority(): int + public function supportsNormalization($data, $format = null): bool { - return -100; + return $data instanceof Error && $data->getPrevious() instanceof ValidationException; } } diff --git a/src/GraphQl/Serializer/ItemNormalizer.php b/src/GraphQl/Serializer/ItemNormalizer.php index 12fa55c1f23..42c5d5a9490 100644 --- a/src/GraphQl/Serializer/ItemNormalizer.php +++ b/src/GraphQl/Serializer/ItemNormalizer.php @@ -73,7 +73,7 @@ public function normalize($object, $format = null, array $context = []) $data = parent::normalize($object, $format, $context); if (!\is_array($data)) { - throw new UnexpectedValueException('Expected data to be an array'); + throw new UnexpectedValueException('Expected data to be an array.'); } $data[self::ITEM_RESOURCE_CLASS_KEY] = $this->getObjectClass($object); diff --git a/src/GraphQl/Serializer/ObjectNormalizer.php b/src/GraphQl/Serializer/ObjectNormalizer.php index 2fd5651fe95..56620d5e1d7 100644 --- a/src/GraphQl/Serializer/ObjectNormalizer.php +++ b/src/GraphQl/Serializer/ObjectNormalizer.php @@ -73,7 +73,7 @@ public function normalize($object, $format = null, array $context = []) $data = $this->decorated->normalize($object, $format, $context); if (!\is_array($data)) { - throw new UnexpectedValueException('Expected data to be an array'); + throw new UnexpectedValueException('Expected data to be an array.'); } if (!isset($originalResource)) { diff --git a/src/GraphQl/Type/TypeBuilder.php b/src/GraphQl/Type/TypeBuilder.php index d13beacb973..66b6b23fcc3 100644 --- a/src/GraphQl/Type/TypeBuilder.php +++ b/src/GraphQl/Type/TypeBuilder.php @@ -77,7 +77,7 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadata $ if ($this->typesContainer->has($shortName)) { $resourceObjectType = $this->typesContainer->get($shortName); if (!($resourceObjectType instanceof ObjectType || $resourceObjectType instanceof NonNull)) { - throw new \UnexpectedValueException(sprintf('Expected GraphQL type "%s" to be %s.', $shortName, implode('|', [ObjectType::class, NonNull::class]))); + throw new \LogicException(sprintf('Expected GraphQL type "%s" to be %s.', $shortName, implode('|', [ObjectType::class, NonNull::class]))); } return $resourceObjectType; @@ -137,7 +137,7 @@ public function getNodeInterface(): InterfaceType if ($this->typesContainer->has('Node')) { $nodeInterface = $this->typesContainer->get('Node'); if (!$nodeInterface instanceof InterfaceType) { - throw new \UnexpectedValueException(sprintf('Expected GraphQL type "Node" to be %s.', InterfaceType::class)); + throw new \LogicException(sprintf('Expected GraphQL type "Node" to be %s.', InterfaceType::class)); } return $nodeInterface; diff --git a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php index 2ee09ae6486..e186bbb60bd 100644 --- a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php +++ b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php @@ -18,7 +18,6 @@ use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\DataProviderPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\FilterPass; -use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlMutationResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlQueryResolverPass; use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlTypePass; @@ -43,7 +42,6 @@ public function testBuild() $containerProphecy->addCompilerPass(Argument::type(GraphQlTypePass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(GraphQlQueryResolverPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(GraphQlMutationResolverPass::class))->shouldBeCalled(); - $containerProphecy->addCompilerPass(Argument::type(GraphQlExceptionFormatterPass::class))->shouldBeCalled(); $containerProphecy->addCompilerPass(Argument::type(MetadataAwareNameConverterPass::class))->shouldBeCalled(); $bundle = new ApiPlatformBundle(); diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 01e030cc9d0..84e0867ce47 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -63,7 +63,6 @@ use ApiPlatform\Core\DataTransformer\DataTransformerInterface; use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; use ApiPlatform\Core\GraphQl\Resolver\MutationResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryCollectionResolverInterface; use ApiPlatform\Core\GraphQl\Resolver\QueryItemResolverInterface; @@ -368,10 +367,10 @@ public function testDisableGraphQl() $containerBuilderProphecy->setDefinition('api_platform.graphql.type_converter', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.query_resolver_locator', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.mutation_resolver_locator', Argument::type(Definition::class))->shouldNotBeCalled(); - $containerBuilderProphecy->setDefinition('api_platform.graphql.validation_exception_formatter', Argument::type(Definition::class))->shouldNotBeCalled(); - $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_callback', Argument::type(Definition::class))->shouldNotBeCalled(); - $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_locator', Argument::type(Definition::class))->shouldNotBeCalled(); - $containerBuilderProphecy->setDefinition('api_platform.graphql.exception_formatter_factory', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.normalizer.error', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.normalizer.validation_exception', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.normalizer.http_exception', Argument::type(Definition::class))->shouldNotBeCalled(); + $containerBuilderProphecy->setDefinition('api_platform.graphql.normalizer.runtime_exception', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setDefinition('api_platform.graphql.command.export_command', Argument::type(Definition::class))->shouldNotBeCalled(); $containerBuilderProphecy->setParameter('api_platform.graphql.enabled', true)->shouldNotBeCalled(); $containerBuilderProphecy->setParameter('api_platform.graphql.enabled', false)->shouldBeCalled(); @@ -391,8 +390,6 @@ public function testDisableGraphQl() $this->childDefinitionProphecy->addTag('api_platform.graphql.query_resolver')->shouldNotBeCalled(); $containerBuilderProphecy->registerForAutoconfiguration(MutationResolverInterface::class)->shouldNotBeCalled(); $this->childDefinitionProphecy->addTag('api_platform.graphql.mutation_resolver')->shouldNotBeCalled(); - $containerBuilderProphecy->registerForAutoconfiguration(ExceptionFormatterInterface::class)->shouldNotBeCalled(); - $this->childDefinitionProphecy->addTag('api_platform.graphql.exception_formatter')->shouldNotBeCalled(); $containerBuilderProphecy->setAlias(GraphQlSerializerContextBuilderInterface::class, 'api_platform.graphql.serializer.context_builder')->shouldNotBeCalled(); $containerBuilder = $containerBuilderProphecy->reveal(); @@ -1064,10 +1061,6 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); $this->childDefinitionProphecy->addTag('api_platform.graphql.type')->shouldBeCalledTimes(1); - $containerBuilderProphecy->registerForAutoconfiguration(ExceptionFormatterInterface::class) - ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); - $this->childDefinitionProphecy->addTag('api_platform.graphql.exception_formatter')->shouldBeCalledTimes(1); - $containerBuilderProphecy->registerForAutoconfiguration(QueryItemResolverInterface::class) ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1); $containerBuilderProphecy->registerForAutoconfiguration(QueryCollectionResolverInterface::class) @@ -1194,10 +1187,10 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo 'api_platform.graphql.resolver.stage.write', 'api_platform.graphql.resolver.stage.validate', 'api_platform.graphql.resolver.resource_field', - 'api_platform.graphql.exception_formatter_callback', - 'api_platform.graphql.validation_exception_formatter', - 'api_platform.graphql.exception_formatter_locator', - 'api_platform.graphql.exception_formatter_factory', + 'api_platform.graphql.normalizer.error', + 'api_platform.graphql.normalizer.validation_exception', + 'api_platform.graphql.normalizer.http_exception', + 'api_platform.graphql.normalizer.runtime_exception', 'api_platform.graphql.iterable_type', 'api_platform.graphql.upload_type', 'api_platform.graphql.type_locator', diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php deleted file mode 100644 index 5121309df55..00000000000 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlExceptionFormatterPassTest.php +++ /dev/null @@ -1,95 +0,0 @@ - - * - * 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\Core\Tests\Bridge\Symfony\Bundle\DependencyInjection\Compiler; - -use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\GraphQlExceptionFormatterPass; -use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Reference; - -/** - * @author Mahmood Bazdar - */ -class GraphQlExceptionFormatterPassTest extends TestCase -{ - public function testProcess() - { - $filterPass = new GraphQlExceptionFormatterPass(); - - $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); - - $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::that(function (array $arg) { - return !isset($arg['foo']) && isset($arg['my_id']) && $arg['my_id'] instanceof Reference; - }))->shouldBeCalled(); - - $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterFactoryDefinitionProphecy->addArgument(['my_id'])->shouldBeCalled(); - - $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); - $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(true)->shouldBeCalled(); - $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['id' => 'my_id']]])->shouldBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldBeCalled(); - - $filterPass->process($containerBuilderProphecy->reveal()); - } - - public function testIdNotExist() - { - $filterPass = new GraphQlExceptionFormatterPass(); - - $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); - - $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::that(function (array $arg) { - return !isset($arg['foo']) && isset($arg['bar']) && $arg['bar'] instanceof Reference; - }))->shouldBeCalled(); - - $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterFactoryDefinitionProphecy->addArgument(['bar'])->shouldBeCalled(); - - $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); - $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(true)->shouldBeCalled(); - $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['hi' => 'hello']]])->shouldBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldBeCalled(); - - $filterPass->process($containerBuilderProphecy->reveal()); - } - - public function testDisabled() - { - $filterPass = new GraphQlExceptionFormatterPass(); - - $this->assertInstanceOf(CompilerPassInterface::class, $filterPass); - - $exceptionFormatterLocatorDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterLocatorDefinitionProphecy->addArgument(Argument::any())->shouldNotBeCalled(); - - $exceptionFormatterFactoryDefinitionProphecy = $this->prophesize(Definition::class); - $exceptionFormatterFactoryDefinitionProphecy->addArgument(['my_id'])->shouldNotBeCalled(); - - $containerBuilderProphecy = $this->prophesize(ContainerBuilder::class); - $containerBuilderProphecy->getParameter('api_platform.graphql.enabled')->willReturn(false)->shouldBeCalled(); - $containerBuilderProphecy->findTaggedServiceIds('api_platform.graphql.exception_formatter', true)->willReturn(['foo' => [], 'bar' => [['id' => 'my_id']]])->shouldNotBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_locator')->willReturn($exceptionFormatterLocatorDefinitionProphecy->reveal())->shouldNotBeCalled(); - $containerBuilderProphecy->getDefinition('api_platform.graphql.exception_formatter_factory')->willReturn($exceptionFormatterFactoryDefinitionProphecy->reveal())->shouldNotBeCalled(); - - $filterPass->process($containerBuilderProphecy->reveal()); - } -} diff --git a/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php b/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php deleted file mode 100644 index 45c92d7b9a3..00000000000 --- a/tests/Fixtures/TestBundle/GraphQl/ExceptionFormatter/ValidationExceptionFormatter.php +++ /dev/null @@ -1,71 +0,0 @@ - - * - * 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\Core\Tests\Fixtures\TestBundle\GraphQl\ExceptionFormatter; - -use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; -use GraphQL\Error\Error; -use GraphQL\Error\FormattedError; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Validator\ConstraintViolation; - -/** - * Formats Validation exception. - * - * @author Mahmood Bazdar - */ -class ValidationExceptionFormatter implements ExceptionFormatterInterface -{ - /** - * {@inheritdoc} - */ - public function format(Error $error): array - { - /** - * @var ValidationException - */ - $validationException = $error->getPrevious(); - $error = FormattedError::createFromException($error); - $error['message'] = 'Validation Exception'; - $error['status'] = Response::HTTP_BAD_REQUEST; - $error['extensions']['category'] = Error::CATEGORY_GRAPHQL; - $error['violations'] = []; - - /** @var ConstraintViolation $violation */ - foreach ($validationException->getConstraintViolationList() as $violation) { - $error['violations'][] = [ - 'path' => $violation->getPropertyPath(), - 'message' => $violation->getMessage(), - ]; - } - - return $error; - } - - /** - * {@inheritdoc} - */ - public function supports(\Throwable $exception): bool - { - return $exception instanceof ValidationException; - } - - /** - * {@inheritdoc} - */ - public function getPriority(): int - { - return 1; - } -} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 603b2c9b733..e4c861cbb4a 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -321,8 +321,3 @@ services: decorates: 'api_platform.graphql.type_converter' arguments: ['@app.graphql.type_converter.inner'] public: false - - app.graphql.validation_exception_formatter: - class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\GraphQl\ExceptionFormatter\ValidationExceptionFormatter' - tags: - - { name: 'api_platform.graphql.exception_formatter' } \ No newline at end of file diff --git a/tests/GraphQl/Action/EntrypointActionTest.php b/tests/GraphQl/Action/EntrypointActionTest.php index c15aec9d7c1..02c48be80c2 100644 --- a/tests/GraphQl/Action/EntrypointActionTest.php +++ b/tests/GraphQl/Action/EntrypointActionTest.php @@ -16,8 +16,9 @@ use ApiPlatform\Core\GraphQl\Action\EntrypointAction; use ApiPlatform\Core\GraphQl\Action\GraphiQlAction; use ApiPlatform\Core\GraphQl\Action\GraphQlPlaygroundAction; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallbackInterface; use ApiPlatform\Core\GraphQl\ExecutorInterface; +use ApiPlatform\Core\GraphQl\Serializer\Exception\ErrorNormalizer; +use ApiPlatform\Core\GraphQl\Serializer\Exception\HttpExceptionNormalizer; use ApiPlatform\Core\GraphQl\Type\SchemaBuilderInterface; use GraphQL\Executor\ExecutionResult; use GraphQL\Type\Schema; @@ -28,6 +29,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\RouterInterface; +use Symfony\Component\Serializer\Serializer; use Twig\Environment as TwigEnvironment; /** @@ -38,7 +40,7 @@ class EntrypointActionTest extends TestCase /** * Hack to avoid transient failing test because of Date header. */ - private function assertEqualsWithoutDateHeader(JsonResponse $expected, Response $actual) + private function assertEqualsWithoutDateHeader(JsonResponse $expected, Response $actual): void { $expected->headers->remove('Date'); $actual->headers->remove('Date'); @@ -54,7 +56,7 @@ public function testGetHtmlAction(): void $this->assertInstanceOf(Response::class, $mockedEntrypoint($request)); } - public function testGetAction() + public function testGetAction(): void { $request = new Request(['query' => 'graphqlQuery', 'variables' => '["graphqlVariable"]', 'operation' => 'graphqlOperationName']); $request->setRequestFormat('json'); @@ -63,7 +65,7 @@ public function testGetAction() $this->assertEqualsWithoutDateHeader(new JsonResponse(['GraphQL']), $mockedEntrypoint($request)); } - public function testPostRawAction() + public function testPostRawAction(): void { $request = new Request(['variables' => '["graphqlVariable"]', 'operation' => 'graphqlOperationName'], [], [], [], [], [], 'graphqlQuery'); $request->setFormat('graphql', 'application/graphql'); @@ -74,7 +76,7 @@ public function testPostRawAction() $this->assertEqualsWithoutDateHeader(new JsonResponse(['GraphQL']), $mockedEntrypoint($request)); } - public function testPostJsonAction() + public function testPostJsonAction(): void { $request = new Request([], [], [], [], [], [], '{"query": "graphqlQuery", "variables": "[\"graphqlVariable\"]", "operation": "graphqlOperationName"}'); $request->setMethod('POST'); @@ -87,7 +89,7 @@ public function testPostJsonAction() /** * @dataProvider multipartRequestProvider */ - public function testMultipartRequestAction(?string $operations, ?string $map, array $files, array $variables, Response $expectedResponse) + public function testMultipartRequestAction(?string $operations, ?string $map, array $files, array $variables, Response $expectedResponse): void { $requestParams = []; if ($operations) { @@ -145,82 +147,82 @@ public function multipartRequestProvider(): array '{"file": ["variables.file"]}', ['file' => $file], ['file' => $file], - new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"},"status":400}]}'), ], 'upload without providing map' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', null, ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"},"status":400}]}'), ], 'upload with invalid json' => [ '{invalid}', '{"file": ["file"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL data is not valid JSON.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL data is not valid JSON.","extensions":{"category":"user"},"status":400}]}'), ], 'upload with invalid map JSON' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{invalid}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request map is not valid JSON.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request map is not valid JSON.","extensions":{"category":"user"},"status":400}]}'), ], 'upload with no file' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["file"]}', [], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request file has not been sent correctly.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request file has not been sent correctly.","extensions":{"category":"user"},"status":400}]}'), ], 'upload with wrong map' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["file"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request path in map is invalid.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request path in map is invalid.","extensions":{"category":"user"},"status":400}]}'), ], 'upload when variable path does not exist' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["variables.wrong"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request path in map does not match the variables.","extensions":{"category":"user"}}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request path in map does not match the variables.","extensions":{"category":"user"},"status":400}]}'), ], ]; } - public function testBadContentTypePostAction() + public function testBadContentTypePostAction(): void { $request = new Request(); $request->setMethod('POST'); $request->headers->set('Content-Type', 'application/xml'); $mockedEntrypoint = $this->getEntrypointAction(); - $this->assertEquals(400, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"}}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); + $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); } - public function testBadMethodAction() + public function testBadMethodAction(): void { $request = new Request(); $request->setMethod('PUT'); $mockedEntrypoint = $this->getEntrypointAction(); - $this->assertEquals(400, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"}}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); + $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); } - public function testBadVariablesAction() + public function testBadVariablesAction(): void { $request = new Request(['query' => 'graphqlQuery', 'variables' => 'graphqlVariable', 'operation' => 'graphqlOperationName']); $request->setRequestFormat('json'); $mockedEntrypoint = $this->getEntrypointAction(); - $this->assertEquals(400, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL variables are not valid JSON.","extensions":{"category":"user"}}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); + $this->assertEquals('{"errors":[{"message":"GraphQL variables are not valid JSON.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); } private function getEntrypointAction(array $variables = ['graphqlVariable']): EntrypointAction @@ -229,11 +231,14 @@ private function getEntrypointAction(array $variables = ['graphqlVariable']): En $schemaBuilderProphecy = $this->prophesize(SchemaBuilderInterface::class); $schemaBuilderProphecy->getSchema()->willReturn($schema->reveal()); - $exceptionFormatterCallback = $this->prophesize(ExceptionFormatterCallbackInterface::class)->reveal(); + $normalizer = new Serializer([ + new HttpExceptionNormalizer(), + new ErrorNormalizer(), + ]); $executionResultProphecy = $this->prophesize(ExecutionResult::class); $executionResultProphecy->toArray(false)->willReturn(['GraphQL']); - $executionResultProphecy->setErrorFormatter($exceptionFormatterCallback)->willReturn($executionResultProphecy); + $executionResultProphecy->setErrorFormatter([$normalizer, 'normalize'])->willReturn($executionResultProphecy); $executorProphecy = $this->prophesize(ExecutorInterface::class); $executorProphecy->executeQuery(Argument::is($schema->reveal()), 'graphqlQuery', null, null, $variables, 'graphqlOperationName')->willReturn($executionResultProphecy->reveal()); @@ -243,6 +248,6 @@ private function getEntrypointAction(array $variables = ['graphqlVariable']): En $graphiQlAction = new GraphiQlAction($twigProphecy->reveal(), $routerProphecy->reveal(), true); $graphQlPlaygroundAction = new GraphQlPlaygroundAction($twigProphecy->reveal(), $routerProphecy->reveal(), true); - return new EntrypointAction($schemaBuilderProphecy->reveal(), $executorProphecy->reveal(), $graphiQlAction, $graphQlPlaygroundAction, $exceptionFormatterCallback, false, true, true, 'graphiql'); + return new EntrypointAction($schemaBuilderProphecy->reveal(), $executorProphecy->reveal(), $graphiQlAction, $graphQlPlaygroundAction, $normalizer, false, true, true, 'graphiql'); } } diff --git a/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php b/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php deleted file mode 100644 index 49ea27f7763..00000000000 --- a/tests/GraphQl/Exception/ExceptionFormatterCallbackTest.php +++ /dev/null @@ -1,93 +0,0 @@ - - * - * 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\Core\Tests\GraphQl\Exception; - -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterCallback; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterFactory; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; -use GraphQL\Error\Error; -use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; - -/** - * @author Mahmood Bazdar - */ -class ExceptionFormatterCallbackTest extends TestCase -{ - /** - * @dataProvider formatExceptionProvider - */ - public function testFormatException(bool $supports, array $expect) - { - $errorProphecy = $this->prophesize(\Exception::class)->reveal(); - $error = new Error('Something', null, null, null, null, $errorProphecy); - - $exceptionFormatterProphecy = $this->prophesize(ExceptionFormatterInterface::class); - $exceptionFormatterProphecy->format($error)->willReturn(['error' => 'test']); - $exceptionFormatterProphecy->supports($error->getPrevious())->willReturn($supports); - $exceptionFormatterProphecy->getPriority()->willReturn(1); - $exceptionFormatter = $exceptionFormatterProphecy->reveal(); - - $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); - $exceptionFormatterLocatorProphecy->get('formatter')->willReturn($exceptionFormatter)->shouldBeCalled(); - $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['formatter']); - - $exceptionFormatterCallback = new ExceptionFormatterCallback($exceptionFormattersFactory); - $this->assertEquals($expect, $exceptionFormatterCallback($error)); - } - - public function formatExceptionProvider(): array - { - return [ - 'format supported exception' => [true, ['error' => 'test']], - 'falling back to default formatter for unsupported exceptions' => [ - false, - [ - 'message' => 'Internal server error', - 'extensions' => [ - 'category' => 'internal', - ], - ], - ], - ]; - } - - public function testFormatterPriority() - { - $errorProphecy = $this->prophesize(\Exception::class)->reveal(); - $error = new Error('Something', null, null, null, null, $errorProphecy); - - $exceptionFormatter1Prophecy = $this->prophesize(ExceptionFormatterInterface::class); - $exceptionFormatter1Prophecy->format($error)->willReturn(['error' => 1]); - $exceptionFormatter1Prophecy->supports($error->getPrevious())->willReturn(true); - $exceptionFormatter1Prophecy->getPriority()->willReturn(1); - $exceptionFormatter1 = $exceptionFormatter1Prophecy->reveal(); - - $exceptionFormatter2Prophecy = $this->prophesize(ExceptionFormatterInterface::class); - $exceptionFormatter2Prophecy->format($error)->willReturn(['error' => 2]); - $exceptionFormatter2Prophecy->supports($error->getPrevious())->willReturn(true); - $exceptionFormatter2Prophecy->getPriority()->willReturn(2); - $exceptionFormatter2 = $exceptionFormatter2Prophecy->reveal(); - - $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); - $exceptionFormatterLocatorProphecy->get('formatter1')->willReturn($exceptionFormatter1)->shouldBeCalled(); - $exceptionFormatterLocatorProphecy->get('formatter2')->willReturn($exceptionFormatter2)->shouldBeCalled(); - - $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['formatter1', 'formatter2']); - - $exceptionFormatterCallback = new ExceptionFormatterCallback($exceptionFormattersFactory); - - $this->assertEquals(['error' => 2], $exceptionFormatterCallback($error)); - } -} diff --git a/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php b/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php deleted file mode 100644 index 8d576d89567..00000000000 --- a/tests/GraphQl/Exception/ExceptionFormatterFactoryTest.php +++ /dev/null @@ -1,43 +0,0 @@ - - * - * 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\Core\Tests\GraphQl\Exception; - -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterFactory; -use ApiPlatform\Core\GraphQl\Exception\ExceptionFormatterInterface; -use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; - -/** - * @author Mahmood Bazdar - */ -class ExceptionFormatterFactoryTest extends TestCase -{ - public function testGetExceptionFormatters() - { - $exceptionFormatterProphecy = $this->prophesize(ExceptionFormatterInterface::class); - $exceptionFormatterProphecy->supports()->willReturn(true); - $exceptionFormatterProphecy->getPriority()->willReturn(1); - $exceptionFormatter = $exceptionFormatterProphecy->reveal(); - - $exceptionFormatterLocatorProphecy = $this->prophesize(ContainerInterface::class); - $exceptionFormatterLocatorProphecy->get('foo')->willReturn($exceptionFormatter)->shouldBeCalled(); - - $exceptionFormattersFactory = new ExceptionFormatterFactory($exceptionFormatterLocatorProphecy->reveal(), ['foo']); - - $formatters = $exceptionFormattersFactory->getExceptionFormatters(); - $this->assertArrayHasKey('foo', $formatters); - $this->assertInstanceOf(ExceptionFormatterInterface::class, $formatters['foo']); - $this->assertEquals(['foo' => $exceptionFormatter], $formatters); - } -} diff --git a/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php b/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php deleted file mode 100644 index aef54bdafcc..00000000000 --- a/tests/GraphQl/Exception/Formatter/ValidationExceptionFormatterTest.php +++ /dev/null @@ -1,62 +0,0 @@ - - * - * 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\Core\Tests\GraphQl\Exception\Formatter; - -use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; -use ApiPlatform\Core\GraphQl\Exception\Formatter\ValidationExceptionFormatter; -use GraphQL\Error\Error; -use PHPUnit\Framework\TestCase; -use Symfony\Component\Validator\ConstraintViolation; -use Symfony\Component\Validator\ConstraintViolationList; - -/** - * @author Mahmood Bazdar - */ -class ValidationExceptionFormatterTest extends TestCase -{ - public function testFormat() - { - $exception = new ValidationException(new ConstraintViolationList([ - new ConstraintViolation('message 1', '', [], '', 'Field 1', 'invalid'), - new ConstraintViolation('message 2', '', [], '', 'Field 2', 'invalid'), - ])); - $error = new Error('test message', null, null, null, null, $exception); - $formatter = new ValidationExceptionFormatter(); - - $formattedError = $formatter->format($error); - $this->assertArrayHasKey('violations', $formattedError); - $this->assertEquals(400, $formattedError['status']); - $this->assertEquals([ - [ - 'path' => 'Field 1', - 'message' => 'message 1', - ], - [ - 'path' => 'Field 2', - 'message' => 'message 2', - ], - ], $formattedError['violations']); - $this->assertEquals(Error::CATEGORY_GRAPHQL, $formattedError['extensions']['category']); - } - - public function testSupports() - { - $exception = new ValidationException(new ConstraintViolationList([ - new ConstraintViolation('message 1', '', [], '', 'Field 1', 'invalid'), - ])); - $formatter = new ValidationExceptionFormatter(); - - $this->assertTrue($formatter->supports($exception)); - } -} diff --git a/tests/GraphQl/Resolver/Factory/ItemMutationResolverFactoryTest.php b/tests/GraphQl/Resolver/Factory/ItemMutationResolverFactoryTest.php index 8f9f5ae178f..ce283a65e33 100644 --- a/tests/GraphQl/Resolver/Factory/ItemMutationResolverFactoryTest.php +++ b/tests/GraphQl/Resolver/Factory/ItemMutationResolverFactoryTest.php @@ -24,7 +24,6 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -321,7 +320,7 @@ public function testResolveCustomBadItem(): void return $customItem; }); - $this->expectException(Error::class); + $this->expectException(\LogicException::class); $this->expectExceptionMessage('Custom mutation resolver "query_resolver_id" has to return an item of class shortName but returned an item of class Dummy.'); ($this->itemMutationResolverFactory)($resourceClass, $rootClass, $operationName)($source, $args, null, $info); diff --git a/tests/GraphQl/Resolver/Factory/ItemResolverFactoryTest.php b/tests/GraphQl/Resolver/Factory/ItemResolverFactoryTest.php index 1093daf904d..bae61f2a5b2 100644 --- a/tests/GraphQl/Resolver/Factory/ItemResolverFactoryTest.php +++ b/tests/GraphQl/Resolver/Factory/ItemResolverFactoryTest.php @@ -21,7 +21,6 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; @@ -148,7 +147,7 @@ public function testResolveNoResourceNoItem(): void $readStageItem = null; $this->readStageProphecy->__invoke($resourceClass, $rootClass, $operationName, $resolverContext)->shouldBeCalled()->willReturn($readStageItem); - $this->expectException(Error::class); + $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage('Resource class cannot be determined.'); ($this->itemResolverFactory)($resourceClass, $rootClass, $operationName)($source, $args, null, $info); @@ -167,7 +166,7 @@ public function testResolveBadItem(): void $readStageItem = new \stdClass(); $this->readStageProphecy->__invoke($resourceClass, $rootClass, $operationName, $resolverContext)->shouldBeCalled()->willReturn($readStageItem); - $this->expectException(Error::class); + $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage('Resolver only handles items of class Dummy but retrieved item is of class stdClass.'); ($this->itemResolverFactory)($resourceClass, $rootClass, $operationName)($source, $args, null, $info); @@ -236,7 +235,7 @@ public function testResolveCustomBadItem(): void return $customItem; }); - $this->expectException(Error::class); + $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage('Custom query resolver "query_resolver_id" has to return an item of class stdClass but returned an item of class Dummy.'); ($this->itemResolverFactory)($resourceClass, $rootClass, $operationName)($source, $args, null, $info); diff --git a/tests/GraphQl/Resolver/Stage/ReadStageTest.php b/tests/GraphQl/Resolver/Stage/ReadStageTest.php index d63e39e2fbd..fce8e9e6c65 100644 --- a/tests/GraphQl/Resolver/Stage/ReadStageTest.php +++ b/tests/GraphQl/Resolver/Stage/ReadStageTest.php @@ -22,9 +22,9 @@ use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** * @author Alan Poulain @@ -175,8 +175,8 @@ public function itemMutationProvider(): array return [ 'no identifier' => ['myResource', null, $item, false, null], 'identifier' => ['stdClass', 'identifier', $item, false, $item], - 'identifier bad item' => ['myResource', 'identifier', $item, false, $item, Error::class, 'Item "identifier" did not match expected type "shortName".'], - 'identifier not found' => ['myResource', 'identifier_not_found', $item, true, null, Error::class, 'Item "identifier_not_found" not found.'], + 'identifier bad item' => ['myResource', 'identifier', $item, false, $item, \UnexpectedValueException::class, 'Item "identifier" did not match expected type "shortName".'], + 'identifier not found' => ['myResource', 'identifier_not_found', $item, true, null, NotFoundHttpException::class, 'Item "identifier_not_found" not found.'], ]; } diff --git a/tests/GraphQl/Resolver/Stage/SecurityPostDenormalizeStageTest.php b/tests/GraphQl/Resolver/Stage/SecurityPostDenormalizeStageTest.php index 301963c836a..1595e10e438 100644 --- a/tests/GraphQl/Resolver/Stage/SecurityPostDenormalizeStageTest.php +++ b/tests/GraphQl/Resolver/Stage/SecurityPostDenormalizeStageTest.php @@ -17,10 +17,10 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Security\ResourceAccessCheckerInterface; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * @author Alan Poulain @@ -107,7 +107,7 @@ public function testNotGranted(): void $info = $this->prophesize(ResolveInfo::class)->reveal(); - $this->expectException(Error::class); + $this->expectException(AccessDeniedHttpException::class); $this->expectExceptionMessage('Access Denied.'); ($this->securityPostDenormalizeStage)($resourceClass, 'item_query', [ diff --git a/tests/GraphQl/Resolver/Stage/SecurityStageTest.php b/tests/GraphQl/Resolver/Stage/SecurityStageTest.php index a49207679a2..e3d431a2c72 100644 --- a/tests/GraphQl/Resolver/Stage/SecurityStageTest.php +++ b/tests/GraphQl/Resolver/Stage/SecurityStageTest.php @@ -17,10 +17,10 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Security\ResourceAccessCheckerInterface; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * @author Alan Poulain @@ -88,7 +88,7 @@ public function testNotGranted(): void $info = $this->prophesize(ResolveInfo::class)->reveal(); - $this->expectException(Error::class); + $this->expectException(AccessDeniedHttpException::class); $this->expectExceptionMessage('Access Denied.'); ($this->securityStage)($resourceClass, 'item_query', [ diff --git a/tests/GraphQl/Resolver/Stage/SerializeStageTest.php b/tests/GraphQl/Resolver/Stage/SerializeStageTest.php index 640d15cfa90..0db5f9d5e9e 100644 --- a/tests/GraphQl/Resolver/Stage/SerializeStageTest.php +++ b/tests/GraphQl/Resolver/Stage/SerializeStageTest.php @@ -20,7 +20,6 @@ use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; -use GraphQL\Error\Error; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -140,13 +139,13 @@ public function testApplyCollectionWithPagination(iterable $collection, array $a public function applyCollectionWithPaginationProvider(): array { return [ - 'not paginator' => [[], [], null, Error::class, 'Collection returned by the collection data provider must implement ApiPlatform\Core\DataProvider\PaginatorInterface'], + 'not paginator' => [[], [], null, \LogicException::class, 'Collection returned by the collection data provider must implement ApiPlatform\Core\DataProvider\PaginatorInterface'], 'empty paginator' => [new ArrayPaginator([], 0, 0), [], ['totalCount' => 0., 'edges' => [], 'pageInfo' => ['startCursor' => null, 'endCursor' => null, 'hasNextPage' => false, 'hasPreviousPage' => false]]], 'paginator' => [new ArrayPaginator([new \stdClass(), new \stdClass(), new \stdClass()], 0, 2), [], ['totalCount' => 3., 'edges' => [['node' => ['normalized_item'], 'cursor' => 'MA=='], ['node' => ['normalized_item'], 'cursor' => 'MQ==']], 'pageInfo' => ['startCursor' => 'MA==', 'endCursor' => 'MQ==', 'hasNextPage' => true, 'hasPreviousPage' => false]]], 'paginator with after cursor' => [new ArrayPaginator([new \stdClass(), new \stdClass(), new \stdClass()], 1, 2), ['after' => 'MA=='], ['totalCount' => 3., 'edges' => [['node' => ['normalized_item'], 'cursor' => 'MQ=='], ['node' => ['normalized_item'], 'cursor' => 'Mg==']], 'pageInfo' => ['startCursor' => 'MQ==', 'endCursor' => 'Mg==', 'hasNextPage' => false, 'hasPreviousPage' => true]]], - 'paginator with bad after cursor' => [new ArrayPaginator([], 0, 0), ['after' => '-'], null, Error::class, 'Cursor - is invalid'], + 'paginator with bad after cursor' => [new ArrayPaginator([], 0, 0), ['after' => '-'], null, \UnexpectedValueException::class, 'Cursor - is invalid'], 'paginator with before cursor' => [new ArrayPaginator([new \stdClass(), new \stdClass(), new \stdClass()], 1, 1), ['before' => 'Mg=='], ['totalCount' => 3., 'edges' => [['node' => ['normalized_item'], 'cursor' => 'MQ==']], 'pageInfo' => ['startCursor' => 'MQ==', 'endCursor' => 'MQ==', 'hasNextPage' => false, 'hasPreviousPage' => true]]], - 'paginator with bad before cursor' => [new ArrayPaginator([], 0, 0), ['before' => '-'], null, Error::class, 'Cursor - is invalid'], + 'paginator with bad before cursor' => [new ArrayPaginator([], 0, 0), ['before' => '-'], null, \UnexpectedValueException::class, 'Cursor - is invalid'], 'paginator with last' => [new ArrayPaginator([new \stdClass(), new \stdClass(), new \stdClass()], 1, 2), ['last' => 2], ['totalCount' => 3., 'edges' => [['node' => ['normalized_item'], 'cursor' => 'MQ=='], ['node' => ['normalized_item'], 'cursor' => 'Mg==']], 'pageInfo' => ['startCursor' => 'MQ==', 'endCursor' => 'Mg==', 'hasNextPage' => false, 'hasPreviousPage' => true]]], ]; } @@ -163,7 +162,7 @@ public function testApplyBadNormalizedData(): void $this->normalizerProphecy->normalize(Argument::type('stdClass'), ItemNormalizer::FORMAT, $normalizationContext)->willReturn(new \stdClass()); - $this->expectException(Error::class); + $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage('Expected serialized data to be a nullable array.'); ($this->createSerializeStage(false))(new \stdClass(), $resourceClass, $operationName, $context); diff --git a/tests/GraphQl/Serializer/Exception/ErrorNormalizerTest.php b/tests/GraphQl/Serializer/Exception/ErrorNormalizerTest.php new file mode 100644 index 00000000000..ccceabc9e5a --- /dev/null +++ b/tests/GraphQl/Serializer/Exception/ErrorNormalizerTest.php @@ -0,0 +1,53 @@ + + * + * 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\Core\Tests\GraphQl\Serializer\Exception; + +use ApiPlatform\Core\GraphQl\Serializer\Exception\ErrorNormalizer; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; + +/** + * @author Alan Poulain + */ +class ErrorNormalizerTest extends TestCase +{ + private $errorNormalizer; + + /** + * {@inheritdoc} + */ + protected function setUp(): void + { + parent::setUp(); + + $this->errorNormalizer = new ErrorNormalizer(); + } + + public function testNormalize(): void + { + $errorMessage = 'test message'; + $error = new Error($errorMessage); + + $normalizedError = $this->errorNormalizer->normalize($error); + $this->assertSame($errorMessage, $normalizedError['message']); + $this->assertSame(Error::CATEGORY_GRAPHQL, $normalizedError['extensions']['category']); + } + + public function testSupportsNormalization(): void + { + $error = new Error('test message'); + + $this->assertTrue($this->errorNormalizer->supportsNormalization($error)); + } +} diff --git a/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php new file mode 100644 index 00000000000..1efcf32f7bb --- /dev/null +++ b/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php @@ -0,0 +1,70 @@ + + * + * 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\Core\Tests\GraphQl\Serializer\Exception; + +use ApiPlatform\Core\GraphQl\Serializer\Exception\HttpExceptionNormalizer; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; + +/** + * @author Alan Poulain + */ +class HttpExceptionNormalizerTest extends TestCase +{ + private $httpExceptionNormalizer; + + /** + * {@inheritdoc} + */ + protected function setUp(): void + { + parent::setUp(); + + $this->httpExceptionNormalizer = new HttpExceptionNormalizer(); + } + + /** + * @dataProvider exceptionProvider + */ + public function testNormalize(HttpException $exception, string $expectedExceptionMessage, int $expectedStatus, string $expectedCategory): void + { + $error = new Error('test message', null, null, null, null, $exception); + + $normalizedError = $this->httpExceptionNormalizer->normalize($error); + $this->assertSame($expectedExceptionMessage, $normalizedError['message']); + $this->assertSame($expectedStatus, $normalizedError['status']); + $this->assertSame($expectedCategory, $normalizedError['extensions']['category']); + } + + public function exceptionProvider(): array + { + $exceptionMessage = 'exception message'; + + return [ + 'client error' => [new BadRequestHttpException($exceptionMessage), $exceptionMessage, 400, 'user'], + 'server error' => [new ServiceUnavailableHttpException(null, $exceptionMessage), $exceptionMessage, 503, Error::CATEGORY_INTERNAL], + ]; + } + + public function testSupportsNormalization(): void + { + $exception = new BadRequestHttpException(); + $error = new Error('test message', null, null, null, null, $exception); + + $this->assertTrue($this->httpExceptionNormalizer->supportsNormalization($error)); + } +} diff --git a/tests/GraphQl/Serializer/Exception/RuntimeExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/RuntimeExceptionNormalizerTest.php new file mode 100644 index 00000000000..2050dc8f772 --- /dev/null +++ b/tests/GraphQl/Serializer/Exception/RuntimeExceptionNormalizerTest.php @@ -0,0 +1,55 @@ + + * + * 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\Core\Tests\GraphQl\Serializer\Exception; + +use ApiPlatform\Core\GraphQl\Serializer\Exception\RuntimeExceptionNormalizer; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; + +/** + * @author Alan Poulain + */ +class RuntimeExceptionNormalizerTest extends TestCase +{ + private $runtimeExceptionNormalizer; + + /** + * {@inheritdoc} + */ + protected function setUp(): void + { + parent::setUp(); + + $this->runtimeExceptionNormalizer = new RuntimeExceptionNormalizer(); + } + + public function testNormalize(): void + { + $exceptionMessage = 'exception message'; + $exception = new \RuntimeException($exceptionMessage); + $error = new Error('test message', null, null, null, null, $exception); + + $normalizedError = $this->runtimeExceptionNormalizer->normalize($error); + $this->assertSame($exceptionMessage, $normalizedError['message']); + $this->assertSame(Error::CATEGORY_INTERNAL, $normalizedError['extensions']['category']); + } + + public function testSupportsNormalization(): void + { + $exception = new \RuntimeException(); + $error = new Error('test message', null, null, null, null, $exception); + + $this->assertTrue($this->runtimeExceptionNormalizer->supportsNormalization($error)); + } +} diff --git a/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php new file mode 100644 index 00000000000..e3f02961639 --- /dev/null +++ b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php @@ -0,0 +1,73 @@ + + * + * 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\Core\Tests\GraphQl\Serializer\Exception; + +use ApiPlatform\Core\Bridge\Symfony\Validator\Exception\ValidationException; +use ApiPlatform\Core\GraphQl\Serializer\Exception\ValidationExceptionNormalizer; +use GraphQL\Error\Error; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationList; + +/** + * @author Mahmood Bazdar + */ +class ValidationExceptionNormalizerTest extends TestCase +{ + private $validationExceptionNormalizer; + + /** + * {@inheritdoc} + */ + protected function setUp(): void + { + parent::setUp(); + + $this->validationExceptionNormalizer = new ValidationExceptionNormalizer(); + } + + public function testNormalize(): void + { + $exceptionMessage = 'exception message'; + $exception = new ValidationException(new ConstraintViolationList([ + new ConstraintViolation('message 1', '', [], '', 'field 1', 'invalid'), + new ConstraintViolation('message 2', '', [], '', 'field 2', 'invalid'), + ]), $exceptionMessage); + $error = new Error('test message', null, null, null, null, $exception); + + $normalizedError = $this->validationExceptionNormalizer->normalize($error); + $this->assertSame($exceptionMessage, $normalizedError['message']); + $this->assertSame(400, $normalizedError['status']); + $this->assertSame('user', $normalizedError['extensions']['category']); + $this->assertArrayHasKey('violations', $normalizedError); + $this->assertSame([ + [ + 'path' => 'field 1', + 'message' => 'message 1', + ], + [ + 'path' => 'field 2', + 'message' => 'message 2', + ], + ], $normalizedError['violations']); + } + + public function testSupportsNormalization(): void + { + $exception = new ValidationException(new ConstraintViolationList([])); + $error = new Error('test message', null, null, null, null, $exception); + + $this->assertTrue($this->validationExceptionNormalizer->supportsNormalization($error)); + } +} From 2c6080b2c627e3eaccf4698d040caa5061d8870d Mon Sep 17 00:00:00 2001 From: mahmood bazdar Date: Sun, 27 Oct 2019 15:26:03 +0330 Subject: [PATCH 3/3] Changing error format to follow GraphQL spec --- features/graphql/authorization.feature | 12 +++++------ features/graphql/introspection.feature | 2 +- features/graphql/mutation.feature | 8 ++++---- .../Exception/HttpExceptionNormalizer.php | 2 +- .../ValidationExceptionNormalizer.php | 6 +++--- tests/GraphQl/Action/EntrypointActionTest.php | 20 +++++++++---------- .../Exception/HttpExceptionNormalizerTest.php | 2 +- .../ValidationExceptionNormalizerTest.php | 6 +++--- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/features/graphql/authorization.feature b/features/graphql/authorization.feature index 4bce4a2bb25..a4cc77c4ccd 100644 --- a/features/graphql/authorization.feature +++ b/features/graphql/authorization.feature @@ -18,7 +18,7 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." @@ -40,7 +40,7 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." @@ -83,7 +83,7 @@ Feature: Authorization checking And the response should be in JSON And the header "Content-Type" should be equal to "application/json" And the JSON node "data.securedDummies" should be null - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." @@ -102,7 +102,7 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Only admins can create a secured dummy." @@ -159,7 +159,7 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." @@ -196,7 +196,7 @@ Feature: Authorization checking Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 403 + And the JSON node "errors[0].extensions.status" should be equal to 403 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "Access Denied." diff --git a/features/graphql/introspection.feature b/features/graphql/introspection.feature index 4bffad9492b..62c34ae86e2 100644 --- a/features/graphql/introspection.feature +++ b/features/graphql/introspection.feature @@ -6,7 +6,7 @@ Feature: GraphQL introspection support Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to 400 + And the JSON node "errors[0].extensions.status" should be equal to 400 And the JSON node "errors[0].extensions.category" should be equal to user And the JSON node "errors[0].message" should be equal to "GraphQL query is not valid." diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index 9a5b54d7b4a..0baa2ce0b10 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -674,11 +674,11 @@ Feature: GraphQL mutation support Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].status" should be equal to "400" + And the JSON node "errors[0].extensions.status" should be equal to "400" And the JSON node "errors[0].message" should be equal to "name: This value should not be blank." - And the JSON node "errors[0].violations" should exist - And the JSON node "errors[0].violations[0].path" should be equal to "name" - And the JSON node "errors[0].violations[0].message" should be equal to "This value should not be blank." + And the JSON node "errors[0].extensions.violations" should exist + And the JSON node "errors[0].extensions.violations[0].path" should be equal to "name" + And the JSON node "errors[0].extensions.violations[0].message" should be equal to "This value should not be blank." Scenario: Execute a custom mutation Given there are 1 dummyCustomMutation objects diff --git a/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php b/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php index c40927bf38c..061cfb3199e 100644 --- a/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php +++ b/src/GraphQl/Serializer/Exception/HttpExceptionNormalizer.php @@ -36,7 +36,7 @@ public function normalize($object, $format = null, array $context = []): array $httpException = $object->getPrevious(); $error = FormattedError::createFromException($object); $error['message'] = $httpException->getMessage(); - $error['status'] = $statusCode = $httpException->getStatusCode(); + $error['extensions']['status'] = $statusCode = $httpException->getStatusCode(); $error['extensions']['category'] = $statusCode < 500 ? 'user' : Error::CATEGORY_INTERNAL; return $error; diff --git a/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php index a281036d713..d62986ef561 100644 --- a/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php +++ b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php @@ -39,13 +39,13 @@ public function normalize($object, $format = null, array $context = []): array $validationException = $object->getPrevious(); $error = FormattedError::createFromException($object); $error['message'] = $validationException->getMessage(); - $error['status'] = Response::HTTP_BAD_REQUEST; + $error['extensions']['status'] = Response::HTTP_BAD_REQUEST; $error['extensions']['category'] = 'user'; - $error['violations'] = []; + $error['extensions']['violations'] = []; /** @var ConstraintViolation $violation */ foreach ($validationException->getConstraintViolationList() as $violation) { - $error['violations'][] = [ + $error['extensions']['violations'][] = [ 'path' => $violation->getPropertyPath(), 'message' => $violation->getMessage(), ]; diff --git a/tests/GraphQl/Action/EntrypointActionTest.php b/tests/GraphQl/Action/EntrypointActionTest.php index 02c48be80c2..9da2d7edc7f 100644 --- a/tests/GraphQl/Action/EntrypointActionTest.php +++ b/tests/GraphQl/Action/EntrypointActionTest.php @@ -147,49 +147,49 @@ public function multipartRequestProvider(): array '{"file": ["variables.file"]}', ['file' => $file], ['file' => $file], - new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user","status":400}}]}'), ], 'upload without providing map' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', null, ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request does not respect the specification.","extensions":{"category":"user","status":400}}]}'), ], 'upload with invalid json' => [ '{invalid}', '{"file": ["file"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL data is not valid JSON.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL data is not valid JSON.","extensions":{"category":"user","status":400}}]}'), ], 'upload with invalid map JSON' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{invalid}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request map is not valid JSON.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request map is not valid JSON.","extensions":{"category":"user","status":400}}]}'), ], 'upload with no file' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["file"]}', [], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request file has not been sent correctly.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request file has not been sent correctly.","extensions":{"category":"user","status":400}}]}'), ], 'upload with wrong map' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["file"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request path in map is invalid.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request path in map is invalid.","extensions":{"category":"user","status":400}}]}'), ], 'upload when variable path does not exist' => [ '{"query": "graphqlQuery", "variables": {"file": null}, "operation": "graphqlOperationName"}', '{"file": ["variables.wrong"]}', ['file' => $file], ['file' => null], - new Response('{"errors":[{"message":"GraphQL multipart request path in map does not match the variables.","extensions":{"category":"user"},"status":400}]}'), + new Response('{"errors":[{"message":"GraphQL multipart request path in map does not match the variables.","extensions":{"category":"user","status":400}}]}'), ], ]; } @@ -202,7 +202,7 @@ public function testBadContentTypePostAction(): void $mockedEntrypoint = $this->getEntrypointAction(); $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user","status":400}}]}', $mockedEntrypoint($request)->getContent()); } public function testBadMethodAction(): void @@ -212,7 +212,7 @@ public function testBadMethodAction(): void $mockedEntrypoint = $this->getEntrypointAction(); $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals('{"errors":[{"message":"GraphQL query is not valid.","extensions":{"category":"user","status":400}}]}', $mockedEntrypoint($request)->getContent()); } public function testBadVariablesAction(): void @@ -222,7 +222,7 @@ public function testBadVariablesAction(): void $mockedEntrypoint = $this->getEntrypointAction(); $this->assertEquals(200, $mockedEntrypoint($request)->getStatusCode()); - $this->assertEquals('{"errors":[{"message":"GraphQL variables are not valid JSON.","extensions":{"category":"user"},"status":400}]}', $mockedEntrypoint($request)->getContent()); + $this->assertEquals('{"errors":[{"message":"GraphQL variables are not valid JSON.","extensions":{"category":"user","status":400}}]}', $mockedEntrypoint($request)->getContent()); } private function getEntrypointAction(array $variables = ['graphqlVariable']): EntrypointAction diff --git a/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php index 1efcf32f7bb..fadf5b421a7 100644 --- a/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php +++ b/tests/GraphQl/Serializer/Exception/HttpExceptionNormalizerTest.php @@ -46,7 +46,7 @@ public function testNormalize(HttpException $exception, string $expectedExceptio $normalizedError = $this->httpExceptionNormalizer->normalize($error); $this->assertSame($expectedExceptionMessage, $normalizedError['message']); - $this->assertSame($expectedStatus, $normalizedError['status']); + $this->assertSame($expectedStatus, $normalizedError['extensions']['status']); $this->assertSame($expectedCategory, $normalizedError['extensions']['category']); } diff --git a/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php index e3f02961639..37d10665279 100644 --- a/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php +++ b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php @@ -48,9 +48,9 @@ public function testNormalize(): void $normalizedError = $this->validationExceptionNormalizer->normalize($error); $this->assertSame($exceptionMessage, $normalizedError['message']); - $this->assertSame(400, $normalizedError['status']); + $this->assertSame(400, $normalizedError['extensions']['status']); $this->assertSame('user', $normalizedError['extensions']['category']); - $this->assertArrayHasKey('violations', $normalizedError); + $this->assertArrayHasKey('violations', $normalizedError['extensions']); $this->assertSame([ [ 'path' => 'field 1', @@ -60,7 +60,7 @@ public function testNormalize(): void 'path' => 'field 2', 'message' => 'message 2', ], - ], $normalizedError['violations']); + ], $normalizedError['extensions']['violations']); } public function testSupportsNormalization(): void