Skip to content

Commit bc5ecf3

Browse files
WyriHaximusWyriHaximus
authored
Merge pull request #189 from clue-labs/quit-quitting
Consistently emit close event after quit even if server rejects
2 parents 228bedd + 1e14223 commit bc5ecf3

6 files changed

Lines changed: 194 additions & 47 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,11 @@ $mysql->query('CREATE TABLE test ...');
348348
$mysql->quit();
349349
```
350350

351+
This method will gracefully close the connection to the MySQL database
352+
server once all outstanding commands are completed. See also
353+
[`close()`](#close) if you want to force-close the connection without
354+
waiting for any commands to complete instead.
355+
351356
#### close()
352357

353358
The `close(): void` method can be used to

‎src/Io/Connection.php‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,16 @@ public function ping()
135135
public function quit()
136136
{
137137
return new Promise(function ($resolve, $reject) {
138-
$this->_doCommand(new QuitCommand())
139-
->on('error', function ($reason) use ($reject) {
140-
$reject($reason);
141-
})
142-
->on('success', function () use ($resolve) {
143-
$this->state = self::STATE_CLOSED;
144-
$this->emit('end', [$this]);
145-
$this->emit('close', [$this]);
146-
$resolve(null);
147-
});
138+
$command = $this->_doCommand(new QuitCommand());
148139
$this->state = self::STATE_CLOSING;
140+
$command->on('success', function () use ($resolve) {
141+
$resolve(null);
142+
$this->close();
143+
});
144+
$command->on('error', function ($reason) use ($reject) {
145+
$reject($reason);
146+
$this->close();
147+
});
149148
});
150149
}
151150

‎src/MysqlClient.php‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
use React\EventLoop\LoopInterface;
88
use React\Mysql\Io\Connection;
99
use React\Mysql\Io\Factory;
10-
use React\Stream\ReadableStreamInterface;
10+
use React\Promise\Promise;
1111
use React\Socket\ConnectorInterface;
12+
use React\Stream\ReadableStreamInterface;
1213

1314
/**
1415
* This class represents a connection that is responsible for communicating
@@ -135,9 +136,8 @@ function () {
135136
// successfully disconnected => remove reference
136137
$this->disconnecting = null;
137138
},
138-
function () use ($connection) {
139-
// soft-close failed => force-close connection
140-
$connection->close();
139+
function () {
140+
// soft-close failed but will close anyway => remove reference
141141
$this->disconnecting = null;
142142
}
143143
);
@@ -361,6 +361,11 @@ function (\Exception $e) {
361361
* $mysql->quit();
362362
* ```
363363
*
364+
* This method will gracefully close the connection to the MySQL database
365+
* server once all outstanding commands are completed. See also
366+
* [`close()`](#close) if you want to force-close the connection without
367+
* waiting for any commands to complete instead.
368+
*
364369
* @return PromiseInterface<void>
365370
* Resolves with a `void` value on success or rejects with an `Exception` on error.
366371
*/
@@ -376,17 +381,25 @@ public function quit()
376381
return \React\Promise\resolve(null);
377382
}
378383

379-
return $this->connecting()->then(function (Connection $connection) {
380-
$this->awake();
381-
return $connection->quit()->then(
382-
function () {
383-
$this->close();
384-
},
385-
function (\Exception $e) {
386-
$this->close();
387-
throw $e;
388-
}
389-
);
384+
return new Promise(function (callable $resolve, callable $reject) {
385+
$this->connecting()->then(function (Connection $connection) use ($resolve, $reject) {
386+
$this->awake();
387+
// soft-close connection and emit close event afterwards both on success or on error
388+
$connection->quit()->then(
389+
function () use ($resolve){
390+
$resolve(null);
391+
$this->close();
392+
},
393+
function (\Exception $e) use ($reject) {
394+
$reject($e);
395+
$this->close();
396+
}
397+
);
398+
}, function (\Exception $e) use ($reject) {
399+
// emit close event afterwards when no connection can be established
400+
$reject($e);
401+
$this->close();
402+
});
390403
});
391404
}
392405

‎tests/Io/ConnectionTest.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,72 @@ public function testQuitWillEnqueueOneCommand()
1717
$conn->quit();
1818
}
1919

20+
public function testQuitWillResolveBeforeEmittingCloseEventWhenQuitCommandEmitsSuccess()
21+
{
22+
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
23+
24+
$pingCommand = null;
25+
$executor = $this->getMockBuilder('React\MySQL\Io\Executor')->setMethods(['enqueue'])->getMock();
26+
$executor->expects($this->once())->method('enqueue')->willReturnCallback(function ($command) use (&$pingCommand) {
27+
return $pingCommand = $command;
28+
});
29+
30+
$connection = new Connection($stream, $executor);
31+
32+
$events = '';
33+
$connection->on('close', function () use (&$events) {
34+
$events .= 'closed.';
35+
});
36+
37+
$this->assertEquals('', $events);
38+
39+
$promise = $connection->quit();
40+
41+
$promise->then(function () use (&$events) {
42+
$events .= 'fulfilled.';
43+
});
44+
45+
$this->assertEquals('', $events);
46+
47+
$this->assertNotNull($pingCommand);
48+
$pingCommand->emit('success');
49+
50+
$this->assertEquals('fulfilled.closed.', $events);
51+
}
52+
53+
public function testQuitWillRejectBeforeEmittingCloseEventWhenQuitCommandEmitsError()
54+
{
55+
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
56+
57+
$pingCommand = null;
58+
$executor = $this->getMockBuilder('React\MySQL\Io\Executor')->setMethods(['enqueue'])->getMock();
59+
$executor->expects($this->once())->method('enqueue')->willReturnCallback(function ($command) use (&$pingCommand) {
60+
return $pingCommand = $command;
61+
});
62+
63+
$connection = new Connection($stream, $executor);
64+
65+
$events = '';
66+
$connection->on('close', function () use (&$events) {
67+
$events .= 'closed.';
68+
});
69+
70+
$this->assertEquals('', $events);
71+
72+
$promise = $connection->quit();
73+
74+
$promise->then(null, function () use (&$events) {
75+
$events .= 'rejected.';
76+
});
77+
78+
$this->assertEquals('', $events);
79+
80+
$this->assertNotNull($pingCommand);
81+
$pingCommand->emit('error', [new \RuntimeException()]);
82+
83+
$this->assertEquals('rejected.closed.', $events);
84+
}
85+
2086
public function testQueryAfterQuitRejectsImmediately()
2187
{
2288
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();

‎tests/MysqlClientTest.php‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace React\Tests\Mysql\Io;
3+
namespace React\Tests\Mysql;
44

55
use React\Mysql\Io\Connection;
66
use React\Mysql\MysqlClient;
@@ -10,7 +10,6 @@
1010
use React\Promise\PromiseInterface;
1111
use React\Stream\ReadableStreamInterface;
1212
use React\Stream\ThroughStream;
13-
use React\Tests\Mysql\BaseTestCase;
1413

1514
class MysqlClientTest extends BaseTestCase
1615
{
@@ -197,12 +196,12 @@ public function testPingFollowedByIdleTimerWillQuitUnderlyingConnection()
197196
$timeout();
198197
}
199198

200-
public function testPingFollowedByIdleTimerWillCloseUnderlyingConnectionWhenQuitFails()
199+
public function testPingFollowedByIdleTimerWillNotHaveToCloseUnderlyingConnectionWhenQuitFailsBecauseUnderlyingConnectionEmitsCloseAutomatically()
201200
{
202201
$base = $this->getMockBuilder('React\Mysql\Io\Connection')->setMethods(['ping', 'quit', 'close'])->disableOriginalConstructor()->getMock();
203202
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
204203
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\reject(new \RuntimeException()));
205-
$base->expects($this->once())->method('close');
204+
$base->expects($this->never())->method('close');
206205

207206
$factory = $this->getMockBuilder('React\Mysql\Io\Factory')->disableOriginalConstructor()->getMock();
208207
$factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
@@ -227,6 +226,15 @@ public function testPingFollowedByIdleTimerWillCloseUnderlyingConnectionWhenQuit
227226

228227
$this->assertNotNull($timeout);
229228
$timeout();
229+
230+
assert($base instanceof Connection);
231+
$base->emit('close');
232+
233+
$ref = new \ReflectionProperty($connection, 'connecting');
234+
$ref->setAccessible(true);
235+
$connecting = $ref->getValue($connection);
236+
237+
$this->assertNull($connecting);
230238
}
231239

232240
public function testPingAfterIdleTimerWillCloseUnderlyingConnectionBeforeCreatingSecondConnection()
@@ -757,6 +765,32 @@ public function testQuitAfterPingReturnsPendingPromiseWhenConnectionIsPending()
757765
$ret->then($this->expectCallableNever(), $this->expectCallableNever());
758766
}
759767

768+
public function testQuitAfterPingRejectsAndThenEmitsCloseWhenFactoryFailsToCreateUnderlyingConnection()
769+
{
770+
$deferred = new Deferred();
771+
$factory = $this->getMockBuilder('React\MySQL\Io\Factory')->disableOriginalConstructor()->getMock();
772+
$factory->expects($this->once())->method('createConnection')->willReturn($deferred->promise());
773+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
774+
775+
$connection = new MysqlClient('', null, $loop);
776+
777+
$ref = new \ReflectionProperty($connection, 'factory');
778+
$ref->setAccessible(true);
779+
$ref->setValue($connection, $factory);
780+
781+
$connection->ping()->then(null, $this->expectCallableOnce());
782+
783+
$this->expectOutputString('reject.close.');
784+
$connection->on('close', function () {
785+
echo 'close.';
786+
});
787+
$connection->quit()->then(null, function () {
788+
echo 'reject.';
789+
});
790+
791+
$deferred->reject(new \RuntimeException());
792+
}
793+
760794
public function testQuitAfterPingWillQuitUnderlyingConnectionWhenResolved()
761795
{
762796
$base = $this->getMockBuilder('React\Mysql\Io\Connection')->disableOriginalConstructor()->getMock();
@@ -777,11 +811,12 @@ public function testQuitAfterPingWillQuitUnderlyingConnectionWhenResolved()
777811
$connection->quit();
778812
}
779813

780-
public function testQuitAfterPingResolvesAndEmitsCloseWhenUnderlyingConnectionQuits()
814+
public function testQuitAfterPingResolvesAndThenEmitsCloseWhenUnderlyingConnectionQuits()
781815
{
782816
$base = $this->getMockBuilder('React\Mysql\Io\Connection')->disableOriginalConstructor()->getMock();
817+
$deferred = new Deferred();
783818
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
784-
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\resolve(null));
819+
$base->expects($this->once())->method('quit')->willReturn($deferred->promise());
785820

786821
$factory = $this->getMockBuilder('React\Mysql\Io\Factory')->disableOriginalConstructor()->getMock();
787822
$factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
@@ -793,21 +828,25 @@ public function testQuitAfterPingResolvesAndEmitsCloseWhenUnderlyingConnectionQu
793828
$ref->setAccessible(true);
794829
$ref->setValue($connection, $factory);
795830

796-
$connection->on('close', $this->expectCallableOnce());
797-
798831
$connection->ping();
799-
$ret = $connection->quit();
800832

801-
$this->assertTrue($ret instanceof PromiseInterface);
802-
$ret->then($this->expectCallableOnce(), $this->expectCallableNever());
833+
$this->expectOutputString('quit.close.');
834+
$connection->on('close', function () {
835+
echo 'close.';
836+
});
837+
$connection->quit()->then(function () {
838+
echo 'quit.';
839+
});
840+
841+
$deferred->resolve(null);
803842
}
804843

805-
public function testQuitAfterPingRejectsAndEmitsCloseWhenUnderlyingConnectionFailsToQuit()
844+
public function testQuitAfterPingRejectsAndThenEmitsCloseWhenUnderlyingConnectionFailsToQuit()
806845
{
807-
$error = new \RuntimeException();
846+
$deferred = new Deferred();
808847
$base = $this->getMockBuilder('React\Mysql\Io\Connection')->disableOriginalConstructor()->getMock();
809848
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
810-
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\reject($error));
849+
$base->expects($this->once())->method('quit')->willReturn($deferred->promise());
811850

812851
$factory = $this->getMockBuilder('React\Mysql\Io\Factory')->disableOriginalConstructor()->getMock();
813852
$factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
@@ -819,13 +858,17 @@ public function testQuitAfterPingRejectsAndEmitsCloseWhenUnderlyingConnectionFai
819858
$ref->setAccessible(true);
820859
$ref->setValue($connection, $factory);
821860

822-
$connection->on('close', $this->expectCallableOnce());
823-
824861
$connection->ping();
825-
$ret = $connection->quit();
826862

827-
$this->assertTrue($ret instanceof PromiseInterface);
828-
$ret->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
863+
$this->expectOutputString('reject.close.');
864+
$connection->on('close', function () {
865+
echo 'close.';
866+
});
867+
$connection->quit()->then(null, function () {
868+
echo 'reject.';
869+
});
870+
871+
$deferred->reject(new \RuntimeException());
829872
}
830873

831874
public function testCloseEmitsCloseImmediatelyWhenConnectionIsNotAlreadyPending()

‎tests/NoResultQueryTest.php‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,35 @@ public function testPingWithValidAuthWillRunUntilQuitAfterPing()
133133
Loop::run();
134134
}
135135

136-
/**
137-
* @doesNotPerformAssertions
138-
*/
136+
public function testPingAndQuitWillFulfillPingBeforeQuitBeforeCloseEvent()
137+
{
138+
$this->expectOutputString('ping.quit.close.');
139+
140+
$uri = $this->getConnectionString();
141+
$connection = new MysqlClient($uri);
142+
143+
$connection->on('close', function () {
144+
echo 'close.';
145+
});
146+
147+
$connection->ping()->then(function () {
148+
echo 'ping.';
149+
});
150+
151+
$connection->quit()->then(function () {
152+
echo 'quit.';
153+
});
154+
155+
Loop::run();
156+
}
157+
139158
public function testPingWithValidAuthWillRunUntilIdleTimerAfterPingEvenWithoutQuit()
140159
{
141160
$uri = $this->getConnectionString();
142161
$connection = new MysqlClient($uri);
143162

163+
$connection->on('close', $this->expectCallableNever());
164+
144165
$connection->ping();
145166

146167
Loop::run();

0 commit comments

Comments
 (0)