From 4c3787d09f3dc7ad38d23ff658feac2e8845b962 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 11 Feb 2018 14:33:04 +0100 Subject: [PATCH 1/3] Introduce extensible schema --- Definition/Builder/SchemaBuilder.php | 21 +++--- Definition/Type/ExtensibleSchema.php | 47 ++++++++++++ .../DecoratorExtension.php} | 71 ++++++++++--------- .../SchemaExtensionInterface.php | 10 +++ .../SchemaExtension/ValidatorExtension.php | 13 ++++ .../OverblogGraphQLExtension.php | 2 +- Event/ExecutorArgumentsEvent.php | 10 +-- Request/Executor.php | 2 + Resources/config/services.yml | 5 -- .../DecoratorExtensionTest.php} | 8 +-- Tests/Request/ExecutorTest.php | 4 +- 11 files changed, 132 insertions(+), 61 deletions(-) create mode 100644 Definition/Type/ExtensibleSchema.php rename Definition/Type/{SchemaDecorator.php => SchemaExtension/DecoratorExtension.php} (64%) create mode 100644 Definition/Type/SchemaExtension/SchemaExtensionInterface.php create mode 100644 Definition/Type/SchemaExtension/ValidatorExtension.php rename Tests/Definition/Type/{SchemaDecoratorTest.php => SchemaExtension/DecoratorExtensionTest.php} (97%) diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index 96e959008..30be3d270 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -3,8 +3,9 @@ namespace Overblog\GraphQLBundle\Definition\Builder; use GraphQL\Type\Definition\Type; -use GraphQL\Type\Schema; -use Overblog\GraphQLBundle\Definition\Type\SchemaDecorator; +use Overblog\GraphQLBundle\Definition\Type\ExtensibleSchema; +use Overblog\GraphQLBundle\Definition\Type\SchemaExtension\DecoratorExtension; +use Overblog\GraphQLBundle\Definition\Type\SchemaExtension\ValidatorExtension; use Overblog\GraphQLBundle\Resolver\ResolverMapInterface; use Overblog\GraphQLBundle\Resolver\ResolverMaps; use Overblog\GraphQLBundle\Resolver\TypeResolver; @@ -14,16 +15,12 @@ class SchemaBuilder /** @var TypeResolver */ private $typeResolver; - /** @var SchemaDecorator */ - private $decorator; - /** @var bool */ private $enableValidation; - public function __construct(TypeResolver $typeResolver, SchemaDecorator $decorator, $enableValidation = false) + public function __construct(TypeResolver $typeResolver, $enableValidation = false) { $this->typeResolver = $typeResolver; - $this->decorator = $decorator; $this->enableValidation = $enableValidation; } @@ -34,7 +31,7 @@ public function __construct(TypeResolver $typeResolver, SchemaDecorator $decorat * @param ResolverMapInterface[] $resolverMaps * @param string[] $types * - * @return Schema + * @return ExtensibleSchema */ public function create($queryAlias = null, $mutationAlias = null, $subscriptionAlias = null, array $resolverMaps = [], array $types = []) { @@ -42,13 +39,13 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA $mutation = $this->typeResolver->resolve($mutationAlias); $subscription = $this->typeResolver->resolve($subscriptionAlias); - $schema = new Schema($this->buildSchemaArguments($query, $mutation, $subscription, $types)); - reset($resolverMaps); - $this->decorator->decorate($schema, 1 === count($resolverMaps) ? current($resolverMaps) : new ResolverMaps($resolverMaps)); + $schema = new ExtensibleSchema($this->buildSchemaArguments($query, $mutation, $subscription, $types)); + $extensions = [new DecoratorExtension(1 === count($resolverMaps) ? current($resolverMaps) : new ResolverMaps($resolverMaps))]; if ($this->enableValidation) { - $schema->assertValid(); + $extensions[] = new ValidatorExtension(); } + $schema->setExtensions($extensions); return $schema; } diff --git a/Definition/Type/ExtensibleSchema.php b/Definition/Type/ExtensibleSchema.php new file mode 100644 index 000000000..1dc4169c2 --- /dev/null +++ b/Definition/Type/ExtensibleSchema.php @@ -0,0 +1,47 @@ +extensions = []; + foreach ($extensions as $extension) { + $this->addExtension($extension); + } + + return $this; + } + + /** + * @param SchemaExtensionInterface $extension + */ + public function addExtension(SchemaExtensionInterface $extension) + { + $this->extensions[] = $extension; + } + + /** + * @return $this + */ + public function processExtensions() + { + foreach ($this->extensions as $extension) { + $extension->process($this); + } + + return $this; + } +} diff --git a/Definition/Type/SchemaDecorator.php b/Definition/Type/SchemaExtension/DecoratorExtension.php similarity index 64% rename from Definition/Type/SchemaDecorator.php rename to Definition/Type/SchemaExtension/DecoratorExtension.php index b8fa4ee99..6bdd98f2e 100644 --- a/Definition/Type/SchemaDecorator.php +++ b/Definition/Type/SchemaExtension/DecoratorExtension.php @@ -1,7 +1,8 @@ resolverMap = $resolverMap; + } + + public function process(Schema $schema) { - foreach ($resolverMap->covered() as $typeName) { + foreach ($this->resolverMap->covered() as $typeName) { $type = $schema->getType($typeName); if ($type instanceof ObjectType) { - $this->decorateObjectType($type, $resolverMap); + $this->decorateObjectType($type); } elseif ($type instanceof InterfaceType || $type instanceof UnionType) { - $this->decorateInterfaceOrUnionType($type, $resolverMap); + $this->decorateInterfaceOrUnionType($type); } elseif ($type instanceof EnumType) { - $this->decorateEnumType($type, $resolverMap); + $this->decorateEnumType($type); } elseif ($type instanceof CustomScalarType) { - $this->decorateCustomScalarType($type, $resolverMap); + $this->decorateCustomScalarType($type); } else { - $covered = $resolverMap->covered($type->name); + $covered = $this->resolverMap->covered($type->name); if (!empty($covered)) { throw new \InvalidArgumentException( sprintf( @@ -41,31 +49,30 @@ public function decorate(Schema $schema, ResolverMapInterface $resolverMap) } } - private function decorateObjectType(ObjectType $type, ResolverMapInterface $resolverMap) + private function decorateObjectType(ObjectType $type) { $fieldsResolved = []; - foreach ($resolverMap->covered($type->name) as $fieldName) { + foreach ($this->resolverMap->covered($type->name) as $fieldName) { if (ResolverMapInterface::IS_TYPEOF === $fieldName) { - $this->configTypeMapping($type, $resolverMap, ResolverMapInterface::IS_TYPEOF); + $this->configTypeMapping($type, ResolverMapInterface::IS_TYPEOF); } elseif (ResolverMapInterface::RESOLVE_FIELD === $fieldName) { - $resolveFieldFn = Resolver::wrapArgs($resolverMap->resolve($type->name, ResolverMapInterface::RESOLVE_FIELD)); + $resolveFieldFn = Resolver::wrapArgs($this->resolverMap->resolve($type->name, ResolverMapInterface::RESOLVE_FIELD)); $type->config[substr(ResolverMapInterface::RESOLVE_FIELD, 2)] = $resolveFieldFn; $type->resolveFieldFn = $resolveFieldFn; } else { $fieldsResolved[] = $fieldName; } } - $this->decorateObjectTypeFields($type, $resolverMap, $fieldsResolved); + $this->decorateObjectTypeFields($type, $fieldsResolved); } /** * @param InterfaceType|UnionType $type - * @param ResolverMapInterface $resolverMap */ - private function decorateInterfaceOrUnionType($type, ResolverMapInterface $resolverMap) + private function decorateInterfaceOrUnionType($type) { - $this->configTypeMapping($type, $resolverMap, ResolverMapInterface::RESOLVE_TYPE); - $covered = $resolverMap->covered($type->name); + $this->configTypeMapping($type, ResolverMapInterface::RESOLVE_TYPE); + $covered = $this->resolverMap->covered($type->name); if (!empty($covered)) { $unknownFields = array_diff($covered, [ResolverMapInterface::RESOLVE_TYPE]); if (!empty($unknownFields)) { @@ -81,7 +88,7 @@ private function decorateInterfaceOrUnionType($type, ResolverMapInterface $resol } } - private function decorateCustomScalarType(CustomScalarType $type, ResolverMapInterface $resolverMap) + private function decorateCustomScalarType(CustomScalarType $type) { static $allowedFields = [ ResolverMapInterface::SCALAR_TYPE, @@ -91,10 +98,10 @@ private function decorateCustomScalarType(CustomScalarType $type, ResolverMapInt ]; foreach ($allowedFields as $fieldName) { - $this->configTypeMapping($type, $resolverMap, $fieldName); + $this->configTypeMapping($type, $fieldName); } - $unknownFields = array_diff($resolverMap->covered($type->name), $allowedFields); + $unknownFields = array_diff($this->resolverMap->covered($type->name), $allowedFields); if (!empty($unknownFields)) { throw new \InvalidArgumentException( sprintf( @@ -107,17 +114,17 @@ private function decorateCustomScalarType(CustomScalarType $type, ResolverMapInt } } - private function decorateEnumType(EnumType $type, ResolverMapInterface $resolverMaps) + private function decorateEnumType(EnumType $type) { $fieldNames = []; foreach ($type->config['values'] as $key => &$value) { $fieldName = isset($value['name']) ? $value['name'] : $key; - if ($resolverMaps->isResolvable($type->name, $fieldName)) { - $value['value'] = $resolverMaps->resolve($type->name, $fieldName); + if ($this->resolverMap->isResolvable($type->name, $fieldName)) { + $value['value'] = $this->resolverMap->resolve($type->name, $fieldName); } $fieldNames[] = $fieldName; } - $unknownFields = array_diff($resolverMaps->covered($type->name), $fieldNames); + $unknownFields = array_diff($this->resolverMap->covered($type->name), $fieldNames); if (!empty($unknownFields)) { throw new \InvalidArgumentException( sprintf( @@ -129,11 +136,11 @@ private function decorateEnumType(EnumType $type, ResolverMapInterface $resolver } } - private function decorateObjectTypeFields(ObjectType $type, ResolverMapInterface $resolverMap, array $fieldsResolved) + private function decorateObjectTypeFields(ObjectType $type, array $fieldsResolved) { $fields = $type->config['fields']; - $decoratedFields = function () use ($fields, $type, $resolverMap, $fieldsResolved) { + $decoratedFields = function () use ($fields, $type, $fieldsResolved) { if (is_callable($fields)) { $fields = $fields(); } @@ -142,8 +149,8 @@ private function decorateObjectTypeFields(ObjectType $type, ResolverMapInterface foreach ($fields as $key => &$field) { $fieldName = isset($field['name']) ? $field['name'] : $key; - if ($resolverMap->isResolvable($type->name, $fieldName)) { - $field['resolve'] = Resolver::wrapArgs($resolverMap->resolve($type->name, $fieldName)); + if ($this->resolverMap->isResolvable($type->name, $fieldName)) { + $field['resolve'] = Resolver::wrapArgs($this->resolverMap->resolve($type->name, $fieldName)); } $fieldNames[] = $fieldName; @@ -162,10 +169,10 @@ private function decorateObjectTypeFields(ObjectType $type, ResolverMapInterface $type->config['fields'] = is_callable($fields) ? $decoratedFields : $decoratedFields(); } - private function configTypeMapping(Type $type, ResolverMapInterface $resolverMap, $fieldName) + private function configTypeMapping(Type $type, $fieldName) { - if ($resolverMap->isResolvable($type->name, $fieldName)) { - $type->config[substr($fieldName, 2)] = $resolverMap->resolve($type->name, $fieldName); + if ($this->resolverMap->isResolvable($type->name, $fieldName)) { + $type->config[substr($fieldName, 2)] = $this->resolverMap->resolve($type->name, $fieldName); } } } diff --git a/Definition/Type/SchemaExtension/SchemaExtensionInterface.php b/Definition/Type/SchemaExtension/SchemaExtensionInterface.php new file mode 100644 index 000000000..01f6692d6 --- /dev/null +++ b/Definition/Type/SchemaExtension/SchemaExtensionInterface.php @@ -0,0 +1,10 @@ +assertValid(); + } +} diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 8e968693f..f1423ce44 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -216,7 +216,7 @@ private function setErrorHandler(array $config, ContainerBuilder $container) private function setSchemaBuilderArguments(array $config, ContainerBuilder $container) { $container->getDefinition($this->getAlias().'.schema_builder') - ->replaceArgument(2, $config['definitions']['config_validation']); + ->replaceArgument(1, $config['definitions']['config_validation']); } private function setSchemaArguments(array $config, ContainerBuilder $container) diff --git a/Event/ExecutorArgumentsEvent.php b/Event/ExecutorArgumentsEvent.php index dcc9ee4db..d8d2058bf 100644 --- a/Event/ExecutorArgumentsEvent.php +++ b/Event/ExecutorArgumentsEvent.php @@ -2,12 +2,12 @@ namespace Overblog\GraphQLBundle\Event; -use GraphQL\Type\Schema; +use Overblog\GraphQLBundle\Definition\Type\ExtensibleSchema; use Symfony\Component\EventDispatcher\Event; final class ExecutorArgumentsEvent extends Event { - /** @var Schema */ + /** @var ExtensibleSchema */ private $schema; /** @var string */ @@ -26,7 +26,7 @@ final class ExecutorArgumentsEvent extends Event private $operationName; public static function create( - Schema $schema, + ExtensibleSchema $schema, $requestString, \ArrayObject $contextValue, $rootValue = null, @@ -78,13 +78,13 @@ public function setVariableValue(array $variableValue = null) $this->variableValue = $variableValue; } - public function setSchema(Schema $schema) + public function setSchema(ExtensibleSchema $schema) { $this->schema = $schema; } /** - * @return Schema + * @return ExtensibleSchema */ public function getSchema() { diff --git a/Request/Executor.php b/Request/Executor.php index fd88db1f2..00062b8b2 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -131,6 +131,8 @@ public function execute($schemaName, array $request, $rootValue = null) isset($request[ParserInterface::PARAM_OPERATION_NAME]) ? $request[ParserInterface::PARAM_OPERATION_NAME] : null ); + $executorArgumentsEvent->getSchema()->processExtensions(); + $result = $this->executor->execute( $executorArgumentsEvent->getSchema(), $executorArgumentsEvent->getRequestString(), diff --git a/Resources/config/services.yml b/Resources/config/services.yml index a17535dac..9b480eed1 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -33,13 +33,8 @@ services: public: false arguments: - "@overblog_graphql.type_resolver" - - '@Overblog\GraphQLBundle\Definition\Type\SchemaDecorator' - false - Overblog\GraphQLBundle\Definition\Type\SchemaDecorator: - class: Overblog\GraphQLBundle\Definition\Type\SchemaDecorator - public: false - overblog_graphql.type_resolver: class: Overblog\GraphQLBundle\Resolver\TypeResolver public: true diff --git a/Tests/Definition/Type/SchemaDecoratorTest.php b/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php similarity index 97% rename from Tests/Definition/Type/SchemaDecoratorTest.php rename to Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php index 77c24b406..453629c95 100644 --- a/Tests/Definition/Type/SchemaDecoratorTest.php +++ b/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php @@ -1,6 +1,6 @@ decorate($this->createSchemaMock($types), $this->createResolverMapMock($map)); + (new DecoratorExtension($this->createResolverMapMock($map)))->process($this->createSchemaMock($types)); } /** diff --git a/Tests/Request/ExecutorTest.php b/Tests/Request/ExecutorTest.php index 42d719be1..751a2172e 100644 --- a/Tests/Request/ExecutorTest.php +++ b/Tests/Request/ExecutorTest.php @@ -6,7 +6,7 @@ use GraphQL\Executor\Promise\PromiseAdapter; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; -use GraphQL\Type\Schema; +use Overblog\GraphQLBundle\Definition\Type\ExtensibleSchema; use Overblog\GraphQLBundle\Executor\Executor; use Overblog\GraphQLBundle\Request\Executor as RequestExecutor; use PHPUnit\Framework\TestCase; @@ -39,7 +39,7 @@ public function setUp() ], ], ]); - $this->executor->addSchema('global', new Schema(['query' => $queryType])); + $this->executor->addSchema('global', new ExtensibleSchema(['query' => $queryType])); } /** From b53e5748a2ce62d5fa6764fd2dd44b2f46b2a59a Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 12 Feb 2018 07:35:02 +0100 Subject: [PATCH 2/3] Replace ResolverMapInterface special field prefix by `%%` Move from `__` to `%%` prefix because this could conflict with object fields as is valid names. --- .../Type/SchemaExtension/DecoratorExtension.php | 9 +++++---- Resolver/ResolverMapInterface.php | 14 +++++++------- .../SchemaExtension/DecoratorExtensionTest.php | 8 ++++---- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Definition/Type/SchemaExtension/DecoratorExtension.php b/Definition/Type/SchemaExtension/DecoratorExtension.php index 6bdd98f2e..21d375503 100644 --- a/Definition/Type/SchemaExtension/DecoratorExtension.php +++ b/Definition/Type/SchemaExtension/DecoratorExtension.php @@ -78,10 +78,10 @@ private function decorateInterfaceOrUnionType($type) if (!empty($unknownFields)) { throw new \InvalidArgumentException( sprintf( - '"%s".{"%s"} defined in resolverMap, but only "%s" is allowed.', + '"%s".{"%s"} defined in resolverMap, but only "%s::RESOLVE_TYPE" is allowed.', $type->name, implode('", "', $unknownFields), - ResolverMapInterface::RESOLVE_TYPE + ResolverMapInterface::class ) ); } @@ -105,10 +105,11 @@ private function decorateCustomScalarType(CustomScalarType $type) if (!empty($unknownFields)) { throw new \InvalidArgumentException( sprintf( - '"%s".{"%s"} defined in resolverMap, but only "%s" is allowed.', + '"%s".{"%s"} defined in resolverMap, but only "%s::{%s}" is allowed.', $type->name, implode('", "', $unknownFields), - implode('", "', $allowedFields) + ResolverMapInterface::class, + implode(', ', ['SCALAR_TYPE', 'SERIALIZE', 'PARSE_VALUE', 'PARSE_LITERAL']) ) ); } diff --git a/Resolver/ResolverMapInterface.php b/Resolver/ResolverMapInterface.php index 8238179fd..536c5fb66 100644 --- a/Resolver/ResolverMapInterface.php +++ b/Resolver/ResolverMapInterface.php @@ -5,15 +5,15 @@ interface ResolverMapInterface { // union and interface - const RESOLVE_TYPE = '__resolveType'; + const RESOLVE_TYPE = '%%resolveType'; // object - const RESOLVE_FIELD = '__resolveField'; - const IS_TYPEOF = '__isTypeOf'; + const RESOLVE_FIELD = '%%resolveField'; + const IS_TYPEOF = '%%isTypeOf'; // custom scalar - const SCALAR_TYPE = '__scalarType'; - const SERIALIZE = '__serialize'; - const PARSE_VALUE = '__parseValue'; - const PARSE_LITERAL = '__parseLiteral'; + const SCALAR_TYPE = '%%scalarType'; + const SERIALIZE = '%%serialize'; + const PARSE_VALUE = '%%parseValue'; + const PARSE_LITERAL = '%%parseLiteral'; /** * Returns the resolver for type category if exists. diff --git a/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php b/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php index 453629c95..bee0d20f8 100644 --- a/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php +++ b/Tests/Definition/Type/SchemaExtension/DecoratorExtensionTest.php @@ -10,8 +10,8 @@ use GraphQL\Type\Definition\UnionType; use GraphQL\Type\Schema; use Overblog\GraphQLBundle\Definition\Argument; -use Overblog\GraphQLBundle\Definition\Type\SchemaExtension\DecoratorExtension; use Overblog\GraphQLBundle\Definition\Type\CustomScalarType; +use Overblog\GraphQLBundle\Definition\Type\SchemaExtension\DecoratorExtension; use Overblog\GraphQLBundle\Resolver\ResolverMap; use Overblog\GraphQLBundle\Resolver\ResolverMapInterface; use PHPUnit\Framework\TestCase; @@ -170,7 +170,7 @@ public function testUnionTypeUnknownField() ], ], \InvalidArgumentException::class, - '"Foo".{"baz"} defined in resolverMap, but only "__resolveType" is allowed.' + '"Foo".{"baz"} defined in resolverMap, but only "Overblog\GraphQLBundle\Resolver\ResolverMapInterface::RESOLVE_TYPE" is allowed.' ); } @@ -186,7 +186,7 @@ public function testInterfaceTypeUnknownField() ], ], \InvalidArgumentException::class, - '"Foo".{"baz"} defined in resolverMap, but only "__resolveType" is allowed.' + '"Foo".{"baz"} defined in resolverMap, but only "Overblog\GraphQLBundle\Resolver\ResolverMapInterface::RESOLVE_TYPE" is allowed.' ); } @@ -202,7 +202,7 @@ public function testCustomScalarTypeUnknownField() ], ], \InvalidArgumentException::class, - '"Foo".{"baz"} defined in resolverMap, but only "__scalarType", "__serialize", "__parseValue", "__parseLiteral" is allowed.' + '"Foo".{"baz"} defined in resolverMap, but only "Overblog\GraphQLBundle\Resolver\ResolverMapInterface::{SCALAR_TYPE, SERIALIZE, PARSE_VALUE, PARSE_LITERAL}" is allowed.' ); } From 97ae1b22976ea6a8cbb7fff5fb3571634d44d51f Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 12 Feb 2018 07:41:47 +0100 Subject: [PATCH 3/3] Fix travis depth --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 44f529d8f..958a5c769 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ dist: trusty sudo: false git: - depth: 1 + depth: 50 branches: only: