From 201212c8f8ad8eb4c1cd5cfc3c677ba6222114aa Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 17 May 2021 09:18:32 +0100 Subject: [PATCH] Issue #3177660 by mondrake, andypost, anmolgoyal74, daffie, alexpott: Remove public properties from StatementInterface implementations --- core/lib/Drupal/Core/Database/Connection.php | 10 ++- .../Core/Database/Driver/pgsql/Connection.php | 4 +- .../Core/Database/Driver/pgsql/Update.php | 12 +--- .../Core/Database/Driver/pgsql/Upsert.php | 5 +- .../Database/Driver/sqlite/Connection.php | 28 ++++---- core/lib/Drupal/Core/Database/Log.php | 5 +- .../lib/Drupal/Core/Database/Query/Delete.php | 9 ++- .../Drupal/Core/Database/Query/Truncate.php | 17 ++++- .../lib/Drupal/Core/Database/Query/Update.php | 9 ++- .../lib/Drupal/Core/Database/Query/Upsert.php | 9 ++- core/lib/Drupal/Core/Database/Statement.php | 7 ++ .../Core/Database/StatementInterface.php | 29 +++----- .../Core/Database/StatementPrefetch.php | 72 ++++++++++++++++--- .../Drupal/Core/Database/StatementWrapper.php | 51 ++++++++++--- .../Core/Database/DeleteTruncateTest.php | 18 +++++ .../Core/Database/StatementTest.php | 26 +++++++ .../KernelTests/Core/Database/UpdateTest.php | 27 +++++++ .../KernelTests/Core/Database/UpsertTest.php | 16 +++++ 18 files changed, 278 insertions(+), 76 deletions(-) diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php index f1029ab21ca0..e0f9c5f63136 100644 --- a/core/lib/Drupal/Core/Database/Connection.php +++ b/core/lib/Drupal/Core/Database/Connection.php @@ -543,6 +543,9 @@ public function getFullQualifiedTableName($table) { * An associative array of options to control how the query is run. See * the documentation for self::defaultOptions() for details. The content of * the 'pdo' key will be passed to the prepared statement. + * @param bool $allow_row_count + * (optional) A flag indicating if row count is allowed on the statement + * object. Defaults to FALSE. * * @return \Drupal\Core\Database\StatementInterface * A PDO prepared statement ready for its execute() method. @@ -552,14 +555,14 @@ public function getFullQualifiedTableName($table) { * not allowed in the query. * @throws \Drupal\Core\Database\DatabaseExceptionWrapper */ - public function prepareStatement(string $query, array $options): StatementInterface { + public function prepareStatement(string $query, array $options, bool $allow_row_count = FALSE): StatementInterface { try { $query = $this->preprocessStatement($query, $options); // @todo in Drupal 10, only return the StatementWrapper. // @see https://www.drupal.org/node/3177490 $statement = $this->statementWrapperClass ? - new $this->statementWrapperClass($this, $this->connection, $query, $options['pdo'] ?? []) : + new $this->statementWrapperClass($this, $this->connection, $query, $options['pdo'] ?? [], $allow_row_count) : $this->connection->prepare($query, $options['pdo'] ?? []); } catch (\Exception $e) { @@ -887,6 +890,9 @@ public function query($query, array $args = [], $options = []) { case Database::RETURN_STATEMENT: return $stmt; + // Database::RETURN_AFFECTED should not be used; enable row counting + // by passing the appropriate argument to the constructor instead. + // @see https://www.drupal.org/node/3186368 case Database::RETURN_AFFECTED: $stmt->allowRowCount = TRUE; return $stmt->rowCount(); diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php index 35d1f3e24bf3..a5b6ae3296ca 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php @@ -201,13 +201,13 @@ public function query($query, array $args = [], $options = []) { /** * {@inheritdoc} */ - public function prepareStatement(string $query, array $options): StatementInterface { + public function prepareStatement(string $query, array $options, bool $allow_row_count = FALSE): StatementInterface { // mapConditionOperator converts some operations (LIKE, REGEXP, etc.) to // PostgreSQL equivalents (ILIKE, ~*, etc.). However PostgreSQL doesn't // automatically cast the fields to the right type for these operators, // so we need to alter the query and add the type-cast. $query = preg_replace('/ ([^ ]+) +(I*LIKE|NOT +I*LIKE|~\*|!~\*) /i', ' ${1}::text ${2} ', $query); - return parent::prepareStatement($query, $options); + return parent::prepareStatement($query, $options, $allow_row_count); } public function queryRange($query, $from, $count, array $args = [], array $options = []) { diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php index 8a3d918c9096..993f763aa20b 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php @@ -2,7 +2,6 @@ namespace Drupal\Core\Database\Driver\pgsql; -use Drupal\Core\Database\Database; use Drupal\Core\Database\Query\Update as QueryUpdate; use Drupal\Core\Database\Query\SelectInterface; @@ -18,7 +17,7 @@ public function execute() { // Because we filter $fields the same way here and in __toString(), the // placeholders will all match up properly. - $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions); + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); // Fetch the list of blobs and sequences used on that table. $table_information = $this->connection->schema()->queryTableInformation($this->table); @@ -69,20 +68,15 @@ public function execute() { } } - $options = $this->queryOptions; - $options['already_prepared'] = TRUE; - $options['return'] = Database::RETURN_AFFECTED; - $this->connection->addSavepoint(); try { - $stmt->execute(NULL, $options); + $stmt->execute(NULL, $this->queryOptions); $this->connection->releaseSavepoint(); - $stmt->allowRowCount = TRUE; return $stmt->rowCount(); } catch (\Exception $e) { $this->connection->rollbackSavepoint(); - throw $e; + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, [], $this->queryOptions); } } diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Upsert.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Upsert.php index 8e0fcce999bb..6f8d72582f9b 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Upsert.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Upsert.php @@ -19,8 +19,7 @@ public function execute() { return NULL; } - $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions); - $stmt->allowRowCount = TRUE; + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); // Fetch the list of blobs and sequences used on that table. $table_information = $this->connection->schema()->queryTableInformation($this->table); @@ -87,7 +86,7 @@ public function execute() { } catch (\Exception $e) { $this->connection->rollbackSavepoint(); - throw $e; + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, [], $options); } } diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php index 4f18449b2e07..4a43d11aa567 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php @@ -2,7 +2,6 @@ namespace Drupal\Core\Database\Driver\sqlite; -use Drupal\Core\Database\Database; use Drupal\Core\Database\DatabaseNotFoundException; use Drupal\Core\Database\Connection as DatabaseConnection; use Drupal\Core\Database\StatementInterface; @@ -431,10 +430,10 @@ public function mapConditionOperator($operator) { /** * {@inheritdoc} */ - public function prepareStatement(string $query, array $options): StatementInterface { + public function prepareStatement(string $query, array $options, bool $allow_row_count = FALSE): StatementInterface { try { $query = $this->preprocessStatement($query, $options); - $statement = new Statement($this->connection, $this, $query, $options['pdo'] ?? []); + $statement = new Statement($this->connection, $this, $query, $options['pdo'] ?? [], $allow_row_count); } catch (\Exception $e) { $this->exceptionHandler()->handleStatementException($e, $query, $options); @@ -449,20 +448,21 @@ public function nextId($existing_id = 0) { // override nextId. However, this is unlikely as we deal with short strings // and integers and no known databases require special handling for those // simple cases. If another transaction wants to write the same row, it will - // wait until this transaction commits. Also, the return value needs to be - // set to RETURN_AFFECTED as if it were a real update() query otherwise it - // is not possible to get the row count properly. - $affected = $this->query('UPDATE {sequences} SET value = GREATEST(value, :existing_id) + 1', [ - ':existing_id' => $existing_id, - ], ['return' => Database::RETURN_AFFECTED]); - if (!$affected) { - $this->query('INSERT INTO {sequences} (value) VALUES (:existing_id + 1)', [ - ':existing_id' => $existing_id, - ]); + // wait until this transaction commits. + $stmt = $this->prepareStatement('UPDATE {sequences} SET [value] = GREATEST([value], :existing_id) + 1', [], TRUE); + $args = [':existing_id' => $existing_id]; + try { + $stmt->execute($args); + } + catch (\Exception $e) { + $this->exceptionHandler()->handleExecutionException($e, $stmt, $args, []); + } + if ($stmt->rowCount() === 0) { + $this->query('INSERT INTO {sequences} ([value]) VALUES (:existing_id + 1)', $args); } // The transaction gets committed when the transaction object gets destroyed // because it gets out of scope. - return $this->query('SELECT value FROM {sequences}')->fetchField(); + return $this->query('SELECT [value] FROM {sequences}')->fetchField(); } /** diff --git a/core/lib/Drupal/Core/Database/Log.php b/core/lib/Drupal/Core/Database/Log.php index 191a3493ac9d..6499a2b6703f 100644 --- a/core/lib/Drupal/Core/Database/Log.php +++ b/core/lib/Drupal/Core/Database/Log.php @@ -116,10 +116,13 @@ public function end($logging_key) { */ public function log(StatementInterface $statement, $args, $time, float $start = NULL) { foreach (array_keys($this->queryLog) as $key) { + // @todo Remove the method_exists check for getConnectionTarget in + // Drupal 10. + // @see https://www.drupal.org/project/drupal/issues/3210310 $this->queryLog[$key][] = [ 'query' => $statement->getQueryString(), 'args' => $args, - 'target' => $statement->dbh->getTarget(), + 'target' => method_exists($statement, 'getConnectionTarget') ? $statement->getConnectionTarget() : $statement->dbh->getTarget(), 'caller' => $this->findCaller(), 'time' => $time, 'start' => $start, diff --git a/core/lib/Drupal/Core/Database/Query/Delete.php b/core/lib/Drupal/Core/Database/Query/Delete.php index 658b8342ccde..2fb0b91449d8 100644 --- a/core/lib/Drupal/Core/Database/Query/Delete.php +++ b/core/lib/Drupal/Core/Database/Query/Delete.php @@ -52,7 +52,14 @@ public function execute() { $values = $this->condition->arguments(); } - return $this->connection->query((string) $this, $values, $this->queryOptions); + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); + try { + $stmt->execute($values, $this->queryOptions); + return $stmt->rowCount(); + } + catch (\Exception $e) { + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, $values, $this->queryOptions); + } } /** diff --git a/core/lib/Drupal/Core/Database/Query/Truncate.php b/core/lib/Drupal/Core/Database/Query/Truncate.php index 1d8fd36e0faf..2c0adcb31012 100644 --- a/core/lib/Drupal/Core/Database/Query/Truncate.php +++ b/core/lib/Drupal/Core/Database/Query/Truncate.php @@ -51,10 +51,23 @@ public function compiled() { * Executes the TRUNCATE query. * * @return - * Return value is dependent on the database type. + * Return value is dependent on whether the executed SQL statement is a + * TRUNCATE or a DELETE. TRUNCATE is DDL and no information on affected + * rows is available. DELETE is DML and will return the number of affected + * rows. In general, do not rely on the value returned by this method in + * calling code. + * + * @see https://learnsql.com/blog/difference-between-truncate-delete-and-drop-table-in-sql */ public function execute() { - return $this->connection->query((string) $this, [], $this->queryOptions); + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); + try { + $stmt->execute([], $this->queryOptions); + return $stmt->rowCount(); + } + catch (\Exception $e) { + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, [], $this->queryOptions); + } } /** diff --git a/core/lib/Drupal/Core/Database/Query/Update.php b/core/lib/Drupal/Core/Database/Query/Update.php index 2b08e20d0018..88af918647e0 100644 --- a/core/lib/Drupal/Core/Database/Query/Update.php +++ b/core/lib/Drupal/Core/Database/Query/Update.php @@ -145,7 +145,14 @@ public function execute() { $update_values = array_merge($update_values, $this->condition->arguments()); } - return $this->connection->query((string) $this, $update_values, $this->queryOptions); + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); + try { + $stmt->execute($update_values, $this->queryOptions); + return $stmt->rowCount(); + } + catch (\Exception $e) { + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, $update_values, $this->queryOptions); + } } /** diff --git a/core/lib/Drupal/Core/Database/Query/Upsert.php b/core/lib/Drupal/Core/Database/Query/Upsert.php index 596b347a58c0..524b145b55ab 100644 --- a/core/lib/Drupal/Core/Database/Query/Upsert.php +++ b/core/lib/Drupal/Core/Database/Query/Upsert.php @@ -108,7 +108,14 @@ public function execute() { } } - $affected_rows = $this->connection->query((string) $this, $values, $this->queryOptions); + $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions, TRUE); + try { + $stmt->execute($values, $this->queryOptions); + $affected_rows = $stmt->rowCount(); + } + catch (\Exception $e) { + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, $values, $this->queryOptions); + } // Re-initialize the values array so that we can re-use this query. $this->insertValues = []; diff --git a/core/lib/Drupal/Core/Database/Statement.php b/core/lib/Drupal/Core/Database/Statement.php index 7a52c35cee2f..fb9a1b8eb66b 100644 --- a/core/lib/Drupal/Core/Database/Statement.php +++ b/core/lib/Drupal/Core/Database/Statement.php @@ -44,6 +44,13 @@ protected function __construct(Connection $dbh) { $this->setFetchMode(\PDO::FETCH_OBJ); } + /** + * {@inheritdoc} + */ + public function getConnectionTarget(): string { + return $this->connection->getTarget(); + } + /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Database/StatementInterface.php b/core/lib/Drupal/Core/Database/StatementInterface.php index 90a25c50dd1f..8bfb76201329 100644 --- a/core/lib/Drupal/Core/Database/StatementInterface.php +++ b/core/lib/Drupal/Core/Database/StatementInterface.php @@ -20,25 +20,6 @@ */ interface StatementInterface extends \Traversable { - /** - * Constructs a new PDOStatement object. - * - * The PDO manual does not document this constructor, but when overriding the - * PDOStatement class with a custom without this constructor, PDO will throw - * the internal exception/warning: - * - * "PDO::query(): SQLSTATE[HY000]: General error: user-supplied statement does - * not accept constructor arguments" - * - * PDO enforces that the access type of this constructor must be protected, - * and lastly, it also enforces that a custom PDOStatement interface (like - * this) omits the constructor (declaring it results in fatal errors - * complaining about "the access type must not be public" if it is public, and - * "the access type must be omitted" if it is protected; i.e., conflicting - * statements). The access type has to be protected. - */ - // protected function __construct(Connection $dbh); - /** * Executes a prepared statement. * @@ -61,6 +42,16 @@ public function execute($args = [], $options = []); */ public function getQueryString(); + /** + * Returns the target connection this statement is associated with. + * + * @return string + * The target connection string of this statement. + */ + // @todo Include this method in the interface in Drupal 10. + // @see https://www.drupal.org/project/drupal/issues/3210310 + // public function getConnectionTarget(): string; + /** * Returns the number of rows affected by the last SQL statement. * diff --git a/core/lib/Drupal/Core/Database/StatementPrefetch.php b/core/lib/Drupal/Core/Database/StatementPrefetch.php index 3b672739d1ec..1f2f1709cb7e 100644 --- a/core/lib/Drupal/Core/Database/StatementPrefetch.php +++ b/core/lib/Drupal/Core/Database/StatementPrefetch.php @@ -25,11 +25,11 @@ class StatementPrefetch implements \Iterator, StatementInterface { protected $driverOptions; /** - * Reference to the Drupal database connection object for this statement. + * The Drupal database connection object. * * @var \Drupal\Core\Database\Connection */ - public $dbh; + protected $connection; /** * Reference to the PDO connection object for this statement. @@ -124,13 +124,65 @@ class StatementPrefetch implements \Iterator, StatementInterface { * * @var bool */ - public $allowRowCount = FALSE; + protected $rowCountEnabled = FALSE; - public function __construct(\PDO $pdo_connection, Connection $connection, $query, array $driver_options = []) { + /** + * Constructs a StatementPrefetch object. + * + * @param \PDO $pdo_connection + * An object of the PDO class representing a database connection. + * @param \Drupal\Core\Database\Connection $connection + * The database connection. + * @param string $query + * The query string. + * @param array $driver_options + * Driver-specific options. + * @param bool $row_count_enabled + * (optional) Enables counting the rows affected. Defaults to FALSE. + */ + public function __construct(\PDO $pdo_connection, Connection $connection, $query, array $driver_options = [], bool $row_count_enabled = FALSE) { $this->pdoConnection = $pdo_connection; - $this->dbh = $connection; + $this->connection = $connection; $this->queryString = $query; $this->driverOptions = $driver_options; + $this->rowCountEnabled = $row_count_enabled; + } + + /** + * Implements the magic __get() method. + * + * @todo Remove the method before Drupal 10. + * @see https://www.drupal.org/i/3210310 + */ + public function __get($name) { + if ($name === 'dbh') { + @trigger_error(__CLASS__ . '::$dbh should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->connection instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + return $this->connection; + } + if ($name === 'allowRowCount') { + @trigger_error(__CLASS__ . '::$allowRowCount should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->rowCountEnabled instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + return $this->rowCountEnabled; + } + } + + /** + * Implements the magic __set() method. + * + * @todo Remove the method before Drupal 10. + * @see https://www.drupal.org/i/3210310 + */ + public function __set($name, $value) { + if ($name === 'allowRowCount') { + @trigger_error(__CLASS__ . '::$allowRowCount should not be written in drupal:9.3.0 and will error in drupal:10.0.0. Enable row counting by passing the appropriate argument to the constructor instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + $this->rowCountEnabled = $value; + } + } + + /** + * {@inheritdoc} + */ + public function getConnectionTarget(): string { + return $this->connection->getTarget(); } /** @@ -149,7 +201,7 @@ public function execute($args = [], $options = []) { } } - $logger = $this->dbh->getLogger(); + $logger = $this->connection->getLogger(); if (!empty($logger)) { $query_start = microtime(TRUE); } @@ -165,7 +217,7 @@ public function execute($args = [], $options = []) { $this->throwPDOException(); } - if ($options['return'] == Database::RETURN_AFFECTED) { + if ($this->rowCountEnabled) { $this->rowCount = $statement->rowCount(); } // Fetch all the data from the reply, in order to release any lock @@ -199,7 +251,7 @@ public function execute($args = [], $options = []) { * Throw a PDO Exception based on the last PDO error. */ protected function throwPDOException() { - $error_info = $this->dbh->errorInfo(); + $error_info = $this->connection->errorInfo(); // We rebuild a message formatted in the same way as PDO. $exception = new \PDOException("SQLSTATE[" . $error_info[0] . "]: General error " . $error_info[1] . ": " . $error_info[2]); $exception->errorInfo = $error_info; @@ -221,7 +273,7 @@ protected function throwPDOException() { * A PDOStatement object. */ protected function getStatement($query, &$args = []) { - return $this->dbh->prepare($query, $this->driverOptions); + return $this->connection->prepare($query, $this->driverOptions); } /** @@ -364,7 +416,7 @@ public function valid() { */ public function rowCount() { // SELECT query should not use the method. - if ($this->allowRowCount) { + if ($this->rowCountEnabled) { return $this->rowCount; } else { diff --git a/core/lib/Drupal/Core/Database/StatementWrapper.php b/core/lib/Drupal/Core/Database/StatementWrapper.php index 7132efb62791..4bef35c22d2c 100644 --- a/core/lib/Drupal/Core/Database/StatementWrapper.php +++ b/core/lib/Drupal/Core/Database/StatementWrapper.php @@ -14,7 +14,7 @@ class StatementWrapper implements \IteratorAggregate, StatementInterface { * * @var \Drupal\Core\Database\Connection */ - public $dbh; + protected $connection; /** * The client database Statement object. @@ -30,7 +30,7 @@ class StatementWrapper implements \IteratorAggregate, StatementInterface { * * @var bool */ - public $allowRowCount = FALSE; + protected $rowCountEnabled = FALSE; /** * Constructs a StatementWrapper object. @@ -43,26 +43,48 @@ class StatementWrapper implements \IteratorAggregate, StatementInterface { * The SQL query string. * @param array $options * Array of query options. + * @param bool $row_count_enabled + * (optional) Enables counting the rows affected. Defaults to FALSE. */ - public function __construct(Connection $connection, $client_connection, string $query, array $options) { - $this->dbh = $connection; + public function __construct(Connection $connection, $client_connection, string $query, array $options, bool $row_count_enabled = FALSE) { + $this->connection = $connection; $this->clientStatement = $client_connection->prepare($query, $options); + $this->rowCountEnabled = $row_count_enabled; $this->setFetchMode(\PDO::FETCH_OBJ); } /** * Implements the magic __get() method. * - * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Access the - * client-level statement object via ::getClientStatement(). - * - * @see https://www.drupal.org/node/3177488 + * @todo Remove the method before Drupal 10. + * @see https://www.drupal.org/i/3210310 */ public function __get($name) { if ($name === 'queryString') { - @trigger_error("StatementWrapper::\${$name} should not be accessed in drupal:9.1.0 and will error in drupal:10.0.0. Access the client-level statement object via ::getClientStatement(). See https://www.drupal.org/node/3177488", E_USER_DEPRECATED); + @trigger_error("StatementWrapper::\$queryString should not be accessed in drupal:9.1.0 and will error in drupal:10.0.0. Access the client-level statement object via ::getClientStatement(). See https://www.drupal.org/node/3177488", E_USER_DEPRECATED); return $this->getClientStatement()->queryString; } + if ($name === 'dbh') { + @trigger_error(__CLASS__ . '::$dbh should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->connection instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + return $this->connection; + } + if ($name === 'allowRowCount') { + @trigger_error(__CLASS__ . '::$allowRowCount should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->rowCountEnabled instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + return $this->rowCountEnabled; + } + } + + /** + * Implements the magic __set() method. + * + * @todo Remove the method before Drupal 10. + * @see https://www.drupal.org/i/3210310 + */ + public function __set($name, $value) { + if ($name === 'allowRowCount') { + @trigger_error(__CLASS__ . '::$allowRowCount should not be written in drupal:9.3.0 and will error in drupal:10.0.0. Enable row counting by passing the appropriate argument to the constructor instead. See https://www.drupal.org/node/3186368', E_USER_DEPRECATED); + $this->rowCountEnabled = $value; + } } /** @@ -93,6 +115,13 @@ public function getClientStatement() { return $this->clientStatement; } + /** + * {@inheritdoc} + */ + public function getConnectionTarget(): string { + return $this->connection->getTarget(); + } + /** * {@inheritdoc} */ @@ -108,7 +137,7 @@ public function execute($args = [], $options = []) { } } - $logger = $this->dbh->getLogger(); + $logger = $this->connection->getLogger(); if (!empty($logger)) { $query_start = microtime(TRUE); } @@ -202,7 +231,7 @@ public function fetchObject(string $class_name = NULL, array $constructor_argume */ public function rowCount() { // SELECT query should not use the method. - if ($this->allowRowCount) { + if ($this->rowCountEnabled) { return $this->clientStatement->rowCount(); } else { diff --git a/core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php b/core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php index f1e384cb6079..0bd4b2913682 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php @@ -2,6 +2,8 @@ namespace Drupal\KernelTests\Core\Database; +use Drupal\Core\Database\DatabaseExceptionWrapper; + /** * Tests delete and truncate queries. * @@ -149,4 +151,20 @@ public function testSpecialColumnDelete() { $this->assertEquals($num_records_before, $num_records_after + $num_deleted, 'Deletion adds up.'); } + /** + * Deleting from a not existing table throws a DatabaseExceptionWrapper. + */ + public function testDeleteFromNonExistingTable(): void { + $this->expectException(DatabaseExceptionWrapper::class); + $this->connection->delete('a-table-that-does-not-exist')->execute(); + } + + /** + * Truncating a not existing table throws a DatabaseExceptionWrapper. + */ + public function testTruncateNonExistingTable(): void { + $this->expectException(DatabaseExceptionWrapper::class); + $this->connection->truncate('a-table-that-does-not-exist')->execute(); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php b/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php index 65b8c47f6330..b0071a4bf07f 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php @@ -45,4 +45,30 @@ public function testRepeatedInsertStatementReuse() { $this->assertSame('31', $this->connection->query('SELECT [age] FROM {test} WHERE [name] = :name', [':name' => 'Curly'])->fetchField()); } + /** + * Tests accessing deprecated properties. + * + * @group legacy + */ + public function testGetDeprecatedProperties(): void { + $statement = $this->connection->prepareStatement('SELECT * FROM {test}', []); + $this->expectDeprecation('%s$dbh should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->connection instead. See https://www.drupal.org/node/3186368'); + $this->assertNotNull($statement->dbh); + $this->expectDeprecation('%s$allowRowCount should not be accessed in drupal:9.3.0 and will error in drupal:10.0.0. Use $this->rowCountEnabled instead. See https://www.drupal.org/node/3186368'); + $this->assertFalse($statement->allowRowCount); + } + + /** + * Tests writing deprecated properties. + * + * @group legacy + */ + public function testSetDeprecatedProperties(): void { + $statement = $this->connection->prepareStatement('UPDATE {test} SET [age] = :age', []); + $this->expectDeprecation('%s$allowRowCount should not be written in drupal:9.3.0 and will error in drupal:10.0.0. Enable row counting by passing the appropriate argument to the constructor instead. See https://www.drupal.org/node/3186368'); + $statement->allowRowCount = TRUE; + $statement->execute([':age' => 12]); + $this->assertEquals(4, $statement->rowCount()); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/Database/UpdateTest.php b/core/tests/Drupal/KernelTests/Core/Database/UpdateTest.php index c3bdde6b617a..0fd41c654187 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/UpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/UpdateTest.php @@ -2,6 +2,9 @@ namespace Drupal\KernelTests\Core\Database; +use Drupal\Core\Database\DatabaseExceptionWrapper; +use Drupal\Core\Database\IntegrityConstraintViolationException; + /** * Tests the update query builder. * @@ -143,4 +146,28 @@ public function testSpecialColumnUpdate() { $this->assertEquals('New update value', $saved_value); } + /** + * Updating a not existing table throws a DatabaseExceptionWrapper. + */ + public function testUpdateNonExistingTable(): void { + $this->expectException(DatabaseExceptionWrapper::class); + $this->connection->update('a-table-that-does-not-exist') + ->fields([ + 'update' => 'New update value', + ]) + ->condition('id', 1) + ->execute(); + } + + /** + * Updating a serial field throws a IntegrityConstraintViolationException. + */ + public function testUpdateValueInSerial(): void { + $this->expectException(IntegrityConstraintViolationException::class); + $this->connection->update('test') + ->fields(['id' => 2]) + ->condition('id', 1) + ->execute(); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/Database/UpsertTest.php b/core/tests/Drupal/KernelTests/Core/Database/UpsertTest.php index 36919d1274da..a3c02126a35f 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/UpsertTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/UpsertTest.php @@ -3,6 +3,7 @@ namespace Drupal\KernelTests\Core\Database; use Drupal\Core\Database\Database; +use Drupal\Core\Database\DatabaseExceptionWrapper; /** * Tests the Upsert query builder. @@ -91,4 +92,19 @@ public function testUpsertWithKeywords() { $this->assertEquals('Update value 2', $record->update); } + /** + * Upsert on a not existing table throws a DatabaseExceptionWrapper. + */ + public function testUpsertNonExistingTable(): void { + $this->expectException(DatabaseExceptionWrapper::class); + $upsert = $this->connection->upsert('a-table-that-does-not-exist') + ->key('id') + ->fields(['id', 'update']); + $upsert->values([ + 'id' => 1, + 'update' => 'Update value 1 updated', + ]); + $upsert->execute(); + } + } -- GitLab