Browse Source

Fix setting connection roles with non-manager connection

Corey Taylor 3 years ago
parent
commit
0a71e7ab02

+ 15 - 17
src/Database/Query.php

@@ -237,13 +237,7 @@ class Query implements ExpressionInterface, IteratorAggregate
      */
     public function useReadRole()
     {
-        if ($this->_connection->role() !== Connection::ROLE_READ) {
-            /** @var \Cake\Database\Connection $read */
-            $read = ConnectionManager::get($this->_connection->configName(), true, Connection::ROLE_READ);
-            $this->setConnection($read);
-        }
-
-        return $this;
+        return $this->useRole(Connection::ROLE_READ);
     }
 
     /**
@@ -253,13 +247,7 @@ class Query implements ExpressionInterface, IteratorAggregate
      */
     public function useWriteRole()
     {
-        if ($this->_connection->role() !== Connection::ROLE_WRITE) {
-            /** @var \Cake\Database\Connection $write */
-            $write = ConnectionManager::get($this->_connection->configName(), true, Connection::ROLE_WRITE);
-            $this->setConnection($write);
-        }
-
-        return $this;
+        return $this->useRole(Connection::ROLE_WRITE);
     }
 
     /**
@@ -270,11 +258,21 @@ class Query implements ExpressionInterface, IteratorAggregate
      */
     public function useRole(string $role)
     {
-        if ($role === Connection::ROLE_READ) {
-            return $this->useReadRole();
+        assert(in_array($role, [Connection::ROLE_READ, Connection::ROLE_WRITE], true));
+        if ($this->_connection->role() === $role) {
+            return $this;
         }
 
-        return $this->useWriteRole();
+        $name = $this->_connection->configName();
+        $roleName = ConnectionManager::getName($role, $name);
+        if ($roleName === $name) {
+            return $this;
+        }
+
+        /** @var \Cake\Database\Connection $connection */
+        $connection = ConnectionManager::get($roleName);
+
+        return $this->setConnection($connection);
     }
 
     /**

+ 6 - 15
src/Datasource/ConnectionManager.php

@@ -214,14 +214,14 @@ class ConnectionManager
     public static function get(string $name, bool $useAliases = true, ?string $role = null)
     {
         if ($role) {
-            $name = static::getName($role, $name, $useAliases);
+            $roleName = static::getName($role, $name, $useAliases);
         }
 
         if ($useAliases && isset(static::$_aliasMap[$name])) {
             $name = static::$_aliasMap[$name];
         }
 
-        if (empty(static::$_config[$name])) {
+        if (!isset(static::$_config[$name])) {
             throw new MissingDatasourceConfigException(['name' => $name]);
         }
 
@@ -242,6 +242,7 @@ class ConnectionManager
      */
     public static function getName(string $role, string $name, $useAliases = true): string
     {
+        assert(in_array($role, [ConnectionInterface::ROLE_READ, ConnectionInterface::ROLE_WRITE], true));
         if ($role === ConnectionInterface::ROLE_READ) {
             return static::getReadName($name, $useAliases);
         }
@@ -266,21 +267,11 @@ class ConnectionManager
             $readName = $name . ':read';
         }
 
-        if ($useAliases) {
-            if (isset(static::$_aliasMap[$readName])) {
-                return $readName;
-            }
-
-            if (isset(static::$_aliasMap[$writeName])) {
-                return $writeName;
-            }
-        }
-
-        if (isset(static::$_config[$readName])) {
+        if (($useAliases && isset(static::$_aliasMap[$readName])) || isset(static::$_config[$readName])) {
             return $readName;
         }
 
-        return isset(static::$_config[$writeName]) ? $writeName : $name;
+        return $writeName;
     }
 
     /**
@@ -297,6 +288,6 @@ class ConnectionManager
             $writeName = $matches[1];
         }
 
-        return $useAliases ? static::$_aliasMap[$writeName] ?? $writeName : $writeName;
+        return $writeName;
     }
 }

+ 37 - 7
tests/TestCase/Database/QueryTest.php

@@ -36,6 +36,7 @@ use Cake\Database\TypeFactory;
 use Cake\Database\TypeMap;
 use Cake\Database\ValueBinder;
 use Cake\Datasource\ConnectionManager;
+use Cake\Datasource\Exception\MissingDatasourceConfigException;
 use Cake\TestSuite\TestCase;
 use DateTime;
 use DateTimeImmutable;
@@ -101,20 +102,49 @@ class QueryTest extends TestCase
     {
         $query = new Query($this->connection);
 
-        // Defaults to "test" write connection
-        $query->useRole(Connection::ROLE_READ);
+        // Defaults to write "test" connection
+        $query->useReadRole();
         $this->assertSame('test', $query->getConnection()->configName());
-        $this->assertSame('write', $query->getConnection()->role());
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnection()->role());
 
         ConnectionManager::setConfig('test:read', ['url' => getenv('DB_URL')]);
 
-        $query->useRole(Connection::ROLE_READ);
+        $query->useReadRole();
         $this->assertSame('test:read', $query->getConnection()->configName());
-        $this->assertSame('read', $query->getConnection()->role());
+        $this->assertSame(Connection::ROLE_READ, $query->getConnection()->role());
 
-        $query->useRole(Connection::ROLE_WRITE);
+        $query->useWriteRole();
         $this->assertSame('test', $query->getConnection()->configName());
-        $this->assertSame('write', $query->getConnection()->role());
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnection()->role());
+    }
+
+    public function testConnectionRolesManualWriteConnection(): void
+    {
+        $config = $this->connection->config();
+        $config['name'] = 'not-in-manager';
+
+        $query = new Query(new Connection($config));
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnection()->role());
+
+        $query->useWriteRole();
+        $this->assertSame('not-in-manager', $query->getConnection()->configName());
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnection()->role());
+
+        $query->useReadRole();
+        $this->assertSame('not-in-manager', $query->getConnection()->configName());
+        $this->assertSame(Connection::ROLE_WRITE, $query->getConnection()->role());
+    }
+
+    public function testConnectionRolesManualReadConnection(): void
+    {
+        $config = $this->connection->config();
+        $config['name'] = 'not-in-manager:read';
+
+        $query = new Query(new Connection($config));
+        $this->assertSame(Connection::ROLE_READ, $query->getConnection()->role());
+
+        $this->expectException(MissingDatasourceConfigException::class);
+        $query->useWriteRole();
     }
 
     /**

+ 11 - 5
tests/TestCase/Datasource/ConnectionManagerTest.php

@@ -216,8 +216,8 @@ class ConnectionManagerTest extends TestCase
         $writeRole = ConnectionInterface::ROLE_WRITE;
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test'));
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test:read'));
-        $this->assertSame('test', ConnectionManager::getName($writeRole, 'default'));
-        $this->assertSame('test', ConnectionManager::getName($writeRole, 'default:read'));
+        $this->assertSame('default', ConnectionManager::getName($writeRole, 'default'));
+        $this->assertSame('default', ConnectionManager::getName($writeRole, 'default:read'));
 
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test', false));
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test:read', false));
@@ -226,7 +226,7 @@ class ConnectionManagerTest extends TestCase
 
         ConnectionManager::alias('test', 'test:read');
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test:read'));
-        $this->assertSame('test', ConnectionManager::getName($writeRole, 'default:read'));
+        $this->assertSame('default', ConnectionManager::getName($writeRole, 'default:read'));
 
         $this->assertSame('test', ConnectionManager::getName($writeRole, 'test:read', false));
         $this->assertSame('default', ConnectionManager::getName($writeRole, 'default:read', false));
@@ -243,7 +243,7 @@ class ConnectionManagerTest extends TestCase
         $this->assertSame('test', ConnectionManager::getName($readRole, 'test', false));
         $this->assertSame('test', ConnectionManager::getName($readRole, 'test:read', false));
         $this->assertSame('default', ConnectionManager::getName($readRole, 'default', false));
-        $this->assertSame('default:read', ConnectionManager::getName($readRole, 'default:read', false));
+        $this->assertSame('default', ConnectionManager::getName($readRole, 'default:read', false));
 
         ConnectionManager::alias('test', 'test:read');
         $this->assertSame('test:read', ConnectionManager::getName($readRole, 'test'));
@@ -256,7 +256,13 @@ class ConnectionManagerTest extends TestCase
         // With no alias, defaults to the physical test connection
         $this->assertSame('test', ConnectionManager::getName($readRole, 'test:read', false));
         $this->assertSame('default', ConnectionManager::getName($readRole, 'default', false));
-        $this->assertSame('default:read', ConnectionManager::getName($readRole, 'default:read', false));
+        $this->assertSame('default', ConnectionManager::getName($readRole, 'default:read', false));
+
+        ConnectionManager::alias('test', 'default:read');
+        $this->assertSame('default:read', ConnectionManager::getName($readRole, 'default'));
+        $this->assertSame('default', ConnectionManager::getName($readRole, 'default', false));
+        $this->assertSame('default:read', ConnectionManager::getName($readRole, 'default:read'));
+        $this->assertSame('default', ConnectionManager::getName($readRole, 'default:read', false));
     }
 
     public function testMissingWriteConnection(): void