Skip to content

Commit 8f6b811

Browse files
authored
Merge pull request #6 from mcg-web/abstract-promise
Abstract Promise
2 parents a82efd7 + 28306ac commit 8f6b811

File tree

7 files changed

+165
-128
lines changed

7 files changed

+165
-128
lines changed

.travis.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ cache:
2323
before_install:
2424
- if [[ "$TRAVIS_PHP_VERSION" != "5.6" && "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then phpenv config-rm xdebug.ini || true; fi
2525
- composer selfupdate
26+
- composer require "guzzlehttp/promises"
2627

2728
install: composer update --prefer-dist --no-interaction
2829

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ data sources such as databases or web services via batching and caching.
1111

1212
## Requirements
1313

14-
This library require [React/Promise](https://github.com/reactphp/promise) and PHP >= 5.5 to works.
14+
This library require PHP >= 5.5 to works.
1515

1616
## Getting Started
1717

@@ -31,8 +31,9 @@ Create loaders by providing a batch loading instance.
3131
use Overblog\DataLoader\DataLoader;
3232

3333
$myBatchGetUsers = function ($keys) { /* ... */ };
34+
$promiseFactory = new MyPromiseFactory();
3435

35-
$userLoader = new DataLoader($myBatchGetUsers);
36+
$userLoader = new DataLoader($myBatchGetUsers, $promiseFactory);
3637
```
3738

3839
A batch loading callable / callback accepts an Array of keys, and returns a Promise which
@@ -122,11 +123,12 @@ Each `DataLoaderPHP` instance contains a unique memoized cache. Use caution when
122123
used in long-lived applications or those which serve many users with different
123124
access permissions and consider creating a new instance per web request.
124125

125-
##### `new DataLoader(callable $batchLoadFn [, Option $options])`
126+
##### `new DataLoader(callable $batchLoadFn, PromiseFactoryInterface $promiseFactory [, Option $options])`
126127

127128
Create a new `DataLoaderPHP` given a batch loading instance and options.
128129

129130
- *$batchLoadFn*: A callable / callback which accepts an Array of keys, and returns a Promise which resolves to an Array of values.
131+
- *$promiseFactory*: Any object that implements `McGWeb\PromiseFactory\PromiseFactoryInterface`. (see [McGWeb/Promise-Factory](https://github.com/mcg-web/promise-factory))
130132
- *$options*: An optional object of options:
131133

132134
- *batch*: Default `true`. Set to `false` to disable batching, instead

composer.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
"name": "overblog/dataloader-php",
33
"type": "library",
44
"license": "MIT",
5+
"description": "DataLoaderPhp is a generic utility to be used as part of your application's data fetching layer to provide a simplified and consistent API over various remote data sources such as databases or web services via batching and caching.",
6+
"keywords": ["dataLoader", "caching", "batching"],
57
"autoload": {
68
"psr-4": {
79
"Overblog\\DataLoader\\": "src/"
@@ -14,7 +16,7 @@
1416
},
1517
"require": {
1618
"php": "^5.5|^7.0",
17-
"react/promise": "^2.4"
19+
"mcg-web/promise-factory": "^0.2"
1820
},
1921
"require-dev": {
2022
"phpunit/phpunit": "^4.1|^5.1"

src/DataLoader.php

Lines changed: 60 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace Overblog\DataLoader;
1313

14-
use React\Promise\Promise;
14+
use McGWeb\PromiseFactory\PromiseFactoryInterface;
1515

1616
class DataLoader
1717
{
@@ -31,7 +31,7 @@ class DataLoader
3131
private $promiseCache;
3232

3333
/**
34-
* @var Promise[]
34+
* @var array
3535
*/
3636
private $queue = [];
3737

@@ -40,9 +40,15 @@ class DataLoader
4040
*/
4141
private static $instances = [];
4242

43-
public function __construct(callable $batchLoadFn, Option $options = null)
43+
/**
44+
* @var PromiseFactoryInterface
45+
*/
46+
private $promiseFactory;
47+
48+
public function __construct(callable $batchLoadFn, PromiseFactoryInterface $promiseFactory, Option $options = null)
4449
{
4550
$this->batchLoadFn = $batchLoadFn;
51+
$this->promiseFactory = $promiseFactory;
4652
$this->options = $options ?: new Option();
4753
$this->promiseCache = $this->options->getCacheMap();
4854
self::$instances[] = $this;
@@ -53,7 +59,7 @@ public function __construct(callable $batchLoadFn, Option $options = null)
5359
*
5460
* @param string $key
5561
*
56-
* @return Promise
62+
* @return mixed return a Promise
5763
*/
5864
public function load($key)
5965
{
@@ -70,33 +76,34 @@ public function load($key)
7076
return $cachedPromise;
7177
}
7278
}
73-
$promise = null;
7479

7580
// Otherwise, produce a new Promise for this value.
76-
$promise = new Promise(
77-
function ($resolve, $reject) use (&$promise, $key, $shouldBatch) {
78-
$this->queue[] = [
79-
'key' => $key,
80-
'resolve' => $resolve,
81-
'reject' => $reject,
82-
'promise' => &$promise,
83-
];
84-
85-
// Determine if a dispatch of this queue should be scheduled.
86-
// A single dispatch should be scheduled per queue at the time when the
87-
// queue changes from "empty" to "full".
88-
if (count($this->queue) === 1) {
89-
if (!$shouldBatch) {
90-
// Otherwise dispatch the (queue of one) immediately.
91-
$this->dispatchQueue();
92-
}
93-
}
94-
},
95-
function (callable $resolve, callable $reject) {
81+
$promise = $this->getPromiseFactory()->create(
82+
$resolve,
83+
$reject,
84+
function () {
9685
// Cancel/abort any running operations like network connections, streams etc.
9786

98-
$reject(new \RuntimeException('DataLoader destroyed before promise complete.'));
99-
});
87+
throw new \RuntimeException('DataLoader destroyed before promise complete.');
88+
}
89+
);
90+
91+
$this->queue[] = [
92+
'key' => $key,
93+
'resolve' => $resolve,
94+
'reject' => $reject,
95+
'promise' => $promise,
96+
];
97+
98+
// Determine if a dispatch of this queue should be scheduled.
99+
// A single dispatch should be scheduled per queue at the time when the
100+
// queue changes from "empty" to "full".
101+
if (count($this->queue) === 1) {
102+
if (!$shouldBatch) {
103+
// Otherwise dispatch the (queue of one) immediately.
104+
$this->dispatchQueue();
105+
}
106+
}
100107
// If caching, cache this promise.
101108
if ($shouldCache) {
102109
$this->promiseCache->set($cacheKey, $promise);
@@ -118,14 +125,14 @@ function (callable $resolve, callable $reject) {
118125
* ]);
119126
* @param array $keys
120127
*
121-
* @return Promise
128+
* @return mixed return a Promise
122129
*/
123130
public function loadMany($keys)
124131
{
125132
if (!is_array($keys) && !$keys instanceof \Traversable) {
126133
throw new \InvalidArgumentException(sprintf('The "%s" method must be called with Array<key> but got: %s.', __METHOD__, gettype($keys)));
127134
}
128-
return \React\Promise\all(array_map(
135+
return $this->getPromiseFactory()->createAll(array_map(
129136
function ($key) {
130137
return $this->load($key);
131138
},
@@ -178,7 +185,7 @@ public function prime($key, $value)
178185
if (!$this->promiseCache->has($cacheKey)) {
179186
// Cache a rejected promise if the value is an Error, in order to match
180187
// the behavior of load(key).
181-
$promise = $value instanceof \Exception ? \React\Promise\reject($value) : \React\Promise\resolve($value);
188+
$promise = $value instanceof \Exception ? $this->getPromiseFactory()->createReject($value) : $this->getPromiseFactory()->createResolve($value);
182189

183190
$this->promiseCache->set($cacheKey, $promise);
184191
}
@@ -191,13 +198,12 @@ public function __destruct()
191198
if ($this->needProcess()) {
192199
foreach ($this->queue as $data) {
193200
try {
194-
/** @var Promise $promise */
195-
$promise = $data['promise'];
196-
$promise->cancel();
201+
$this->getPromiseFactory()->cancel($data['promise']);
197202
} catch (\Exception $e) {
198203
// no need to do nothing if cancel failed
199204
}
200205
}
206+
$this->await();
201207
}
202208
foreach (self::$instances as $i => $instance) {
203209
if ($this !== $instance) {
@@ -215,10 +221,16 @@ protected function needProcess()
215221
protected function process()
216222
{
217223
if ($this->needProcess()) {
224+
$this->getPromiseFactory()->await();
218225
$this->dispatchQueue();
219226
}
220227
}
221228

229+
protected function getPromiseFactory()
230+
{
231+
return $this->promiseFactory;
232+
}
233+
222234
/**
223235
* @param $promise
224236
* @param bool $unwrap controls whether or not the value of the promise is returned for a fulfilled promise or if an exception is thrown if the promise is rejected
@@ -227,48 +239,28 @@ protected function process()
227239
*/
228240
public static function await($promise = null, $unwrap = true)
229241
{
230-
self::awaitInstances();
231-
232-
if (null === $promise) {
233-
return null;
234-
}
235-
$resolvedValue = null;
236-
$exception = null;
237-
238-
if (!is_callable([$promise, 'then'])) {
239-
throw new \InvalidArgumentException(sprintf('The "%s" method must be called with a Promise ("then" method).', __METHOD__));
240-
}
241-
242-
$promise->then(function ($values) use (&$resolvedValue) {
243-
$resolvedValue = $values;
244-
}, function ($reason) use (&$exception) {
245-
$exception = $reason;
246-
});
247-
if ($exception instanceof \Exception) {
248-
if (!$unwrap) {
249-
return $exception;
250-
}
251-
throw $exception;
242+
if (empty(self::$instances)) {
243+
throw new \RuntimeException('Found no active DataLoader instance.');
252244
}
245+
self::awaitInstances();
253246

254-
return $resolvedValue;
247+
return self::$instances[0]->getPromiseFactory()->await($promise, $unwrap);
255248
}
256249

257250
private static function awaitInstances()
258251
{
259252
$dataLoaders = self::$instances;
260-
if (!empty($dataLoaders)) {
261-
$wait = true;
262-
263-
while ($wait) {
264-
foreach ($dataLoaders as $dataLoader) {
265-
if (!$dataLoader || !$dataLoader->needProcess()) {
266-
$wait = false;
267-
continue;
268-
}
269-
$wait = true;
270-
$dataLoader->process();
253+
254+
$wait = true;
255+
256+
while ($wait) {
257+
foreach ($dataLoaders as $dataLoader) {
258+
if (!$dataLoader || !$dataLoader->needProcess()) {
259+
$wait = false;
260+
continue;
271261
}
262+
$wait = true;
263+
$dataLoader->process();
272264
}
273265
}
274266
}
@@ -322,7 +314,6 @@ private function dispatchQueueBatch(array $queue)
322314

323315
// Call the provided batchLoadFn for this loader with the loader queue's keys.
324316
$batchLoadFn = $this->batchLoadFn;
325-
/** @var Promise $batchPromise */
326317
$batchPromise = $batchLoadFn($keys);
327318

328319
// Assert the expected response from batchLoadFn
@@ -374,7 +365,7 @@ function ($values) use ($keys, $queue) {
374365
/**
375366
* Do not cache individual loads if the entire batch dispatch fails,
376367
* but still reject each request so they do not hang.
377-
* @param Promise[] $queue
368+
* @param array $queue
378369
* @param \Exception $error
379370
*/
380371
private function failedDispatch($queue, \Exception $error)

tests/AbuseTest.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use Overblog\DataLoader\DataLoader;
1515

16-
class AbuseTest extends \PHPUnit_Framework_TestCase
16+
class AbuseTest extends TestCase
1717
{
1818
/**
1919
* @group provides-descriptive-error-messages-for-api-abuse
@@ -75,7 +75,7 @@ public function testBatchFunctionMustReturnAPromiseNotAValue()
7575
public function testBatchFunctionMustReturnAPromiseOfAnArrayNotNull()
7676
{
7777
DataLoader::await(self::idLoader(function () {
78-
return \React\Promise\resolve();
78+
return self::$promiseFactory->createResolve(null);
7979
})->load(1));
8080
}
8181

@@ -87,20 +87,33 @@ public function testBatchFunctionMustReturnAPromiseOfAnArrayNotNull()
8787
public function testBatchFunctionMustPromiseAnArrayOfCorrectLength()
8888
{
8989
DataLoader::await(self::idLoader(function () {
90-
return \React\Promise\resolve([]);
90+
return self::$promiseFactory->createResolve([]);
9191
})->load(1));
9292
}
9393

9494
/**
9595
* @group provides-descriptive-error-messages-for-api-abuse
9696
* @expectedException \InvalidArgumentException
97-
* @expectedExceptionMessage The "Overblog\DataLoader\DataLoader::await" method must be called with a Promise ("then" method).
97+
* @expectedExceptionMessage ::await" method must be called with a Promise ("then" method).
98+
* @runInSeparateProcess
9899
*/
99100
public function testAwaitPromiseMustHaveAThenMethod()
100101
{
102+
self::idLoader();
101103
DataLoader::await([]);
102104
}
103105

106+
/**
107+
* @group provides-descriptive-error-messages-for-api-abuse
108+
* @expectedException \RuntimeException
109+
* @expectedExceptionMessage Found no active DataLoader instance.
110+
* @runInSeparateProcess
111+
*/
112+
public function testAwaitWithoutNoInstance()
113+
{
114+
DataLoader::await();
115+
}
116+
104117
/**
105118
* @param callable $batchLoadFn
106119
* @return DataLoader
@@ -109,10 +122,10 @@ private static function idLoader(callable $batchLoadFn = null)
109122
{
110123
if (null === $batchLoadFn) {
111124
$batchLoadFn = function ($keys) {
112-
return \React\Promise\all($keys);
125+
return self::$promiseFactory->createAll($keys);
113126
};
114127
}
115128

116-
return new DataLoader($batchLoadFn);
129+
return new DataLoader($batchLoadFn, self::$promiseFactory);
117130
}
118131
}

0 commit comments

Comments
 (0)