Skip to content

Commit c4dd048

Browse files
author
Jeremiah VALERIE
committed
Add API Abuse tests
1 parent 1a58b6a commit c4dd048

File tree

5 files changed

+155
-97
lines changed

5 files changed

+155
-97
lines changed

‎.travis.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,4 @@ install: composer update --prefer-dist --no-interaction
2929
script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then phpunit -d xdebug.max_nesting_level=1000 --debug --coverage-clover build/logs/clover.xml; else phpunit --debug; fi
3030

3131
after_success:
32-
- if [[ "$TRAVIS_PHP_VERSION" != "5.6"]]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php vendor/bin/coveralls -v; fi
32+
- if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then composer require "satooshi/php-coveralls:^1.0" && travis_retry php vendor/bin/coveralls -v; fi

‎src/DataLoader.php‎

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function (callable $resolve, callable $reject) {
123123
public function loadMany($keys)
124124
{
125125
if (!is_array($keys) && !$keys instanceof \Traversable) {
126-
throw new \InvalidArgumentException(sprintf('The %s function must be called with Array<key> but got: %s.', __METHOD__, gettype($keys)));
126+
throw new \InvalidArgumentException(sprintf('The "%s" method must be called with Array<key> but got: %s.', __METHOD__, gettype($keys)));
127127
}
128128
return \React\Promise\all(array_map(
129129
function ($key) {
@@ -229,51 +229,45 @@ public static function await($promise = null, $unwrap = true)
229229
{
230230
self::awaitInstances();
231231

232-
if (null !== $promise) {
233-
$resolvedValue = null;
234-
$exception = null;
232+
if (null === $promise) {
233+
return null;
234+
}
235+
$resolvedValue = null;
236+
$exception = null;
235237

236-
if (!is_callable([$promise, 'then'])) {
237-
throw new \InvalidArgumentException('Promise must have a "then" method.');
238-
}
238+
if (!is_callable([$promise, 'then'])) {
239+
throw new \InvalidArgumentException(sprintf('The "%s" method must be called with a Promise ("then" method).', __METHOD__));
240+
}
239241

240-
$promise->then(function ($values) use (&$resolvedValue) {
241-
$resolvedValue = $values;
242-
}, function ($reason) use (&$exception) {
243-
$exception = $reason;
244-
});
245-
if ($exception instanceof \Exception) {
246-
if (!$unwrap) {
247-
return $exception;
248-
}
249-
throw $exception;
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;
250250
}
251-
252-
return $resolvedValue;
251+
throw $exception;
253252
}
253+
254+
return $resolvedValue;
254255
}
255256

256257
private static function awaitInstances()
257258
{
258259
$dataLoaders = self::$instances;
259-
if (empty($dataLoaders)) {
260-
return;
261-
}
260+
if (!empty($dataLoaders)) {
261+
$wait = true;
262262

263-
$wait = true;
264-
265-
while ($wait) {
266-
foreach ($dataLoaders as $dataLoader) {
267-
try {
263+
while ($wait) {
264+
foreach ($dataLoaders as $dataLoader) {
268265
if (!$dataLoader || !$dataLoader->needProcess()) {
269266
$wait = false;
270267
continue;
271268
}
272269
$wait = true;
273270
$dataLoader->process();
274-
} catch (\Exception $e) {
275-
$wait = false;
276-
continue;
277271
}
278272
}
279273
}
@@ -291,7 +285,7 @@ private function checkKey($key, $method)
291285
{
292286
if (null === $key) {
293287
throw new \InvalidArgumentException(
294-
sprintf('The %s function must be called with a value, but got: %s.', $method, gettype($key))
288+
sprintf('The "%s" method must be called with a value, but got: %s.', $method, gettype($key))
295289
);
296290
}
297291
}

‎src/Option.php‎

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
namespace Overblog\DataLoader;
1313

14-
use Symfony\Component\Cache\Adapter\AdapterInterface;
15-
1614
class Option
1715
{
1816
/**
@@ -52,9 +50,8 @@ public function __construct(array $params = [])
5250

5351
$options = array_merge($defaultOptions, $params);
5452

55-
foreach ($options as $name => $value) {
56-
$method = 'set'.ucfirst($name);
57-
$this->$method($value);
53+
foreach ($options as $property => $value) {
54+
$this->$property = $value;
5855
}
5956
}
6057

@@ -66,16 +63,6 @@ public function shouldBatch()
6663
return $this->batch;
6764
}
6865

69-
/**
70-
* @param boolean $batch
71-
* @return Option
72-
*/
73-
public function setBatch($batch)
74-
{
75-
$this->batch = $batch;
76-
return $this;
77-
}
78-
7966
/**
8067
* @return int
8168
*/
@@ -84,16 +71,6 @@ public function getMaxBatchSize()
8471
return $this->maxBatchSize;
8572
}
8673

87-
/**
88-
* @param int $maxBatchSize
89-
* @return Option
90-
*/
91-
public function setMaxBatchSize($maxBatchSize)
92-
{
93-
$this->maxBatchSize = $maxBatchSize;
94-
return $this;
95-
}
96-
9774
/**
9875
* @return boolean
9976
*/
@@ -102,16 +79,6 @@ public function shouldCache()
10279
return $this->cache;
10380
}
10481

105-
/**
106-
* @param boolean $cache
107-
* @return Option
108-
*/
109-
public function setCache($cache)
110-
{
111-
$this->cache = $cache;
112-
return $this;
113-
}
114-
11582
/**
11683
* @return callable
11784
*/
@@ -120,31 +87,11 @@ public function getCacheKeyFn()
12087
return $this->cacheKeyFn;
12188
}
12289

123-
/**
124-
* @param callable $cacheKeyFn
125-
* @return Option
126-
*/
127-
public function setCacheKeyFn(callable $cacheKeyFn = null)
128-
{
129-
$this->cacheKeyFn = $cacheKeyFn;
130-
return $this;
131-
}
132-
13390
/**
13491
* @return CacheMap
13592
*/
13693
public function getCacheMap()
13794
{
13895
return $this->cacheMap;
13996
}
140-
141-
/**
142-
* @param CacheMap $cacheMap
143-
* @return Option
144-
*/
145-
public function setCacheMap(CacheMap $cacheMap)
146-
{
147-
$this->cacheMap = $cacheMap;
148-
return $this;
149-
}
15097
}

‎tests/AbuseTest.php‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the DataLoaderPhp package.
5+
*
6+
* (c) Overblog <http://github.com/overblog/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Overblog\DataLoader\Tests;
13+
14+
use Overblog\DataLoader\DataLoader;
15+
16+
class AbuseTest extends \PHPUnit_Framework_TestCase
17+
{
18+
/**
19+
* @group provides-descriptive-error-messages-for-api-abuse
20+
*
21+
* @expectedException \InvalidArgumentException
22+
* @expectedExceptionMessage The "Overblog\DataLoader\DataLoader::load" method must be called with a value, but got: NULL.
23+
*/
24+
public function testLoadFunctionRequiresAKeyNotNull()
25+
{
26+
self::idLoader()->load(null);
27+
}
28+
29+
/**
30+
* @group provides-descriptive-error-messages-for-api-abuse
31+
*/
32+
public function testLoadFunctionRequiresAKeyWith0()
33+
{
34+
self::idLoader()->load(0);
35+
}
36+
37+
/**
38+
* @group provides-descriptive-error-messages-for-api-abuse
39+
*
40+
* @expectedException \InvalidArgumentException
41+
* @expectedExceptionMessage The "Overblog\DataLoader\DataLoader::loadMany" method must be called with Array<key> but got: integer.
42+
*/
43+
public function testLoadManyFunctionRequiresAListOfKey()
44+
{
45+
self::idLoader()->loadMany(1, 2, 3);
46+
}
47+
48+
/**
49+
* @group provides-descriptive-error-messages-for-api-abuse
50+
*/
51+
public function testLoadManyFunctionRequiresAListEmptyArrayAccepted()
52+
{
53+
self::idLoader()->loadMany([]);
54+
}
55+
56+
/**
57+
* @group provides-descriptive-error-messages-for-api-abuse
58+
*
59+
* @expectedException \RuntimeException
60+
* @expectedExceptionMessage DataLoader must be constructed with a function which accepts Array<key> and returns Promise<Array<value>>, but the function did not return a Promise: array.
61+
*/
62+
public function testBatchFunctionMustReturnAPromiseNotAValue()
63+
{
64+
DataLoader::await(self::idLoader(function ($keys) {
65+
return $keys;
66+
})->load(1));
67+
}
68+
69+
/**
70+
* @group provides-descriptive-error-messages-for-api-abuse
71+
*
72+
* @expectedException \RuntimeException
73+
* @expectedExceptionMessage DataLoader must be constructed with a function which accepts Array<key> and returns Promise<Array<value>>, but the function did not return a Promise of an Array: NULL.
74+
*/
75+
public function testBatchFunctionMustReturnAPromiseOfAnArrayNotNull()
76+
{
77+
DataLoader::await(self::idLoader(function () {
78+
return \React\Promise\resolve();
79+
})->load(1));
80+
}
81+
82+
/**
83+
* @group provides-descriptive-error-messages-for-api-abuse
84+
* @expectedException \RuntimeException
85+
* @expectedExceptionMessage DataLoader must be constructed with a function which accepts Array<key> and returns Promise<Array<value>>, but the function did not return a Promise of an Array of the same length as the Array of keys.
86+
*/
87+
public function testBatchFunctionMustPromiseAnArrayOfCorrectLength()
88+
{
89+
DataLoader::await(self::idLoader(function () {
90+
return \React\Promise\resolve([]);
91+
})->load(1));
92+
}
93+
94+
/**
95+
* @group provides-descriptive-error-messages-for-api-abuse
96+
* @expectedException \InvalidArgumentException
97+
* @expectedExceptionMessage The "Overblog\DataLoader\DataLoader::await" method must be called with a Promise ("then" method).
98+
*/
99+
public function testAwaitPromiseMustHaveAThenMethod()
100+
{
101+
DataLoader::await([]);
102+
}
103+
104+
/**
105+
* @param callable $batchLoadFn
106+
* @return DataLoader
107+
*/
108+
private static function idLoader(callable $batchLoadFn = null)
109+
{
110+
if (null === $batchLoadFn) {
111+
$batchLoadFn = function ($keys) {
112+
return \React\Promise\all($keys);
113+
};
114+
}
115+
116+
return new DataLoader($batchLoadFn);
117+
}
118+
}

‎tests/DataLoadTest.php‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -279,12 +279,7 @@ public function testResolvesToErrorToIndicateFailure()
279279
*/
280280
list($evenLoader, $loadCalls) = self::eventLoader();
281281

282-
$caughtError = null;
283-
try {
284-
DataLoader::await($evenLoader->load(1));
285-
} catch (\Exception $error) {
286-
$caughtError = $error;
287-
}
282+
$caughtError = DataLoader::await($evenLoader->load(1), false);
288283
$this->assertInstanceOf(\Exception::class, $caughtError);
289284
$this->assertEquals($caughtError->getMessage(), 'Odd: 1');
290285
$value2 = DataLoader::await($evenLoader->load(2));
@@ -308,11 +303,10 @@ public function testCanRepresentFailuresAndSuccessesSimultaneously()
308303
$promise2 = $evenLoader->load(2);
309304

310305
$caughtError = null;
311-
try {
312-
DataLoader::await($promise1);
313-
} catch (\Exception $error) {
306+
$promise1->then(null, function ($error) use (&$caughtError) {
314307
$caughtError = $error;
315-
}
308+
});
309+
DataLoader::await();
316310
$this->assertInstanceOf(\Exception::class, $caughtError);
317311
$this->assertEquals($caughtError->getMessage(), 'Odd: 1');
318312

@@ -788,6 +782,11 @@ public function testOnDestructionAllPromiseInQueueShouldBeCancelled()
788782
$this->assertEquals($exception->getMessage(), 'DataLoader destroyed before promise complete.');
789783
}
790784

785+
public function testCallingAwaitFunctionWhenNoInstanceOfDataLoaderShouldNotThrowError()
786+
{
787+
DataLoader::await();
788+
}
789+
791790
public function cacheKey($key)
792791
{
793792
$cacheKey = [];

0 commit comments

Comments
 (0)