Issue #2307681 by cbr, Berdir, moshe weitzman: GetEntityTypeFromStaticClass - poor performance.

core/lib/Drupal/Core/Entity/Entity.php

@@ -424,76 +424,26 @@ public function getListCacheTags() {
    * {@inheritdoc}
    */
   public static function load($id) {
-    return \Drupal::entityManager()->getStorage(static::getEntityTypeFromStaticClass())->load($id);
+    $entity_manager = \Drupal::entityManager();
+    return $entity_manager->getStorage($entity_manager->getEntityTypeFromClass(get_called_class()))->load($id);
   }
 
   /**
    * {@inheritdoc}
    */
   public static function loadMultiple(array $ids = NULL) {
-    return \Drupal::entityManager()->getStorage(static::getEntityTypeFromStaticClass())->loadMultiple($ids);
+    $entity_manager = \Drupal::entityManager();
+    return $entity_manager->getStorage($entity_manager->getEntityTypeFromClass(get_called_class()))->loadMultiple($ids);
   }
 
   /**
    * {@inheritdoc}
    */
   public static function create(array $values = array()) {
-    return \Drupal::entityManager()->getStorage(static::getEntityTypeFromStaticClass())->create($values);
+    $entity_manager = \Drupal::entityManager();
+    return $entity_manager->getStorage($entity_manager->getEntityTypeFromClass(get_called_class()))->create($values);
   }
 
-  /**
-   * Returns the entity type ID based on the class that is called on.
-   *
-   * Compares the class this is called on against the known entity classes
-   * and returns the entity type ID of a direct match or a subclass as fallback,
-   * to support entity type definitions that were altered.
-   *
-   * @return string
-   *   The entity type ID.
-   *
-   * @throws \Drupal\Core\Entity\Exception\AmbiguousEntityClassException
-   *   Thrown when multiple subclasses correspond to the called class.
-   * @throws \Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException
-   *   Thrown when no entity class corresponds to the called class.
-   *
-   * @see \Drupal\Core\Entity\Entity::load()
-   * @see \Drupal\Core\Entity\Entity::loadMultiple()
-   */
-  protected static function getEntityTypeFromStaticClass() {
-    $called_class = get_called_class();
-    $subclasses = 0;
-    $same_class = 0;
-    $entity_type_id = NULL;
-    $subclass_entity_type_id = NULL;
-    foreach (\Drupal::entityManager()->getDefinitions() as $entity_type) {
-      // Check if this is the same class, throw an exception if there is more
-      // than one match.
-      if ($entity_type->getClass() == $called_class) {
-        $entity_type_id = $entity_type->id();
-        if ($same_class++) {
-          throw new AmbiguousEntityClassException($called_class);
-        }
-      }
-      // Check for entity types that are subclasses of the called class, but
-      // throw an exception if we have multiple matches.
-      elseif (is_subclass_of($entity_type->getClass(), $called_class)) {
-        $subclass_entity_type_id = $entity_type->id();
-        if ($subclasses++) {
-          throw new AmbiguousEntityClassException($called_class);
-        }
-      }
-    }
-
-    // Return the matching entity type ID or the subclass match if there is one
-    // as a secondary priority.
-    if ($entity_type_id) {
-      return $entity_type_id;
-    }
-    if ($subclass_entity_type_id) {
-      return $subclass_entity_type_id;
-    }
-    throw new NoCorrespondingEntityClassException($called_class);
-  }
 
   /**
    * Acts on an entity after it was saved or deleted.

core/lib/Drupal/Core/Entity/EntityManager.php

@@ -14,6 +14,8 @@
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Entity\Exception\AmbiguousEntityClassException;
+use Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Language\LanguageManagerInterface;
 use Drupal\Core\Language\LanguageInterface;
@@ -144,6 +146,13 @@ class EntityManager extends DefaultPluginManager implements EntityManagerInterfa
    */
   protected $fieldMapByFieldType = array();
 
+  /**
+   * Contains cached mappings of class names to entity types.
+   *
+   * @var array
+   */
+  protected $classNameEntityTypeMap = array();
+
   /**
    * Constructs a new Entity plugin manager.
    *
@@ -180,6 +189,7 @@ public function clearCachedDefinitions() {
     parent::clearCachedDefinitions();
     $this->clearCachedBundles();
     $this->clearCachedFieldDefinitions();
+    $this->classNameEntityTypeMap = array();
   }
 
   /**
@@ -933,4 +943,34 @@ public function loadEntityByUuid($entity_type_id, $uuid) {
     return reset($entities);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getEntityTypeFromClass($class_name) {
+
+    // Check the already calculated classes first.
+    if (isset($this->classNameEntityTypeMap[$class_name])) {
+      return $this->classNameEntityTypeMap[$class_name];
+    }
+
+    $same_class = 0;
+    $entity_type_id = NULL;
+    foreach ($this->getDefinitions() as $entity_type) {
+      if ($entity_type->getOriginalClass() == $class_name  || $entity_type->getClass() == $class_name) {
+        $entity_type_id = $entity_type->id();
+        if ($same_class++) {
+          throw new AmbiguousEntityClassException($class_name);
+        }
+      }
+    }
+
+    // Return the matching entity type ID if there is one.
+    if ($entity_type_id) {
+      $this->classNameEntityTypeMap[$class_name] = $entity_type_id;
+      return $entity_type_id;
+    }
+
+    throw new NoCorrespondingEntityClassException($class_name);
+  }
+
 }

core/lib/Drupal/Core/Entity/EntityManagerInterface.php

@@ -388,4 +388,27 @@ public function getFormModeOptions($entity_type_id, $include_disabled = FALSE);
    */
   public function loadEntityByUuid($entity_type_id, $uuid);
 
+  /**
+   * Returns the entity type ID based on the class that is called on.
+   *
+   * Compares the class this is called on against the known entity classes
+   * and returns the entity type ID of a direct match or a subclass as fallback,
+   * to support entity type definitions that were altered.
+   *
+   * @param string $class_name
+   *   Class name to use for searching the entity type ID.
+   *
+   * @return string
+   *   The entity type ID.
+   *
+   * @throws \Drupal\Core\Entity\Exception\AmbiguousEntityClassException
+   *   Thrown when multiple subclasses correspond to the called class.
+   * @throws \Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException
+   *   Thrown when no entity class corresponds to the called class.
+   *
+   * @see \Drupal\Core\Entity\Entity::load()
+   * @see \Drupal\Core\Entity\Entity::loadMultiple()
+   */
+  public function getEntityTypeFromClass($class_name);
+
 }

core/lib/Drupal/Core/Entity/EntityType.php

@@ -70,6 +70,15 @@ class EntityType implements EntityTypeInterface {
    */
   protected $class;
 
+  /**
+   * The name of the original entity type class.
+   *
+   * This is only set if the class name is changed.
+   *
+   * @var string
+   */
+  protected $originalClass;
+
   /**
    * An array of controllers.
    *
@@ -308,10 +317,22 @@ public function getClass() {
     return $this->class;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getOriginalClass() {
+    return $this->originalClass ?: $this->class;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function setClass($class) {
+    if (!$this->originalClass && $this->class) {
+      // If the original class is currently not set, set it to the current
+      // class, assume that is the original class name.
+      $this->originalClass = $this->class;
+    }
     $this->class = $class;
     return $this;
   }

core/lib/Drupal/Core/Entity/EntityTypeInterface.php

@@ -74,6 +74,18 @@ public function getProvider();
    */
   public function getClass();
 
+  /**
+   * Returns the name of the original entity type class.
+   *
+   * In case the class name was changed with setClass(), this will return
+   * the initial value. Useful when trying to identify the entity type ID based
+   * on the class.
+   *
+   * @return string
+   *   The name of the original entity type class.
+   */
+  public function getOriginalClass();
+
   /**
    * Returns an array of entity keys.
    *

core/lib/Drupal/Core/Entity/Exception/AmbiguousEntityClassException.php

@@ -8,12 +8,7 @@
 namespace Drupal\Core\Entity\Exception;
 
 /**
- * Exception thrown if multiple subclasses exist for an entity.
- *
- * This might occur if an entity is subclassed multiple times and the base
- * class is altered to use one of the subclasses instead. If a static method on
- * the base class is then invoked it is impossible to determine which of the
- * subclasses is responsible for it.
+ * Exception thrown if multiple entity types exist for an entity class.
  *
  * @see hook_entity_info_alter()
  * @see \Drupal\Core\Entity\Entity::getEntityTypeFromStaticClass()
@@ -27,7 +22,7 @@ class AmbiguousEntityClassException extends \Exception {
    *   The entity parent class.
    */
   public function __construct($class) {
-    $message = sprintf('Multiple subclasses provide an entity type for %s.', $class);
+    $message = sprintf('Multiple entity types found for %s.', $class);
     parent::__construct($message);
   }
 

core/modules/node/node.api.php

@@ -326,22 +326,19 @@ function hook_node_grants_alter(&$grants, \Drupal\Core\Session\AccountInterface
 function hook_node_access(\Drupal\node\NodeInterface $node, $op, \Drupal\Core\Session\AccountInterface $account, $langcode) {
   $type = is_string($node) ? $node : $node->getType();
 
-  $configured_types = node_permissions_get_configured_types();
-  if (isset($configured_types[$type])) {
-    if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) {
-      return NODE_ACCESS_ALLOW;
-    }
+  if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) {
+    return NODE_ACCESS_ALLOW;
+  }
 
-    if ($op == 'update') {
-      if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) {
-        return NODE_ACCESS_ALLOW;
-      }
+  if ($op == 'update') {
+    if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) {
+      return NODE_ACCESS_ALLOW;
     }
+  }
 
-    if ($op == 'delete') {
-      if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) {
-        return NODE_ACCESS_ALLOW;
-      }
+  if ($op == 'delete') {
+    if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))) {
+      return NODE_ACCESS_ALLOW;
     }
   }
 

core/modules/node/node.module

@@ -716,8 +716,8 @@ function node_permission() {
     ),
   );
 
-  // Generate standard node permissions for all applicable node types.
-  foreach (node_permissions_get_configured_types() as $type) {
+  // Generate node permissions for all node types.
+  foreach (NodeType::loadMultiple() as $type) {
     $perms += node_list_permissions($type);
   }
 
@@ -1158,22 +1158,19 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo
 function node_node_access(NodeInterface $node, $op, $account) {
   $type = $node->bundle();
 
-  $configured_types = node_permissions_get_configured_types();
-  if (isset($configured_types[$type])) {
-    if ($op == 'create' && $account->hasPermission('create ' . $type . ' content', $account)) {
-      return NODE_ACCESS_ALLOW;
-    }
+  if ($op == 'create' && $account->hasPermission('create ' . $type . ' content', $account)) {
+    return NODE_ACCESS_ALLOW;
+  }
 
-    if ($op == 'update') {
-      if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) {
-        return NODE_ACCESS_ALLOW;
-      }
+  if ($op == 'update') {
+    if ($account->hasPermission('edit any ' . $type . ' content', $account) || ($account->hasPermission('edit own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) {
+      return NODE_ACCESS_ALLOW;
     }
+  }
 
-    if ($op == 'delete') {
-      if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) {
-        return NODE_ACCESS_ALLOW;
-      }
+  if ($op == 'delete') {
+    if ($account->hasPermission('delete any ' . $type . ' content', $account) || ($account->hasPermission('delete own ' . $type . ' content', $account) && ($account->id() == $node->getOwnerId()))) {
+      return NODE_ACCESS_ALLOW;
     }
   }
 
@@ -1222,31 +1219,6 @@ function node_list_permissions($type) {
   return $perms;
 }
 
-/**
- * Returns an array of node types that should be managed by permissions.
- *
- * By default, this will include all node types in the system. To exclude a
- * specific node from getting permissions defined for it, set the
- * node_permissions_$type variable to 0. Core does not provide an interface for
- * doing so. However, contrib modules may exclude their own nodes in
- * hook_install(). Alternatively, contrib modules may configure all node types
- * at once, or decide to apply some other hook_node_access() implementation to
- * some or all node types.
- *
- * @return
- *   An array of node types managed by this module.
- */
-function node_permissions_get_configured_types() {
-  $configured_types = array();
-  foreach (node_type_get_types() as $name => $type) {
-    $node_settings = $type->getModuleSettings('node');
-    if (!isset($node_settings['permissions']) || !empty($node_settings['permissions'])) {
-      $configured_types[$name] = $type;
-    }
-  }
-  return $configured_types;
-}
-
 /**
  * Fetches an array of permission IDs granted to the given user ID.
  *

core/modules/node/src/Access/NodeAddAccessCheck.php

@@ -53,7 +53,7 @@ public function access(AccountInterface $account, NodeTypeInterface $node_type =
       return $access_control_handler->createAccess($node_type->id(), $account) ? static::ALLOW : static::DENY;
     }
     // If checking whether a node of any type may be created.
-    foreach (node_permissions_get_configured_types() as $node_type) {
+    foreach ($this->entityManager->getStorage('node_type')->loadMultiple() as $node_type) {
       if ($access_control_handler->createAccess($node_type->id(), $account)) {
         return static::ALLOW;
       }

core/modules/node/src/NodeAccessControlHandler.php

@@ -122,10 +122,7 @@ protected function checkAccess(EntityInterface $node, $operation, $langcode, Acc
    * {@inheritdoc}
    */
   protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
-    $configured_types = node_permissions_get_configured_types();
-    if (isset($configured_types[$entity_bundle])) {
-      return $account->hasPermission('create ' . $entity_bundle . ' content');
-    }
+    return $account->hasPermission('create ' . $entity_bundle . ' content');
   }
 
   /**

core/tests/Drupal/Tests/Core/Entity/EntityManagerTest.php

@@ -1254,6 +1254,87 @@ public function testGetFieldMapByFieldType() {
     $this->assertArrayNotHasKey('id', $stringFields['test_entity_type']);
   }
 
+  /**
+   * @covers ::getEntityTypeFromClass()
+   */
+  public function testGetEntityTypeFromClass() {
+    $apple = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $apple->expects($this->exactly(2))
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\apple\Entity\Apple'));
+    $banana = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $banana->expects($this->exactly(2))
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\banana\Entity\Banana'));
+    $banana->expects($this->once())
+      ->method('getClass')
+      ->will($this->returnValue('\Drupal\mango\Entity\Mango'));
+    $banana->expects($this->exactly(2))
+      ->method('id')
+      ->will($this->returnValue('banana'));
+    $this->setUpEntityManager(array(
+      'apple' => $apple,
+      'banana' => $banana,
+    ));
+
+    $entity_type_id = $this->entityManager->getEntityTypeFromClass('\Drupal\banana\Entity\Banana');
+    $this->assertSame('banana', $entity_type_id);
+    $entity_type_id = $this->entityManager->getEntityTypeFromClass('\Drupal\mango\Entity\Mango');
+    $this->assertSame('banana', $entity_type_id);
+  }
+
+  /**
+   * @covers ::getEntityTypeFromClass()
+   *
+   * @expectedException \Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException
+   * @expectedExceptionMessage The \Drupal\pear\Entity\Pear class does not correspond to an entity type.
+   */
+  public function testGetEntityTypeFromClassNoMatch() {
+    $apple = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $apple->expects($this->once())
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\apple\Entity\Apple'));
+    $banana = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $banana->expects($this->once())
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\banana\Entity\Banana'));
+    $this->setUpEntityManager(array(
+      'apple' => $apple,
+      'banana' => $banana,
+    ));
+
+    $this->entityManager->getEntityTypeFromClass('\Drupal\pear\Entity\Pear');
+  }
+
+  /**
+   * @covers ::getEntityTypeFromClass()
+   *
+   * @expectedException \Drupal\Core\Entity\Exception\AmbiguousEntityClassException
+   * @expectedExceptionMessage Multiple entity types found for \Drupal\apple\Entity\Apple.
+   */
+  public function testGetEntityTypeFromClassAmbiguous() {
+    $boskoop = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $boskoop->expects($this->once())
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\apple\Entity\Apple'));
+    $boskoop->expects($this->once())
+      ->method('id')
+      ->will($this->returnValue('boskop'));
+    $gala = $this->getMock('Drupal\Core\Entity\EntityTypeInterface');
+    $gala->expects($this->once())
+      ->method('getOriginalClass')
+      ->will($this->returnValue('\Drupal\apple\Entity\Apple'));
+    $gala->expects($this->once())
+      ->method('id')
+      ->will($this->returnValue('gala'));
+    $this->setUpEntityManager(array(
+      'boskoop' => $boskoop,
+      'gala' => $gala,
+    ));
+
+    $this->entityManager->getEntityTypeFromClass('\Drupal\apple\Entity\Apple');
+  }
+
   /**
    * Gets a mock controller class name.
    *

core/tests/Drupal/Tests/Core/Entity/EntityTypeTest.php

@@ -202,6 +202,26 @@ public function testIdExceedsMaxLength() {
     $this->setUpEntityType(array('id' => $id));
   }
 
+  /**
+   * @covers ::getOriginalClass
+   */
+  public function testgetOriginalClassUnchanged() {
+    $class = $this->randomMachineName();
+    $entity_type = $this->setUpEntityType(array('class' => $class));
+    $this->assertEquals($class, $entity_type->getOriginalClass());
+  }
+
+  /**
+   * @covers ::setClass
+   * @covers ::getOriginalClass
+   */
+  public function testgetOriginalClassChanged() {
+    $class = $this->randomMachineName();
+    $entity_type = $this->setUpEntityType(array('class' => $class));
+    $entity_type->setClass($this->randomMachineName());
+    $this->assertEquals($class, $entity_type->getOriginalClass());
+  }
+
   /**
    * @covers ::id
    */

core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php

@@ -9,6 +9,7 @@
 
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Entity\Entity;
+use Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException;
 use Drupal\Core\Language\Language;
 use Drupal\entity_test\Entity\EntityTest;
 use Drupal\entity_test\Entity\EntityTestMul;
@@ -234,16 +235,6 @@ public function testLanguage() {
    * Setup for the tests of the ::load() method.
    */
   function setupTestLoad() {
-    // Use an entity type object which has the methods enabled which are being
-    // called by the protected method Entity::getEntityTypeFromStaticClass().
-    $methods = get_class_methods('Drupal\Core\Entity\EntityType');
-    unset($methods[array_search('getClass', $methods)]);
-    unset($methods[array_search('setClass', $methods)]);
-    $this->entityType = $this->getMockBuilder('\Drupal\Core\Entity\EntityType')
-      ->disableOriginalConstructor()
-      ->setMethods($methods)
-      ->getMock();
-
     // Base our mocked entity on a real entity class so we can test if calling
     // Entity::load() on the base class will bubble up to an actual entity.
     $this->entityTypeId = 'entity_test_mul';
@@ -255,146 +246,23 @@ function setupTestLoad() {
       ->disableOriginalConstructor()
       ->setMethods($methods)
       ->getMock();
-    $this->entityType->setClass(get_class($this->entity));
-
-    $this->entityManager->expects($this->once())
-      ->method('getDefinitions')
-      ->will($this->returnValue(array($this->entityTypeId => $this->entityType)));
-
-    $this->entityType->expects($this->any())
-      ->method('id')
-      ->will($this->returnValue($this->entityTypeId));
   }
 
   /**
    * @covers ::load
    * @covers ::getEntityTypeFromStaticClass
    *
-   * Tests Entity::load() when called statically on the Entity base class.
+   * Tests Entity::load() when called statically on a subclass of Entity.
    */
   public function testLoad() {
     $this->setupTestLoad();
 
-    $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
-    $storage->expects($this->once())
-      ->method('load')
-      ->with(1)
-      ->will($this->returnValue($this->entity));
-    $this->entityManager->expects($this->once())
-      ->method('getStorage')
-      ->with($this->entityTypeId)
-      ->will($this->returnValue($storage));
-
-    // Call Entity::load statically and check that it returns the mock entity.
-    $this->assertSame($this->entity, Entity::load(1));
-  }
-
-  /**
-   * @covers ::load
-   * @covers ::getEntityTypeFromStaticClass
-   *
-   * Tests if an assertion is thrown if Entity::load() is called on a base class
-   * which is subclassed multiple times.
-   *
-   * @expectedException \Drupal\Core\Entity\Exception\AmbiguousEntityClassException
-   */
-  public function testLoadWithAmbiguousSubclasses() {
-    // Use an entity type object which has the methods enabled which are being
-    // called by the protected method Entity::getEntityTypeFromStaticClass().
-    $methods = get_class_methods('Drupal\Core\Entity\EntityType');
-    unset($methods[array_search('getClass', $methods)]);
-    unset($methods[array_search('setClass', $methods)]);
-
-    $first_entity_type = $this->getMockBuilder('\Drupal\Core\Entity\EntityType')
-      ->disableOriginalConstructor()
-      ->setMethods($methods)
-      ->getMock();
-    $first_entity_type->setClass('Drupal\entity_test\Entity\EntityTestMul');
-
-    $second_entity_type = $this->getMockBuilder('\Drupal\Core\Entity\EntityType')
-      ->disableOriginalConstructor()
-      ->setMethods($methods)