Unverified Commit d1461d6b authored by alexpott's avatar alexpott
Browse files

Issue #3189680 by mondrake, daffie, alexpott: Deprecate the 'throw_exception'...

Issue #3189680 by mondrake, daffie, alexpott: Deprecate the 'throw_exception' option in the Database API

(cherry picked from commit 0bff06df)
parent 6967b53c
...@@ -347,11 +347,11 @@ public function __destruct() { ...@@ -347,11 +347,11 @@ public function __destruct() {
* - Database::RETURN_NULL: Do not return anything, as there is no * - Database::RETURN_NULL: Do not return anything, as there is no
* meaningful value to return. That is the case for INSERT queries on * meaningful value to return. That is the case for INSERT queries on
* tables that do not contain a serial column. * tables that do not contain a serial column.
* - throw_exception: By default, the database system will catch any errors * - throw_exception: (deprecated) By default, the database system will catch
* on a query as an Exception, log it, and then rethrow it so that code * any errors on a query as an Exception, log it, and then rethrow it so
* further up the call chain can take an appropriate action. To suppress * that code further up the call chain can take an appropriate action. To
* that behavior and simply return NULL on failure, set this option to * suppress that behavior and simply return NULL on failure, set this
* FALSE. * option to FALSE.
* - allow_delimiter_in_query: By default, queries which have the ; delimiter * - allow_delimiter_in_query: By default, queries which have the ; delimiter
* any place in them will cause an exception. This reduces the chance of SQL * any place in them will cause an exception. This reduces the chance of SQL
* injection attacks that terminate the original query and add one or more * injection attacks that terminate the original query and add one or more
...@@ -376,7 +376,6 @@ protected function defaultOptions() { ...@@ -376,7 +376,6 @@ protected function defaultOptions() {
return [ return [
'fetch' => \PDO::FETCH_OBJ, 'fetch' => \PDO::FETCH_OBJ,
'return' => Database::RETURN_STATEMENT, 'return' => Database::RETURN_STATEMENT,
'throw_exception' => TRUE,
'allow_delimiter_in_query' => FALSE, 'allow_delimiter_in_query' => FALSE,
'allow_square_brackets' => FALSE, 'allow_square_brackets' => FALSE,
'pdo' => [], 'pdo' => [],
...@@ -841,9 +840,7 @@ protected function filterComment($comment = '') { ...@@ -841,9 +840,7 @@ protected function filterComment($comment = '') {
* (not the number matched). * (not the number matched).
* - If $options['return'] === self::RETURN_INSERT_ID, * - If $options['return'] === self::RETURN_INSERT_ID,
* returns the generated insert ID of the last query as a string. * returns the generated insert ID of the last query as a string.
* - If either $options['return'] === self::RETURN_NULL, or * - If $options['return'] === self::RETURN_NULL, returns NULL.
* an exception occurs and $options['throw_exception'] evaluates to FALSE,
* returns NULL.
* *
* @throws \Drupal\Core\Database\DatabaseExceptionWrapper * @throws \Drupal\Core\Database\DatabaseExceptionWrapper
* @throws \Drupal\Core\Database\IntegrityConstraintViolationException * @throws \Drupal\Core\Database\IntegrityConstraintViolationException
...@@ -947,7 +944,7 @@ public function query($query, array $args = [], $options = []) { ...@@ -947,7 +944,7 @@ public function query($query, array $args = [], $options = []) {
*/ */
protected function handleQueryException(\PDOException $e, $query, array $args = [], $options = []) { protected function handleQueryException(\PDOException $e, $query, array $args = [], $options = []) {
@trigger_error('Connection::handleQueryException() is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Get a handler through $this->exceptionHandler() instead, and use one of its methods. See https://www.drupal.org/node/3187222', E_USER_DEPRECATED); @trigger_error('Connection::handleQueryException() is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Get a handler through $this->exceptionHandler() instead, and use one of its methods. See https://www.drupal.org/node/3187222', E_USER_DEPRECATED);
if ($options['throw_exception']) { if ($options['throw_exception'] ?? TRUE) {
// Wrap the exception in another exception, because PHP does not allow // Wrap the exception in another exception, because PHP does not allow
// overriding Exception::getMessage(). Its message is the extra database // overriding Exception::getMessage(). Its message is the extra database
// debug information. // debug information.
......
...@@ -17,8 +17,11 @@ class ExceptionHandler extends BaseExceptionHandler { ...@@ -17,8 +17,11 @@ class ExceptionHandler extends BaseExceptionHandler {
* {@inheritdoc} * {@inheritdoc}
*/ */
public function handleExecutionException(\Exception $exception, StatementInterface $statement, array $arguments = [], array $options = []): void { public function handleExecutionException(\Exception $exception, StatementInterface $statement, array $arguments = [], array $options = []): void {
if (!($options['throw_exception'] ?? TRUE)) { if (array_key_exists('throw_exception', $options)) {
return; @trigger_error('Passing a \'throw_exception\' option to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187', E_USER_DEPRECATED);
if (!($options['throw_exception'])) {
return;
}
} }
if ($exception instanceof \PDOException) { if ($exception instanceof \PDOException) {
......
...@@ -25,8 +25,11 @@ class ExceptionHandler { ...@@ -25,8 +25,11 @@ class ExceptionHandler {
* @throws \Drupal\Core\Database\DatabaseExceptionWrapper * @throws \Drupal\Core\Database\DatabaseExceptionWrapper
*/ */
public function handleStatementException(\Exception $exception, string $sql, array $options = []): void { public function handleStatementException(\Exception $exception, string $sql, array $options = []): void {
if (!($options['throw_exception'] ?? TRUE)) { if (array_key_exists('throw_exception', $options)) {
return; @trigger_error('Passing a \'throw_exception\' option to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187', E_USER_DEPRECATED);
if (!($options['throw_exception'])) {
return;
}
} }
if ($exception instanceof \PDOException) { if ($exception instanceof \PDOException) {
...@@ -57,8 +60,11 @@ public function handleStatementException(\Exception $exception, string $sql, arr ...@@ -57,8 +60,11 @@ public function handleStatementException(\Exception $exception, string $sql, arr
* @throws \Drupal\Core\Database\IntegrityConstraintViolationException * @throws \Drupal\Core\Database\IntegrityConstraintViolationException
*/ */
public function handleExecutionException(\Exception $exception, StatementInterface $statement, array $arguments = [], array $options = []): void { public function handleExecutionException(\Exception $exception, StatementInterface $statement, array $arguments = [], array $options = []): void {
if (!($options['throw_exception'] ?? TRUE)) { if (array_key_exists('throw_exception', $options)) {
return; @trigger_error('Passing a \'throw_exception\' option to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187', E_USER_DEPRECATED);
if (!($options['throw_exception'])) {
return;
}
} }
if ($exception instanceof \PDOException) { if ($exception instanceof \PDOException) {
......
...@@ -352,10 +352,6 @@ public function __toString() { ...@@ -352,10 +352,6 @@ public function __toString() {
} }
public function execute() { public function execute() {
// Default options for merge queries.
$this->queryOptions += [
'throw_exception' => TRUE,
];
try { try {
if (!count($this->condition)) { if (!count($this->condition)) {
...@@ -397,12 +393,15 @@ public function execute() { ...@@ -397,12 +393,15 @@ public function execute() {
} }
} }
catch (\Exception $e) { catch (\Exception $e) {
if ($this->queryOptions['throw_exception']) { // @todo 'throw_exception' option is deprecated. Remove in D10.
throw $e; // @see https://www.drupal.org/project/drupal/issues/3210310
} if (array_key_exists('throw_exception', $this->queryOptions)) {
else { @trigger_error('Passing a \'throw_exception\' option to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187', E_USER_DEPRECATED);
return NULL; if (!($this->queryOptions['throw_exception'])) {
return NULL;
}
} }
throw $e;
} }
} }
......
...@@ -97,6 +97,26 @@ public function testPrepareStatementFailOnPreparation() { ...@@ -97,6 +97,26 @@ public function testPrepareStatementFailOnPreparation() {
$stmt = $foo_connection->prepareStatement('bananas', []); $stmt = $foo_connection->prepareStatement('bananas', []);
} }
/**
* Tests Connection::prepareStatement with throw_exception option set.
*
* @group legacy
*/
public function testPrepareStatementFailOnPreparationWithThrowExceptionOption(): void {
$driver = Database::getConnection()->driver();
if ($driver !== 'mysql') {
$this->markTestSkipped("MySql tests can not run for driver '$driver'.");
}
$connection_info = Database::getConnectionInfo('default');
$connection_info['default']['pdo'][\PDO::ATTR_EMULATE_PREPARES] = FALSE;
Database::addConnectionInfo('default', 'foo', $connection_info['default']);
$foo_connection = Database::getConnection('foo', 'default');
$this->expectException(DatabaseExceptionWrapper::class);
$this->expectDeprecation('Passing a \'throw_exception\' option to %AExceptionHandler::handleStatementException is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187');
$stmt = $foo_connection->prepareStatement('bananas', ['throw_exception' => FALSE]);
}
/** /**
* Tests the expected database exception thrown for inexistent tables. * Tests the expected database exception thrown for inexistent tables.
*/ */
......
...@@ -196,35 +196,27 @@ public function testMergeUpdateWithoutUpdate() { ...@@ -196,35 +196,27 @@ public function testMergeUpdateWithoutUpdate() {
* Tests that an invalid merge query throws an exception. * Tests that an invalid merge query throws an exception.
*/ */
public function testInvalidMerge() { public function testInvalidMerge() {
try { $this->expectException(InvalidMergeQueryException::class);
// This query will fail because there is no key field specified. // This merge will fail because there is no key field specified.
// Normally it would throw an exception but we are suppressing it with $this->connection
// the throw_exception option. ->merge('test_people')
$options['throw_exception'] = FALSE; ->fields(['age' => 31, 'name' => 'Tiffany'])
$this->connection->merge('test_people', $options) ->execute();
->fields([ }
'age' => 31,
'name' => 'Tiffany', /**
]) * Tests deprecation of the 'throw_exception' option.
->execute(); *
} * @group legacy
catch (InvalidMergeQueryException $e) { */
$this->fail('$options[\'throw_exception\'] is FALSE, but InvalidMergeQueryException thrown for invalid query.'); public function testLegacyThrowExceptionOption(): void {
} $this->expectDeprecation("Passing a 'throw_exception' option to %AMerge::execute is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187");
// This merge will fail because there is no key field specified.
try { $this->assertNull($this->connection
// This query will fail because there is no key field specified. ->merge('test_people', ['throw_exception' => FALSE])
$this->connection->merge('test_people') ->fields(['age' => 31, 'name' => 'Tiffany'])
->fields([ ->execute()
'age' => 31, );
'name' => 'Tiffany',
])
->execute();
$this->fail('InvalidMergeQueryException should be thrown.');
}
catch (\Exception $e) {
$this->assertInstanceOf(InvalidMergeQueryException::class, $e);
}
} }
/** /**
......
...@@ -550,35 +550,31 @@ public function testSelectDuplicateAlias() { ...@@ -550,35 +550,31 @@ public function testSelectDuplicateAlias() {
$this->assertNotSame($alias1, $alias2, 'Duplicate aliases are renamed.'); $this->assertNotSame($alias1, $alias2, 'Duplicate aliases are renamed.');
} }
/**
* Tests deprecation of the 'throw_exception' option.
*
* @group legacy
*/
public function testLegacyThrowExceptionOption(): void {
$this->expectDeprecation("Passing a 'throw_exception' option to %AExceptionHandler::handleExecutionException is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Always catch exceptions. See https://www.drupal.org/node/3201187");
// This query will fail because the table does not exist.
$this->assertNull($this->connection->select('some_table_that_does_not_exist', 't', ['throw_exception' => FALSE])
->fields('t')
->countQuery()
->execute()
);
}
/** /**
* Tests that an invalid count query throws an exception. * Tests that an invalid count query throws an exception.
*/ */
public function testInvalidSelectCount() { public function testInvalidSelectCount() {
try { $this->expectException(DatabaseExceptionWrapper::class);
// This query will fail because the table does not exist. // This query will fail because the table does not exist.
// Normally it would throw an exception but we are suppressing $this->connection->select('some_table_that_does_not_exist', 't')
// it with the throw_exception option. ->fields('t')
$options['throw_exception'] = FALSE; ->countQuery()
$this->connection->select('some_table_that_does_not_exist', 't', $options) ->execute();
->fields('t')
->countQuery()
->execute();
}
catch (\Exception $e) {
$this->fail('$options[\'throw_exception\'] is FALSE, but Exception thrown for invalid query.');
}
try {
// This query will fail because the table does not exist.
$this->connection->select('some_table_that_does_not_exist', 't')
->fields('t')
->countQuery()
->execute();
$this->fail('No Exception thrown.');
}
catch (\Exception $e) {
$this->assertInstanceOf(DatabaseExceptionWrapper::class, $e);
}
} }
/** /**
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment