8000 Fix deprecated DBAL calls by derrabus · Pull Request #8794 · doctrine/orm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix deprecated DBAL calls #8794

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 28, 2021
Merged

Fix deprecated DBAL calls #8794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"doctrine/cache": "^1.11.3|^2.0.3",
"doctrine/collections": "^1.5",
"doctrine/common": "^3.0.3",
"doctrine/dbal": "^2.13.0",
"doctrine/dbal": "^2.13.1",
"doctrine/deprecations": "^0.5.3",
"doctrine/event-manager": "^1.1",
"doctrine/inflector": "^1.4|^2.0",
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Id/SequenceGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function generate(EntityManager $em, $entity)
$sql = $conn->getDatabasePlatform()->getSequenceNextValSQL($this->_sequenceName);

// Using `query` to force usage of the master server in MasterSlaveConnection
$this->_nextValue = (int) $conn->query($sql)->fetchColumn();
$this->_nextValue = (int) $conn->executeQuery($sql)->fetchOne();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change made the ORM incompatible with the PrimaryReadReplicaConnection (the new name of MasterSlaveConnection) by not respecting the comment just above it. executeQuery is the API where reads are allowed from the read replica
The alternative is to force connecting to the master here instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can your provide a PR to fix that? I don't use PrimaryReadReplicaConnection anywhere at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started a Slack discussion about that, to see what is the right fix for that.

Semantically, we can have 3 kind of execution:

  • read-only queries, for which we want the result -> executeQuery, and PrimaryReadReplicaConnection let them go through the read replica (if not already connected to the primary)
  • write statements without a result (e.g. UPDATE for instance) -> executeStatement (returning the number of affected rows), , and PrimaryReadReplicaConnection automatically ensures that they go to the primary
  • statements needing write priviledges but returning a result (e.g. moving to the next value of a sequence like here) -> query and PrimaryReadReplicaConnection ensures that they go to the primary

The issue here is that query got deprecated in favor of executeQuery, meaning that we don't actually have a method for the third case anymore (which would force to add support for PrimaryReadReplicaConnection in each place needing that kind of query). To me, the right fix would be to undeprecate query (or add a new method name for that behavior, using the new return type) in DBAL, and then using it in the ORM

$this->_maxValue = $this->_nextValue + $this->_allocationSize;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/ORM/Id/TableGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function generate(
if ($conn->getTransactionNestingLevel() === 0) {
// use select for update
$sql = $conn->getDatabasePlatform()->getTableHiLoCurrentValSql($this->_tableName, $this->_sequenceName);
$currentLevel = $conn->fetchColumn($sql);
$currentLevel = $conn->fetchOne($sql);

if ($currentLevel !== null) {
$this->_nextValue = $currentLevel;
Expand All @@ -80,7 +80,7 @@ public function generate(
$this->_allocationSize
);

if ($conn->executeUpdate($updateSql, [1 => $currentLevel, 2 => $currentLevel + 1]) !== 1) {
if ($conn->executeStatement($updateSql, [1 => $currentLevel, 2 => $currentLevel + 1]) !== 1) {
// no affected rows, concurrency issue, throw exception
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Id/UuidGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ public function generate(EntityManager $em, $entity)
$conn = $em->getConnection();
$sql = 'SELECT ' . $conn->getDatabasePlatform()->getGuidExpression();

return $conn->query($sql)->fetchColumn(0);
return $conn->executeQuery($sql)->fetchOne();
}
}
10 changes: 5 additions & 5 deletions lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function delete(PersistentCollection $collection)
$types[] = PersisterHelper::getTypeOfColumn($joinColumn['referencedColumnName'], $class, $this->em);
}

$this->conn->executeUpdate($this->getDeleteSQL($collection), $this->getDeleteSQLParameters($collection), $types);
$this->conn->executeStatement($this->getDeleteSQL($collection), $this->getDeleteSQLParameters($collection), $types);
}

/**
Expand All @@ -79,15 +79,15 @@ public function update(PersistentCollection $collection)
[$insertSql, $insertTypes] = $this->getInsertRowSQL($collection);

foreach ($collection->getDeleteDiff() as $element) {
$this->conn->executeUpdate(
$this->conn->executeStatement(
$deleteSql,
$this->getDeleteRowSQLParameters($collection, $element),
$deleteTypes
);
}

foreach ($collection->getInsertDiff() as $element) {
$this->conn->executeUpdate(
$this->conn->executeStatement(
$insertSql,
$this->getInsertRowSQLParameters($collection, $element),
$insertTypes
Expand Down Expand Up @@ -170,7 +170,7 @@ public function count(PersistentCollection $collection)
. $joinTargetEntitySQL
. ' WHERE ' . implode(' AND ', $conditions);

return $this->conn->fetchColumn($sql, $params, 0, $types);
return (int) $this->conn->fetchOne($sql, $params, $types);
}

/**
Expand Down Expand Up @@ -223,7 +223,7 @@ public function contains(PersistentCollection $collection, $element)

$sql = 'SELECT 1 FROM ' . $quotedJoinTable . ' WHERE ' . implode(' AND ', $whereClauses);

return (bool) $this->conn->fetchColumn($sql, $params, 0, $types);
return (bool) $this->conn->fetchOne($sql, $params, $types);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private function deleteEntityCollection(PersistentCollection $collection): int
$statement = 'DELETE FROM ' . $this->quoteStrategy->getTableName($targetClass, $this->platform)
. ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';

return $this->conn->executeUpdate($statement, $parameters);
return $this->conn->executeStatement($statement, $parameters);
}

/**
Expand Down Expand Up @@ -233,7 +233,7 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):
$statement = $this->platform->getCreateTemporaryTableSnippetSQL() . ' ' . $tempTable
. ' (' . $this->platform->getColumnDeclarationListSQL($columnDefinitions) . ')';

$this->conn->executeUpdate($statement);
$this->conn->executeStatement($statement);

// 2) Build insert table records into temporary table
$query = $this->em->createQuery(
Expand All @@ -243,7 +243,7 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):

$statement = 'INSERT INTO ' . $tempTable . ' (' . $idColumnList . ') ' . $query->getSQL();
$parameters = array_values($sourceClass->getIdentifierValues($collection->getOwner()));
$numDeleted = $this->conn->executeUpdate($statement, $parameters);
$numDeleted = $this->conn->executeStatement($statement, $parameters);

// 3) Delete records on each table in the hierarchy
$classNames = array_merge($targetClass->parentClasses, [$targetClass->name], $targetClass->subClasses);
Expand All @@ -253,13 +253,13 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):
$statement = 'DELETE FROM ' . $tableName . ' WHERE (' . $idColumnList . ')'
. ' IN (SELECT ' . $idColumnList . ' FROM ' . $tempTable . ')';

$this->conn->executeUpdate($statement);
$this->conn->executeStatement($statement);
}

// 4) Drop temporary table
$statement = $this->platform->getDropTemporaryTableSQL($tempTable);

$this->conn->executeUpdate($statement);
$this->conn->executeStatement($statement);

return $numDeleted;
}
Expand Down
12 changes: 5 additions & 7 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public function executeInserts()
}
}

$stmt->execute();
$stmt->executeStatement();

if ($isPostInsertId) {
$generatedId = $idGenerator->generate($this->em, $entity);
Expand All @@ -307,7 +307,6 @@ public function executeInserts()
}
}

$stmt->closeCursor();
$this->queuedInserts = [];

return $postInsertIds;
Expand Down Expand Up @@ -353,10 +352,9 @@ protected function fetchVersionValue($versionedClass, array $id)

$flatId = $this->identifierFlattener->flattenIdentifier($versionedClass, $id);

$value = $this->conn->fetchColumn(
$value = $this->conn->fetchOne(
$sql,
array_values($flatId),
0,
$this->extractIdentifierTypes($id, $versionedClass)
);

Expand Down Expand Up @@ -514,7 +512,7 @@ final protected function updateTable(
. ' SET ' . implode(', ', $set)
. ' WHERE ' . implode(' = ? AND ', $where) . ' = ?';

$result = $this->conn->executeUpdate($sql, $params, $types);
$result = $this->conn->executeStatement($sql, $params, $types);

if ($versioned && ! $result) {
throw OptimisticLockException::lockFailed($entity);
Expand Down Expand Up @@ -837,7 +835,7 @@ public function count($criteria = [])
? $this->expandCriteriaParameters($criteria)
: $this->expandParameters($criteria);

return (int) $this->conn->executeQuery($sql, $params, $types)->fetchColumn();
return (int) $this->conn->executeQuery($sql, $params, $types)->fetchOne();
}

/**
Expand Down Expand Up @@ -2015,7 +2013,7 @@ public function exists($entity, ?Criteria $extraConditions = null)
$sql .= ' AND ' . $filterSql;
}

return (bool) $this->conn->fetchColumn($sql, $params, 0, $types);
return (bool) $this->conn->fetchOne($sql, $params, $types);
}

/**
Expand Down
10 changes: 2 additions & 8 deletions lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public function executeInserts()
$rootTableStmt->bindValue($paramIndex++, $value, $this->columnTypes[$columnName]);
}

$rootTableStmt->execute();
$rootTableStmt->executeStatement();

if ($isPostInsertId) {
$generatedId = $idGenerator->generate($this->em, $entity);
Expand Down Expand Up @@ -204,16 +204,10 @@ public function executeInserts()
}
}

$stmt->execute();
$stmt->executeStatement();
}
}

$rootTableStmt->closeCursor();

foreach ($subTableStmts as $stmt) {
$stmt->closeCursor();
}

$this->queuedInserts = [];

return $postInsertIds;
Expand Down
18 changes: 6 additions & 12 deletions lib/Doctrine/ORM/Query/Exec/MultiTableUpdateExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,11 @@ public function __construct(AST\Node $AST, $sqlWalker)
public function execute(Connection $conn, array $params, array $types)
{
// Create temporary id table
$conn->executeUpdate($this->_createTempTableSql);
$conn->executeStatement($this->_createTempTableSql);

try {
// Insert identifiers. Parameters from the update clause are cut off.
$numUpdated = $conn->executeUpdate(
$numUpdated = $conn->executeStatement(
$this->_insertSql,
array_slice($params, $this->_numParametersInUpdateClause),
array_slice($types, $this->_numParametersInUpdateClause)
Expand All @@ -186,19 +186,13 @@ public function execute(Connection $conn, array $params, array $types)
}
}

$conn->executeUpdate($statement, $paramValues, $paramTypes);
$conn->executeStatement($statement, $paramValues, $paramTypes);
}
} catch (Throwable $exception) {
// FAILURE! Drop temporary table to avoid possible collisions
$conn->executeUpdate($this->_dropTempTableSql);

// Re-throw exception
throw $exception;
} finally {
// Drop temporary table
$conn->executeStatement($this->_dropTempTableSql);
}

// Drop temporary table
$conn->executeUpdate($this->_dropTempTableSql);

return $numUpdated;
}
}
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Query/Exec/SingleTableDeleteUpdateExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ public function __construct(AST\Node $AST, $sqlWalker)
*/
public function execute(Connection $conn, array $params, array $types)
{
return $conn->executeUpdate($this->_sqlStatements, $params, $types);
return $conn->executeStatement($this->_sqlStatements, $params, $types);
}
}
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Tools/SchemaTool.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ private function gatherRelationJoinColumns(
$blacklistedFks[$compositeName] = true;
} elseif (! isset($blacklistedFks[$compositeName])) {
$addedFks[$compositeName] = ['foreignTableName' => $foreignTableName, 'foreignColumns' => $foreignColumns];
$theJoinTable->addUnnamedForeignKeyConstraint(
$theJoinTable->addForeignKeyConstraint(
$foreignTableName,
$localColumns,
$foreignColumns,
Expand Down
33 changes: 26 additions & 7 deletions tests/Doctrine/Tests/Mocks/ConnectionMock.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@

namespace Doctrine\Tests\Mocks;

use BadMethodCallException;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Cache\QueryCacheProfile;
use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Driver\Statement;
use Doctrine\DBAL\ForwardCompatibility\Result as ForwardCompatibilityResult;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Exception;

use function is_string;
use function sprintf;

/**
* Mock class for Connection.
Expand All @@ -25,7 +29,7 @@ class ConnectionMock extends Connection
/** @var Exception|null */
private $_fetchOneException;

/** @var Statement|null */
/** @var ForwardCompatibilityResult|null */
private $_queryResult;

/** @var DatabasePlatformMock */
Expand All @@ -38,7 +42,7 @@ class ConnectionMock extends Connection
private $_inserts = [];

/** @var array */
private $_executeUpdates = [];
private $_executeStatements = [];

/** @var array */
private $_deletes = [];
Expand Down Expand Up @@ -74,7 +78,17 @@ public function insert($tableName, array $data, array $types = [])
*/
public function executeUpdate($query, array $params = [], array $types = [])
{
$this->_executeUpdates[] = ['query' => $query, 'params' => $params, 'types' => $types];
throw new BadMethodCallException(sprintf('Call to deprecated method %s().', __METHOD__));
}

/**
* {@inheritdoc}
*/
public function executeStatement($sql, array $params = [], array $types = []): int
{
$this->_executeStatements[] = ['sql' => $sql, 'params' => $params, 'types' => $types];

return 1;
}

/**
Expand All @@ -96,7 +110,7 @@ public function lastInsertId($seqName = null)
/**
* {@inheritdoc}
*/
public function fetchColumn($statement, array $params = [], $colnum = 0, array $types = [])
public function fetchColumn($statement, array $params = [], $colunm = 0, array $types = [])
{
if ($this->_fetchOneException !== null) {
throw $this->_fetchOneException;
Expand All @@ -110,6 +124,11 @@ public function query(): Statement
return $this->_queryResult;
}

public function executeQuery($sql, array $params = [], $types = [], ?QueryCacheProfile $qcp = null): ForwardCompatibilityResult
{
return $this->_queryResult ?? parent::executeQuery($sql, $params, $types, $qcp);
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -149,7 +168,7 @@ public function setLastInsertId(int $id): void

public function setQueryResult(Statement $result): void
{
$this->_queryResult = $result;
$this->_queryResult = ForwardCompatibilityResult::ensure($result);
}

/**
Expand All @@ -163,9 +182,9 @@ public function getInserts(): array
/**
* @return array
*/
public function getExecuteUpdates(): array
public function getExecuteStatements(): array
{
return $this->_executeUpdates;
return $this->_executeStatements;
}

/**
Expand Down
7 changes: 3 additions & 4 deletions tests/Doctrine/Tests/Mocks/StatementMock.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,9 @@ public function execute($params = null)
{
}

/**
* {@inheritdoc}
*/
public function rowCount()
public function rowCount(): int
{
return 1;
}

/**
Expand Down Expand Up @@ -81,6 +79,7 @@ public function setFetchMode($fetchStyle, $arg2 = null, $arg3 = null)
*/
public function fetch($fetchMode = null, $cursorOrientation = PDO::FETCH_ORI_NEXT, $cursorOffset = 0)
{
return false;
}

/**
Expand Down
Loading
0