Serial execution should support sync execution (+tests for the serial execution)

src/Executor/Executor.php

@@ -285,29 +285,31 @@ class Executor
         // field and its descendants will be omitted, and sibling fields will still
         // be executed. An execution which encounters errors will still result in a
         // resolved Promise.
-        $result = $this->exeContext->promises->create(function (callable $resolve) {
+        $data = $this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue);
-            return $resolve($this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue));
+        $result = $this->buildResponse($data);
-        });
 
-        return $result
+        // Note: we deviate here from the reference implementation a bit by always returning promise
-            ->then(
+        // But for the "sync" case it is always fulfilled
-                null,
+        return $this->isPromise($result)
-                function ($error) {
+            ? $result
-                    // Errors from sub-fields of a NonNull type may propagate to the top level,
+            : $this->exeContext->promises->createFulfilled($result);
-                    // at which point we still log the error and null the parent field, which
+    }
-                    // in this case is the entire response.
+
-                    $this->exeContext->addError($error);
+    /**
-
+     * @param mixed|null|Promise $data
-                    return null;
+     * @return ExecutionResult|Promise
+     */
+    private function buildResponse($data)
+    {
+        if ($this->isPromise($data)) {
+            return $data->then(function ($resolved) {
+                return $this->buildResponse($resolved);
+            });
         }
-            )
-            ->then(function ($data) {
         if ($data !== null) {
             $data = (array) $data;
         }
-
         return new ExecutionResult($data, $this->exeContext->errors);
-            });
     }
 
     /**
@@ -333,14 +335,12 @@ class Executor
                 $this->executeFieldsSerially($type, $rootValue, $path, $fields) :
                 $this->executeFields($type, $rootValue, $path, $fields);
 
-            $promise = $this->getPromise($result);
+            if ($this->isPromise($result)) {
-            if ($promise) {
+                return $result->then(
-                return $promise->then(
                     null,
                     function ($error) {
                         $this->exeContext->addError($error);
-
+                        return $this->exeContext->promises->createFulfilled(null);
-                        return null;
                     }
                 );
             }
@@ -348,7 +348,6 @@ class Executor
             return $result;
         } catch (Error $error) {
             $this->exeContext->addError($error);
-
             return null;
         }
     }
@@ -555,9 +554,10 @@ class Executor
      */
     private function executeFieldsSerially(ObjectType $parentType, $sourceValue, $path, $fields)
     {
-        $prevPromise = $this->exeContext->promises->createFulfilled([]);
+        $result = $this->promiseReduce(
-
+            array_keys($fields->getArrayCopy()),
-        $process = function ($results, $responseName, $path, $parentType, $sourceValue, $fieldNodes) {
+            function ($results, $responseName) use ($path, $parentType, $sourceValue, $fields) {
+                $fieldNodes  = $fields[$responseName];
                 $fieldPath   = $path;
                 $fieldPath[] = $responseName;
                 $result      = $this->resolveField($parentType, $sourceValue, $fieldNodes, $fieldPath);
@@ -568,32 +568,21 @@ class Executor
                 if ($promise) {
                     return $promise->then(function ($resolvedResult) use ($responseName, $results) {
                         $results[$responseName] = $resolvedResult;
-
                         return $results;
                     });
                 }
                 $results[$responseName] = $result;
-
                 return $results;
-        };
+            },
-
+            []
-        foreach ($fields as $responseName => $fieldNodes) {
+        );
-            $prevPromise = $prevPromise->then(function ($resolvedResults) use (
+        if ($this->isPromise($result)) {
-                $process,
+            return $result->then(function ($resolvedResults) {
-                $responseName,
-                $path,
-                $parentType,
-                $sourceValue,
-                $fieldNodes
-            ) {
-                return $process($resolvedResults, $responseName, $path, $parentType, $sourceValue, $fieldNodes);
-            });
-        }
-
-        return $prevPromise->then(function ($resolvedResults) {
                 return self::fixResultsIfEmptyArray($resolvedResults);
             });
         }
+        return self::fixResultsIfEmptyArray($result);
+    }
 
     /**
      * Resolves the field on the given source object. In particular, this
@@ -962,6 +951,15 @@ class Executor
         throw new \RuntimeException(sprintf('Cannot complete value of unexpected type "%s".', $returnType));
     }
 
+    /**
+     * @param mixed $value
+     * @return bool
+     */
+    private function isPromise($value)
+    {
+        return $value instanceof Promise || $this->exeContext->promises->isThenable($value);
+    }
+
     /**
      * Only returns the value if it acts like a Promise, i.e. has a "then" function,
      * otherwise returns null.
@@ -990,6 +988,31 @@ class Executor
         return null;
     }
 
+    /**
+     * Similar to array_reduce(), however the reducing callback may return
+     * a Promise, in which case reduction will continue after each promise resolves.
+     *
+     * If the callback does not return a Promise, then this function will also not
+     * return a Promise.
+     *
+     * @param mixed[] $values
+     * @param \Closure $callback
+     * @param Promise|mixed|null $initialValue
+     * @return mixed[]
+     */
+    private function promiseReduce(array $values, \Closure $callback, $initialValue)
+    {
+        return array_reduce($values, function ($previous, $value) use ($callback) {
+            $promise = $this->getPromise($previous);
+            if ($promise) {
+                return $promise->then(function ($resolved) use ($callback, $value) {
+                    return $callback($resolved, $value);
+                });
+            }
+            return $callback($previous, $value);
+        }, $initialValue);
+    }
+
     /**
      * Complete a list value by completing each item in the list with the
      * inner type

tests/Executor/SyncTest.php

@@ -0,0 +1,207 @@
+<?php
+namespace GraphQL\Tests\Executor;
+
+use GraphQL\Deferred;
+use GraphQL\Error\Error;
+use GraphQL\Error\FormattedError;
+use GraphQL\Executor\ExecutionResult;
+use GraphQL\Executor\Executor;
+use GraphQL\Executor\Promise\Adapter\SyncPromise;
+use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter;
+use GraphQL\Executor\Promise\Promise;
+use GraphQL\GraphQL;
+use GraphQL\Language\Parser;
+use GraphQL\Type\Definition\ObjectType;
+use GraphQL\Type\Definition\Type;
+use GraphQL\Type\Schema;
+use GraphQL\Utils\Utils;
+use GraphQL\Validator\DocumentValidator;
+use PHPUnit\Framework\TestCase;
+
+class SyncTest extends TestCase
+{
+    /** @var Schema */
+    private $schema;
+
+    /** @var SyncPromiseAdapter */
+    private $promiseAdapter;
+
+    public function setUp()
+    {
+        $this->schema = new Schema([
+            'query' => new ObjectType([
+                'name' => 'Query',
+                'fields' => [
+                    'syncField' => [
+                        'type' => Type::string(),
+                        'resolve' => function ($rootValue) {
+                            return $rootValue;
+                        }
+                    ],
+                    'asyncField' => [
+                        'type' => Type::string(),
+                        'resolve' => function ($rootValue) {
+                            return new Deferred(function () use ($rootValue) {
+                                return $rootValue;
+                            });
+                        }
+                    ]
+                ]
+            ]),
+            'mutation' => new ObjectType([
+                'name' => 'Mutation',
+                'fields' => [
+                    'syncMutationField' => [
+                        'type' => Type::string(),
+                        'resolve' => function ($rootValue) {
+                            return $rootValue;
+                        }
+                    ]
+                ]
+            ])
+        ]);
+        $this->promiseAdapter = new SyncPromiseAdapter();
+    }
+
+    // Describe: Execute: synchronously when possible
+
+    /**
+     * @it does not return a Promise for initial errors
+     */
+    public function testDoesNotReturnAPromiseForInitialErrors()
+    {
+        $doc = 'fragment Example on Query { syncField }';
+        $result = $this->execute(
+            $this->schema,
+            Parser::parse($doc),
+            'rootValue'
+        );
+        $this->assertSync(['errors' => [['message' => 'Must provide an operation.']]], $result);
+    }
+
+    /**
+     * @it does not return a Promise if fields are all synchronous
+     */
+    public function testDoesNotReturnAPromiseIfFieldsAreAllSynchronous()
+    {
+        $doc = 'query Example { syncField }';
+        $result = $this->execute(
+            $this->schema,
+            Parser::parse($doc),
+            'rootValue'
+        );
+        $this->assertSync(['data' => ['syncField' => 'rootValue']], $result);
+    }
+
+    /**
+     * @it does not return a Promise if mutation fields are all synchronous
+     */
+    public function testDoesNotReturnAPromiseIfMutationFieldsAreAllSynchronous()
+    {
+        $doc = 'mutation Example { syncMutationField }';
+        $result = $this->execute(
+            $this->schema,
+            Parser::parse($doc),
+            'rootValue'
+        );
+        $this->assertSync(['data' => ['syncMutationField' => 'rootValue']], $result);
+    }
+
+    /**
+     * @it returns a Promise if any field is asynchronous
+     */
+    public function testReturnsAPromiseIfAnyFieldIsAsynchronous()
+    {
+        $doc = 'query Example { syncField, asyncField }';
+        $result = $this->execute(
+            $this->schema,
+            Parser::parse($doc),
+            'rootValue'
+        );
+        $this->assertAsync(['data' => ['syncField' => 'rootValue', 'asyncField' => 'rootValue']], $result);
+    }
+
+    // Describe: graphqlSync
+
+    /**
+     * @it does not return a Promise for syntax errors
+     */
+    public function testDoesNotReturnAPromiseForSyntaxErrors()
+    {
+        $doc = 'fragment Example on Query { { { syncField }';
+        $result = $this->graphqlSync(
+            $this->schema,
+            $doc
+        );
+        $this->assertSync([
+            'errors' => [
+                ['message' => 'Syntax Error: Expected Name, found {',
+                'locations' => [['line' => 1, 'column' => 29]]]
+            ]
+        ], $result);
+    }
+
+    /**
+     * @it does not return a Promise for validation errors
+     */
+    public function testDoesNotReturnAPromiseForValidationErrors()
+    {
+        $doc = 'fragment Example on Query { unknownField }';
+        $validationErrors = DocumentValidator::validate($this->schema, Parser::parse($doc));
+        $result = $this->graphqlSync(
+            $this->schema,
+            $doc
+        );
+        $expected = [
+            'errors' => Utils::map($validationErrors, function ($e) {
+                return FormattedError::createFromException($e);
+            })
+        ];
+        $this->assertSync($expected, $result);
+    }
+
+    /**
+     * @it does not return a Promise for sync execution
+     */
+    public function testDoesNotReturnAPromiseForSyncExecution()
+    {
+        $doc = 'query Example { syncField }';
+        $result = $this->graphqlSync(
+            $this->schema,
+            $doc,
+            'rootValue'
+        );
+        $this->assertSync(['data' => ['syncField' => 'rootValue']], $result);
+    }
+
+    private function graphqlSync($schema, $doc, $rootValue = null)
+    {
+        return GraphQL::promiseToExecute($this->promiseAdapter, $schema, $doc, $rootValue);
+    }
+
+    private function execute($schema, $doc, $rootValue = null)
+    {
+        return Executor::promiseToExecute($this->promiseAdapter, $schema, $doc, $rootValue);
+    }
+
+    private function assertSync($expectedFinalArray, $actualResult)
+    {
+        $message = 'Failed assertion that execution was synchronous';
+        $this->assertInstanceOf(Promise::class, $actualResult, $message);
+        $this->assertInstanceOf(SyncPromise::class, $actualResult->adoptedPromise, $message);
+        $this->assertEquals(SyncPromise::FULFILLED, $actualResult->adoptedPromise->state, $message);
+        $this->assertInstanceOf(ExecutionResult::class, $actualResult->adoptedPromise->result, $message);
+        $this->assertArraySubset($expectedFinalArray, $actualResult->adoptedPromise->result->toArray(), $message);
+    }
+
+    private function assertAsync($expectedFinalArray, $actualResult)
+    {
+        $message = 'Failed assertion that execution was asynchronous';
+        $this->assertInstanceOf(Promise::class, $actualResult, $message);
+        $this->assertInstanceOf(SyncPromise::class, $actualResult->adoptedPromise, $message);
+        $this->assertEquals(SyncPromise::PENDING, $actualResult->adoptedPromise->state, $message);
+        $resolvedResult = $this->promiseAdapter->wait($actualResult);
+        $this->assertInstanceOf(ExecutionResult::class, $resolvedResult, $message);
+        $this->assertArraySubset($expectedFinalArray, $resolvedResult->toArray(), $message);
+    }
+}