From 7e427d5c5e45602b03cfdbb99681b458ea750b6f Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org> Date: Fri, 15 Sep 2017 12:19:44 +0100 Subject: [PATCH] Issue #2391829 by amateescu, cilefen, Berdir, yched, fago: ContentUninstallValidator relies on the not required ContentEntityStorage::hasData() method --- .../Core/Config/Entity/ConfigEntityStorage.php | 7 +++++++ .../Core/Entity/ContentEntityStorageBase.php | 10 ---------- ...namicallyFieldableEntityStorageInterface.php | 8 -------- .../Drupal/Core/Entity/EntityStorageBase.php | 10 ++++++++++ .../Core/Entity/EntityStorageInterface.php | 8 ++++++++ .../KeyValueStore/KeyValueEntityStorage.php | 7 +++++++ .../modules/keyvalue_test/keyvalue_test.module | 1 + .../Core/Config/ConfigEntityStorageTest.php | 14 ++++++++++++++ .../KeyValueContentEntityStorageTest.php | 17 +++++++++++++++++ 9 files changed, 64 insertions(+), 18 deletions(-) diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php index 11b66fc7a193..62fb13efc0d8 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php @@ -316,6 +316,13 @@ protected function has($id, EntityInterface $entity) { return !$config->isNew(); } + /** + * {@inheritdoc} + */ + public function hasData() { + return (bool) $this->configFactory->listAll($this->getPrefix()); + } + /** * Gets entities from the static cache. * diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php index 9d6b8d64f508..515db0d2f6d3 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php @@ -62,16 +62,6 @@ public static function createInstance(ContainerInterface $container, EntityTypeI ); } - /** - * {@inheritdoc} - */ - public function hasData() { - return (bool) $this->getQuery() - ->accessCheck(FALSE) - ->range(0, 1) - ->execute(); - } - /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php b/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php index feed5653bb15..04d1b1e05c8f 100644 --- a/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php +++ b/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php @@ -18,14 +18,6 @@ */ interface DynamicallyFieldableEntityStorageInterface extends FieldableEntityStorageInterface, FieldStorageDefinitionListenerInterface, FieldDefinitionListenerInterface { - /** - * Determines if the storage contains any data. - * - * @return bool - * TRUE if the storage contains data, FALSE if not. - */ - public function hasData(); - /** * Purges a batch of field data. * diff --git a/core/lib/Drupal/Core/Entity/EntityStorageBase.php b/core/lib/Drupal/Core/Entity/EntityStorageBase.php index 7335af119692..e5bf880aef2a 100644 --- a/core/lib/Drupal/Core/Entity/EntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php @@ -505,6 +505,16 @@ public function loadByProperties(array $values = []) { return $result ? $this->loadMultiple($result) : []; } + /** + * {@inheritdoc} + */ + public function hasData() { + return (bool) $this->getQuery() + ->accessCheck(FALSE) + ->range(0, 1) + ->execute(); + } + /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Entity/EntityStorageInterface.php b/core/lib/Drupal/Core/Entity/EntityStorageInterface.php index 50166d6e2787..9f50674d0df3 100644 --- a/core/lib/Drupal/Core/Entity/EntityStorageInterface.php +++ b/core/lib/Drupal/Core/Entity/EntityStorageInterface.php @@ -142,6 +142,14 @@ public function delete(array $entities); */ public function save(EntityInterface $entity); + /** + * Determines if the storage contains any data. + * + * @return bool + * TRUE if the storage contains data, FALSE if not. + */ + public function hasData(); + /** * Gets an entity query instance. * diff --git a/core/lib/Drupal/Core/Entity/KeyValueStore/KeyValueEntityStorage.php b/core/lib/Drupal/Core/Entity/KeyValueStore/KeyValueEntityStorage.php index de77e09d20f3..cd2f26efbd69 100644 --- a/core/lib/Drupal/Core/Entity/KeyValueStore/KeyValueEntityStorage.php +++ b/core/lib/Drupal/Core/Entity/KeyValueStore/KeyValueEntityStorage.php @@ -187,6 +187,13 @@ protected function has($id, EntityInterface $entity) { return $this->keyValueStore->has($id); } + /** + * {@inheritdoc} + */ + public function hasData() { + return (bool) $this->keyValueStore->getAll(); + } + /** * {@inheritdoc} */ diff --git a/core/modules/system/tests/modules/keyvalue_test/keyvalue_test.module b/core/modules/system/tests/modules/keyvalue_test/keyvalue_test.module index 1e7da67fad92..4316719dd3e4 100644 --- a/core/modules/system/tests/modules/keyvalue_test/keyvalue_test.module +++ b/core/modules/system/tests/modules/keyvalue_test/keyvalue_test.module @@ -14,5 +14,6 @@ function keyvalue_test_entity_type_alter(array &$entity_types) { $entity_types['entity_test_label']->setStorageClass('Drupal\Core\Entity\KeyValueStore\KeyValueContentEntityStorage'); $entity_keys = $entity_types['entity_test_label']->getKeys(); $entity_types['entity_test_label']->set('entity_keys', $entity_keys + ['uuid' => 'uuid']); + $entity_types['entity_test_label']->set('provider', 'keyvalue_test'); } } diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStorageTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStorageTest.php index bf41e1de5050..ce9a928d53a3 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStorageTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStorageTest.php @@ -51,4 +51,18 @@ public function testUUIDConflict() { $this->assertIdentical($entity->toArray(), $original_properties); } + /** + * Tests the hasData() method for config entity storage. + * + * @covers \Drupal\Core\Config\Entity\ConfigEntityStorage::hasData + */ + public function testHasData() { + $storage = \Drupal::entityTypeManager()->getStorage('config_test'); + $this->assertFalse($storage->hasData()); + + // Add a test config entity and check again. + $storage->create(['id' => $this->randomMachineName()])->save(); + $this->assertTrue($storage->hasData()); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/KeyValueStore/KeyValueContentEntityStorageTest.php b/core/tests/Drupal/KernelTests/Core/KeyValueStore/KeyValueContentEntityStorageTest.php index 31300dbcd77a..98850055a5fb 100644 --- a/core/tests/Drupal/KernelTests/Core/KeyValueStore/KeyValueContentEntityStorageTest.php +++ b/core/tests/Drupal/KernelTests/Core/KeyValueStore/KeyValueContentEntityStorageTest.php @@ -31,9 +31,15 @@ protected function setUp() { /** * Tests CRUD operations. + * + * @covers \Drupal\Core\Entity\KeyValueStore\KeyValueEntityStorage::hasData */ public function testCRUD() { $default_langcode = \Drupal::languageManager()->getDefaultLanguage()->getId(); + + $storage = \Drupal::entityTypeManager()->getStorage('entity_test_label'); + $this->assertFalse($storage->hasData()); + // Verify default properties on a newly created empty entity. $empty = EntityTestLabel::create(); $this->assertIdentical($empty->id->value, NULL); @@ -108,6 +114,9 @@ public function testCRUD() { $this->fail('EntityMalformedException was not thrown.'); } + // Verify that hasData() returns the expected result. + $this->assertTrue($storage->hasData()); + // Verify that the correct status is returned and properties did not change. $this->assertIdentical($status, SAVED_NEW); $this->assertIdentical($entity_test->id(), $expected['id']); @@ -157,4 +166,12 @@ public function testCRUD() { } } + /** + * Tests uninstallation of a module that does not use the SQL entity storage. + */ + public function testUninstall() { + $uninstall_validator_reasons = \Drupal::service('content_uninstall_validator')->validate('keyvalue_test'); + $this->assertEmpty($uninstall_validator_reasons); + } + } -- GitLab