Skip to content

Commit 595ae52

Browse files
vladarvladar
authored
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
2 parents ff3a40d + 8626e0b commit 595ae52

File tree

4 files changed

+179
-2
lines changed

4 files changed

+179
-2
lines changed

‎.travis.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ before_install:
2222

2323
install:
2424
- composer install --dev --prefer-dist
25+
- composer require react/promise:2.*
2526

26-
script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then phpunit --coverage-clover build/logs/clover.xml; else phpunit; fi
27+
script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then phpunit --coverage-clover build/logs/clover.xml --group default,ReactPromise; else phpunit --group default,ReactPromise; fi
2728

2829
after_success:
2930
- if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php bin/coveralls -v; fi

‎phpunit.xml.dist‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
</testsuite>
1818
</testsuites>
1919

20+
<groups>
21+
<exclude>
22+
<group>ReactPromise</group>
23+
</exclude>
24+
</groups>
25+
2026
<filter>
2127
<whitelist>
2228
<directory suffix=".php">./src</directory>

‎src/Executor/Promise/Adapter/ReactPromiseAdapter.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,16 @@ public function all(array $promisesOrValues)
7171
$promisesOrValues = Utils::map($promisesOrValues, function ($item) {
7272
return $item instanceof Promise ? $item->adoptedPromise : $item;
7373
});
74-
$promise = \React\Promise\all($promisesOrValues);
74+
75+
$promise = \React\Promise\all($promisesOrValues)->then(function($values) use ($promisesOrValues) {
76+
$orderedResults = [];
77+
78+
foreach ($promisesOrValues as $key => $value) {
79+
$orderedResults[$key] = $values[$key];
80+
}
81+
82+
return $orderedResults;
83+
});
7584
return new Promise($promise, $this);
7685
}
7786
}

‎tests/Executor/Promise/ReactPromiseAdapterTest.php‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
<?php
2+
3+
4+
namespace GraphQL\Tests\Executor\Promise;
5+
6+
7+
use GraphQL\Executor\Promise\Adapter\ReactPromiseAdapter;
8+
use GraphQL\Executor\Promise\Promise;
9+
use React\Promise\Deferred;
10+
use React\Promise\FulfilledPromise;
11+
use React\Promise\LazyPromise;
12+
use React\Promise\Promise as ReactPromise;
13+
use React\Promise\RejectedPromise;
14+
15+
/**
16+
* @group ReactPromise
17+
*/
18+
class ReactPromiseAdapterTest extends \PHPUnit_Framework_TestCase
19+
{
20+
public function setUp()
21+
{
22+
if(! class_exists('React\Promise\Promise')) {
23+
$this->markTestSkipped('react/promise package must be installed to run GraphQL\Tests\Executor\Promise\ReactPromiseAdapterTest');
24+
}
25+
}
26+
27+
public function testIsThenableReturnsTrueWhenAReactPromiseIsGiven()
28+
{
29+
$reactAdapter = new ReactPromiseAdapter();
30+
31+
$this->assertSame(true, $reactAdapter->isThenable(new ReactPromise(function() {})));
32+
$this->assertSame(true, $reactAdapter->isThenable(new FulfilledPromise()));
33+
$this->assertSame(true, $reactAdapter->isThenable(new RejectedPromise()));
34+
$this->assertSame(true, $reactAdapter->isThenable(new LazyPromise(function() {})));
35+
$this->assertSame(false, $reactAdapter->isThenable(false));
36+
$this->assertSame(false, $reactAdapter->isThenable(true));
37+
$this->assertSame(false, $reactAdapter->isThenable(1));
38+
$this->assertSame(false, $reactAdapter->isThenable(0));
39+
$this->assertSame(false, $reactAdapter->isThenable('test'));
40+
$this->assertSame(false, $reactAdapter->isThenable(''));
41+
$this->assertSame(false, $reactAdapter->isThenable([]));
42+
$this->assertSame(false, $reactAdapter->isThenable(new \stdClass()));
43+
}
44+
45+
public function testConvertsReactPromisesToGraphQlOnes()
46+
{
47+
$reactAdapter = new ReactPromiseAdapter();
48+
$reactPromise = new FulfilledPromise(1);
49+
50+
$promise = $reactAdapter->convertThenable($reactPromise);
51+
52+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $promise);
53+
$this->assertInstanceOf('React\Promise\FulfilledPromise', $promise->adoptedPromise);
54+
}
55+
56+
public function testThen()
57+
{
58+
$reactAdapter = new ReactPromiseAdapter();
59+
$reactPromise = new FulfilledPromise(1);
60+
$promise = $reactAdapter->convertThenable($reactPromise);
61+
62+
$result = null;
63+
64+
$resultPromise = $reactAdapter->then($promise, function ($value) use (&$result) {
65+
$result = $value;
66+
});
67+
68+
$this->assertSame(1, $result);
69+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $resultPromise);
70+
$this->assertInstanceOf('React\Promise\FulfilledPromise', $resultPromise->adoptedPromise);
71+
}
72+
73+
public function testCreate()
74+
{
75+
$reactAdapter = new ReactPromiseAdapter();
76+
$resolvedPromise = $reactAdapter->create(function ($resolve) {
77+
$resolve(1);
78+
});
79+
80+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $resolvedPromise);
81+
$this->assertInstanceOf('React\Promise\Promise', $resolvedPromise->adoptedPromise);
82+
83+
$result = null;
84+
85+
$resolvedPromise->then(function ($value) use (&$result) {
86+
$result = $value;
87+
});
88+
89+
$this->assertSame(1, $result);
90+
}
91+
92+
public function testCreateFulfilled()
93+
{
94+
$reactAdapter = new ReactPromiseAdapter();
95+
$fulfilledPromise = $reactAdapter->createFulfilled(1);
96+
97+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $fulfilledPromise);
98+
$this->assertInstanceOf('React\Promise\FulfilledPromise', $fulfilledPromise->adoptedPromise);
99+
100+
$result = null;
101+
102+
$fulfilledPromise->then(function ($value) use (&$result) {
103+
$result = $value;
104+
});
105+
106+
$this->assertSame(1, $result);
107+
}
108+
109+
public function testCreateRejected()
110+
{
111+
$reactAdapter = new ReactPromiseAdapter();
112+
$rejectedPromise = $reactAdapter->createRejected(new \Exception('I am a bad promise'));
113+
114+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $rejectedPromise);
115+
$this->assertInstanceOf('React\Promise\RejectedPromise', $rejectedPromise->adoptedPromise);
116+
117+
$exception = null;
118+
119+
$rejectedPromise->then(null, function ($error) use (&$exception) {
120+
$exception = $error;
121+
});
122+
123+
$this->assertInstanceOf('\Exception', $exception);
124+
$this->assertEquals('I am a bad promise', $exception->getMessage());
125+
}
126+
127+
public function testAll()
128+
{
129+
$reactAdapter = new ReactPromiseAdapter();
130+
$promises = [new FulfilledPromise(1), new FulfilledPromise(2), new FulfilledPromise(3)];
131+
132+
$allPromise = $reactAdapter->all($promises);
133+
134+
$this->assertInstanceOf('GraphQL\Executor\Promise\Promise', $allPromise);
135+
$this->assertInstanceOf('React\Promise\FulfilledPromise', $allPromise->adoptedPromise);
136+
137+
$result = null;
138+
139+
$allPromise->then(function ($values) use (&$result) {
140+
$result = $values;
141+
});
142+
143+
$this->assertSame([1, 2, 3], $result);
144+
}
145+
146+
public function testAllShouldPreserveTheOrderOfTheArrayWhenResolvingAsyncPromises()
147+
{
148+
$reactAdapter = new ReactPromiseAdapter();
149+
$deferred = new Deferred();
150+
$promises = [new FulfilledPromise(1), $deferred->promise(), new FulfilledPromise(3)];
151+
$result = null;
152+
153+
$reactAdapter->all($promises)->then(function ($values) use (&$result) {
154+
$result = $values;
155+
});
156+
157+
// Resolve the async promise
158+
$deferred->resolve(2);
159+
$this->assertSame([1, 2, 3], $result);
160+
}
161+
}

0 commit comments

Comments
 (0)