Skip to content

Commit 718b46b

Browse files
author
childish-sambino
authored
fix: drop the page limit calculation (#654)
Dropping the page limit calculation since it doesn't make sense to be calculated from a record limit.
1 parent 1fc7b86 commit 718b46b

File tree

2 files changed

+87
-60
lines changed

2 files changed

+87
-60
lines changed

‎src/Twilio/Version.php

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,21 +167,16 @@ public function delete(string $method, string $uri,
167167
}
168168

169169
public function readLimits(int $limit = null, int $pageSize = null): array {
170-
$pageLimit = Values::NONE;
171-
172-
if ($limit) {
173-
if ($pageSize === null) {
174-
$pageSize = \min($limit, self::MAX_PAGE_SIZE);
175-
}
176-
$pageLimit = (int)(\ceil($limit / (float)$pageSize));
170+
if ($limit && $pageSize === null) {
171+
$pageSize = $limit;
177172
}
178173

179174
$pageSize = \min($pageSize, self::MAX_PAGE_SIZE);
180175

181176
return [
182177
'limit' => $limit ?: Values::NONE,
183178
'pageSize' => $pageSize ?: Values::NONE,
184-
'pageLimit' => $pageLimit,
179+
'pageLimit' => Values::NONE,
185180
];
186181
}
187182

‎tests/Twilio/Unit/VersionTest.php

Lines changed: 84 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Twilio\Exceptions\RestException;
88
use Twilio\Http\CurlClient;
99
use Twilio\Http\Response;
10+
use Twilio\Page;
1011
use Twilio\Rest\Client;
1112
use Twilio\Values;
1213
use Twilio\Version;
@@ -51,6 +52,13 @@ public function setVersion(string $version): void {
5152
}
5253
}
5354

55+
56+
class TestPage extends Page {
57+
public function buildInstance(array $payload): array {
58+
return $payload;
59+
}
60+
}
61+
5462
class VersionTest extends UnitTest {
5563
protected $curlClient;
5664
/** @var Client $client */
@@ -77,76 +85,100 @@ protected function setUp(): void {
7785
* @param int|null $pageSize PageSize provided by the user
7886
* @param int|Values::NONE $expectedLimit Expected limit returned by readLimits
7987
* @param int|Values::NONE $expectedPageSize Expected page size returned by readLimits
80-
* @param int|Values::NONe $expectedPageLimit Expected page limit returned by readLimits
8188
* @dataProvider readLimitProvider
8289
*/
83-
public function testReadLimits(string $message, ?int $limit, ?int $pageSize, $expectedLimit, $expectedPageSize, $expectedPageLimit): void {
90+
public function testReadLimits(string $message, ?int $limit, ?int $pageSize, $expectedLimit, $expectedPageSize): void {
8491
$actual = $this->version->readLimits($limit, $pageSize);
85-
$this->assertEquals($expectedLimit, $actual['limit'], "$message: Limit does not match");
86-
$this->assertEquals($expectedPageSize, $actual['pageSize'], "$message: PageSize does not match");
87-
$this->assertEquals($expectedPageLimit, $actual['pageLimit'], "$message: PageLimit does not match");
92+
self::assertEquals($expectedLimit, $actual['limit'], "$message: Limit does not match");
93+
self::assertEquals($expectedPageSize, $actual['pageSize'], "$message: PageSize does not match");
8894
}
8995

9096
public function readLimitProvider(): array {
9197
return [
9298
[
9399
'Nothing Specified',
94100
null, null,
95-
Values::NONE, Values::NONE, Values::NONE,
101+
Values::NONE, Values::NONE,
96102
],
97103
[
98104
'Limit Specified - Under Max Page Size',
99105
Version::MAX_PAGE_SIZE - 1, null,
100-
Version::MAX_PAGE_SIZE - 1, Version::MAX_PAGE_SIZE - 1, 1,
106+
Version::MAX_PAGE_SIZE - 1, Version::MAX_PAGE_SIZE - 1,
101107
],
102108
[
103109
'Limit Specified - At Max Page Size',
104110
Version::MAX_PAGE_SIZE, null,
105-
Version::MAX_PAGE_SIZE, Version::MAX_PAGE_SIZE, 1,
111+
Version::MAX_PAGE_SIZE, Version::MAX_PAGE_SIZE,
106112
],
107113
[
108114
'Limit Specified - Over Max Page Size',
109115
Version::MAX_PAGE_SIZE + 1, null,
110-
Version::MAX_PAGE_SIZE + 1, Version::MAX_PAGE_SIZE, 2,
116+
Version::MAX_PAGE_SIZE + 1, Version::MAX_PAGE_SIZE,
111117
],
112118
[
113119
'Page Size Specified - Under Max Page Size',
114120
null, Version::MAX_PAGE_SIZE - 1,
115-
Values::NONE, Version::MAX_PAGE_SIZE - 1, Values::NONE,
121+
Values::NONE, Version::MAX_PAGE_SIZE - 1
116122
],
117123
[
118124
'Page Size Specified - At Max Page Size',
119125
null, Version::MAX_PAGE_SIZE,
120-
Values::NONE, Version::MAX_PAGE_SIZE, Values::NONE,
126+
Values::NONE, Version::MAX_PAGE_SIZE
121127
],
122128
[
123129
'Page Size Specified - Over Max Page Size',
124130
null, Version::MAX_PAGE_SIZE + 1,
125-
Values::NONE, Version::MAX_PAGE_SIZE, Values::NONE
126-
],
127-
[
128-
'Limit less than Page Size',
129-
50, 100,
130-
50, 100, 1,
131-
],
132-
[
133-
'Limit equal to Page Size',
134-
100, 100,
135-
100, 100, 1,
136-
],
137-
[
138-
'Limit greater than Page Size - evenly divisible',
139-
100, 50,
140-
100, 50, 2,
141-
],
142-
[
143-
'Limit greater than Page Size - not evenly divisible',
144-
100, 30,
145-
100, 30, 4
131+
Values::NONE, Version::MAX_PAGE_SIZE
146132
],
147133
];
148134
}
149135

136+
/**
137+
* @param string $message Case message to display on assertion error
138+
* @param int|null $limit Limit provided by the user
139+
* @param int|null $pageLimit Page limit provided by the user
140+
* @param int $expectedCount Expected record count returned by stream
141+
* @dataProvider streamProvider
142+
*/
143+
public function testStream(string $message, ?int $limit, ?int $pageLimit, int $expectedCount): void {
144+
$this->curlClient
145+
->method('request')
146+
->will(self::onConsecutiveCalls(
147+
new Response(
148+
200,
149+
'{
150+
"next_page_uri": "/2010-04-01/Accounts/AC123/Messages.json?Page=1",
151+
"messages": [{"body": "payload0"}, {"body": "payload1"}]
152+
}'),
153+
new Response(
154+
200,
155+
'{
156+
"next_page_uri": "/2010-04-01/Accounts/AC123/Messages.json?Page=2",
157+
"messages": [{"body": "payload2"}, {"body": "payload3"}]
158+
}'),
159+
new Response(
160+
200,
161+
'{
162+
"next_page_uri": null,
163+
"messages": [{"body": "payload4"}]
164+
}')
165+
));
166+
167+
$response = $this->version->page('GET', '/Accounts/AC123/Messages.json');
168+
$page = new TestPage($this->version, $response);
169+
$messages = $this->version->stream($page, $limit, $pageLimit);
170+
171+
self::assertEquals($expectedCount, \iterator_count($messages), "$message: Count does not match");
172+
}
173+
174+
public function streamProvider(): array {
175+
return [
176+
['No limits', null, null, 5],
177+
['Item limit', 3, null, 3],
178+
['Page limit', null, 1, 2],
179+
];
180+
}
181+
150182
/**
151183
* @param string $message Case message to display on assertion error
152184
* @param string $prefix Version prefix to test
@@ -157,7 +189,7 @@ public function readLimitProvider(): array {
157189
public function testRelativeUri(string $message, string $prefix, string $uri, string $expected): void {
158190
$this->version->setVersion($prefix);
159191
$actual = $this->version->relativeUri($uri);
160-
$this->assertEquals($expected, $actual, $message);
192+
self::assertEquals($expected, $actual, $message);
161193
}
162194

163195
public function relativeUriProvider(): array {
@@ -225,12 +257,12 @@ public function relativeUriProvider(): array {
225257
public function testAbsoluteUrl(string $message, string $prefix, string $uri, string $expected): void {
226258
$this->version->setVersion($prefix);
227259
$actual = $this->version->absoluteUrl($uri);
228-
$this->assertEquals($expected, $actual, $message);
260+
self::assertEquals($expected, $actual, $message);
229261
}
230262

231263
public function testRestExceptionWithDetails(): void {
232264
$this->curlClient
233-
->expects($this->once())
265+
->expects(self::once())
234266
->method('request')
235267
->willReturn(new Response(400, '{
236268
"code": 20001,
@@ -244,17 +276,17 @@ public function testRestExceptionWithDetails(): void {
244276
$this->version->fetch('get', 'http://foo.bar');
245277
self::fail();
246278
}catch (RestException $rex){
247-
$this->assertEquals(20001, $rex->getCode());
248-
$this->assertEquals(400, $rex->getStatusCode());
249-
$this->assertEquals('[HTTP 400] Unable to fetch record: Bad request', $rex->getMessage());
250-
$this->assertEquals('https://www.twilio.com/docs/errors/20001', $rex->getMoreInfo());
251-
$this->assertEquals(["foo" => "bar"], $rex->getDetails());
279+
self::assertEquals(20001, $rex->getCode());
280+
self::assertEquals(400, $rex->getStatusCode());
281+
self::assertEquals('[HTTP 400] Unable to fetch record: Bad request', $rex->getMessage());
282+
self::assertEquals('https://www.twilio.com/docs/errors/20001', $rex->getMoreInfo());
283+
self::assertEquals(["foo" => "bar"], $rex->getDetails());
252284
}
253285
}
254286

255287
public function testRestExceptionWithoutDetails(): void {
256288
$this->curlClient
257-
->expects($this->once())
289+
->expects(self::once())
258290
->method('request')
259291
->willReturn(new Response(400, '{
260292
"code": 20001,
@@ -266,28 +298,28 @@ public function testRestExceptionWithoutDetails(): void {
266298
$this->version->fetch('get', 'http://foo.bar');
267299
self::fail();
268300
}catch (RestException $rex){
269-
$this->assertEquals(20001, $rex->getCode());
270-
$this->assertEquals(400, $rex->getStatusCode());
271-
$this->assertEquals('[HTTP 400] Unable to fetch record: Bad request', $rex->getMessage());
272-
$this->assertEquals('https://www.twilio.com/docs/errors/20001', $rex->getMoreInfo());
273-
$this->assertEmpty($rex->getDetails());
301+
self::assertEquals(20001, $rex->getCode());
302+
self::assertEquals(400, $rex->getStatusCode());
303+
self::assertEquals('[HTTP 400] Unable to fetch record: Bad request', $rex->getMessage());
304+
self::assertEquals('https://www.twilio.com/docs/errors/20001', $rex->getMoreInfo());
305+
self::assertEmpty($rex->getDetails());
274306
}
275307
}
276308

277309
public function testRestException(): void {
278310
$this->curlClient
279-
->expects($this->once())
311+
->expects(self::once())
280312
->method('request')
281313
->willReturn(new Response(400, ''));
282314
try {
283315
$this->version->fetch('get', 'http://foo.bar');
284316
self::fail();
285317
}catch (RestException $rex){
286-
$this->assertEquals(400, $rex->getCode());
287-
$this->assertEquals(400, $rex->getStatusCode());
288-
$this->assertEquals('[HTTP 400] Unable to fetch record', $rex->getMessage());
289-
$this->assertEquals('', $rex->getMoreInfo());
290-
$this->assertEmpty($rex->getDetails());
318+
self::assertEquals(400, $rex->getCode());
319+
self::assertEquals(400, $rex->getStatusCode());
320+
self::assertEquals('[HTTP 400] Unable to fetch record', $rex->getMessage());
321+
self::assertEquals('', $rex->getMoreInfo());
322+
self::assertEmpty($rex->getDetails());
291323
}
292324
}
293325

0 commit comments

Comments
 (0)