From 39170302039b7da5658988e95d7e83945efcf280 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 19 Jun 2020 11:08:18 +0100
Subject: [PATCH] Issue #2345451 by mondrake, daffie, david_garcia,
 shobhit_juyal, sokru, markdorison, alexpott, Beakerboy: Introduce a
 Connection::prepareStatement method allowing to pass options to the
 Statement, deprecate Connection::prepareQuery() and Connection::prepare()

---
 core/lib/Drupal/Core/Database/Connection.php  | 60 +++++++++++++++----
 .../Core/Database/Driver/pgsql/Connection.php |  9 ++-
 .../Core/Database/Driver/pgsql/Insert.php     |  2 +-
 .../Database/Driver/pgsql/NativeUpsert.php    |  2 +-
 .../Core/Database/Driver/pgsql/Update.php     |  2 +-
 .../Database/Driver/sqlite/Connection.php     |  8 ++-
 .../Core/Database/StatementPrefetch.php       |  2 +-
 .../Database/DatabaseExceptionWrapperTest.php | 28 ++++++++-
 .../Core/Database/StatementTest.php           | 48 +++++++++++++++
 .../Tests/Core/Database/ConnectionTest.php    |  6 +-
 10 files changed, 144 insertions(+), 23 deletions(-)
 create mode 100644 core/tests/Drupal/KernelTests/Core/Database/StatementTest.php

diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php
index 1ff55b14ece2..f081b05ee0ae 100644
--- a/core/lib/Drupal/Core/Database/Connection.php
+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -315,6 +315,11 @@ public function destroy() {
    *   database type. In rare cases, such as creating an SQL function, []
    *   characters might be needed and can be allowed by changing this option to
    *   TRUE.
+   * - pdo: By default, queries will execute with the PDO options set on the
+   *   connection. In particular cases, it could be necessary to override the
+   *   PDO driver options on the statement level. In such case, pass the
+   *   required setting as an array here, and they will be passed to the
+   *   prepared statement. See https://www.php.net/manual/en/pdo.prepare.php.
    *
    * @return array
    *   An array of default query options.
@@ -326,6 +331,7 @@ protected function defaultOptions() {
       'throw_exception' => TRUE,
       'allow_delimiter_in_query' => FALSE,
       'allow_square_brackets' => FALSE,
+      'pdo' => [],
     ];
   }
 
@@ -476,6 +482,31 @@ public function getFullQualifiedTableName($table) {
     return $options['database'] . '.' . $prefix . $table;
   }
 
+  /**
+   * Returns a prepared statement given a SQL string.
+   *
+   * This method caches prepared statements, reusing them when possible. It also
+   * prefixes tables names enclosed in curly braces and, optionally, quotes
+   * identifiers enclosed in square brackets.
+   *
+   * @param string $query
+   *   The query string as SQL, with curly braces surrounding the table names.
+   * @param array $options
+   *   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.
+   *
+   * @return \Drupal\Core\Database\StatementInterface
+   *   A PDO prepared statement ready for its execute() method.
+   */
+  public function prepareStatement(string $query, array $options): StatementInterface {
+    $query = $this->prefixTables($query);
+    if (!($options['allow_square_brackets'] ?? FALSE)) {
+      $query = $this->quoteIdentifiers($query);
+    }
+    return $this->connection->prepare($query, $options['pdo'] ?? []);
+  }
+
   /**
    * Prepares a query string and returns the prepared statement.
    *
@@ -492,14 +523,15 @@ public function getFullQualifiedTableName($table) {
    *
    * @return \Drupal\Core\Database\StatementInterface
    *   A PDO prepared statement ready for its execute() method.
+   *
+   * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use
+   *   ::prepareStatement instead.
+   *
+   * @see https://www.drupal.org/node/3137786
    */
   public function prepareQuery($query, $quote_identifiers = TRUE) {
-    $query = $this->prefixTables($query);
-    if ($quote_identifiers) {
-      $query = $this->quoteIdentifiers($query);
-    }
-
-    return $this->connection->prepare($query);
+    @trigger_error('Connection::prepareQuery() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use ::prepareStatement() instead. See https://www.drupal.org/node/3137786', E_USER_DEPRECATED);
+    return $this->prepareStatement($query, ['allow_square_brackets' => !$quote_identifiers]);
   }
 
   /**
@@ -675,9 +707,7 @@ protected function filterComment($comment = '') {
    *   object to this method. It is used primarily for database drivers for
    *   databases that require special LOB field handling.
    * @param array $args
-   *   An array of arguments for the prepared statement. If the prepared
-   *   statement uses ? placeholders, this array must be an indexed array.
-   *   If it contains named placeholders, it must be an associative array.
+   *   The associative array of arguments for the prepared statement.
    * @param array $options
    *   An associative array of options to control how the query is run. The
    *   given options will be merged with self::defaultOptions(). See the
@@ -734,7 +764,7 @@ public function query($query, array $args = [], $options = []) {
         if (strpos($query, ';') !== FALSE && empty($options['allow_delimiter_in_query'])) {
           throw new \InvalidArgumentException('; is not supported in SQL strings. Use only one statement at a time.');
         }
-        $stmt = $this->prepareQuery($query, !$options['allow_square_brackets']);
+        $stmt = $this->prepareStatement($query, $options);
         $stmt->execute($args, $options);
       }
 
@@ -1668,9 +1698,17 @@ abstract public function nextId($existing_id = 0);
    *
    * @throws \PDOException
    *
-   * @see \PDO::prepare()
+   * @see https://www.php.net/manual/en/pdo.prepare.php
+   *
+   * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Database
+   *   drivers should instantiate \PDOStatement objects by calling
+   *   \PDO::prepare in their Collection::prepareStatement method instead.
+   *   \PDO::prepare should not be called outside of driver code.
+   *
+   * @see https://www.drupal.org/node/3137786
    */
   public function prepare($statement, array $driver_options = []) {
+    @trigger_error('Connection::prepare() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Database drivers should instantiate \PDOStatement objects by calling \PDO::prepare in their Collection::prepareStatement method instead. \PDO::prepare should not be called outside of driver code. See https://www.drupal.org/node/3137786', E_USER_DEPRECATED);
     return $this->connection->prepare($statement, $driver_options);
   }
 
diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
index c2aef48a0f37..30b8eae94f4c 100644
--- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
@@ -6,6 +6,7 @@
 use Drupal\Core\Database\Connection as DatabaseConnection;
 use Drupal\Core\Database\DatabaseAccessDeniedException;
 use Drupal\Core\Database\DatabaseNotFoundException;
+use Drupal\Core\Database\StatementInterface;
 
 /**
  * @addtogroup database
@@ -184,12 +185,16 @@ public function query($query, array $args = [], $options = []) {
     return $return;
   }
 
-  public function prepareQuery($query, $quote_identifiers = TRUE) {
+  /**
+   * {@inheritdoc}
+   */
+  public function prepareStatement(string $query, array $options): 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.
-    return parent::prepareQuery(preg_replace('/ ([^ ]+) +(I*LIKE|NOT +I*LIKE|~\*|!~\*) /i', ' ${1}::text ${2} ', $query), $quote_identifiers);
+    $query = preg_replace('/ ([^ ]+) +(I*LIKE|NOT +I*LIKE|~\*|!~\*) /i', ' ${1}::text ${2} ', $query);
+    return parent::prepareStatement($query, $options);
   }
 
   public function queryRange($query, $from, $count, array $args = [], array $options = []) {
diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Insert.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Insert.php
index ce25e647f6c8..2a7ef3686b53 100644
--- a/core/lib/Drupal/Core/Database/Driver/pgsql/Insert.php
+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Insert.php
@@ -20,7 +20,7 @@ public function execute() {
       return NULL;
     }
 
-    $stmt = $this->connection->prepareQuery((string) $this);
+    $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions);
 
     // Fetch the list of blobs and sequences used on that table.
     $table_information = $this->connection->schema()->queryTableInformation($this->table);
diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/NativeUpsert.php b/core/lib/Drupal/Core/Database/Driver/pgsql/NativeUpsert.php
index 03f9db0ddf18..8d338c54e45d 100644
--- a/core/lib/Drupal/Core/Database/Driver/pgsql/NativeUpsert.php
+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/NativeUpsert.php
@@ -19,7 +19,7 @@ public function execute() {
       return NULL;
     }
 
-    $stmt = $this->connection->prepareQuery((string) $this);
+    $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions);
 
     // Fetch the list of blobs and sequences used on that table.
     $table_information = $this->connection->schema()->queryTableInformation($this->table);
diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php
index e937f6c8eace..c4297d1e8cec 100644
--- a/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php
+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Update.php
@@ -18,7 +18,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->prepareQuery((string) $this);
+    $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions);
 
     // Fetch the list of blobs and sequences used on that table.
     $table_information = $this->connection->schema()->queryTableInformation($this->table);
diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
index 8c9a7d41d571..44ef57c6bcb2 100644
--- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
+++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
@@ -5,6 +5,7 @@
 use Drupal\Core\Database\Database;
 use Drupal\Core\Database\DatabaseNotFoundException;
 use Drupal\Core\Database\Connection as DatabaseConnection;
+use Drupal\Core\Database\StatementInterface;
 
 /**
  * SQLite implementation of \Drupal\Core\Database\Connection.
@@ -336,6 +337,7 @@ public static function sqlFunctionLikeBinary($pattern, $subject) {
    * {@inheritdoc}
    */
   public function prepare($statement, array $driver_options = []) {
+    @trigger_error('Connection::prepare() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Database drivers should instantiate \PDOStatement objects by calling \PDO::prepare in their Collection::prepareStatement method instead. \PDO::prepare should not be called outside of driver code. See https://www.drupal.org/node/3137786', E_USER_DEPRECATED);
     return new Statement($this->connection, $this, $statement, $driver_options);
   }
 
@@ -403,12 +405,12 @@ public function mapConditionOperator($operator) {
   /**
    * {@inheritdoc}
    */
-  public function prepareQuery($query, $quote_identifiers = TRUE) {
+  public function prepareStatement(string $query, array $options): StatementInterface {
     $query = $this->prefixTables($query);
-    if ($quote_identifiers) {
+    if (!($options['allow_square_brackets'] ?? FALSE)) {
       $query = $this->quoteIdentifiers($query);
     }
-    return $this->prepare($query);
+    return new Statement($this->connection, $this, $query, $options['pdo'] ?? []);
   }
 
   public function nextId($existing_id = 0) {
diff --git a/core/lib/Drupal/Core/Database/StatementPrefetch.php b/core/lib/Drupal/Core/Database/StatementPrefetch.php
index da0fea5b68c4..3f6efdf3644d 100644
--- a/core/lib/Drupal/Core/Database/StatementPrefetch.php
+++ b/core/lib/Drupal/Core/Database/StatementPrefetch.php
@@ -221,7 +221,7 @@ protected function throwPDOException() {
    *   A PDOStatement object.
    */
   protected function getStatement($query, &$args = []) {
-    return $this->dbh->prepare($query);
+    return $this->dbh->prepare($query, $this->driverOptions);
   }
 
   /**
diff --git a/core/tests/Drupal/KernelTests/Core/Database/DatabaseExceptionWrapperTest.php b/core/tests/Drupal/KernelTests/Core/Database/DatabaseExceptionWrapperTest.php
index 54cc6cd3fae3..b371d968a6b4 100644
--- a/core/tests/Drupal/KernelTests/Core/Database/DatabaseExceptionWrapperTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Database/DatabaseExceptionWrapperTest.php
@@ -14,9 +14,12 @@
 class DatabaseExceptionWrapperTest extends KernelTestBase {
 
   /**
-   * Tests the expected database exception thrown for prepared statements.
+   * Tests deprecation of Connection::prepare.
+   *
+   * @group legacy
+   * @expectedDeprecation Connection::prepare() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Database drivers should instantiate \PDOStatement objects by calling \PDO::prepare in their Collection::prepareStatement method instead. \PDO::prepare should not be called outside of driver code. See https://www.drupal.org/node/3137786
    */
-  public function testPreparedStatement() {
+  public function testPrepare() {
     $connection = Database::getConnection();
     try {
       // SQLite validates the syntax upon preparing a statement already.
@@ -40,6 +43,27 @@ public function testPreparedStatement() {
     }
   }
 
+  /**
+   * Tests deprecation of Connection::prepareQuery.
+   *
+   * @group legacy
+   * @expectedDeprecation Connection::prepareQuery() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use ::prepareStatement() instead. See https://www.drupal.org/node/3137786
+   */
+  public function testPrepareQuery() {
+    $this->expectException(\PDOException::class);
+    $stmt = Database::getConnection()->prepareQuery('bananas');
+    $stmt->execute();
+  }
+
+  /**
+   * Tests Connection::prepareStatement exceptions.
+   */
+  public function testPrepareStatement() {
+    $this->expectException(\PDOException::class);
+    $stmt = Database::getConnection()->prepareStatement('bananas', []);
+    $stmt->execute();
+  }
+
   /**
    * Tests the expected database exception thrown for inexistent tables.
    */
diff --git a/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php b/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php
new file mode 100644
index 000000000000..6cf7bf923550
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Database/StatementTest.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace Drupal\KernelTests\Core\Database;
+
+use Drupal\Core\Database\Database;
+use Drupal\Core\Database\StatementInterface;
+
+/**
+ * Tests the Statement classes.
+ *
+ * @group Database
+ */
+class StatementTest extends DatabaseTestBase {
+
+  /**
+   * Tests that a prepared statement object can be reused for multiple inserts.
+   */
+  public function testRepeatedInsertStatementReuse() {
+    $num_records_before = $this->connection->select('test')->countQuery()->execute()->fetchField();
+
+    $sql = "INSERT INTO {test} ([name], [age]) VALUES (:name, :age)";
+    $args = [
+      ':name' => 'Larry',
+      ':age' => '30',
+    ];
+    $options = [
+      'return' => Database::RETURN_STATEMENT,
+      'allow_square_brackets' => FALSE,
+    ];
+
+    $stmt = $this->connection->prepareStatement($sql, $options);
+    $this->assertInstanceOf(StatementInterface::class, $stmt);
+    $this->assertTrue($stmt->execute($args, $options));
+
+    // We should be able to specify values in any order if named.
+    $args = [
+      ':age' => '31',
+      ':name' => 'Curly',
+    ];
+    $this->assertTrue($stmt->execute($args, $options));
+
+    $num_records_after = $this->connection->select('test')->countQuery()->execute()->fetchField();
+    $this->assertEquals($num_records_before + 2, $num_records_after);
+    $this->assertSame('30', $this->connection->query('SELECT age FROM {test} WHERE name = :name', [':name' => 'Larry'])->fetchField());
+    $this->assertSame('31', $this->connection->query('SELECT age FROM {test} WHERE name = :name', [':name' => 'Curly'])->fetchField());
+  }
+
+}
diff --git a/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
index d5a2dec3163c..a3bc9973aee5 100644
--- a/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
@@ -3,6 +3,7 @@
 namespace Drupal\Tests\Core\Database;
 
 use Composer\Autoload\ClassLoader;
+use Drupal\Core\Database\Statement;
 use Drupal\Tests\Core\Database\Stub\StubConnection;
 use Drupal\Tests\Core\Database\Stub\StubPDO;
 use Drupal\Tests\UnitTestCase;
@@ -585,13 +586,16 @@ public function testQueryTrim($expected, $query, $options) {
     $mock_pdo = $this->getMockBuilder(StubPdo::class)
       ->setMethods(['execute', 'prepare', 'setAttribute'])
       ->getMock();
+    $mock_statement = $this->getMockBuilder(Statement::class)
+      ->disableOriginalConstructor()
+      ->getMock();
 
     // Ensure that PDO::prepare() is called only once, and with the
     // correctly trimmed query string.
     $mock_pdo->expects($this->once())
       ->method('prepare')
       ->with($expected)
-      ->willReturnSelf();
+      ->willReturn($mock_statement);
     $connection = new StubConnection($mock_pdo, []);
     $connection->query($query, [], $options);
   }
-- 
GitLab