From 1680c473492dea880efdf3d98b57ff08d83871b6 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Mon, 26 Aug 2019 14:08:11 -0700 Subject: [PATCH] Collection improvements / fixes --- lib/ApiOperations/All.php | 2 +- lib/Collection.php | 131 ++++++++++++++++++++++++++++---- tests/Stripe/CollectionTest.php | 70 +++++++++++++++++ 3 files changed, 189 insertions(+), 14 deletions(-) diff --git a/lib/ApiOperations/All.php b/lib/ApiOperations/All.php index 6ae462713..e59e80b41 100644 --- a/lib/ApiOperations/All.php +++ b/lib/ApiOperations/All.php @@ -28,7 +28,7 @@ public static function all($params = null, $opts = null) ); } $obj->setLastResponse($response); - $obj->setRequestParams($params); + $obj->setFilters($params); return $obj; } } diff --git a/lib/Collection.php b/lib/Collection.php index f51a3ce41..07f87bb16 100644 --- a/lib/Collection.php +++ b/lib/Collection.php @@ -18,7 +18,7 @@ class Collection extends StripeObject implements \IteratorAggregate use ApiOperations\Request; - protected $_requestParams = []; + protected $filters = []; /** * @return string The base URL for the given class. @@ -28,31 +28,67 @@ public static function baseUrl() return Stripe::$apiBase; } - public function setRequestParams($params) + /** + * Returns the filters. + * + * @param array $filters The filters. + */ + public function getFilters($filters) { - $this->_requestParams = $params; + return $this->filters; + } + + /** + * Sets the filters. + * + * @return array The filters. + */ + public function setFilters($filters) + { + $this->filters = $filters; + } + + public function offsetGet($k) + { + if (is_string($k)) { + return parent::offsetGet($k); + } else { + $msg = "You tried to access the {$k} index, but Collection " . + "types only support string keys. (HINT: List calls " . + "return an object with a `data` (which is the data " . + "array). You likely want to call ->data[{$k}])"; + throw new \InvalidArgumentException($msg); + } } public function all($params = null, $opts = null) { + self::_validateParams($params); list($url, $params) = $this->extractPathAndUpdateParams($params); list($response, $opts) = $this->_request('get', $url, $params, $opts); - $this->_requestParams = $params; - return Util\Util::convertToStripeObject($response, $opts); + $obj = Util\Util::convertToStripeObject($response, $opts); + if (!($obj instanceof \Stripe\Collection)) { + throw new \Stripe\Error\Api( + 'Expected type ' . \Stripe\Collection::class . ', got "' . get_class($obj) . '" instead.' + ); + } + $obj->setFilters($params); + return $obj; } public function create($params = null, $opts = null) { + self::_validateParams($params); list($url, $params) = $this->extractPathAndUpdateParams($params); list($response, $opts) = $this->_request('post', $url, $params, $opts); - $this->_requestParams = $params; return Util\Util::convertToStripeObject($response, $opts); } public function retrieve($id, $params = null, $opts = null) { + self::_validateParams($params); list($url, $params) = $this->extractPathAndUpdateParams($params); $id = Util\Util::utf8($id); @@ -63,7 +99,6 @@ public function retrieve($id, $params = null, $opts = null) $params, $opts ); - $this->_requestParams = $params; return Util\Util::convertToStripeObject($response, $opts); } @@ -88,19 +123,89 @@ public function autoPagingIterator() $params = $this->_requestParams; while (true) { - $itemId = null; foreach ($page as $item) { - $itemId = $item['id']; yield $item; } - if (!$page['has_more'] || is_null($itemId)) { - return; + $page = $page->nextPage(); + + if ($page->isEmpty()) { + break; } + } + } + + /** + * Returns an empty collection. This is returned from {@see nextPage()} + * when we know that there isn't a next page in order to replicate the + * behavior of the API when it attempts to return a page beyond the last. + * + * @param array|string|null $opts + * @return Collection + */ + public static function emptyCollection($opts = null) + { + return Collection::constructFrom(['data' => []], $opts); + } - $params = array_merge($params ?: [], ['starting_after' => $itemId]); - $page = $this->all($params, $this->_opts); + /** + * Returns true if the page object contains no element. + * + * @return boolean + */ + public function isEmpty() + { + return empty($this->data); + } + + /** + * Fetches the next page in the resource list (if there is one). + * + * This method will try to respect the limit of the current page. If none + * was given, the default limit will be fetched again. + * + * @param array|null $params + * @param array|string|null $opts + * @return Collection + */ + public function nextPage($params = null, $opts = null) + { + if (!$this->has_more) { + return static::emptyCollection($opts); } + + $lastId = end($this->data)->id; + + $params = array_merge( + $this->filters, + ['starting_after' => $lastId], + $params ?: [] + ); + + return $this->all($params, $opts); + } + + /** + * Fetches the previous page in the resource list (if there is one). + * + * This method will try to respect the limit of the current page. If none + * was given, the default limit will be fetched again. + * + * @param array|null $params + * @param array|string|null $opts + * @return Collection + */ + public function previousPage($params = null, $opts = null) + { + $firstId = $this->data[0]->id; + + $params = array_merge( + $this->filters, + ['ending_before' => $firstId], + $params ?: [] + ); + + return $this->all($params, $opts); } private function extractPathAndUpdateParams($params) diff --git a/tests/Stripe/CollectionTest.php b/tests/Stripe/CollectionTest.php index 35b110653..890c6537c 100644 --- a/tests/Stripe/CollectionTest.php +++ b/tests/Stripe/CollectionTest.php @@ -16,6 +16,15 @@ public function setUpFixture() ]); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessageRegExp /You tried to access the \d index/ + */ + public function testOffsetGetNumericIndex() + { + $this->fixture[0]; + } + public function testCanList() { $this->stubRequest( @@ -167,4 +176,65 @@ public function testHeaders() 'idempotency_key' => 'qwertyuiop', ]); } + + public function testEmptyCollection() + { + $emptyCollection = Collection::emptyCollection(); + $this->assertEquals([], $emptyCollection->data); + } + + public function testIsEmpty() + { + $empty = Collection::constructFrom(['data' => []]); + $this->assertTrue($empty->isEmpty()); + + $notEmpty = Collection::constructFrom(['data' => [['id' => 1]]]); + $this->assertFalse($notEmpty->isEmpty()); + } + + public function testNextPage() + { + $this->stubRequest( + 'GET', + '/things', + [ + 'starting_after' => 1, + ], + null, + false, + [ + 'object' => 'list', + 'data' => [['id' => 2], ['id' => 3]], + 'has_more' => false, + ] + ); + + $nextPage = $this->fixture->nextPage(); + $ids = []; + foreach ($nextPage->data as $element) { + array_push($ids, $element['id']); + } + $this->assertEquals([2, 3], $ids); + } + + public function testPreviousPage() + { + $this->stubRequest( + 'GET', + '/things', + [ + 'ending_before' => 1, + ], + null, + false, + [ + 'object' => 'list', + 'data' => [], + 'has_more' => false, + ] + ); + + $previousPage = $this->fixture->previousPage(); + $this->assertEquals([], $previousPage->data); + } }