Skip to content

Commit e0d17af

Browse files
AxaliaNtbolier
AxaliaN
authored and
tbolier
committed
Fixed cursor iteration seek and rewind bugs
1 parent 54342e8 commit e0d17af

File tree

6 files changed

+126
-34
lines changed

6 files changed

+126
-34
lines changed

‎src/Connection/Connection.php‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
declare(strict_types = 1);
2+
declare(strict_types=1);
33

44
namespace TBolier\RethinkQL\Connection;
55

@@ -132,10 +132,7 @@ public function reconnect($noreplyWait = true): Connection
132132
*/
133133
public function continueQuery(int $token): ResponseInterface
134134
{
135-
$message = (new Message())->setQuery(
136-
[QueryType::CONTINUE]
137-
);
138-
135+
$message = new Message(QueryType::CONTINUE);
139136
$this->writeQuery($token, $message);
140137

141138
// Await the response
@@ -224,10 +221,10 @@ public function server(): ResponseInterface
224221
try {
225222
$token = $this->generateToken();
226223

227-
$query = new Message(QueryType::SERVER_INFO);
228-
$this->writeQuery($token, $query);
224+
$message = new Message(QueryType::SERVER_INFO);
225+
$this->writeQuery($token, $message);
229226

230-
$response = $this->receiveResponse($token, $query);
227+
$response = $this->receiveResponse($token, $message);
231228

232229
if ($response->getType() !== 5) {
233230
throw new ConnectionException('Unexpected response type for server query.');
@@ -245,9 +242,7 @@ public function server(): ResponseInterface
245242
*/
246243
public function stopQuery(int $token): ResponseInterface
247244
{
248-
$message = (new Message())->setQuery(
249-
[QueryType::STOP]
250-
);
245+
$message = new Message(QueryType::STOP);
251246

252247
$this->writeQuery($token, $message);
253248

@@ -283,9 +278,9 @@ public function writeQuery(int $token, MessageInterface $message): int
283278
}
284279

285280
$requestSize = pack('V', \strlen($request));
286-
$binaryToken = pack('V', $token).pack('V', 0);
281+
$binaryToken = pack('V', $token) . pack('V', 0);
287282

288-
return $this->stream->write($binaryToken.$requestSize.$request);
283+
return $this->stream->write($binaryToken . $requestSize . $request);
289284
}
290285

291286
/**
@@ -298,10 +293,10 @@ public function noreplyWait(): void
298293
try {
299294
$token = $this->generateToken();
300295

301-
$query = new Message(QueryType::NOREPLY_WAIT);
302-
$this->writeQuery($token, $query);
296+
$message = new Message(QueryType::NOREPLY_WAIT);
297+
$this->writeQuery($token, $message);
303298

304-
$response = $this->receiveResponse($token, $query);
299+
$response = $this->receiveResponse($token, $message);
305300

306301
if ($response->getType() !== 4) {
307302
throw new ConnectionException('Unexpected response type for noreplyWait query.');
@@ -396,22 +391,22 @@ private function validateResponse(
396391
}
397392

398393
if ($response->getType() === ResponseType::CLIENT_ERROR) {
399-
throw new ConnectionException('Client error: '.$response->getData()[0].' jsonQuery: '.json_encode($message));
394+
throw new ConnectionException('Client error: ' . $response->getData()[0] . ' jsonQuery: ' . json_encode($message));
400395
}
401396

402397
if ($responseToken !== $token) {
403398
throw new ConnectionException(
404399
'Received wrong token. Response does not match the request. '
405-
. 'Expected '.$token.', received '.$responseToken
400+
. 'Expected ' . $token . ', received ' . $responseToken
406401
);
407402
}
408403

409404
if ($response->getType() === ResponseType::COMPILE_ERROR) {
410-
throw new ConnectionException('Compile error: '.$response->getData()[0].', jsonQuery: '.json_encode($message));
405+
throw new ConnectionException('Compile error: ' . $response->getData()[0] . ', jsonQuery: ' . json_encode($message));
411406
}
412407

413408
if ($response->getType() === ResponseType::RUNTIME_ERROR) {
414-
throw new ConnectionException('Runtime error: '.$response->getData()[0].', jsonQuery: '.json_encode($message));
409+
throw new ConnectionException('Runtime error: ' . $response->getData()[0] . ', jsonQuery: ' . json_encode($message));
415410
}
416411
}
417412
}

‎src/Response/Cursor.php‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,11 @@ public function valid(): bool
109109
*/
110110
public function rewind(): void
111111
{
112-
$this->close();
112+
if ($this->index === 0) {
113+
return;
114+
}
113115

116+
$this->close();
114117
$this->addResponse($this->connection->rewindFromCursor($this->message));
115118
}
116119

@@ -139,11 +142,7 @@ private function addResponse(ResponseInterface $response)
139142
*/
140143
private function seek(): void
141144
{
142-
while ($this->index === $this->size) {
143-
if ($this->isComplete) {
144-
return;
145-
}
146-
145+
if ($this->index >= $this->size && !$this->isComplete) {
147146
$this->request();
148147
}
149148
}

‎test/integration/AbstractTestCase.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,19 @@ protected function r()
4646
return $this->r;
4747
}
4848

49+
$name = 'phpunit_default';
50+
$options = new Options(PHPUNIT_CONNECTIONS[$name]);
51+
4952
/** @var ConnectionInterface $connection */
50-
$connection = $this->createConnection('phpunit_default')->connect();
51-
$connection->connect()->use('test');
53+
$connection = $this->createConnection($name)->connect();
54+
$connection->connect()->use($options->getDbName());
5255

5356
$this->r = new Rethink($connection);
5457

5558
/** @var ResponseInterface $res */
5659
$res = $this->r->dbList()->run();
57-
if (\is_array($res->getData()) && !\in_array('test', $res->getData(), true)) {
58-
$this->r->dbCreate('test')->run();
60+
if (\is_array($res->getData()) && !\in_array($options->getDbName(), $res->getData(), true)) {
61+
$this->r->dbCreate($options->getDbName())->run();
5962
}
6063

6164
return $this->r;
@@ -67,10 +70,13 @@ protected function tearDown()
6770
return;
6871
}
6972

73+
$name = 'phpunit_default';
74+
$options = new Options(PHPUNIT_CONNECTIONS[$name]);
75+
7076
/** @var ResponseInterface $res */
7177
$res = $this->r->dbList()->run();
72-
if (\is_array($res->getData()) && \in_array('test', $res->getData(), true)) {
73-
$this->r->dbDrop('test')->run();
78+
if (\is_array($res->getData()) && \in_array($options->getDbName(), $res->getData(), true)) {
79+
$this->r->dbDrop($options->getDbName())->run();
7480
}
7581
}
7682

‎test/integration/Query/AbstractTableTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ public function tearDown()
2424
if (\is_array($res->getData()) && \in_array('tabletest', $res->getData(), true)) {
2525
$this->r()->db()->tableDrop('tabletest')->run();
2626
}
27-
28-
parent::tearDown();
2927
}
3028

3129
/**

‎test/integration/Query/CursorTest.php‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,31 @@
33

44
namespace TBolier\RethinkQL\IntegrationTest\Query;
55

6+
use TBolier\RethinkQL\Response\Cursor;
67
use TBolier\RethinkQL\Response\ResponseInterface;
78

89
class CursorTest extends AbstractTableTest
910
{
11+
public function setUp()
12+
{
13+
parent::setUp();
14+
15+
$res = $this->r()->db()->tableList()->run();
16+
if (\is_array($res->getData()) && !\in_array('tabletest_big', $res->getData(), true)) {
17+
$this->r()->db()->tableCreate('tabletest_big')->run();
18+
}
19+
}
20+
21+
public function tearDown()
22+
{
23+
parent::tearDown();
24+
25+
$res = $this->r()->db()->tableList()->run();
26+
if (\is_array($res->getData()) && \in_array('tabletest_big', $res->getData(), true)) {
27+
$this->r()->db()->tableDrop('tabletest_big')->run();
28+
}
29+
}
30+
1031
/**
1132
* @throws \Exception
1233
*/
@@ -22,6 +43,52 @@ public function testCursor()
2243
$this->assertEquals(1000, $response->getData());
2344
}
2445

46+
/**
47+
* @throws \Exception
48+
*/
49+
public function testCursorBigDocuments()
50+
{
51+
$data = [
52+
'Professor X' => 'Charles Francis Xavier',
53+
'Cyclops' => 'Scott Summers',
54+
'Iceman' => 'Robert Louis "Bobby" Drake',
55+
'Angel' => 'Warren Kenneth Worthington III',
56+
'Beast' => 'Henry Philip "Hank" McCoy',
57+
'Marvel Girl/Phoenix' => 'Jean Elaine Grey/Jean Elaine Grey-Summers',
58+
'Magnetrix/Polaris' => 'Lorna Sally Dane',
59+
'Nightcrawler' => 'Kurt Wagner',
60+
'Wolverine' => 'James "Logan" Howlett',
61+
'Storm' => 'Ororo Monroe',
62+
'Colossus' => 'Piotr Nikolaievitch "Peter" Rasputin',
63+
'Sprite/Ariel/Shadowcat' => 'Katherine Anne "Kitty" Pryde',
64+
'Rogue' => 'Anna Marie',
65+
'Phoenix/Marvel Girl/Prestige' => 'Rachel Anne Grey-Summers',
66+
'Psylocke' => 'Elizabeth "Betsy" Braddock',
67+
'Gambit' => 'Rémy Etienne LeBeau',
68+
'Jubilee' => 'Jubilation Lee',
69+
'Bishop' => 'Lucas Bishop',
70+
];
71+
72+
$this->insertBigDocuments($data);
73+
74+
$cursor = $this->r()->table('tabletest_big')->run();
75+
76+
$i = 0;
77+
foreach ($cursor as $document) {
78+
// Assert the document every 1000s documents.
79+
if ($i % 1000 === 0) {
80+
$this->assertArraySubset($data, $document);
81+
}
82+
$i++;
83+
}
84+
85+
$this->assertEquals($i, $this->r()->table('tabletest_big')->count()->run()->getData());
86+
87+
$this->assertInstanceOf(Cursor::class, $cursor);
88+
89+
$this->r()->table('tabletest_big')->delete()->run();
90+
}
91+
2592
/**
2693
* @return ResponseInterface
2794
* @throws \Exception
@@ -48,4 +115,30 @@ private function insertDocuments(): ResponseInterface
48115

49116
return $res;
50117
}
118+
119+
/**
120+
* @param array $data
121+
* @return ResponseInterface
122+
* @throws \Exception
123+
*/
124+
private function insertBigDocuments(array $data): ResponseInterface
125+
{
126+
for ($i = 1; $i <= 100; $i++) {
127+
$documents = [];
128+
129+
for ($x = 1; $x <= 100; $x++) {
130+
$documents[] = $data;
131+
}
132+
133+
/** @var ResponseInterface $res */
134+
$res = $this->r()
135+
->table('tabletest_big')
136+
->insert($documents)
137+
->run();
138+
139+
$this->assertEquals(100, $res->getData()['inserted']);
140+
}
141+
142+
return $res;
143+
}
51144
}

‎test/unit/Connection/ConnectionQueryTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
use TBolier\RethinkQL\Message\MessageInterface;
77
use TBolier\RethinkQL\Response\ResponseInterface;
8+
use TBolier\RethinkQL\Types\Query\QueryType;
89
use TBolier\RethinkQL\Types\Response\ResponseType;
910

1011
class ConnectionQueryTest extends ConnectionTestCase

0 commit comments

Comments
 (0)