[DBAL-335] Fix nasty bug in MasterSlaveConnection, connecting and writing to...

[DBAL-335] Fix nasty bug in MasterSlaveConnection, connecting and writing to slave when using keepSlave option.

lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php

@@ -138,16 +138,26 @@ class MasterSlaveConnection extends Connection
     /**
      * {@inheritDoc}
      */
-    public function connect($connectionName = 'slave')
+    public function connect($connectionName = null)
     {
+        $requestedConnectionChange = ($connectionName !== null);
+        $connectionName            = $connectionName ?: 'slave';
+
         if ( $connectionName !== 'slave' && $connectionName !== 'master' ) {
             throw new \InvalidArgumentException("Invalid option to connect(), only master or slave allowed.");
         }
 
+        // If we have a connection open, and this is not an explicit connection
+        // change request, then abort right here, because we are already done.
+        // This prevents writes to the slave in case of "keepSlave" option enabled.
+        if ($this->_conn && !$requestedConnectionChange) {
+            return false;
+        }
+
         $forceMasterAsSlave = false;
 
         if ($this->getTransactionNestingLevel() > 0) {
-            $connectionName = 'master';
+            $connectionName     = 'master';
             $forceMasterAsSlave = true;
         }
 
@@ -161,7 +171,7 @@ class MasterSlaveConnection extends Connection
         }
 
         if ($connectionName === 'master') {
-            /** Set slave connection to master to avoid invalid reads */
+            // Set slave connection to master to avoid invalid reads
             if ($this->connections['slave'] && ! $this->keepSlave) {
                 unset($this->connections['slave']);
             }

tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php

@@ -32,11 +32,12 @@ class MasterSlaveConnectionTest extends DbalFunctionalTestCase
         }
     }
 
-    public function createMasterSlaveConnection()
+    public function createMasterSlaveConnection($keepSlave = false)
     {
         $params = $this->_conn->getParams();
-        $params['master'] = $params;
-        $params['slaves'] = array($params, $params);
+        $params['master']       = $params;
+        $params['slaves']       = array($params, $params);
+        $params['keepSlave']    = $keepSlave;
         $params['wrapperClass'] = 'Doctrine\DBAL\Connections\MasterSlaveConnection';
 
         return DriverManager::getConnection($params);
@@ -79,4 +80,23 @@ class MasterSlaveConnectionTest extends DbalFunctionalTestCase
         $this->assertEquals(2, $data[0]['num']);
         $this->assertTrue($conn->isConnectedToMaster());
     }
+
+    /**
+     * @group DBAL-335
+     */
+    public function testKeepSlaveBeginTransactionStaysOnMaster()
+    {
+        $conn = $this->createMasterSlaveConnection($keepSlave = true);
+        $conn->connect('slave');
+
+        $conn->insert('master_slave_table', array('test_int' => 30));
+
+        $this->assertTrue($conn->isConnectedToMaster());
+
+        $conn->connect();
+        $this->assertTrue($conn->isConnectedToMaster());
+
+        $conn->connect('slave');
+        $this->assertFalse($conn->isConnectedToMaster());
+    }
 }