Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistently emit close event after quit even if server rejects #189

Merged
merged 1 commit into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
{
$base = $this->getMockBuilder('React\Mysql\Io\Connection')->disableOriginalConstructor()->getMock();
$deferred = new Deferred();
$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