Merge pull request #496 from mcg-web/executor

Refactor executor/ request executor
overblog · Jun 2, 2019 · a14b662 · a14b662
2 parents 53ae4db + a59bdcc
commit a14b662
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 184 deletions.
diff --git a/UPGRADE-0.12.md b/UPGRADE-0.12.md
@@ -6,6 +6,7 @@ UPGRADE FROM 0.11 to 0.12
 - [Remove auto mapping configuration](#remove-auto-mapping-configuration)
 - [Relay Paginator, Connections & Edges](#relay-paginator-connections--edges)
 - [Remove obsoletes deprecations](#remove-obsoletes-deprecations)
+- [Simplify executor interface](#simplify-executor-interface)
 
 ### Remove auto mapping configuration
 
@@ -108,3 +109,24 @@ Foo:
 -                argsBuilder: ConnectionArgs
 +                argsBuilder: "Relay::Connection"
 ```
+
+### Simplify executor interface
+
+This section is only for users using custom executor.
+
+The interface move to be look a little be more to `GraphQL\GraphQL`
+execute method.
+
+In `Overblog\GraphQLBundle\Executor\ExecutorInterface`
+`setPromiseAdapter` and `setDefaultFieldResolver` has been removed.
+To use promise adapter should now inject it using DI like this:
+
+```yaml
+services:
+    App\CustomExecutor:
+        calls:
+            - ["setPromiseAdapter", ['@overblog_graphql.promise_adapter']]
+```
+
+Default field resolver is now the 7th argument (`$fieldResolver`) of
+`Overblog\GraphQLBundle\Executor\ExecutorInterface::execute` method.
diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php
@@ -4,9 +4,11 @@
 
 namespace Overblog\GraphQLBundle\Executor;
 
+use GraphQL\Executor\ExecutionResult;
 use GraphQL\Executor\Promise\PromiseAdapter;
 use GraphQL\GraphQL;
 use GraphQL\Type\Schema;
+use Overblog\GraphQLBundle\Executor\Promise\PromiseAdapterInterface;
 
 class Executor implements ExecutorInterface
 {
@@ -16,27 +18,31 @@ class Executor implements ExecutorInterface
     /**
      * {@inheritdoc}
      */
-    public function execute(Schema $schema, string $requestString, array $rootValue = null, $contextValue = null, $variableValues = null, $operationName = null)
-    {
-        $args = \func_get_args();
-        \array_unshift($args, $this->promiseAdapter);
-
-        return \call_user_func_array([GraphQL::class, 'promiseToExecute'], $args);
+    public function execute(
+        Schema $schema,
+        string $requestString,
+        $rootValue = null,
+        $contextValue = null,
+        $variableValues = null,
+        $operationName = null,
+        ?callable $fieldResolver = null,
+        ?array $validationRules = null
+    ): ExecutionResult {
+        if ($this->promiseAdapter && !$this->promiseAdapter instanceof PromiseAdapterInterface && !\is_callable([$this->promiseAdapter, 'wait'])) {
+            throw new \RuntimeException(
+                \sprintf(
+                    'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.',
+                    PromiseAdapterInterface::class,
+                    PromiseAdapter::class
+                )
+            );
+        }
+
+        return $this->promiseAdapter->wait(GraphQL::promiseToExecute($this->promiseAdapter, ...\func_get_args()));
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function setPromiseAdapter(PromiseAdapter $promiseAdapter): void
     {
         $this->promiseAdapter = $promiseAdapter;
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setDefaultFieldResolver(callable $fn): void
-    {
-        \call_user_func_array([GraphQL::class, 'setDefaultFieldResolver'], \func_get_args());
-    }
 }
diff --git a/src/Executor/ExecutorInterface.php b/src/Executor/ExecutorInterface.php
@@ -5,31 +5,30 @@
 namespace Overblog\GraphQLBundle\Executor;
 
 use GraphQL\Executor\ExecutionResult;
-use GraphQL\Executor\Promise\Promise;
-use GraphQL\Executor\Promise\PromiseAdapter;
 use GraphQL\Type\Schema;
 
 interface ExecutorInterface
 {
     /**
-     * @param Schema      $schema
-     * @param string      $requestString
-     * @param array|null  $rootValue
-     * @param array|null  $contextValue
-     * @param array|null  $variableValues
-     * @param string|null $operationName
+     * @param Schema        $schema
+     * @param string        $requestString
+     * @param mixed         $rootValue
+     * @param array|null    $contextValue
+     * @param array|null    $variableValues
+     * @param string|null   $operationName
+     * @param callable|null $fieldResolver
+     * @param array|null    $validationRules
      *
-     * @return ExecutionResult|Promise
+     * @return ExecutionResult
      */
-    public function execute(Schema $schema, string $requestString, array $rootValue = null, $contextValue = null, $variableValues = null, $operationName = null);
-
-    /**
-     * @param PromiseAdapter|null $promiseAdapter
-     */
-    public function setPromiseAdapter(PromiseAdapter $promiseAdapter);
-
-    /**
-     * @param callable $fn
-     */
-    public function setDefaultFieldResolver(callable $fn);
+    public function execute(
+        Schema $schema,
+        string $requestString,
+        $rootValue = null,
+        $contextValue = null,
+        $variableValues = null,
+        $operationName = null,
+        ?callable $fieldResolver = null,
+        ?array $validationRules = null
+    ): ExecutionResult;
 }
diff --git a/src/Request/Executor.php b/src/Request/Executor.php
@@ -5,8 +5,6 @@
 namespace Overblog\GraphQLBundle\Request;
 
 use GraphQL\Executor\ExecutionResult;
-use GraphQL\Executor\Promise\Promise;
-use GraphQL\Executor\Promise\PromiseAdapter;
 use GraphQL\Type\Schema;
 use GraphQL\Validator\DocumentValidator;
 use GraphQL\Validator\Rules\DisableIntrospection;
@@ -18,7 +16,6 @@
 use Overblog\GraphQLBundle\Event\ExecutorContextEvent;
 use Overblog\GraphQLBundle\Event\ExecutorResultEvent;
 use Overblog\GraphQLBundle\Executor\ExecutorInterface;
-use Overblog\GraphQLBundle\Executor\Promise\PromiseAdapterInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
@@ -35,21 +32,16 @@ class Executor
     /** @var ExecutorInterface */
     private $executor;
 
-    /** @var PromiseAdapter */
-    private $promiseAdapter;
-
     /** @var callable|null */
     private $defaultFieldResolver;
 
     public function __construct(
         ExecutorInterface $executor,
         EventDispatcherInterface $dispatcher,
-        PromiseAdapter $promiseAdapter,
         ?callable $defaultFieldResolver = null
     ) {
         $this->executor = $executor;
         $this->dispatcher = $dispatcher;
-        $this->promiseAdapter = $promiseAdapter;
         $this->defaultFieldResolver = $defaultFieldResolver;
     }
 
@@ -60,13 +52,6 @@ public function setExecutor(ExecutorInterface $executor): self
         return $this;
     }
 
-    public function setPromiseAdapter(PromiseAdapter $promiseAdapter): self
-    {
-        $this->promiseAdapter = $promiseAdapter;
-
-        return $this;
-    }
-
     /**
      * @param string $name
      * @param Schema $schema
@@ -153,7 +138,8 @@ public function execute(?string $schemaName = null, array $request, $rootValue =
             $executorArgumentsEvent->getRootValue(),
             $executorArgumentsEvent->getContextValue(),
             $executorArgumentsEvent->getVariableValue(),
-            $executorArgumentsEvent->getOperationName()
+            $executorArgumentsEvent->getOperationName(),
+            $this->defaultFieldResolver
         );
 
         $result = $this->postExecute($result);
@@ -169,13 +155,6 @@ private function preExecute(
         ?array $variableValue = null,
         ?string $operationName = null
     ): ExecutorArgumentsEvent {
-        $this->checkPromiseAdapter();
-
-        $this->executor->setPromiseAdapter($this->promiseAdapter);
-        // this is needed when not using only generated types
-        if ($this->defaultFieldResolver) {
-            $this->executor->setDefaultFieldResolver($this->defaultFieldResolver);
-        }
         EventDispatcherVersionHelper::dispatch(
             $this->dispatcher,
             new ExecutorContextEvent($contextValue),
@@ -189,46 +168,12 @@ private function preExecute(
         );
     }
 
-    /**
-     * @param ExecutionResult|Promise $result
-     *
-     * @return ExecutionResult
-     */
-    private function postExecute($result): ExecutionResult
+    private function postExecute(ExecutionResult $result): ExecutionResult
     {
-        if ($result instanceof Promise) {
-            $result = $this->promiseAdapter->wait($result);
-        }
-
-        $this->checkExecutionResult($result);
-        $event = EventDispatcherVersionHelper::dispatch(
+        return EventDispatcherVersionHelper::dispatch(
             $this->dispatcher,
             new ExecutorResultEvent($result),
             Events::POST_EXECUTOR
-        );
-
-        return $event->getResult();
-    }
-
-    private function checkPromiseAdapter(): void
-    {
-        if ($this->promiseAdapter && !$this->promiseAdapter instanceof PromiseAdapterInterface && !\is_callable([$this->promiseAdapter, 'wait'])) {
-            throw new \RuntimeException(
-                \sprintf(
-                    'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.',
-                    PromiseAdapterInterface::class,
-                    PromiseAdapter::class
-                )
-            );
-        }
-    }
-
-    private function checkExecutionResult($result): void
-    {
-        if (!\is_object($result) || !$result instanceof ExecutionResult) {
-            throw new \RuntimeException(
-                \sprintf('Execution result should be an object instantiating "%s".', ExecutionResult::class)
-            );
-        }
+        )->getResult();
     }
 }
diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml
@@ -5,15 +5,16 @@ services:
     overblog_graphql.executor.default:
         class: Overblog\GraphQLBundle\Executor\Executor
         public: false
+        calls:
+            - ["setPromiseAdapter", ['@overblog_graphql.promise_adapter']]
 
     overblog_graphql.request_executor:
         class: Overblog\GraphQLBundle\Request\Executor
         public: true
         arguments:
             - "@overblog_graphql.executor"
             - "@event_dispatcher"
-            - "@overblog_graphql.promise_adapter"
-            - "%overblog_graphql.default_resolver%"
+            - '%overblog_graphql.default_resolver%'
         calls:
             - ["setMaxQueryComplexity", ["%overblog_graphql.query_max_complexity%"]]
             - ["setMaxQueryDepth", ["%overblog_graphql.query_max_depth%"]]

diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Overblog\GraphQLBundle\Tests\Executor;
+
+use GraphQL\Executor\Promise\Adapter\ReactPromiseAdapter;
+use GraphQL\Type\Schema;
+use Overblog\GraphQLBundle\Executor\Executor;
+use PHPUnit\Framework\TestCase;
+
+class ExecutorTest extends TestCase
+{
+    /**
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage PromiseAdapter should be an object instantiating "Overblog\GraphQLBundle\Executor\Promise\PromiseAdapterInterface" or "GraphQL\Executor\Promise\PromiseAdapter" with a "wait" method.
+     */
+    public function testInvalidExecutorAdapterPromise(): void
+    {
+        $schema = $this->getMockBuilder(Schema::class)->disableOriginalConstructor()->getMock();
+        $executor = new Executor();
+        $executor->setPromiseAdapter(new ReactPromiseAdapter());
+        $executor->execute($schema, '');
+    }
+}