Execution: refactored promise adapters

src/Executor/Executor.php

@@ -107,8 +107,12 @@ class Executor
         }
 
         $exeContext = self::buildExecutionContext($schema, $ast, $rootValue, $contextValue, $variableValues, $operationName);
-        $executor = new self($exeContext, self::getPromiseAdapter());
+        $promiseAdapter = self::getPromiseAdapter();
-        return $executor->executeQuery();
+
+        $executor = new self($exeContext, $promiseAdapter);
+        $result = $executor->executeQuery();
+
+        return $result;
     }
 
     /**
@@ -205,37 +209,32 @@ class Executor
         $this->promises = $promiseAdapter;
     }
 
+    /**
+     * @return Promise
+     */
     private function executeQuery()
     {
-        try {
+        // Return a Promise that will eventually resolve to the data described by
-            // Return a Promise that will eventually resolve to the data described by
+        // The "Response" section of the GraphQL specification.
-            // The "Response" section of the GraphQL specification.
+        //
-            //
+        // If errors are encountered while executing a GraphQL field, only that
-            // If errors are encountered while executing a GraphQL field, only that
+        // field and its descendants will be omitted, and sibling fields will still
-            // field and its descendants will be omitted, and sibling fields will still
+        // be executed. An execution which encounters errors will still result in a
-            // be executed. An execution which encounters errors will still result in a
+        // resolved Promise.
-            // resolved Promise.
+        $result = $this->promises->createPromise(function (callable $resolve) {
-            $result = $this->promises->createPromise(function (callable $resolve) {
+            return $resolve($this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue));
-                return $resolve($this->executeOperation($this->exeContext->operation, $this->exeContext->rootValue));
+        });
-            });
+        return $result
-            $result = $this->promises->then($result, null, function ($error) {
+            ->then(null, function ($error) {
                 // Errors from sub-fields of a NonNull type may propagate to the top level,
                 // 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);
                 return null;
-            });
+            })
-            $result = $this->promises->then($result, function ($data) {
+            ->then(function ($data) {
                 return new ExecutionResult((array) $data, $this->exeContext->errors);
             });
-
-            return $result;
-        } catch (Error $e) {
-            $this->exeContext->addError($e);
-            $data = null;
-        }
-
-        return new ExecutionResult((array) $data, $this->exeContext->errors);
     }
 
     /**
@@ -310,7 +309,7 @@ class Executor
      */
     private function executeFieldsSerially(ObjectType $parentType, $sourceValue, $path, $fields)
     {
-        $results = $this->promises->createResolvedPromise([]);
+        $prevPromise = $this->promises->createResolvedPromise([]);
 
         $process = function ($results, $responseName, $path, $parentType, $sourceValue, $fieldNodes) {
             $fieldPath = $path;
@@ -319,8 +318,8 @@ class Executor
             if ($result === self::$UNDEFINED) {
                 return $results;
             }
-            if ($this->promises->isPromise($result)) {
+            if ($result instanceof Promise) {
-                return $this->promises->then($result, function ($resolvedResult) use ($responseName, $results) {
+                return $result->then(function ($resolvedResult) use ($responseName, $results) {
                     $results[$responseName] = $resolvedResult;
                     return $results;
                 });
@@ -330,22 +329,14 @@ class Executor
         };
 
         foreach ($fields as $responseName => $fieldNodes) {
-            if ($this->promises->isPromise($results)) {
+            $prevPromise = $prevPromise->then(function ($resolvedResults) use ($process, $responseName, $path, $parentType, $sourceValue, $fieldNodes) {
-                $results = $this->promises->then($results, function ($resolvedResults) use ($process, $responseName, $path, $parentType, $sourceValue, $fieldNodes) {
+                return $process($resolvedResults, $responseName, $path, $parentType, $sourceValue, $fieldNodes);
-                    return $process($resolvedResults, $responseName, $path, $parentType, $sourceValue, $fieldNodes);
-                });
-            } else {
-                $results = $process($results, $responseName, $path, $parentType, $sourceValue, $fieldNodes);
-            }
-        }
-
-        if ($this->promises->isPromise($results)) {
-            return $this->promises->then($results, function ($resolvedResults) {
-                return self::fixResultsIfEmptyArray($resolvedResults);
             });
         }
 
-        return self::fixResultsIfEmptyArray($results);
+        return $prevPromise->then(function ($resolvedResults) {
+            return self::fixResultsIfEmptyArray($resolvedResults);
+        });
     }
 
     /**
@@ -370,7 +361,7 @@ class Executor
             if ($result === self::$UNDEFINED) {
                 continue;
             }
-            if (!$containsPromise && $this->promises->isPromise($result)) {
+            if (!$containsPromise && $result instanceof Promise) {
                 $containsPromise = true;
             }
             $finalResults[$responseName] = $result;
@@ -405,7 +396,7 @@ class Executor
 
         $promise = $this->promises->createPromiseAll($valuesAndPromises);
 
-        return $this->promises->then($promise, function($values) use ($keys) {
+        return $promise->then(function($values) use ($keys) {
             $resolvedResults = [];
             foreach ($values as $i => $value) {
                 $resolvedResults[$keys[$i]] = $value;
@@ -669,7 +660,7 @@ class Executor
      * @param mixed $source
      * @param mixed $context
      * @param ResolveInfo $info
-     * @return \Exception|mixed
+     * @return \Exception|Promise|mixed
      */
     private function resolveOrError($fieldDef, $fieldNode, $resolveFn, $source, $context, $info)
     {
@@ -682,7 +673,15 @@ class Executor
                 $this->exeContext->variableValues
             );
 
-            return call_user_func($resolveFn, $source, $args, $context, $info);
+            $value = call_user_func($resolveFn, $source, $args, $context, $info);
+
+            // Adopt promises from external system:
+            if ($this->promises->isThenable($value)) {
+                $value = $this->promises->convert($value);
+                Utils::invariant($value instanceof Promise);
+            }
+
+            return $value;
         } catch (\Exception $error) {
             return $error;
         }
@@ -731,8 +730,8 @@ class Executor
                 $path,
                 $result
             );
-            if ($this->promises->isPromise($completed)) {
+            if ($completed instanceof Promise) {
-                return $this->promises->then($completed, null, function ($error) use ($exeContext) {
+                return $completed->then(null, function ($error) use ($exeContext) {
                     $exeContext->addError($error);
                     return $this->promises->createResolvedPromise(null);
                 });
@@ -775,8 +774,8 @@ class Executor
                 $path,
                 $result
             );
-            if ($this->promises->isPromise($completed)) {
+            if ($completed instanceof Promise) {
-                return $this->promises->then($completed, null, function ($error) use ($fieldNodes, $path) {
+                return $completed->then(null, function ($error) use ($fieldNodes, $path) {
                     return $this->promises->createRejectedPromise(Error::createLocatedError($error, $fieldNodes, $path));
                 });
             }
@@ -825,8 +824,8 @@ class Executor
     )
     {
         // If result is a Promise, apply-lift over completeValue.
-        if ($this->promises->isPromise($result)) {
+        if ($result instanceof Promise) {
-            return $this->promises->then($result, function (&$resolved) use ($returnType, $fieldNodes, $info, $path) {
+            return $result->then(function (&$resolved) use ($returnType, $fieldNodes, $info, $path) {
                 return $this->completeValue($returnType, $fieldNodes, $info, $path, $resolved);
             });
         }
@@ -1018,7 +1017,7 @@ class Executor
             $fieldPath = $path;
             $fieldPath[] = $i++;
             $completedItem = $this->completeValueCatchingError($itemType, $fieldNodes, $info, $fieldPath, $item);
-            if (!$containsPromise && $this->promises->isPromise($completedItem)) {
+            if (!$containsPromise && $completedItem instanceof Promise) {
                 $containsPromise = true;
             }
             $completedItems[] = $completedItem;

src/Executor/Promise/Adapter/ReactPromiseAdapter.php

@@ -1,81 +1,77 @@
 <?php
-
 namespace GraphQL\Executor\Promise\Adapter;
 
-use GraphQL\Executor\Promise\PromiseAdapter;;
+use GraphQL\Executor\Promise\Promise;
-use React\Promise\FulfilledPromise;
+use GraphQL\Executor\Promise\PromiseAdapter;
-use React\Promise\Promise;
+use GraphQL\Utils;
-use React\Promise\PromiseInterface;
+use React\Promise\Promise as ReactPromise;
+use React\Promise\PromiseInterface as ReactPromiseInterface;
 
 class ReactPromiseAdapter implements PromiseAdapter
 {
     /**
-     * Return true if value is promise
+     * @inheritdoc
-     *
-     * @param  mixed $value
-     *
-     * @return bool
      */
-    public function isPromise($value)
+    public function isThenable($value)
     {
-        return $value instanceof PromiseInterface;
+        return $value instanceof ReactPromiseInterface;
-    }
+    }
-
+
-    /**
+    /**
-     * Accepts value qualified by `isPromise` and returns other promise.
+     * @inheritdoc
-     *
+     */
-     * @param Promise $promise
+    public function convert($promise)
-     * @param callable|null $onFullFilled
+    {
-     * @param callable|null $onRejected
+        return new Promise($promise, $this);
-     * @return mixed
+    }
-     */
+
-    public function then($promise, callable $onFullFilled = null, callable $onRejected = null)
+    /**
-    {
+     * @inheritdoc
-        return $promise->then($onFullFilled, $onRejected);
+     */
+    public function then(Promise $promise, callable $onFulfilled = null, callable $onRejected = null)
+    {
+        /** @var $adoptedPromise ReactPromiseInterface */
+        $adoptedPromise = $promise->adoptedPromise;
+        return new Promise($adoptedPromise->then($onFulfilled, $onRejected), $this);
     }
 
     /**
      * @inheritdoc
-     *
-     * @return PromiseInterface
      */
     public function createPromise(callable $resolver)
     {
-        $promise = new Promise($resolver);
+        $promise = new ReactPromise($resolver);
-
+        return new Promise($promise, $this);
-        return $promise;
     }
 
     /**
      * @inheritdoc
-     *
-     * @return FulfilledPromise
      */
-    public function createResolvedPromise($promiseOrValue = null)
+    public function createResolvedPromise($value = null)
     {
-        return \React\Promise\resolve($promiseOrValue);
+        $promise = \React\Promise\resolve($value);
+        return new Promise($promise, $this);
     }
 
     /**
      * @inheritdoc
-     *
-     * @return \React\Promise\RejectedPromise
      */
-    public function createRejectedPromise($reason)
+    public function createRejectedPromise(\Exception $reason)
     {
-        return \React\Promise\reject($reason);
+        $promise = \React\Promise\reject($reason);
+        return new Promise($promise, $this);
     }
 
     /**
-     * Given an array of promises, return a promise that is fulfilled when all the
+     * @inheritdoc
-     * items in the array are fulfilled.
-     *
-     * @param mixed $promisesOrValues Promises or values.
-     *
-     * @return mixed a Promise
      */
-    public function createPromiseAll($promisesOrValues)
+    public function createPromiseAll(array $promisesOrValues)
     {
-        return \React\Promise\all($promisesOrValues);
+        // TODO: rework with generators when PHP minimum required version is changed to 5.5+
+        $promisesOrValues = Utils::map($promisesOrValues, function ($item) {
+            return $item instanceof Promise ? $item->adoptedPromise : $item;
+        });
+        $promise = \React\Promise\all($promisesOrValues);
+        return new Promise($promise, $this);
     }
 }

src/Executor/Promise/Promise.php

@@ -1,18 +1,39 @@
 <?php
-
 namespace GraphQL\Executor\Promise;
 
+use GraphQL\Utils;
+
 /**
- * A simple Promise representation
+ * Convenience wrapper for promises represented by Promise Adapter
- * this interface helps to document the code
  */
-interface Promise
+class Promise
 {
+    private $adapter;
+
+    public $adoptedPromise;
+
     /**
-     * @param callable|null $onFullFilled
+     * Promise constructor.
+     *
+     * @param mixed $adoptedPromise
+     * @param PromiseAdapter $adapter
+     */
+    public function __construct($adoptedPromise, PromiseAdapter $adapter)
+    {
+        Utils::invariant(!$adoptedPromise instanceof self, 'Expecting promise from adapted system, got ' . __CLASS__);
+
+        $this->adapter = $adapter;
+        $this->adoptedPromise = $adoptedPromise;
+    }
+
+    /**
+     * @param callable|null $onFulfilled
      * @param callable|null $onRejected
      *
      * @return Promise
      */
-    public function then(callable $onFullFilled = null, callable $onRejected = null);
+    public function then(callable $onFulfilled = null, callable $onRejected = null)
+    {
+        return $this->adapter->then($this, $onFulfilled, $onRejected);
+    }
 }

src/Executor/Promise/PromiseAdapter.php

@@ -1,46 +1,53 @@
 <?php
-
 namespace GraphQL\Executor\Promise;
 
 interface PromiseAdapter
 {
     /**
-     * Return true if value is promise
+     * Return true if value is promise of underlying system
      *
      * @param  mixed $value
-     *
      * @return bool
      */
-    public function isPromise($value);
+    public function isThenable($value);
 
     /**
-     * Accepts value qualified by `isPromise` and returns other promise.
+     * Converts promise of underlying system into Promise instance
-     * Underlying mechanics of this process must match Promises/A+ specs
      *
-     * @param $promise
+     * @param $adaptedPromise
-     * @param callable|null $onFullFilled
+     * @return Promise
-     * @param callable|null $onRejected
-     * @return mixed
      */
-    public function then($promise, callable $onFullFilled = null, callable $onRejected = null);
+    public function convert($adaptedPromise);
+
+    /**
+     * Accepts our Promise wrapper, extracts adopted promise out of it and executes actual `then` logic described
+     * in Promises/A+ specs. Then returns new wrapped Promise instance.
+     *
+     * @param Promise $promise
+     * @param callable|null $onFulfilled
+     * @param callable|null $onRejected
+     *
+     * @return Promise
+     */
+    public function then(Promise $promise, callable $onFulfilled = null, callable $onRejected = null);
 
     /**
      * Creates a Promise
      *
      * @param callable $resolver
-     *
+
      * @return Promise
      */
     public function createPromise(callable $resolver);
 
     /**
-     * Creates a full filed Promise for a value if the value is not a promise.
+     * Creates a fulfilled Promise for a value if the value is not a promise.
      *
-     * @param mixed $promiseOrValue
+     * @param mixed $value
      *
-     * @return Promise a full filed Promise
+     * @return Promise
      */
-    public function createResolvedPromise($promiseOrValue = null);
+    public function createResolvedPromise($value = null);
 
     /**
      * Creates a rejected promise for a reason if the reason is not a promise. If
@@ -48,17 +55,17 @@ interface PromiseAdapter
      *
      * @param mixed $reason
      *
-     * @return Promise a rejected promise
+     * @return Promise
      */
-    public function createRejectedPromise($reason);
+    public function createRejectedPromise(\Exception $reason);
 
     /**
-     * Given an array of promises, return a promise that is fulfilled when all the
+     * Given an array of promises (or values), returns a promise that is fulfilled when all the
      * items in the array are fulfilled.
      *
-     * @param mixed $promisesOrValues Promises or values.
+     * @param array $promisesOrValues Promises or values.
      *
-     * @return Promise equivalent to Promise.all result
+     * @return Promise
      */
-    public function createPromiseAll($promisesOrValues);
+    public function createPromiseAll(array $promisesOrValues);
 }