Keep track of underlying connection and create new when connection lost

clue · clue · commit 740e97a13fa8 · 2018-11-11T14:49:29.000+01:00
diff --git a/README.md b/README.md
@@ -171,24 +171,33 @@ $connection->query(…);
 This method immediately returns a "virtual" connection implementing the
 [`ConnectionInterface`](#connectioninterface) that can be used to
 interface with your MySQL database. Internally, it lazily creates the
-underlying database connection (which may take some time) only once the
-first request is invoked on this instance and will queue all outstanding
-requests until the underlying connection is ready.
+underlying database connection only on demand once the first request is
+invoked on this instance and will queue all outstanding requests until
+the underlying connection is ready. Additionally, it will keep track of
+this underlying connection and will create a new underlying connection
+on demand when the current connection is lost.
 
 From a consumer side this means that you can start sending queries to the
-database right away while the actual connection may still be outstanding.
-It will ensure that all commands will be executed in the order they are
-enqueued once the connection is ready. If the database connection fails,
-it will emit an `error` event, reject all outstanding commands and `close`
-the connection as described in the `ConnectionInterface`. In other words,
-it behaves just like a real connection and frees you from having to deal
-with its async resolution.
+database right away while the underlying connection may still be
+outstanding. Because creating this underlying connection may take some
+time, it will enqueue all oustanding commands and will ensure that all
+commands will be executed in correct order once the connection is ready.
+In other words, this "virtual" connection behaves just like a "real"
+connection as described in the `ConnectionInterface` and frees you from
+having to deal with its async resolution.
+
+If the underlying database connection fails, it will reject all
+outstanding commands and will return to the initial "idle" state. This
+means that you can keep sending additional commands at a later time which
+will again try to open the underlying connection.
 
 Note that creating the underlying connection will be deferred until the
 first request is invoked. Accordingly, any eventual connection issues
-will be detected once this instance is first used. Similarly, calling
-`quit()` on this instance before invoking any requests will succeed
-immediately and will not wait for an actual underlying connection.
+will be detected once this instance is first used. You can use the
+`quit()` method to ensure that the "virtual" connection will be soft-closed
+and no further commands can be enqueued. Similarly, calling `quit()` on
+this instance before invoking any requests will succeed immediately and
+will not wait for an actual underlying connection.
 
 Depending on your particular use case, you may prefer this method or the
 underlying `createConnection()` which resolves with a promise. For many
diff --git a/src/Factory.php b/src/Factory.php
@@ -211,24 +211,33 @@ public function createConnection($uri)
      * This method immediately returns a "virtual" connection implementing the
      * [`ConnectionInterface`](#connectioninterface) that can be used to
      * interface with your MySQL database. Internally, it lazily creates the
-     * underlying database connection (which may take some time) only once the
-     * first request is invoked on this instance and will queue all outstanding
-     * requests until the underlying connection is ready.
+     * underlying database connection only on demand once the first request is
+     * invoked on this instance and will queue all outstanding requests until
+     * the underlying connection is ready. Additionally, it will keep track of
+     * this underlying connection and will create a new underlying connection
+     * on demand when the current connection is lost.
      *
      * From a consumer side this means that you can start sending queries to the
-     * database right away while the actual connection may still be outstanding.
-     * It will ensure that all commands will be executed in the order they are
-     * enqueued once the connection is ready. If the database connection fails,
-     * it will emit an `error` event, reject all outstanding commands and `close`
-     * the connection as described in the `ConnectionInterface`. In other words,
-     * it behaves just like a real connection and frees you from having to deal
-     * with its async resolution.
+     * database right away while the underlying connection may still be
+     * outstanding. Because creating this underlying connection may take some
+     * time, it will enqueue all oustanding commands and will ensure that all
+     * commands will be executed in correct order once the connection is ready.
+     * In other words, this "virtual" connection behaves just like a "real"
+     * connection as described in the `ConnectionInterface` and frees you from
+     * having to deal with its async resolution.
+     *
+     * If the underlying database connection fails, it will reject all
+     * outstanding commands and will return to the initial "idle" state. This
+     * means that you can keep sending additional commands at a later time which
+     * will again try to open the underlying connection.
      *
      * Note that creating the underlying connection will be deferred until the
      * first request is invoked. Accordingly, any eventual connection issues
-     * will be detected once this instance is first used. Similarly, calling
-     * `quit()` on this instance before invoking any requests will succeed
-     * immediately and will not wait for an actual underlying connection.
+     * will be detected once this instance is first used. You can use the
+     * `quit()` method to ensure that the "virtual" connection will be soft-closed
+     * and no further commands can be enqueued. Similarly, calling `quit()` on
+     * this instance before invoking any requests will succeed immediately and
+     * will not wait for an actual underlying connection.
      *
      * Depending on your particular use case, you may prefer this method or the
      * underlying `createConnection()` which resolves with a promise. For many
diff --git a/src/Io/LazyConnection.php b/src/Io/LazyConnection.php
@@ -27,29 +27,22 @@ public function __construct(Factory $factory, $uri)
 
     private function connecting()
     {
-        if ($this->connecting === null) {
-            $this->connecting = $this->factory->createConnection($this->uri);
-
-            $this->connecting->then(function (ConnectionInterface $connection) {
-                // connection completed => forward error and close events
-                $connection->on('error', function ($e) {
-                    $this->emit('error', [$e]);
-                });
-                $connection->on('close', function () {
-                    $this->close();
-                });
-            }, function (\Exception $e) {
-                // connection failed => emit error if connection is not already closed
-                if ($this->closed) {
-                    return;
-                }
+        if ($this->connecting !== null) {
+            return $this->connecting;
+        }
 
-                $this->emit('error', [$e]);
-                $this->close();
+        $this->connecting = $connecting = $this->factory->createConnection($this->uri);
+        $this->connecting->then(function (ConnectionInterface $connection) {
+            // connection completed => remember only until closed
+            $connection->on('close', function () {
+                $this->connecting = null;
             });
-        }
+        }, function () {
+            // connection failed => discard connection attempt
+            $this->connecting = null;
+        });
 
-        return $this->connecting;
+        return $connecting;
     }
 
     public function query($sql, array $params = [])
@@ -100,7 +93,15 @@ public function quit()
         }
 
         return $this->connecting()->then(function (ConnectionInterface $connection) {
-            return $connection->quit();
+            return $connection->quit()->then(
+                function () {
+                    $this->close();
+                },
+                function (\Exception $e) {
+                    $this->close();
+                    throw $e;
+                }
+            );
         });
     }
 
diff --git a/tests/FactoryTest.php b/tests/FactoryTest.php
@@ -387,18 +387,18 @@ public function testConnectLazyWithValidAuthWillRunUntilQuitAfterPing()
         $loop->run();
     }
 
-    public function testConnectLazyWithInvalidAuthWillEmitErrorAndCloseAfterPing()
+    public function testConnectLazyWithInvalidAuthWillRejectPingButWillNotEmitErrorOrClose()
     {
         $loop = \React\EventLoop\Factory::create();
         $factory = new Factory($loop);
 
         $uri = $this->getConnectionString(array('passwd' => 'invalidpass'));
         $connection = $factory->createLazyConnection($uri);
 
-        $connection->on('error', $this->expectCallableOnce());
-        $connection->on('close', $this->expectCallableOnce());
+        $connection->on('error', $this->expectCallableNever());
+        $connection->on('close', $this->expectCallableNever());
 
-        $connection->ping();
+        $connection->ping()->then(null, $this->expectCallableOnce());
 
         $loop->run();
     }
diff --git a/tests/Io/LazyConnectionTest.php b/tests/Io/LazyConnectionTest.php
@@ -12,51 +12,47 @@
 
 class LazyConnectionTest extends BaseTestCase
 {
-    public function testPingWillCloseConnectionWithErrorWhenPendingConnectionFails()
+    public function testPingWillNotCloseConnectionWhenPendingConnectionFails()
     {
         $deferred = new Deferred();
         $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
         $factory->expects($this->once())->method('createConnection')->willReturn($deferred->promise());
         $connection = new LazyConnection($factory, '');
 
-        $connection->on('error', $this->expectCallableOnce());
-        $connection->on('close', $this->expectCallableOnce());
+        $connection->on('error', $this->expectCallableNever());
+        $connection->on('close', $this->expectCallableNever());
 
         $connection->ping();
 
         $deferred->reject(new \RuntimeException());
     }
 
-    public function testPingWillCloseConnectionWithoutErrorWhenUnderlyingConnectionCloses()
+    public function testPingWillNotCloseConnectionWhenUnderlyingConnectionCloses()
     {
-        $promise = new Promise(function () { });
-        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
-        $factory->expects($this->once())->method('createConnection')->willReturn($promise);
-        $base = new LazyConnection($factory, '');
+        $base = $this->getMockBuilder('React\MySQL\Io\LazyConnection')->setMethods(array('ping'))->disableOriginalConstructor()->getMock();
+        $base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve());
 
         $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
         $factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
         $connection = new LazyConnection($factory, '');
 
         $connection->on('error', $this->expectCallableNever());
-        $connection->on('close', $this->expectCallableOnce());
+        $connection->on('close', $this->expectCallableNever());
 
         $connection->ping();
         $base->close();
     }
 
-    public function testPingWillForwardErrorFromUnderlyingConnection()
+    public function testPingWillNotForwardErrorFromUnderlyingConnection()
     {
-        $promise = new Promise(function () { });
-        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
-        $factory->expects($this->once())->method('createConnection')->willReturn($promise);
-        $base = new LazyConnection($factory, '');
+        $base = $this->getMockBuilder('React\MySQL\Io\LazyConnection')->setMethods(array('ping'))->disableOriginalConstructor()->getMock();
+        $base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve());
 
         $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
         $factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
         $connection = new LazyConnection($factory, '');
 
-        $connection->on('error', $this->expectCallableOnce());
+        $connection->on('error', $this->expectCallableNever());
         $connection->on('close', $this->expectCallableNever());
 
         $connection->ping();
@@ -178,6 +174,33 @@ public function testPingWillPingUnderlyingConnectionWhenResolved()
         $connection->ping();
     }
 
+    public function testPingTwiceWillBothRejectWithSameErrorWhenUnderlyingConnectionRejects()
+    {
+        $error = new \RuntimeException();
+        $deferred = new Deferred();
+
+        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
+        $factory->expects($this->once())->method('createConnection')->willReturn($deferred->promise());
+        $connection = new LazyConnection($factory, '');
+
+        $connection->ping()->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
+        $connection->ping()->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
+
+        $deferred->reject($error);
+    }
+
+    public function testPingWillTryToCreateNewUnderlyingConnectionAfterPreviousPingFailedToCreateUnderlyingConnection()
+    {
+        $error = new \RuntimeException();
+
+        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
+        $factory->expects($this->exactly(2))->method('createConnection')->willReturn(\React\Promise\reject($error));
+        $connection = new LazyConnection($factory, '');
+
+        $connection->ping()->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
+        $connection->ping()->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
+    }
+
     public function testQuitResolvesAndEmitsCloseImmediatelyWhenConnectionIsNotAlreadyPending()
     {
         $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
@@ -220,6 +243,45 @@ public function testQuitAfterPingWillQuitUnderlyingConnectionWhenResolved()
         $connection->quit();
     }
 
+    public function testQuitAfterPingResolvesAndEmitsCloseWhenUnderlyingConnectionQuits()
+    {
+        $base = $this->getMockBuilder('React\MySQL\ConnectionInterface')->getMock();
+        $base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve());
+        $base->expects($this->once())->method('quit')->willReturn(\React\Promise\resolve());
+
+        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
+        $factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
+        $connection = new LazyConnection($factory, '');
+
+        $connection->on('close', $this->expectCallableOnce());
+
+        $connection->ping();
+        $ret = $connection->quit();
+
+        $this->assertTrue($ret instanceof PromiseInterface);
+        $ret->then($this->expectCallableOnce(), $this->expectCallableNever());
+    }
+
+    public function testQuitAfterPingRejectsAndEmitsCloseWhenUnderlyingConnectionFailsToQuit()
+    {
+        $error = new \RuntimeException();
+        $base = $this->getMockBuilder('React\MySQL\ConnectionInterface')->getMock();
+        $base->expects($this->once())->method('ping')->willReturn(\React\Promise\resolve());
+        $base->expects($this->once())->method('quit')->willReturn(\React\Promise\reject($error));
+
+        $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();
+        $factory->expects($this->once())->method('createConnection')->willReturn(\React\Promise\resolve($base));
+        $connection = new LazyConnection($factory, '');
+
+        $connection->on('close', $this->expectCallableOnce());
+
+        $connection->ping();
+        $ret = $connection->quit();
+
+        $this->assertTrue($ret instanceof PromiseInterface);
+        $ret->then($this->expectCallableNever(), $this->expectCallableOnceWith($error));
+    }
+
     public function testCloseEmitsCloseImmediatelyWhenConnectionIsNotAlreadyPending()
     {
         $factory = $this->getMockBuilder('React\MySQL\Factory')->disableOriginalConstructor()->getMock();

Original file line number	Diff line number	Diff line change
`@@ -387,18 +387,18 @@ public function testConnectLazyWithValidAuthWillRunUntilQuitAfterPing()`
`387`	`387`	`$loop->run();`
`388`	`388`	`}`
`389`	`389`
`390`		`- public function testConnectLazyWithInvalidAuthWillEmitErrorAndCloseAfterPing()`
	`390`	`+ public function testConnectLazyWithInvalidAuthWillRejectPingButWillNotEmitErrorOrClose()`
`391`	`391`	`{`
`392`	`392`	`$loop = \React\EventLoop\Factory::create();`
`393`	`393`	`$factory = new Factory($loop);`
`394`	`394`
`395`	`395`	`$uri = $this->getConnectionString(array('passwd' => 'invalidpass'));`
`396`	`396`	`$connection = $factory->createLazyConnection($uri);`
`397`	`397`
`398`		`- $connection->on('error', $this->expectCallableOnce());`
`399`		`- $connection->on('close', $this->expectCallableOnce());`
	`398`	`+ $connection->on('error', $this->expectCallableNever());`
	`399`	`+ $connection->on('close', $this->expectCallableNever());`
`400`	`400`
`401`		`- $connection->ping();`
	`401`	`+ $connection->ping()->then(null, $this->expectCallableOnce());`
`402`	`402`
`403`	`403`	`$loop->run();`
`404`	`404`	`}`