diff --git a/core/core.services.yml b/core/core.services.yml index 2b27f69..6eb32c2 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -520,6 +520,8 @@ services: arguments: ['@state'] tags: - { name: event_subscriber } + # This service should not be overridden. See + # \Drupal\Core\Entity\Query\QueryFactory. entity.query: class: Drupal\Core\Entity\Query\QueryFactory arguments: ['@entity.manager'] diff --git a/core/lib/Drupal/Core/Entity/ContentEntityNullStorage.php b/core/lib/Drupal/Core/Entity/ContentEntityNullStorage.php index 12aeca3..f31b80e 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityNullStorage.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityNullStorage.php @@ -138,4 +138,11 @@ public function countFieldData($storage_definition, $as_bool = FALSE) { return $as_bool ? FALSE : 0; } + /** + * {@inheritdoc} + */ + public function hasData() { + return FALSE; + } + } diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php index 6415285..3a65176 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php @@ -45,6 +45,16 @@ public static function createInstance(ContainerInterface $container, EntityTypeI /** * {@inheritdoc} */ + public function hasData() { + return (bool) $this->getQuery() + ->accessCheck(FALSE) + ->range(0, 1) + ->execute(); + } + + /** + * {@inheritdoc} + */ protected function doCreate(array $values) { // We have to determine the bundle first. $bundle = FALSE; diff --git a/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php b/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php index 6d12c98..1d20217 100644 --- a/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php +++ b/core/lib/Drupal/Core/Entity/DynamicallyFieldableEntityStorageInterface.php @@ -85,6 +85,13 @@ public function purgeFieldData(FieldDefinitionInterface $field_definition, $batc public function countFieldData($storage_definition, $as_bool = FALSE); /** + * Determines if the storage contains any data. + * + * @return bool + */ + public function hasData(); + + /** * Performs final cleanup after all data of a field has been purged. * * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition diff --git a/core/lib/Drupal/Core/Entity/EntityManager.php b/core/lib/Drupal/Core/Entity/EntityManager.php index 128d21d..7d7eded 100644 --- a/core/lib/Drupal/Core/Entity/EntityManager.php +++ b/core/lib/Drupal/Core/Entity/EntityManager.php @@ -324,19 +324,7 @@ public function getHandler($entity_type, $handler_type) { if (!$class) { throw new InvalidPluginDefinitionException($entity_type, sprintf('The "%s" entity type did not specify a %s handler.', $entity_type, $handler_type)); } - if (is_subclass_of($class, 'Drupal\Core\Entity\EntityHandlerInterface')) { - $handler = $class::createInstance($this->container, $definition); - } - else { - $handler = new $class($definition); - } - if (method_exists($handler, 'setModuleHandler')) { - $handler->setModuleHandler($this->moduleHandler); - } - if (method_exists($handler, 'setStringTranslation')) { - $handler->setStringTranslation($this->translationManager); - } - $this->handlers[$handler_type][$entity_type] = $handler; + $this->handlers[$handler_type][$entity_type] = $this->createHandlerInstance($class, $definition); } return $this->handlers[$handler_type][$entity_type]; } @@ -344,6 +332,25 @@ public function getHandler($entity_type, $handler_type) { /** * {@inheritdoc} */ + public function createHandlerInstance($class, EntityTypeInterface $definition = null) { + if (is_subclass_of($class, 'Drupal\Core\Entity\EntityHandlerInterface')) { + $handler = $class::createInstance($this->container, $definition); + } + else { + $handler = new $class($definition); + } + if (method_exists($handler, 'setModuleHandler')) { + $handler->setModuleHandler($this->moduleHandler); + } + if (method_exists($handler, 'setStringTranslation')) { + $handler->setStringTranslation($this->translationManager); + } + return $handler; + } + + /** + * {@inheritdoc} + */ public function getBaseFieldDefinitions($entity_type_id) { // Check the static cache. if (!isset($this->baseFieldDefinitions[$entity_type_id])) { diff --git a/core/lib/Drupal/Core/Entity/EntityManagerInterface.php b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php index 8cf95ee..a1dea1d 100644 --- a/core/lib/Drupal/Core/Entity/EntityManagerInterface.php +++ b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php @@ -237,7 +237,7 @@ public function getFormObject($entity_type, $operation); public function hasHandler($entity_type, $handler_type); /** - * Creates a new handler instance. + * Creates a new handler instance for a entity type and handler type. * * @param string $entity_type * The entity type for this controller. @@ -252,6 +252,23 @@ public function hasHandler($entity_type, $handler_type); public function getHandler($entity_type, $handler_type); /** + * Creates new handler instance. + * + * Usually \Drupal\Core\Entity\EntityManagerInterface::getHandler() is + * preferred since that method has additional checking that the class exists + * and has static caches. + * + * @param mixed $class + * The handler class to instantiate. + * @param \Drupal\Core\Entity\EntityTypeInterface $definition + * The entity type definition. + * + * @return mixed + * A handler instance. + */ + public function createHandlerInstance($class, EntityTypeInterface $definition = null); + + /** * Get the bundle info of an entity type. * * @param string $entity_type diff --git a/core/lib/Drupal/Core/Entity/EntityStorageBase.php b/core/lib/Drupal/Core/Entity/EntityStorageBase.php index 4a40a9e..5d2c2aa 100644 --- a/core/lib/Drupal/Core/Entity/EntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php @@ -460,7 +460,9 @@ public function loadByProperties(array $values = array()) { * {@inheritdoc} */ public function getQuery($conjunction = 'AND') { - return \Drupal::entityQuery($this->getEntityTypeId(), $conjunction); + // Access the service directly rather than entity.query factory so the + // storage's current entity type is used. + return \Drupal::service($this->getQueryServicename())->get($this->entityType, $conjunction); } } diff --git a/core/lib/Drupal/Core/Entity/Query/QueryFactory.php b/core/lib/Drupal/Core/Entity/Query/QueryFactory.php index 5f1ab13..328c4cf 100644 --- a/core/lib/Drupal/Core/Entity/Query/QueryFactory.php +++ b/core/lib/Drupal/Core/Entity/Query/QueryFactory.php @@ -13,8 +13,17 @@ /** * Factory class Creating entity query objects. + * + * This class is marked as final since it should not be overridden in the + * container. EntityStorageBase::getQuery() accesses the storages query service + * directly without using this factory. + * + * @see \Drupal\Core\Entity\EntityStorageBase::getQuery() + * + * @todo https://www.drupal.org/node/2389335 remove entity.query service and + * replace with using the entity storage's getQuery() method. */ -class QueryFactory implements ContainerAwareInterface { +final class QueryFactory implements ContainerAwareInterface { use ContainerAwareTrait; @@ -48,8 +57,7 @@ public function __construct(EntityManagerInterface $entity_manager) { * The query object that can query the given entity type. */ public function get($entity_type_id, $conjunction = 'AND') { - $service_name = $this->entityManager->getStorage($entity_type_id)->getQueryServiceName(); - return $this->container->get($service_name)->get($this->entityManager->getDefinition($entity_type_id), $conjunction); + return $this->entityManager->getStorage($entity_type_id)->getQuery($conjunction); } /** diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index dac6584..d3087b7 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -178,23 +178,15 @@ public function requiresFieldStorageSchemaChanges(FieldStorageDefinitionInterfac * {@inheritdoc} */ public function requiresEntityDataMigration(EntityTypeInterface $entity_type, EntityTypeInterface $original) { - // If we're updating from NULL storage, then there's no stored data that - // requires migration. - // @todo Remove in https://www.drupal.org/node/2335879. + // If the original storage has existing entities, or it is impossible to + // determine if that is the case, require entity data to be migrated. $original_storage_class = $original->getStorageClass(); - $null_storage_class = 'Drupal\Core\Entity\ContentEntityNullStorage'; - if ($original_storage_class == $null_storage_class || is_subclass_of($original_storage_class, $null_storage_class)) { - return FALSE; + if (!class_exists($original_storage_class)) { + return TRUE; } - - return - // If the original storage class is different, then there might be - // existing entities in that storage even if the new storage's base - // table is empty. - // @todo Ask the old storage handler rather than assuming: - // https://www.drupal.org/node/2335879. - $entity_type->getStorageClass() != $original_storage_class || - !$this->isTableEmpty($this->storage->getBaseTable()); + // Use the original entity type since the storage has not been updated. + $original_storage = $this->entityManager->createHandlerInstance($original_storage_class, $original); + return $original_storage->hasData(); } /** diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php index 3a28de0..4aaa11a 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php @@ -1056,6 +1056,85 @@ public function testDedicatedTableSchemaForEntityWithStringIdentifier() { ); } + public function providerTestRequiresEntityDataMigration() { + $updated_entity_type_definition = $this->getMockBuilder('\Drupal\Core\Entity\EntityTypeInterface') + ->disableOriginalConstructor() + ->getMock(); + $updated_entity_type_definition->expects($this->any()) + ->method('getStorageClass') + ->willReturn('\Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema'); // A class that exists, *any* class. + $original_entity_type_definition = $this->getMockBuilder('\Drupal\Core\Entity\EntityTypeInterface') + ->disableOriginalConstructor() + ->getMock(); + $original_entity_type_definition->expects($this->any()) + ->method('getStorageClass') + ->willReturn('\Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema'); // A class that exists, *any* class. + $original_entity_type_definition_other_nonexisting = $this->getMockBuilder('\Drupal\Core\Entity\EntityTypeInterface') + ->disableOriginalConstructor() + ->getMock(); + $original_entity_type_definition_other_nonexisting->expects($this->any()) + ->method('getStorageClass') + ->willReturn('bar'); + $original_entity_type_definition_other_existing = $this->getMockBuilder('\Drupal\Core\Entity\EntityTypeInterface') + ->disableOriginalConstructor() + ->getMock(); + $original_entity_type_definition_other_existing->expects($this->any()) + ->method('getStorageClass') + ->willReturn('\Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema'); // A class that exists, *any* class. + + return [ + // Case 1: same storage class, ::hasData() === TRUE. + [$updated_entity_type_definition, $original_entity_type_definition, TRUE, TRUE], + // Case 2: same storage class, ::hasData() === FALSE. + [$updated_entity_type_definition, $original_entity_type_definition, FALSE, FALSE], + // Case 3: different storage class, original storage class does not exist. + [$updated_entity_type_definition, $original_entity_type_definition_other_nonexisting, NULL, TRUE], + // Case 4: different storage class, original storage class exists, ::hasData() === TRUE. + [$updated_entity_type_definition, $original_entity_type_definition_other_existing, TRUE, TRUE], + // Case 5: different storage class, original storage class exists, ::hasData() === FALSE. + [$updated_entity_type_definition, $original_entity_type_definition_other_existing, FALSE, FALSE], + ]; + } + + /** + * @covers ::requiresEntityDataMigration + * + * @dataProvider providerTestRequiresEntityDataMigration + */ + public function testRequiresEntityDataMigration($updated_entity_type_definition, $original_entity_type_definition, $original_storage_has_data, $migration_required) { + $this->entityType = new ContentEntityType(array( + 'id' => 'entity_test', + 'entity_keys' => array('id' => 'id'), + )); + + $original_storage = $this->getMockBuilder('Drupal\Core\Entity\Sql\SqlContentEntityStorage') + ->disableOriginalConstructor() + ->getMock(); + + $original_storage->expects($this->exactly(is_null($original_storage_has_data) ? 0 : 1)) + ->method('hasData') + ->willReturn($original_storage_has_data); + + // Assert hasData() is never called on the new storage definition. + $this->storage->expects($this->never()) + ->method('hasData'); + + $connection = $this->getMockBuilder('Drupal\Core\Database\Connection') + ->disableOriginalConstructor() + ->getMock(); + + $this->entityManager->expects($this->any()) + ->method('createHandlerInstance') + ->willReturn($original_storage); + + $this->storageSchema = $this->getMockBuilder('Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema') + ->setConstructorArgs(array($this->entityManager, $this->entityType, $this->storage, $connection)) + ->setMethods(array('installedStorageSchema')) + ->getMock(); + + $this->assertEquals($migration_required, $this->storageSchema->requiresEntityDataMigration($updated_entity_type_definition, $original_entity_type_definition)); + } + /** * Sets up the storage schema object to test. * diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php index 3589a63..3a37f90 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php @@ -1181,6 +1181,56 @@ public function testLoadMultiplePersistentCacheMiss() { } /** + * @covers ::hasData + */ + public function testHasData() { + $query = $this->getMock('Drupal\Core\Entity\Query\QueryInterface'); + $query->expects(($this->once())) + ->method('accessCheck') + ->with(FALSE) + ->willReturn($query); + $query->expects(($this->once())) + ->method('range') + ->with(0, 1) + ->willReturn($query); + $query->expects(($this->once())) + ->method('execute') + ->willReturn(array(5)); + + $factory = $this->getMockBuilder('Drupal\Core\Entity\Query\QueryFactory') + ->disableOriginalConstructor() + ->getMock(); + $factory->expects($this->once()) + ->method('get') + ->with($this->entityType, 'AND') + ->willReturn($query); + + $this->container->set('entity.query.sql', $factory); + + $database = $this->getMockBuilder('Drupal\Core\Database\Connection') + ->disableOriginalConstructor() + ->getMock(); + + $this->entityManager->expects($this->any()) + ->method('getDefinition') + ->will($this->returnValue($this->entityType)); + + $this->entityManager->expects($this->any()) + ->method('getFieldStorageDefinitions') + ->will($this->returnValue($this->fieldDefinitions)); + + $this->entityManager->expects($this->any()) + ->method('getBaseFieldDefinitions') + ->will($this->returnValue($this->fieldDefinitions)); + + $this->entityStorage = new SqlContentEntityStorage($this->entityType, $database, $this->entityManager, $this->cache); + + $result = $this->entityStorage->hasData(); + + $this->assertTrue($result, 'hasData returned TRUE'); + } + + /** * Tests entity ID sanitization. */ public function testCleanIds() {