Skip to content

Commit

Permalink
Consistently emit close event after quit even if server rejects
Browse files Browse the repository at this point in the history
  • Loading branch information
clue committed Nov 27, 2023
1 parent adcf751 commit 08cb202
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 47 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,11 @@ $mysql->query('CREATE TABLE test ...');
$mysql->quit();
```

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

#### close()

The `close(): void` method can be used to
Expand Down
19 changes: 9 additions & 10 deletions src/Io/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,16 @@ public function ping()
public function quit()
{
return new Promise(function ($resolve, $reject) {
$this->_doCommand(new QuitCommand())
->on('error', function ($reason) use ($reject) {
$reject($reason);
})
->on('success', function () use ($resolve) {
$this->state = self::STATE_CLOSED;
$this->emit('end', [$this]);
$this->emit('close', [$this]);
$resolve(null);
});
$command = $this->_doCommand(new QuitCommand());
$this->state = self::STATE_CLOSING;
$command->on('success', function () use ($resolve) {
$resolve(null);
$this->close();
});
$command->on('error', function ($reason) use ($reject) {
$reject($reason);
$this->close();
});
});
}

Expand Down
43 changes: 28 additions & 15 deletions src/MysqlClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
use React\EventLoop\LoopInterface;
use React\MySQL\Io\Connection;
use React\MySQL\Io\Factory;
use React\Stream\ReadableStreamInterface;
use React\Promise\Promise;
use React\Socket\ConnectorInterface;
use React\Stream\ReadableStreamInterface;

/**
* This class represents a connection that is responsible for communicating
Expand Down Expand Up @@ -135,9 +136,8 @@ function () {
// successfully disconnected => remove reference
$this->disconnecting = null;
},
function () use ($connection) {
// soft-close failed => force-close connection
$connection->close();
function () {
// soft-close failed but will close anyway => remove reference
$this->disconnecting = null;
}
);
Expand Down Expand Up @@ -361,6 +361,11 @@ function (\Exception $e) {
* $mysql->quit();
* ```
*
* This method will gracefully close the connection to the MySQL database
* server once all outstanding commands are completed. See also
* [`close()`)(#close) if you want to force-close the connection without
* waiting for any commands to complete instead.
*
* @return PromiseInterface<void>
* Resolves with a `void` value on success or rejects with an `Exception` on error.
*/
Expand All @@ -376,17 +381,25 @@ public function quit()
return \React\Promise\resolve(null);
}

return $this->connecting()->then(function (Connection $connection) {
$this->awake();
return $connection->quit()->then(
function () {
$this->close();
},
function (\Exception $e) {
$this->close();
throw $e;
}
);
return new Promise(function (callable $resolve, callable $reject) {
$this->connecting()->then(function (Connection $connection) use ($resolve, $reject) {
$this->awake();
// soft-close connection and emit close event afterwards both on success or on error
$connection->quit()->then(
function () use ($resolve){
$resolve(null);
$this->close();
},
function (\Exception $e) use ($reject) {
$reject($e);
$this->close();
}
);
}, function (\Exception $e) use ($reject) {
// emit close event afterwards when no connection can be established
$reject($e);
$this->close();
});
});
}

Expand Down
66 changes: 66 additions & 0 deletions tests/Io/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,72 @@ public function testQuitWillEnqueueOneCommand()
$conn->quit();
}

public function testQuitWillResolveBeforeEmittingCloseEventWhenQuitCommandEmitsSuccess()
{
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();

$pingCommand = null;
$executor = $this->getMockBuilder('React\MySQL\Io\Executor')->setMethods(['enqueue'])->getMock();
$executor->expects($this->once())->method('enqueue')->willReturnCallback(function ($command) use (&$pingCommand) {
return $pingCommand = $command;
});

$connection = new Connection($stream, $executor);

$events = '';
$connection->on('close', function () use (&$events) {
$events .= 'closed.';
});

$this->assertEquals('', $events);

$promise = $connection->quit();

$promise->then(function () use (&$events) {
$events .= 'fulfilled.';
});

$this->assertEquals('', $events);

$this->assertNotNull($pingCommand);
$pingCommand->emit('success');

$this->assertEquals('fulfilled.closed.', $events);
}

public function testQuitWillRejectBeforeEmittingCloseEventWhenQuitCommandEmitsError()
{
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();

$pingCommand = null;
$executor = $this->getMockBuilder('React\MySQL\Io\Executor')->setMethods(['enqueue'])->getMock();
$executor->expects($this->once())->method('enqueue')->willReturnCallback(function ($command) use (&$pingCommand) {
return $pingCommand = $command;
});

$connection = new Connection($stream, $executor);

$events = '';
$connection->on('close', function () use (&$events) {
$events .= 'closed.';
});

$this->assertEquals('', $events);

$promise = $connection->quit();

$promise->then(null, function () use (&$events) {
$events .= 'rejected.';
});

$this->assertEquals('', $events);

$this->assertNotNull($pingCommand);
$pingCommand->emit('error', [new \RuntimeException()]);

$this->assertEquals('rejected.closed.', $events);
}

public function testQueryAfterQuitRejectsImmediately()
{
$stream = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
Expand Down
81 changes: 62 additions & 19 deletions tests/MysqlClientTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace React\Tests\MySQL\Io;
namespace React\Tests\MySQL;

use React\MySQL\Io\Connection;
use React\MySQL\MysqlClient;
Expand All @@ -10,7 +10,6 @@
use React\Promise\PromiseInterface;
use React\Stream\ReadableStreamInterface;
use React\Stream\ThroughStream;
use React\Tests\MySQL\BaseTestCase;

class MysqlClientTest extends BaseTestCase
{
Expand Down Expand Up @@ -197,12 +196,12 @@ public function testPingFollowedByIdleTimerWillQuitUnderlyingConnection()
$timeout();
}

public function testPingFollowedByIdleTimerWillCloseUnderlyingConnectionWhenQuitFails()
public function testPingFollowedByIdleTimerWillNotHaveToCloseUnderlyingConnectionWhenQuitFailsBecauseUnderlyingConnectionEmitsCloseAutomatically()
{
$base = $this->getMockBuilder('React\MySQL\Io\Connection')->setMethods(['ping', 'quit', 'close'])->disableOriginalConstructor()->getMock();
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\reject(new \RuntimeException()));
$base->expects($this->once())->method('close');
$base->expects($this->never())->method('close');

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

$this->assertNotNull($timeout);
$timeout();

assert($base instanceof Connection);
$base->emit('close');

$ref = new \ReflectionProperty($connection, 'connecting');
$ref->setAccessible(true);
$connecting = $ref->getValue($connection);

$this->assertNull($connecting);
}

public function testPingAfterIdleTimerWillCloseUnderlyingConnectionBeforeCreatingSecondConnection()
Expand Down Expand Up @@ -757,6 +765,32 @@ public function testQuitAfterPingReturnsPendingPromiseWhenConnectionIsPending()
$ret->then($this->expectCallableNever(), $this->expectCallableNever());
}

public function testQuitAfterPingRejectsAndThenEmitsCloseWhenFactoryFailsToCreateUnderlyingConnection()
{
$deferred = new Deferred();
$factory = $this->getMockBuilder('React\MySQL\Io\Factory')->disableOriginalConstructor()->getMock();
$factory->expects($this->once())->method('createConnection')->willReturn($deferred->promise());
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$connection = new MysqlClient('', null, $loop);

$ref = new \ReflectionProperty($connection, 'factory');
$ref->setAccessible(true);
$ref->setValue($connection, $factory);

$connection->ping()->then(null, $this->expectCallableOnce());

$this->expectOutputString('reject.close.');
$connection->on('close', function () {
echo 'close.';
});
$connection->quit()->then(null, function () {
echo 'reject.';
});

$deferred->reject(new \RuntimeException());
}

public function testQuitAfterPingWillQuitUnderlyingConnectionWhenResolved()
{
$base = $this->getMockBuilder('React\MySQL\Io\Connection')->disableOriginalConstructor()->getMock();
Expand All @@ -777,11 +811,12 @@ public function testQuitAfterPingWillQuitUnderlyingConnectionWhenResolved()
$connection->quit();
}

public function testQuitAfterPingResolvesAndEmitsCloseWhenUnderlyingConnectionQuits()
public function testQuitAfterPingResolvesAndThenEmitsCloseWhenUnderlyingConnectionQuits()
{
$deferred = new Deferred();
$base = $this->getMockBuilder('React\MySQL\Io\Connection')->disableOriginalConstructor()->getMock();
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\resolve(null));
$base->expects($this->once())->method('quit')->willReturn($deferred->promise());

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

$connection->on('close', $this->expectCallableOnce());

$connection->ping();
$ret = $connection->quit();

$this->assertTrue($ret instanceof PromiseInterface);
$ret->then($this->expectCallableOnce(), $this->expectCallableNever());
$this->expectOutputString('quit.close.');
$connection->on('close', function () {
echo 'close.';
});
$connection->quit()->then(function () {
echo 'quit.';
});

$deferred->resolve(null);
}

public function testQuitAfterPingRejectsAndEmitsCloseWhenUnderlyingConnectionFailsToQuit()
public function testQuitAfterPingRejectsAndThenEmitsCloseWhenUnderlyingConnectionFailsToQuit()
{
$error = new \RuntimeException();
$deferred = new Deferred();
$base = $this->getMockBuilder('React\MySQL\Io\Connection')->disableOriginalConstructor()->getMock();
$base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve(null));
$base->expects($this->once())->method('quit')->willReturn(\React\Promise\reject($error));
$base->expects($this->once())->method('quit')->willReturn($deferred->promise());

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

$connection->on('close', $this->expectCallableOnce());

$connection->ping();
$ret = $connection->quit();

$this->assertTrue($ret instanceof PromiseInterface);
$ret->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
$this->expectOutputString('reject.close.');
$connection->on('close', function () {
echo 'close.';
});
$connection->quit()->then(null, function () {
echo 'reject.';
});

$deferred->reject(new \RuntimeException());
}

public function testCloseEmitsCloseImmediatelyWhenConnectionIsNotAlreadyPending()
Expand Down
27 changes: 24 additions & 3 deletions tests/NoResultQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,35 @@ public function testPingWithValidAuthWillRunUntilQuitAfterPing()
Loop::run();
}

/**
* @doesNotPerformAssertions
*/
public function testPingAndQuitWillFulfillPingBeforeQuitBeforeCloseEvent()
{
$this->expectOutputString('ping.quit.close.');

$uri = $this->getConnectionString();
$connection = new MysqlClient($uri);

$connection->on('close', function () {
echo 'close.';
});

$connection->ping()->then(function () {
echo 'ping.';
});

$connection->quit()->then(function () {
echo 'quit.';
});

Loop::run();
}

public function testPingWithValidAuthWillRunUntilIdleTimerAfterPingEvenWithoutQuit()
{
$uri = $this->getConnectionString();
$connection = new MysqlClient($uri);

$connection->on('close', $this->expectCallableNever());

$connection->ping();

Loop::run();
Expand Down

0 comments on commit 08cb202

Please sign in to comment.