Merge pull request #80 from lordthorzonus/nested-async-queries-mess-the-indexes-of-arrays

Nested async queries messes up the order of keys in arrays producing unwanted responses

.travis.yml

@@ -22,8 +22,9 @@ before_install:
 
 install:
     - composer install --dev --prefer-dist
+    - composer require react/promise:2.*
 
-script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then phpunit --coverage-clover build/logs/clover.xml; else phpunit; fi
+script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then phpunit --coverage-clover build/logs/clover.xml --group default,ReactPromise; else phpunit --group default,ReactPromise; fi
 
 after_success:
     - if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php bin/coveralls -v; fi

phpunit.xml.dist

@@ -17,6 +17,12 @@
         </testsuite>
     </testsuites>
 
+    <groups>
+        <exclude>
+            <group>ReactPromise</group>
+        </exclude>
+    </groups>
+
     <filter>
         <whitelist>
             <directory suffix=".php">./src</directory>

src/Executor/Promise/Adapter/ReactPromiseAdapter.php

@@ -71,7 +71,16 @@ class ReactPromiseAdapter implements PromiseAdapter
         $promisesOrValues = Utils::map($promisesOrValues, function ($item) {
             return $item instanceof Promise ? $item->adoptedPromise : $item;
         });
-        $promise = \React\Promise\all($promisesOrValues);
+
+        $promise = \React\Promise\all($promisesOrValues)->then(function($values) use ($promisesOrValues) {
+            $orderedResults = [];
+
+            foreach ($promisesOrValues as $key => $value) {
+                $orderedResults[$key] = $values[$key];
+            }
+
+            return $orderedResults;
+        });
         return new Promise($promise, $this);
     }
 }

tests/Executor/Promise/ReactPromiseAdapterTest.php

@@ -0,0 +1,161 @@
+<?php
+
+
+namespace GraphQL\Tests\Executor\Promise;
+
+
+use GraphQL\Executor\Promise\Adapter\ReactPromiseAdapter;
+use GraphQL\Executor\Promise\Promise;
+use React\Promise\Deferred;
+use React\Promise\FulfilledPromise;
+use React\Promise\LazyPromise;
+use React\Promise\Promise as ReactPromise;
+use React\Promise\RejectedPromise;
+
+/**
+ * @group ReactPromise
+ */
+class ReactPromiseAdapterTest extends \PHPUnit_Framework_TestCase
+{
+    public function setUp()
+    {
+        if(! class_exists('React\Promise\Promise')) {
+            $this->markTestSkipped('react/promise package must be installed to run GraphQL\Tests\Executor\Promise\ReactPromiseAdapterTest');
+        }
+    }
+
+    public function testIsThenableReturnsTrueWhenAReactPromiseIsGiven()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+
+        $this->assertSame(true, $reactAdapter->isThenable(new ReactPromise(function() {})));
+        $this->assertSame(true, $reactAdapter->isThenable(new FulfilledPromise()));
+        $this->assertSame(true, $reactAdapter->isThenable(new RejectedPromise()));
+        $this->assertSame(true, $reactAdapter->isThenable(new LazyPromise(function() {})));
+        $this->assertSame(false, $reactAdapter->isThenable(false));
+        $this->assertSame(false, $reactAdapter->isThenable(true));
+        $this->assertSame(false, $reactAdapter->isThenable(1));
+        $this->assertSame(false, $reactAdapter->isThenable(0));
+        $this->assertSame(false, $reactAdapter->isThenable('test'));
+        $this->assertSame(false, $reactAdapter->isThenable(''));
+        $this->assertSame(false, $reactAdapter->isThenable([]));
+        $this->assertSame(false, $reactAdapter->isThenable(new \stdClass()));
+    }
+
+    public function testConvertsReactPromisesToGraphQlOnes()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $reactPromise = new FulfilledPromise(1);
+
+        $promise = $reactAdapter->convertThenable($reactPromise);
+
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $promise);
+        $this->assertInstanceOf('React\Promise\FulfilledPromise', $promise->adoptedPromise);
+    }
+
+    public function testThen()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $reactPromise = new FulfilledPromise(1);
+        $promise = $reactAdapter->convertThenable($reactPromise);
+
+        $result = null;
+
+        $resultPromise = $reactAdapter->then($promise, function ($value) use (&$result) {
+            $result = $value;
+        });
+
+        $this->assertSame(1, $result);
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $resultPromise);
+        $this->assertInstanceOf('React\Promise\FulfilledPromise', $resultPromise->adoptedPromise);
+    }
+
+    public function testCreate()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $resolvedPromise = $reactAdapter->create(function ($resolve) {
+             $resolve(1);
+        });
+
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $resolvedPromise);
+        $this->assertInstanceOf('React\Promise\Promise', $resolvedPromise->adoptedPromise);
+
+        $result = null;
+
+        $resolvedPromise->then(function ($value) use (&$result) {
+           $result = $value;
+        });
+
+        $this->assertSame(1, $result);
+    }
+
+    public function testCreateFulfilled()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $fulfilledPromise = $reactAdapter->createFulfilled(1);
+
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $fulfilledPromise);
+        $this->assertInstanceOf('React\Promise\FulfilledPromise', $fulfilledPromise->adoptedPromise);
+
+        $result = null;
+
+        $fulfilledPromise->then(function ($value) use (&$result) {
+            $result = $value;
+        });
+
+        $this->assertSame(1, $result);
+    }
+
+    public function testCreateRejected()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $rejectedPromise = $reactAdapter->createRejected(new \Exception('I am a bad promise'));
+
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $rejectedPromise);
+        $this->assertInstanceOf('React\Promise\RejectedPromise', $rejectedPromise->adoptedPromise);
+
+        $exception = null;
+
+        $rejectedPromise->then(null, function ($error) use (&$exception) {
+            $exception = $error;
+        });
+
+        $this->assertInstanceOf('\Exception', $exception);
+        $this->assertEquals('I am a bad promise', $exception->getMessage());
+    }
+
+    public function testAll()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $promises = [new FulfilledPromise(1), new FulfilledPromise(2), new FulfilledPromise(3)];
+
+        $allPromise = $reactAdapter->all($promises);
+
+        $this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $allPromise);
+        $this->assertInstanceOf('React\Promise\FulfilledPromise', $allPromise->adoptedPromise);
+
+        $result = null;
+
+        $allPromise->then(function ($values) use (&$result) {
+           $result = $values;
+        });
+
+        $this->assertSame([1, 2, 3], $result);
+    }
+
+    public function testAllShouldPreserveTheOrderOfTheArrayWhenResolvingAsyncPromises()
+    {
+        $reactAdapter = new ReactPromiseAdapter();
+        $deferred = new Deferred();
+        $promises = [new FulfilledPromise(1), $deferred->promise(), new FulfilledPromise(3)];
+        $result = null;
+
+        $reactAdapter->all($promises)->then(function ($values) use (&$result) {
+            $result = $values;
+        });
+
+        // Resolve the async promise
+        $deferred->resolve(2);
+        $this->assertSame([1, 2, 3], $result);
+    }
+}