Issue #2664322 by dawehner, benjifisher, almaudoh: key_value table is only...

Issue #2664322 by dawehner, benjifisher, almaudoh: key_value table is only used by a core service but it depends on system install

core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php

@@ -5,6 +5,7 @@
 use Drupal\Component\Serialization\SerializationInterface;
 use Drupal\Core\Database\Query\Merge;
 use Drupal\Core\Database\Connection;
+use Drupal\Core\Database\SchemaObjectExistsException;
 use Drupal\Core\DependencyInjection\DependencySerializationTrait;
 
 /**
@@ -61,10 +62,16 @@ public function __construct($collection, SerializationInterface $serializer, Con
    * {@inheritdoc}
    */
   public function has($key) {
-    return (bool) $this->connection->query('SELECT 1 FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name = :key', array(
-      ':collection' => $this->collection,
-      ':key' => $key,
-    ))->fetchField();
+    try {
+      return (bool) $this->connection->query('SELECT 1 FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name = :key', array(
+        ':collection' => $this->collection,
+        ':key' => $key,
+      ))->fetchField();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+      return FALSE;
+    }
   }
 
   /**
@@ -84,6 +91,7 @@ public function getMultiple(array $keys) {
       // @todo: Perhaps if the database is never going to be available,
       // key/value requests should return FALSE in order to allow exception
       // handling to occur but for now, keep it an array, always.
+      $this->catchException($e);
     }
     return $values;
   }
@@ -92,7 +100,14 @@ public function getMultiple(array $keys) {
    * {@inheritdoc}
    */
   public function getAll() {
-    $result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection', array(':collection' => $this->collection));
+    try {
+      $result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection', array(':collection' => $this->collection));
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+      $result = [];
+    }
+
     $values = array();
 
     foreach ($result as $item) {
@@ -107,40 +122,75 @@ public function getAll() {
    * {@inheritdoc}
    */
   public function set($key, $value) {
-    $this->connection->merge($this->table)
-      ->keys(array(
-        'name' => $key,
-        'collection' => $this->collection,
-      ))
-      ->fields(array('value' => $this->serializer->encode($value)))
-      ->execute();
+    $try_again = FALSE;
+    try {
+      $this->connection->merge($this->table)
+        ->keys(array(
+          'name' => $key,
+          'collection' => $this->collection,
+        ))
+        ->fields(array('value' => $this->serializer->encode($value)))
+        ->execute();
+    }
+    catch (\Exception $e) {
+      // If there was an exception, try to create the table.
+      if (!$try_again = $this->ensureTableExists()) {
+        // If the exception happened for other reason than the missing bin
+        // table, propagate the exception.
+        throw $e;
+      }
+    }
+    // Now that the bin has been created, try again if necessary.
+    if ($try_again) {
+      $this->set($key, $value);
+    }
   }
 
   /**
    * {@inheritdoc}
    */
   public function setIfNotExists($key, $value) {
-    $result = $this->connection->merge($this->table)
-      ->insertFields(array(
-        'collection' => $this->collection,
-        'name' => $key,
-        'value' => $this->serializer->encode($value),
-      ))
-      ->condition('collection', $this->collection)
-      ->condition('name', $key)
-      ->execute();
-    return $result == Merge::STATUS_INSERT;
+    $try_again = FALSE;
+    try {
+      $result = $this->connection->merge($this->table)
+        ->insertFields(array(
+          'collection' => $this->collection,
+          'name' => $key,
+          'value' => $this->serializer->encode($value),
+        ))
+        ->condition('collection', $this->collection)
+        ->condition('name', $key)
+        ->execute();
+      return $result == Merge::STATUS_INSERT;
+    }
+    catch (\Exception $e) {
+      // If there was an exception, try to create the table.
+      if (!$try_again = $this->ensureTableExists()) {
+        // If the exception happened for other reason than the missing bin
+        // table, propagate the exception.
+        throw $e;
+      }
+    }
+    // Now that the bin has been created, try again if necessary.
+    if ($try_again) {
+      return $this->setIfNotExists($key, $value);
+    }
   }
 
   /**
    * {@inheritdoc}
    */
   public function rename($key, $new_key) {
-    $this->connection->update($this->table)
-      ->fields(array('name' => $new_key))
-      ->condition('collection', $this->collection)
-      ->condition('name', $key)
-      ->execute();
+    try {
+      $this->connection->update($this->table)
+        ->fields(array('name' => $new_key))
+        ->condition('collection', $this->collection)
+        ->condition('name', $key)
+        ->execute();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+    }
   }
 
   /**
@@ -149,20 +199,98 @@ public function rename($key, $new_key) {
   public function deleteMultiple(array $keys) {
     // Delete in chunks when a large array is passed.
     while ($keys) {
+      try {
+        $this->connection->delete($this->table)
+          ->condition('name', array_splice($keys, 0, 1000), 'IN')
+          ->condition('collection', $this->collection)
+          ->execute();
+      }
+      catch (\Exception $e) {
+        $this->catchException($e);
+      }
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function deleteAll() {
+    try {
       $this->connection->delete($this->table)
-        ->condition('name', array_splice($keys, 0, 1000), 'IN')
         ->condition('collection', $this->collection)
         ->execute();
     }
+    catch (\Exception $e) {
+      $this->catchException($e);
+    }
   }
 
   /**
-   * {@inheritdoc}
+   * Check if the table exists and create it if not.
    */
-  public function deleteAll() {
-    $this->connection->delete($this->table)
-      ->condition('collection', $this->collection)
-      ->execute();
+  protected function ensureTableExists() {
+    try {
+      $database_schema = $this->connection->schema();
+      if (!$database_schema->tableExists($this->table)) {
+        $database_schema->createTable($this->table, $this->schemaDefinition());
+        return TRUE;
+      }
+    }
+    // If the table already exists, then attempting to recreate it will throw an
+    // exception. In this case just catch the exception and do nothing.
+    catch (SchemaObjectExistsException $e) {
+      return TRUE;
+    }
+    return FALSE;
+  }
+
+  /**
+   * Act on an exception when the table might not have been created.
+   *
+   * If the table does not yet exist, that's fine, but if the table exists and
+   * something else cause the exception, then propagate it.
+   *
+   * @param \Exception $e
+   *   The exception.
+   *
+   * @throws \Exception
+   */
+  protected function catchException(\Exception $e) {
+    if ($this->connection->schema()->tableExists($this->table)) {
+      throw $e;
+    }
+  }
+
+  /**
+   * Defines the schema for the key_value table.
+   */
+  public static function schemaDefinition() {
+    return [
+      'description' => 'Generic key-value storage table. See the state system for an example.',
+      'fields' => [
+        'collection' => [
+          'description' => 'A named collection of key and value pairs.',
+          'type' => 'varchar_ascii',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ],
+        'name' => [
+          'description' => 'The key of the key-value pair. As KEY is a SQL reserved keyword, name was chosen instead.',
+          'type' => 'varchar_ascii',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ],
+        'value' => [
+          'description' => 'The value.',
+          'type' => 'blob',
+          'not null' => TRUE,
+          'size' => 'big',
+        ],
+      ],
+      'primary key' => ['collection', 'name'],
+    ];
   }
 
 }

core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php

@@ -34,71 +34,123 @@ public function __construct($collection, SerializationInterface $serializer, Con
    * {@inheritdoc}
    */
   public function has($key) {
-    return (bool) $this->connection->query('SELECT 1 FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name = :key AND expire > :now', array(
-      ':collection' => $this->collection,
-      ':key' => $key,
-      ':now' => REQUEST_TIME,
-    ))->fetchField();
+    try {
+      return (bool) $this->connection->query('SELECT 1 FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name = :key AND expire > :now', array(
+        ':collection' => $this->collection,
+        ':key' => $key,
+        ':now' => REQUEST_TIME,
+      ))->fetchField();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+      return FALSE;
+    }
   }
 
   /**
    * {@inheritdoc}
    */
   public function getMultiple(array $keys) {
-    $values = $this->connection->query(
-      'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE expire > :now AND name IN ( :keys[] ) AND collection = :collection',
-      array(
-        ':now' => REQUEST_TIME,
-        ':keys[]' => $keys,
-        ':collection' => $this->collection,
-      ))->fetchAllKeyed();
-    return array_map(array($this->serializer, 'decode'), $values);
+    try {
+      $values = $this->connection->query(
+        'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE expire > :now AND name IN ( :keys[] ) AND collection = :collection',
+        array(
+          ':now' => REQUEST_TIME,
+          ':keys[]' => $keys,
+          ':collection' => $this->collection,
+        ))->fetchAllKeyed();
+      return array_map(array($this->serializer, 'decode'), $values);
+    }
+    catch (\Exception $e) {
+      // @todo: Perhaps if the database is never going to be available,
+      // key/value requests should return FALSE in order to allow exception
+      // handling to occur but for now, keep it an array, always.
+      // https://www.drupal.org/node/2787737
+      $this->catchException($e);
+    }
+    return [];
   }
 
   /**
    * {@inheritdoc}
    */
   public function getAll() {
-    $values = $this->connection->query(
-      'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND expire > :now',
-      array(
-        ':collection' => $this->collection,
-        ':now' => REQUEST_TIME
-      ))->fetchAllKeyed();
-    return array_map(array($this->serializer, 'decode'), $values);
+    try {
+      $values = $this->connection->query(
+        'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND expire > :now',
+        array(
+          ':collection' => $this->collection,
+          ':now' => REQUEST_TIME
+        ))->fetchAllKeyed();
+      return array_map(array($this->serializer, 'decode'), $values);
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+    }
+    return [];
   }
 
   /**
    * {@inheritdoc}
    */
   function setWithExpire($key, $value, $expire) {
-    $this->connection->merge($this->table)
-      ->keys(array(
-        'name' => $key,
-        'collection' => $this->collection,
-      ))
-      ->fields(array(
-        'value' => $this->serializer->encode($value),
-        'expire' => REQUEST_TIME + $expire,
-      ))
-      ->execute();
+    $try_again = FALSE;
+    try {
+      $this->connection->merge($this->table)
+        ->keys(array(
+          'name' => $key,
+          'collection' => $this->collection,
+        ))
+        ->fields(array(
+          'value' => $this->serializer->encode($value),
+          'expire' => REQUEST_TIME + $expire,
+        ))
+        ->execute();
+    }
+    catch (\Exception $e) {
+      // If there was an exception, try to create the table.
+      if (!$try_again = $this->ensureTableExists()) {
+        // If the exception happened for other reason than the missing bin
+        // table, propagate the exception.
+        throw $e;
+      }
+    }
+    // Now that the bin has been created, try again if necessary.
+    if ($try_again) {
+      $this->setWithExpire($key, $value, $expire);
+    }
   }
 
   /**
    * {@inheritdoc}
    */
   function setWithExpireIfNotExists($key, $value, $expire) {
-    $result = $this->connection->merge($this->table)
-      ->insertFields(array(
-        'collection' => $this->collection,
-        'name' => $key,
-        'value' => $this->serializer->encode($value),
-        'expire' => REQUEST_TIME + $expire,
-      ))
-      ->condition('collection', $this->collection)
-      ->condition('name', $key)
-      ->execute();
-    return $result == Merge::STATUS_INSERT;
+    $try_again = FALSE;
+    try {
+      $result = $this->connection->merge($this->table)
+        ->insertFields(array(
+          'collection' => $this->collection,
+          'name' => $key,
+          'value' => $this->serializer->encode($value),
+          'expire' => REQUEST_TIME + $expire,
+        ))
+        ->condition('collection', $this->collection)
+        ->condition('name', $key)
+        ->execute();
+      return $result == Merge::STATUS_INSERT;
+    }
+    catch (\Exception $e) {
+      // If there was an exception, try to create the table.
+      if (!$try_again = $this->ensureTableExists()) {
+        // If the exception happened for other reason than the missing bin
+        // table, propagate the exception.
+        throw $e;
+      }
+    }
+    // Now that the bin has been created, try again if necessary.
+    if ($try_again) {
+      return $this->setWithExpireIfNotExists($key, $value, $expire);
+    }
   }
 
   /**
@@ -117,4 +169,47 @@ public function deleteMultiple(array $keys) {
     parent::deleteMultiple($keys);
   }
 
+  /**
+   * Defines the schema for the key_value_expire table.
+   */
+  public static function schemaDefinition() {
+    return [
+      'description' => 'Generic key/value storage table with an expiration.',
+      'fields' => [
+        'collection' => [
+          'description' => 'A named collection of key and value pairs.',
+          'type' => 'varchar_ascii',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ],
+        'name' => [
+          // KEY is an SQL reserved word, so use 'name' as the key's field name.
+          'description' => 'The key of the key/value pair.',
+          'type' => 'varchar_ascii',
+          'length' => 128,
+          'not null' => TRUE,
+          'default' => '',
+        ],
+        'value' => [
+          'description' => 'The value of the key/value pair.',
+          'type' => 'blob',
+          'not null' => TRUE,
+          'size' => 'big',
+        ],
+        'expire' => [
+          'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.',
+          'type' => 'int',
+          'not null' => TRUE,
+          'default' => 2147483647,
+        ],
+      ],
+      'primary key' => ['collection', 'name'],
+      'indexes' => [
+        'all' => ['name', 'collection', 'expire'],
+        'expire' => ['expire'],
+      ],
+    ];
+  }
+
 }

core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php

@@ -58,9 +58,31 @@ public function get($collection) {
    * Deletes expired items.
    */
   public function garbageCollection() {
-    $this->connection->delete('key_value_expire')
-      ->condition('expire', REQUEST_TIME, '<')
-      ->execute();
+    try {
+      $this->connection->delete('key_value_expire')
+        ->condition('expire', REQUEST_TIME, '<')
+        ->execute();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+    }
+  }
+
+  /**
+   * Act on an exception when the table might not have been created.
+   *
+   * If the table does not yet exist, that's fine, but if the table exists and
+   * something else cause the exception, then propagate it.
+   *
+   * @param \Exception $e
+   *   The exception.
+   *
+   * @throws \Exception
+   */
+  protected function catchException(\Exception $e) {
+    if ($this->connection->schema()->tableExists('key_value_expire')) {
+      throw $e;
+    }
   }
 
 }

core/lib/Drupal/Core/KeyValueStore/KeyValueStoreInterface.php

@@ -48,7 +48,8 @@ public function get($key, $default = NULL);
    * @return array
    *   An associative array of items successfully returned, indexed by key.
    *
-   * @todo What's returned for non-existing keys?
+   * @todo Determine the best return value for non-existing keys in
+   *   https://www.drupal.org/node/2787737
    */
   public function getMultiple(array $keys);
 

core/modules/dblog/tests/src/Kernel/DbLogFormInjectionTest.php

@@ -79,7 +79,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   protected function setUp() {
     parent::setUp();
     $this->installSchema('dblog', ['watchdog']);
-    $this->installSchema('system', ['key_value_expire', 'sequences']);
+    $this->installSchema('system', ['sequences']);
     $this->installEntitySchema('user');
     $this->logger = \Drupal::logger('test_logger');
     $test_user = User::create(array(

core/modules/editor/tests/src/Kernel/EditorImageDialogTest.php

@@ -36,7 +36,6 @@ class EditorImageDialogTest extends EntityKernelTestBase {
   protected function setUp() {
     parent::setUp();
     $this->installEntitySchema('file');
-    $this->installSchema('system', ['key_value_expire']);
     $this->installSchema('node', array('node_access'));
     $this->installSchema('file', array('file_usage'));
     $this->installConfig(['node']);

core/modules/field/tests/src/Kernel/FieldKernelTestBase.php

@@ -47,7 +47,7 @@ protected function setUp() {
 
     $this->installEntitySchema('entity_test');
     $this->installEntitySchema('user');
-    $this->installSchema('system', ['sequences', 'key_value']);
+    $this->installSchema('system', ['sequences']);
 
     // Set default storage backend and configure the theme system.
     $this->installConfig(array('field', 'system'));

core/modules/rest/tests/fixtures/update/rest-export-with-authentication.php

@@ -66,10 +66,12 @@
   ])
   ->execute();
 
+$config = Yaml::decode(file_get_contents('core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization.yml'));
+$config['uuid'] = '1D52D29E-2DD8-417A-9CDA-1BBC5A013B1F';
 $connection->merge('config')
   ->condition('name', 'views.view.rest_export_with_authorization')
   ->condition('collection', '')
   ->fields([
-    'data' => serialize(Yaml::decode(file_get_contents('core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization.yml'))),
+    'data' => serialize($config),
   ])
   ->execute();

core/modules/rest/tests/fixtures/update/rest.resource.entity.comment_2721595.yml

 id: entity.comment
+uuid: 4093D90C-69E6-4024-B177-25EE075F86D8
 plugin_id: 'entity:comment'
 granularity: method
 configuration:

core/modules/rest/tests/fixtures/update/rest.resource.entity.node_2721595.yml

 id: entity.node
+uuid: 63E588C5-B0EA-4156-8EFE-1A288DAA975F
 plugin_id: 'entity:node'
 granularity: method
 configuration:

core/modules/rest/tests/fixtures/update/rest.resource.entity.user_2721595.yml

 id: entity.user
+uuid: A4674E26-9831-40B4-B3E7-A087C8C9B00D
 plugin_id: 'entity:user'
 granularity: method
 configuration:

core/modules/system/src/Tests/Path/UrlAliasFixtures.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\system\Tests\Path;
 
+use Drupal\Core\KeyValueStore\DatabaseStorage;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Path\AliasStorage;
 
@@ -89,7 +90,7 @@ public function tableDefinition() {
     $schema = system_schema();
 
     $tables['url_alias'] = AliasStorage::schemaDefinition();
-    $tables['key_value'] = $schema['key_value'];
+    $tables['key_value'] = DatabaseStorage::schemaDefinition();
 
     return $tables;
   }

core/modules/system/system.install

@@ -890,71 +890,6 @@ function system_install() {
  * Implements hook_schema().
  */
 function system_schema() {
-  $schema['key_value'] = array(
-    'description' => 'Generic key-value storage table. See the state system for an example.',
-    'fields' => array(
-      'collection' => array(
-        'description' => 'A named collection of key and value pairs.',
-        'type' => 'varchar_ascii',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'name' => array(
-        'description' => 'The key of the key-value pair. As KEY is a SQL reserved keyword, name was chosen instead.',
-        'type' => 'varchar_ascii',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'value' => array(
-        'description' => 'The value.',
-        'type' => 'blob',
-        'not null' => TRUE,
-        'size' => 'big',
-      ),
-    ),
-    'primary key' => array('collection', 'name'),
-  );
-
-  $schema['key_value_expire'] = array(
-    'description' => 'Generic key/value storage table with an expiration.',
-    'fields' => array(
-      'collection' => array(
-        'description' => 'A named collection of key and value pairs.',
-        'type' => 'varchar_ascii',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'name' => array(
-        // KEY is an SQL reserved word, so use 'name' as the key's field name.
-        'description' => 'The key of the key/value pair.',
-        'type' => 'varchar_ascii',
-        'length' => 128,
-        'not null' => TRUE,
-        'default' => '',
-      ),
-      'value' => array(
-        'description' => 'The value of the key/value pair.',
-        'type' => 'blob',
-        'not null' => TRUE,
-        'size' => 'big',
-      ),
-      'expire' => array(
-        'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.',
-        'type' => 'int',
-        'not null' => TRUE,
-        'default' => 2147483647,
-      ),
-    ),
-    'primary key' => array('collection', 'name'),
-    'indexes' => array(
-      'all' => array('name', 'collection', 'expire'),
-      'expire' => array('expire'),
-    ),
-  );
-
   $schema['sequences'] = array(
     'description' => 'Stores IDs.',
     'fields' => array(

core/modules/system/tests/fixtures/update/block.block.testfor2476947.yml

 langcode: en
+uuid: 1F526BF9-F924-49FD-B45A-61F776F3E9E2
 status: true
 dependencies:
   theme:

core/modules/system/tests/fixtures/update/block.block.testfor2569529.yml

 langcode: en
+uuid: 8862ACDA-DDBA-4645-B84D-EB346D48A6F1
 status: true
 dependencies:
   theme:

core/modules/system/tests/fixtures/update/drupal-8.block-test-enabled.php

@@ -39,6 +39,7 @@
 // Install the block configuration.
 $config = file_get_contents(__DIR__ . '/../../../../block/tests/modules/block_test/config/install/block.block.test_block.yml');
 $config = Yaml::parse($config);
+$config['uuid'] = '35B67D42-EF1C-424F-99E6-9DA4E3275A27';
 $connection->insert('config')
   ->fields(['data', 'name', 'collection'])
   ->values([