Skip to content

Commit

Permalink
[doctrineGH-4052] Address review comments, break on connect() with co…
Browse files Browse the repository at this point in the history 
…nnection name in new class.
  • Loading branch information
@beberlei
beberlei committed Jun 8, 2020
1 parent ea60246 commit cbf2827
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 29 deletions.
10 changes: 5 additions & 5 deletions lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ public function __construct(array $params, Driver $driver, ?Configuration $confi
$this->deprecated(self::class, PrimaryReplicaConnection::class);

if (isset($params['master'])) {
$this->deprecated("Params key 'master'", "'primary'");
$this->deprecated('Params key "master"', '"primary"');

$params['primary'] = $params['master'];
unset($params['master']);
}

if (isset($params['slaves'])) {
$this->deprecated("Params key 'slaves'", "'replica'");
$this->deprecated('Params key "slaves"', '"replica"');

$params['replica'] = $params['slave'];
unset($params['slave']);
}

if (isset($params['keepSlave'])) {
$this->deprecated("Params key 'keepSlave'", "'keepReplica'");
$this->deprecated('Params key "keepSlave"', '"keepReplica"');

$params['keepReplica'] = $params['keepSlave'];
unset($params['keepSlave']);
Expand Down Expand Up @@ -72,13 +72,13 @@ public function connect($connectionName = null)
if ($connectionName === 'master') {
$connectionName = 'primary';

$this->deprecated('connect("master")', 'connect("primary")');
$this->deprecated('connect("master")', 'ensureConnectedToPrimary()');
}

if ($connectionName === 'slave') {
$connectionName = 'replica';

$this->deprecated('connect("slave")', 'connect("replica")');
$this->deprecated('connect("slave")', 'ensureConnectedToReplica()');
}

return parent::connect($connectionName);
Expand Down
54 changes: 30 additions & 24 deletions lib/Doctrine/DBAL/Connections/PrimaryReplicaConnection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
* Be aware that Connection#executeQuery is a method specifically for READ
* operations only.
*
* Use Connection#executeUpdate for any SQL statement that changes/updates
* state in the database (UPDATE, INSERT, DELETE or DDL statements).
*
* This connection is limited to replica operations using the
* Connection#executeQuery operation only, because it wouldn't be compatible
* with the ORM or SchemaManager code otherwise. Both use all the other
Expand All @@ -49,7 +52,7 @@
*
* You can manually connect to the primary at any time by calling:
*
* $conn->connect('primary');
* $conn->ensureConnectedToPrimary();
*
* Instantiation through the DriverManager looks like:
*
Expand Down Expand Up @@ -113,10 +116,8 @@ public function __construct(array $params, Driver $driver, ?Configuration $confi

/**
* Checks if the connection is currently towards the primary or not.
*
* @return bool
*/
public function isConnectedToPrimary()
public function isConnectedToPrimary(): bool
{
return $this->_conn !== null && $this->_conn === $this->connections['primary'];
}
Expand All @@ -126,15 +127,20 @@ public function isConnectedToPrimary()
*
* @return bool
*/
public function connect($connectionName = null, bool $triggerDeprecationWhenConnectionNameUsed = true)
public function connect($connectionName = null)
{
if ($connectionName !== null) {
throw new InvalidArgumentException('Passing a connection name as first argument is not supported anymore. Use ensureConnectedToPrimary()/ensureConnectedToReplica() instead.');
}

return $this->performConnect();
}

private function performConnect($connectionName = null)
{
$requestedConnectionChange = ($connectionName !== null);
$connectionName = $connectionName ?: 'replica';

if ($requestedConnectionChange && $triggerDeprecationWhenConnectionNameUsed) {
@trigger_error("PrimaryReplicaConnection::connect() with connection name argument is deprecated, use ensureConnectedToPrimary/Replica() instead", E_USER_DEPRECATED);
}

if ($connectionName !== 'replica' && $connectionName !== 'primary') {
throw new InvalidArgumentException('Invalid option to connect(), only primary or replica allowed.');
}
Expand Down Expand Up @@ -189,7 +195,7 @@ public function connect($connectionName = null, bool $triggerDeprecationWhenConn
*/
public function ensureConnectedToPrimary(): bool
{
return $this->connect('primary', false);
return $this->performConnect('primary');
}

/**
Expand All @@ -201,7 +207,7 @@ public function ensureConnectedToPrimary(): bool
*/
public function ensureConnectedToReplica(): bool
{
return $this->connect('replica', false);
return $this->performConnect('replica');
}

/**
Expand Down Expand Up @@ -251,7 +257,7 @@ protected function chooseConnectionConfiguration($connectionName, $params)
*/
public function executeUpdate($query, array $params = [], array $types = [])
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::executeUpdate($query, $params, $types);
}
Expand All @@ -261,7 +267,7 @@ public function executeUpdate($query, array $params = [], array $types = [])
*/
public function beginTransaction()
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::beginTransaction();
}
Expand All @@ -271,7 +277,7 @@ public function beginTransaction()
*/
public function commit()
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::commit();
}
Expand All @@ -281,7 +287,7 @@ public function commit()
*/
public function rollBack()
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::rollBack();
}
Expand All @@ -291,7 +297,7 @@ public function rollBack()
*/
public function delete($tableName, array $identifier, array $types = [])
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::delete($tableName, $identifier, $types);
}
Expand All @@ -314,7 +320,7 @@ public function close()
*/
public function update($tableName, array $data, array $identifier, array $types = [])
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::update($tableName, $data, $identifier, $types);
}
Expand All @@ -324,7 +330,7 @@ public function update($tableName, array $data, array $identifier, array $types
*/
public function insert($tableName, array $data, array $types = [])
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::insert($tableName, $data, $types);
}
Expand All @@ -334,7 +340,7 @@ public function insert($tableName, array $data, array $types = [])
*/
public function exec($statement)
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::exec($statement);
}
Expand All @@ -344,7 +350,7 @@ public function exec($statement)
*/
public function createSavepoint($savepoint)
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

parent::createSavepoint($savepoint);
}
Expand All @@ -354,7 +360,7 @@ public function createSavepoint($savepoint)
*/
public function releaseSavepoint($savepoint)
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

parent::releaseSavepoint($savepoint);
}
Expand All @@ -364,7 +370,7 @@ public function releaseSavepoint($savepoint)
*/
public function rollbackSavepoint($savepoint)
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

parent::rollbackSavepoint($savepoint);
}
Expand All @@ -374,7 +380,7 @@ public function rollbackSavepoint($savepoint)
*/
public function query()
{
$this->connect('primary');
$this->ensureConnectedToPrimary();
assert($this->_conn instanceof DriverConnection);

$args = func_get_args();
Expand All @@ -400,7 +406,7 @@ public function query()
*/
public function prepare($statement)
{
$this->connect('primary');
$this->ensureConnectedToPrimary();

return parent::prepare($statement);
}
Expand Down

0 comments on commit cbf2827

Please sign in to comment.