Issue #1713332 by amateescu, Xen, droplet, bzrudi71, znerol, czigor, pp,...

Issue #1713332 by amateescu, Xen, droplet, bzrudi71, znerol, czigor, pp, wulff, Damien Tournoud: The SQLite database driver fails to drop simpletest tables

core/includes/database.inc

@@ -666,7 +666,6 @@ function db_field_exists($table, $field) {
  *
  * @param string $table_expression
  *   An SQL expression, for example "simpletest%" (without the quotes).
- *   BEWARE: this is not prefixed, the caller should take care of that.
  *
  * @return array
  *   Array, both the keys and the values are the matching tables.

core/lib/Drupal/Core/Command/DbDumpCommand.php

@@ -129,12 +129,9 @@ protected function generateScript() {
    *   An array of table names.
    */
   protected function getTables() {
-    $pattern = $this->connection->tablePrefix() . '%';
-    $tables = array_values($this->connection->schema()->findTables($pattern));
-    foreach ($tables as $key => $table) {
-      // The prefix is removed for the resultant script.
-      $table = $tables[$key] = str_replace($this->connection->tablePrefix(), '', $table);
+    $tables = array_values($this->connection->schema()->findTables('%'));
 
+    foreach ($tables as $key => $table) {
       // Remove any explicitly excluded tables.
       foreach ($this->excludeTables as $pattern) {
         if (preg_match('/^' . $pattern . '$/', $table)) {
@@ -142,6 +139,7 @@ protected function getTables() {
         }
       }
     }
+
     return $tables;
   }
 

core/lib/Drupal/Core/Database/Connection.php

@@ -138,6 +138,13 @@ abstract class Connection {
    */
   protected $prefixReplace = array();
 
+  /**
+   * List of un-prefixed table names, keyed by prefixed table names.
+   *
+   * @var array
+   */
+  protected $unprefixedTablesMap = [];
+
   /**
    * Constructs a Connection object.
    *
@@ -185,7 +192,9 @@ public function destroy() {
     // Destroy all references to this connection by setting them to NULL.
     // The Statement class attribute only accepts a new value that presents a
     // proper callable, so we reset it to PDOStatement.
-    $this->connection->setAttribute(\PDO::ATTR_STATEMENT_CLASS, array('PDOStatement', array()));
+    if (!empty($this->statementClass)) {
+      $this->connection->setAttribute(\PDO::ATTR_STATEMENT_CLASS, array('PDOStatement', array()));
+    }
     $this->schema = NULL;
   }
 
@@ -289,6 +298,13 @@ protected function setPrefix($prefix) {
     $this->prefixReplace[] = $this->prefixes['default'];
     $this->prefixSearch[] = '}';
     $this->prefixReplace[] = '';
+
+    // Set up a map of prefixed => un-prefixed tables.
+    foreach ($this->prefixes as $table_name => $prefix) {
+      if ($table_name !== 'default') {
+        $this->unprefixedTablesMap[$prefix . $table_name] = $table_name;
+      }
+    }
   }
 
   /**
@@ -327,6 +343,17 @@ public function tablePrefix($table = 'default') {
     }
   }
 
+  /**
+   * Gets a list of individually prefixed table names.
+   *
+   * @return array
+   *   An array of un-prefixed table names, keyed by their fully qualified table
+   *   names (i.e. prefix + table_name).
+   */
+  public function getUnprefixedTablesMap() {
+    return $this->unprefixedTablesMap;
+  }
+
   /**
    * Get a fully qualified table name.
    *

core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php

@@ -154,9 +154,9 @@ public function __destruct() {
 
           // We can prune the database file if it doesn't have any tables.
           if ($count == 0) {
-            // Detach the database.
-            $this->query('DETACH DATABASE :schema', array(':schema' => $prefix));
-            // Destroy the database file.
+            // Detaching the database fails at this point, but no other queries
+            // are executed after the connection is destructed so we can simply
+            // remove the database file.
             unlink($this->connectionOptions['database'] . '-' . $prefix);
           }
         }
@@ -168,6 +168,18 @@ public function __destruct() {
     }
   }
 
+  /**
+   * Gets all the attached databases.
+   *
+   * @return array
+   *   An array of attached database names.
+   *
+   * @see \Drupal\Core\Database\Driver\sqlite\Connection::__construct()
+   */
+  public function getAttachedDatabases() {
+    return $this->attachedDatabases;
+  }
+
   /**
    * SQLite compatibility implementation for the IF() SQL function.
    */

core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php

@@ -696,16 +696,31 @@ public function fieldSetNoDefault($table, $field) {
     $this->alterTable($table, $old_schema, $new_schema);
   }
 
+  /**
+   * {@inheritdoc}
+   */
   public function findTables($table_expression) {
-    // Don't add the prefix, $table_expression already includes the prefix.
-    $info = $this->getPrefixInfo($table_expression, FALSE);
-
-    // Can't use query placeholders for the schema because the query would have
-    // to be :prefixsqlite_master, which does not work.
-    $result = db_query("SELECT name FROM " . $info['schema'] . ".sqlite_master WHERE type = :type AND name LIKE :table_name", array(
-      ':type' => 'table',
-      ':table_name' => $info['table'],
-    ));
-    return $result->fetchAllKeyed(0, 0);
+    $tables = [];
+
+    // The SQLite implementation doesn't need to use the same filtering strategy
+    // as the parent one because individually prefixed tables live in their own
+    // schema (database), which means that neither the main database nor any
+    // attached one will contain a prefixed table name, so we just need to loop
+    // over all known schemas and filter by the user-supplied table expression.
+    $attached_dbs = $this->connection->getAttachedDatabases();
+    foreach ($attached_dbs as $schema) {
+      // Can't use query placeholders for the schema because the query would
+      // have to be :prefixsqlite_master, which does not work. We also need to
+      // ignore the internal SQLite tables.
+      $result = db_query("SELECT name FROM " . $schema . ".sqlite_master WHERE type = :type AND name LIKE :table_name AND name NOT LIKE :pattern", array(
+        ':type' => 'table',
+        ':table_name' => $table_expression,
+        ':pattern' => 'sqlite_%',
+      ));
+      $tables += $result->fetchAllKeyed(0, 0);
+    }
+
+    return $tables;
   }
+
 }

core/lib/Drupal/Core/Database/Schema.php

@@ -16,6 +16,11 @@
  */
 abstract class Schema implements PlaceholderInterface {
 
+  /**
+   * The database connection.
+   *
+   * @var \Drupal\Core\Database\Connection
+   */
   protected $connection;
 
   /**
@@ -173,25 +178,62 @@ public function tableExists($table) {
   }
 
   /**
-   * Find all tables that are like the specified base table name.
+   * Finds all tables that are like the specified base table name.
    *
-   * @param $table_expression
-   *   An SQL expression, for example "simpletest%" (without the quotes).
-   *   BEWARE: this is not prefixed, the caller should take care of that.
+   * @param string $table_expression
+   *   An SQL expression, for example "cache_%" (without the quotes).
    *
-   * @return
-   *   Array, both the keys and the values are the matching tables.
+   * @return array
+   *   Both the keys and the values are the matching tables.
    */
   public function findTables($table_expression) {
-    $condition = $this->buildTableNameCondition($table_expression, 'LIKE', FALSE);
-
+    // Load all the tables up front in order to take into account per-table
+    // prefixes. The actual matching is done at the bottom of the method.
+    $condition = $this->buildTableNameCondition('%', 'LIKE');
     $condition->compile($this->connection, $this);
+
+    $individually_prefixed_tables = $this->connection->getUnprefixedTablesMap();
+    $default_prefix = $this->connection->tablePrefix();
+    $default_prefix_length = strlen($default_prefix);
+    $tables = [];
     // Normally, we would heartily discourage the use of string
     // concatenation for conditionals like this however, we
     // couldn't use db_select() here because it would prefix
     // information_schema.tables and the query would fail.
     // Don't use {} around information_schema.tables table.
-    return $this->connection->query("SELECT table_name FROM information_schema.tables WHERE " . (string) $condition, $condition->arguments())->fetchAllKeyed(0, 0);
+    $results = $this->connection->query("SELECT table_name FROM information_schema.tables WHERE " . (string) $condition, $condition->arguments());
+    foreach ($results as $table) {
+      // Take into account tables that have an individual prefix.
+      if (isset($individually_prefixed_tables[$table->table_name])) {
+        $prefix_length = strlen($this->connection->tablePrefix($individually_prefixed_tables[$table->table_name]));
+      }
+      elseif ($default_prefix && substr($table->table_name, 0, $default_prefix_length) !== $default_prefix) {
+        // This table name does not start the default prefix, which means that
+        // it is not managed by Drupal so it should be excluded from the result.
+        continue;
+      }
+      else {
+        $prefix_length = $default_prefix_length;
+      }
+
+      // Remove the prefix from the returned tables.
+      $unprefixed_table_name = substr($table->table_name, $prefix_length);
+
+      // The pattern can match a table which is the same as the prefix. That
+      // will become an empty string when we remove the prefix, which will
+      // probably surprise the caller, besides not being a prefixed table. So
+      // remove it.
+      if (!empty($unprefixed_table_name)) {
+        $tables[$unprefixed_table_name] = $unprefixed_table_name;
+      }
+    }
+
+    // Convert the table expression from its SQL LIKE syntax to a regular
+    // expression and escape the delimiter that will be used for matching.
+    $table_expression = str_replace(array('%', '_'), array('.*?', '.'), preg_quote($table_expression, '/'));
+    $tables = preg_grep('/^' . $table_expression . '$/i', $tables);
+
+    return $tables;
   }
 
   /**

core/modules/simpletest/src/BrowserTestBase.php

@@ -324,10 +324,9 @@ protected function cleanupEnvironment() {
     $test_connection_info = Database::getConnectionInfo('default');
     $test_prefix = $test_connection_info['default']['prefix']['default'];
     if ($original_prefix != $test_prefix) {
-      $tables = Database::getConnection()->schema()->findTables($test_prefix . '%');
-      $prefix_length = strlen($test_prefix);
+      $tables = Database::getConnection()->schema()->findTables('%');
       foreach ($tables as $table) {
-        if (Database::getConnection()->schema()->dropTable(substr($table, $prefix_length))) {
+        if (Database::getConnection()->schema()->dropTable($table)) {
           unset($tables[$table]);
         }
       }

core/modules/simpletest/src/TestBase.php

@@ -1280,10 +1280,9 @@ private function restoreEnvironment() {
     $test_connection_info = Database::getConnectionInfo('default');
     $test_prefix = $test_connection_info['default']['prefix']['default'];
     if ($original_prefix != $test_prefix) {
-      $tables = Database::getConnection()->schema()->findTables($test_prefix . '%');
-      $prefix_length = strlen($test_prefix);
+      $tables = Database::getConnection()->schema()->findTables('%');
       foreach ($tables as $table) {
-        if (Database::getConnection()->schema()->dropTable(substr($table, $prefix_length))) {
+        if (Database::getConnection()->schema()->dropTable($table)) {
           unset($tables[$table]);
         }
       }

core/modules/simpletest/src/Tests/KernelTestBaseTest.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\simpletest\Tests;
 
+use Drupal\Core\Database\Database;
 use Drupal\simpletest\KernelTestBase;
 
 /**
@@ -324,4 +325,39 @@ public function testDrupalGetProfile() {
     $this->assertNull(drupal_get_profile());
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function run(array $methods = array()) {
+    parent::run($methods);
+
+    // Check that all tables of the test instance have been deleted. At this
+    // point the original database connection is restored so we need to prefix
+    // the tables.
+    $connection = Database::getConnection();
+    if ($connection->databaseType() != 'sqlite') {
+      $tables = $connection->schema()->findTables($this->databasePrefix . '%');
+      $this->assertTrue(empty($tables), 'All test tables have been removed.');
+    }
+    else {
+      // We don't have the test instance connection anymore so we have to
+      // re-attach its database and then use the same query as
+      // \Drupal\Core\Database\Driver\sqlite\Schema::findTables().
+      // @see \Drupal\Core\Database\Driver\sqlite\Connection::__construct()
+      $info = Database::getConnectionInfo();
+      $connection->query('ATTACH DATABASE :database AS :prefix', [
+        ':database' => $info['default']['database'] . '-' . $this->databasePrefix,
+        ':prefix' => $this->databasePrefix
+      ]);
+
+      $result = $connection->query("SELECT name FROM " . $this->databasePrefix . ".sqlite_master WHERE type = :type AND name LIKE :table_name AND name NOT LIKE :pattern", array(
+        ':type' => 'table',
+        ':table_name' => '%',
+        ':pattern' => 'sqlite_%',
+      ))->fetchAllKeyed(0, 0);
+
+      $this->assertTrue(empty($result), 'All test tables have been removed.');
+    }
+  }
+
 }

core/modules/system/src/Tests/Database/SchemaTest.php

@@ -698,4 +698,63 @@ protected function assertFieldChange($old_spec, $new_spec) {
     // Clean-up.
     db_drop_table($table_name);
   }
+
+  /**
+   * Tests the findTables() method.
+   */
+  public function testFindTables() {
+    // We will be testing with three tables, two of them using the default
+    // prefix and the third one with an individually specified prefix.
+
+    // Set up a new connection with different connection info.
+    $connection_info = Database::getConnectionInfo();
+
+    // Add per-table prefix to the second table.
+    $new_connection_info = $connection_info['default'];
+    $new_connection_info['prefix']['test_2_table'] = $new_connection_info['prefix']['default'] . '_shared_';
+    Database::addConnectionInfo('test', 'default', $new_connection_info);
+
+    Database::setActiveConnection('test');
+
+    // Create the tables.
+    $table_specification = [
+      'description' => 'Test table.',
+      'fields' => [
+        'id'  => [
+          'type' => 'int',
+          'default' => NULL,
+        ],
+      ],
+    ];
+    Database::getConnection()->schema()->createTable('test_1_table', $table_specification);
+    Database::getConnection()->schema()->createTable('test_2_table', $table_specification);
+    Database::getConnection()->schema()->createTable('the_third_table', $table_specification);
+
+    // Check the "all tables" syntax.
+    $tables = Database::getConnection()->schema()->findTables('%');
+    sort($tables);
+    $expected = [
+      // The 'config' table is added by
+      // \Drupal\simpletest\KernelTestBase::containerBuild().
+      'config',
+      'test_1_table',
+      // This table uses a per-table prefix, yet it is returned as un-prefixed.
+      'test_2_table',
+      'the_third_table',
+    ];
+    $this->assertEqual($tables, $expected, 'All tables were found.');
+
+    // Check the restrictive syntax.
+    $tables = Database::getConnection()->schema()->findTables('test_%');
+    sort($tables);
+    $expected = [
+      'test_1_table',
+      'test_2_table',
+    ];
+    $this->assertEqual($tables, $expected, 'Two tables were found.');
+
+    // Go back to the initial connection.
+    Database::setActiveConnection('default');
+  }
+
 }