diff --git a/core/lib/Drupal/Core/Config/DatabaseStorage.php b/core/lib/Drupal/Core/Config/DatabaseStorage.php index 30d3fa9..125e90d 100644 --- a/core/lib/Drupal/Core/Config/DatabaseStorage.php +++ b/core/lib/Drupal/Core/Config/DatabaseStorage.php @@ -181,7 +181,7 @@ protected function ensureTableExists() { /** * Defines the schema for the configuration table. */ - protected static function schemaDefinition() { + public static function schemaDefinition() { $schema = array( 'description' => 'The base table for configuration data.', 'fields' => array( diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php index 08e7e19..04af4f4 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php @@ -323,12 +323,14 @@ public function preSave(EntityStorageInterface $storage) { // Ensure this entity's UUID does not exist with a different ID, regardless // of whether it's new or updated. - $matching_entities = $storage->getQuery() - ->condition('uuid', $this->uuid()) - ->execute(); - $matched_entity = reset($matching_entities); - if (!empty($matched_entity) && ($matched_entity != $this->id()) && $matched_entity != $this->getOriginalId()) { - throw new ConfigDuplicateUUIDException("Attempt to save a configuration entity '{$this->id()}' with UUID '{$this->uuid()}' when this UUID is already used for '$matched_entity'"); + if ($uuid = $this->uuid()) { + $matching_entities = $storage->getQuery() + ->condition('uuid', $this->uuid()) + ->execute(); + $matched_entity = reset($matching_entities); + if (!empty($matched_entity) && ($matched_entity != $this->id()) && $matched_entity != $this->getOriginalId()) { + throw new ConfigDuplicateUUIDException("Attempt to save a configuration entity '{$this->id()}' with UUID '{$this->uuid()}' when this UUID is already used for '$matched_entity'"); + } } // If this entity is not new, load the original entity for comparison. diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php index c87b986..f862949 100644 --- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php +++ b/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,19 +199,101 @@ 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 batch 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)) { + $schema_definition = $this->schemaDefinition(); + $database_schema->createTable($this->table, $schema_definition); + return TRUE; + } + } + // If another process has already created the batch table, 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 batch might be stale. + * + * If the table does not yet exist, that's fine, but if the table exists and + * yet the query failed, then the batch is stale and the exception needs + * to propagate. + * + * @param $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 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'], + ]; } } diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php index 8b87d3a..1b09812 100644 --- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php +++ b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php @@ -34,71 +34,122 @@ 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. + $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,5 +168,47 @@ public function deleteMultiple(array $keys) { parent::deleteMultiple($keys); } + /** + * Defines the schema for the key_value_expire table. + */ + public 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'], + ], + ]; + } } diff --git a/core/modules/field/tests/src/Kernel/FieldKernelTestBase.php b/core/modules/field/tests/src/Kernel/FieldKernelTestBase.php index c864de8..b3660f2 100644 --- a/core/modules/field/tests/src/Kernel/FieldKernelTestBase.php +++ b/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')); diff --git a/core/modules/system/src/Tests/KeyValueStore/DatabaseStorageTest.php b/core/modules/system/src/Tests/KeyValueStore/DatabaseStorageTest.php index b433ac1..c8d7fe4 100644 --- a/core/modules/system/src/Tests/KeyValueStore/DatabaseStorageTest.php +++ b/core/modules/system/src/Tests/KeyValueStore/DatabaseStorageTest.php @@ -19,11 +19,6 @@ class DatabaseStorageTest extends StorageTestBase { */ public static $modules = array('system'); - protected function setUp() { - parent::setUp(); - $this->installSchema('system', array('key_value')); - } - /** * {@inheritdoc} */ diff --git a/core/modules/system/src/Tests/Path/UrlAliasFixtures.php b/core/modules/system/src/Tests/Path/UrlAliasFixtures.php index 92984ff..b8b6ea1 100644 --- a/core/modules/system/src/Tests/Path/UrlAliasFixtures.php +++ b/core/modules/system/src/Tests/Path/UrlAliasFixtures.php @@ -2,6 +2,7 @@ namespace Drupal\system\Tests\Path; +use Drupal\Core\Config\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; } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index c716891..ff1a53e 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -855,71 +855,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( diff --git a/core/modules/system/tests/fixtures/HtaccessTest/access_test.module.orig b/core/modules/system/tests/fixtures/HtaccessTest/access_test.module.orig deleted file mode 100644 index e69de29..0000000 diff --git a/core/modules/system/tests/fixtures/HtaccessTest/access_test.php.orig b/core/modules/system/tests/fixtures/HtaccessTest/access_test.php.orig deleted file mode 100644 index e69de29..0000000 diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityAutocompleteTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityAutocompleteTest.php index f03599d..f9c3051 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityAutocompleteTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityAutocompleteTest.php @@ -33,14 +33,6 @@ class EntityAutocompleteTest extends EntityKernelTestBase { protected $bundle = 'entity_test'; /** - * {@inheritdoc} - */ - protected function setUp() { - parent::setUp(); - $this->installSchema('system', ['key_value']); - } - - /** * Tests autocompletion edge cases with slashes in the names. */ function testEntityReferenceAutocompletion() { diff --git a/core/tests/Drupal/KernelTests/Core/Entity/FieldWidgetConstraintValidatorTest.php b/core/tests/Drupal/KernelTests/Core/Entity/FieldWidgetConstraintValidatorTest.php index 50d173b..81c33d6 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/FieldWidgetConstraintValidatorTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/FieldWidgetConstraintValidatorTest.php @@ -23,7 +23,6 @@ class FieldWidgetConstraintValidatorTest extends KernelTestBase { protected function setUp() { parent::setUp(); - $this->installSchema('system', ['key_value']); $this->container->get('router.builder')->rebuild(); $this->installEntitySchema('user'); diff --git a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php index 2c4a01b..f86e667 100644 --- a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php +++ b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php @@ -525,6 +525,7 @@ public function testSaveMismatch() { $config_object = $this->getMockBuilder('Drupal\Core\Config\Config') ->disableOriginalConstructor() ->getMock(); + $config_object->expects($this->atLeastOnce()) ->method('isNew') ->will($this->returnValue(TRUE)); @@ -539,14 +540,16 @@ public function testSaveMismatch() { ->with('the_config_prefix.foo') ->will($this->returnValue($config_object)); + $uuid = '7C1821EF-A96F-4BF0-B654-F683532EECB3'; $this->entityQuery->expects($this->once()) ->method('condition') + ->with('uuid', $uuid) ->will($this->returnSelf()); $this->entityQuery->expects($this->once()) ->method('execute') ->will($this->returnValue(array('baz'))); - $entity = $this->getMockEntity(array('id' => 'foo')); + $entity = $this->getMockEntity(array('id' => 'foo', 'uuid' => $uuid)); $this->entityStorage->save($entity); } @@ -591,7 +594,8 @@ public function testSaveNoMismatch() { ->method('execute') ->will($this->returnValue(array('baz'))); - $entity = $this->getMockEntity(array('id' => 'foo')); + $uuid = '7C1821EF-A96F-4BF0-B654-F683532EECB3'; + $entity = $this->getMockEntity(array('id' => 'foo', 'uuid' => $uuid)); $entity->setOriginalId('baz'); $entity->enforceIsNew(); $this->entityStorage->save($entity);