Skip to content

GitLab

Commit 0ce0a356 authored by catch's avatar catch
Browse files

Issue #2300131 by Berdir, alexpott: Fixed EntityResolverManager instantiates objects unnecessarily.

parent ee394023
Hide whitespace changes
Inline Side-by-side
core/core.services.yml
View file @ 0ce0a356
......@@ -604,7 +604,7 @@ services:
arguments: ['@module_handler']
resolver_manager.entity:
class: Drupal\Core\Entity\EntityResolverManager
arguments: ['@entity.manager', '@controller_resolver', '@class_resolver']
arguments: ['@entity.manager', '@class_resolver']
route_subscriber.entity:
class: Drupal\Core\EventSubscriber\EntityRouteAlterSubscriber
tags:
......
core/lib/Drupal/Core/Entity/EntityResolverManager.php
View file @ 0ce0a356
......@@ -7,8 +7,6 @@
namespace Drupal\Core\Entity;
use Drupal\Component\Plugin\Exception\PluginNotFoundException;
use Drupal\Core\Controller\ControllerResolverInterface;
use Drupal\Core\DependencyInjection\ClassResolverInterface;
use Symfony\Component\Routing\Route;
......@@ -27,13 +25,6 @@ class EntityResolverManager {
*/
protected $entityManager;
/**
* The controller resolver.
*
* @var \Drupal\Core\Controller\ControllerResolverInterface
*/
protected $controllerResolver;
/**
* The class resolver.
*
......@@ -46,49 +37,77 @@ class EntityResolverManager {
*
* @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
* The entity manager.
* @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver
* The controller resolver.
* @param \Drupal\Core\DependencyInjection\ClassResolverInterface $class_resolver
* The class resolver.
*/
public function __construct(EntityManagerInterface $entity_manager, ControllerResolverInterface $controller_resolver, ClassResolverInterface $class_resolver) {
public function __construct(EntityManagerInterface $entity_manager, ClassResolverInterface $class_resolver) {
$this->entityManager = $entity_manager;
$this->controllerResolver = $controller_resolver;
$this->classResolver = $class_resolver;
}
/**
* Creates a controller instance using route defaults.
* Gets the controller class using route defaults.
*
* By design we cannot support all possible routes, but just the ones which
* use the defaults provided by core, which are _content, _controller
* and _form.
*
* Rather than creating an instance of every controller determine the class
* and method that would be used. This is not possible for the service:method
* notation as the runtime container does not allow static introspection.
*
* @see \Drupal\Core\Controller\ControllerResolver::getControllerFromDefinition()
* @see \Drupal\Core\Controller\ClassResolver::getInstanceFromDefinition()
*
* @param array $defaults
* The default values provided by the route.
*
* @return array|null
* Returns the controller instance if it is possible to instantiate it, NULL
* @return string|null
* Returns the controller class, otherwise NULL.
*/
protected function getController(array $defaults) {
protected function getControllerClass(array $defaults) {
$controller = NULL;
if (isset($defaults['_content'])) {
$controller = $this->controllerResolver->getControllerFromDefinition($defaults['_content']);
$controller = $defaults['_content'];
}
if (isset($defaults['_controller'])) {
$controller = $this->controllerResolver->getControllerFromDefinition($defaults['_controller']);
$controller = $defaults['_controller'];
}
if (isset($defaults['_form'])) {
$form_arg = $defaults['_form'];
// Check if the class exists first as the class resolver will throw an
// exception if it doesn't. This also means a service cannot be used here.
if (class_exists($form_arg)) {
$controller = array($this->classResolver->getInstanceFromDefinition($form_arg), 'buildForm');
$controller = $defaults['_form'];
// Check if the class exists and if so use the buildForm() method from the
// interface.
if (class_exists($controller)) {
return array($controller, 'buildForm');
}
}
return $controller;
if (strpos($controller, ':') === FALSE) {
if (method_exists($controller, '__invoke')) {
return array($controller, '__invoke');
}
if (function_exists($controller)) {
return $controller;
}
return NULL;
}
$count = substr_count($controller, ':');
if ($count == 1) {
// Controller in the service:method notation. Get the information from the
// service. This is dangerous as the controller could depend on services
// that could not exist at this point. There is however no other way to
// do it, as the container does not allow static introspection.
list($class_or_service, $method) = explode(':', $controller, 2);
return array($this->classResolver->getInstanceFromDefinition($class_or_service), $method);
}
elseif (strpos($controller, '::') !== FALSE) {
// Controller in the class::method notation.
return explode('::', $controller, 2);
}
return NULL;
}
/**
......@@ -190,7 +209,7 @@ protected function setParametersFromEntityInformation(Route $route) {
* The route object to add the upcasting information onto.
*/
public function setRouteOptions(Route $route) {
if ($controller = $this->getController($route->getDefaults())) {
if ($controller = $this->getControllerClass($route->getDefaults())) {
// Try to use reflection.
if ($this->setParametersFromReflection($controller, $route)) {
return;
......
core/tests/Drupal/Tests/Core/Entity/EntityResolverManagerTest.php
View file @ 0ce0a356
......@@ -36,13 +36,6 @@ class EntityResolverManagerTest extends UnitTestCase {
*/
protected $entityManager;
/**
* The mocked controller resolver.
*
* @var \Drupal\Core\Controller\ControllerResolverInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $controllerResolver;
/**
* The mocked class resolver.
*
......@@ -64,11 +57,10 @@ class EntityResolverManagerTest extends UnitTestCase {
*/
protected function setUp() {
$this->entityManager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface');
$this->controllerResolver = $this->getMock('Drupal\Core\Controller\ControllerResolverInterface');
$this->container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
$this->classResolver = $this->getClassResolverStub();
$this->entityResolverManager = new EntityResolverManager($this->entityManager, $this->controllerResolver, $this->classResolver);
$this->entityResolverManager = new EntityResolverManager($this->entityManager, $this->classResolver);
}
/**
......@@ -76,8 +68,8 @@ protected function setUp() {
*
* We don't have any entity type involved, so we don't need any upcasting.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::setRouteOptions
* @covers ::getControllerClass
*
* @dataProvider providerTestSetRouteOptionsWithStandardRoute
*/
......@@ -85,7 +77,6 @@ public function testSetRouteOptionsWithStandardRoute($controller) {
$route = new Route('/example', array(
'_controller' => $controller,
));
$this->setupControllerResolver($route->getDefault('_controller'));
$defaults = $route->getDefaults();
$this->entityResolverManager->setRouteOptions($route);
......@@ -107,7 +98,7 @@ public function providerTestSetRouteOptionsWithStandardRoute() {
* Tests setRouteOptions() with a controller with a non entity argument.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
*
* @dataProvider providerTestSetRouteOptionsWithStandardRouteWithArgument
*/
......@@ -116,7 +107,6 @@ public function testSetRouteOptionsWithStandardRouteWithArgument($controller) {
'_controller' => $controller,
'argument' => 'test',
));
$this->setupControllerResolver($route->getDefault('_controller'));
$defaults = $route->getDefaults();
$this->entityResolverManager->setRouteOptions($route);
......@@ -138,7 +128,7 @@ public function providerTestSetRouteOptionsWithStandardRouteWithArgument() {
* Tests setRouteOptions() with a _content default.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
*
* @dataProvider providerTestSetRouteOptionsWithContentController
*/
......@@ -147,7 +137,6 @@ public function testSetRouteOptionsWithContentController($controller) {
'_content' => $controller,
'argument' => 'test',
));
$this->setupControllerResolver($route->getDefault('_content'));
$defaults = $route->getDefaults();
$this->entityResolverManager->setRouteOptions($route);
......@@ -169,7 +158,7 @@ public function providerTestSetRouteOptionsWithContentController() {
* Tests setRouteOptions() with an entity type parameter.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
*
......@@ -181,7 +170,6 @@ public function testSetRouteOptionsWithEntityTypeNoUpcasting($controller) {
$route = new Route('/example/{entity_test}', array(
'_content' => $controller,
));
$this->setupControllerResolver($route->getDefault('_content'));
$defaults = $route->getDefaults();
$this->entityResolverManager->setRouteOptions($route);
......@@ -203,7 +191,7 @@ public function providerTestSetRouteOptionsWithEntityTypeNoUpcasting() {
* Tests setRouteOptions() with an entity type parameter, upcasting.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
*
......@@ -215,7 +203,6 @@ public function testSetRouteOptionsWithEntityTypeUpcasting($controller) {
$route = new Route('/example/{entity_test}', array(
'_content' => $controller,
));
$this->setupControllerResolver($route->getDefault('_content'));
$defaults = $route->getDefaults();
$this->entityResolverManager->setRouteOptions($route);
......@@ -238,7 +225,7 @@ public function providerTestSetRouteOptionsWithEntityTypeUpcasting() {
* Tests setRouteOptions() with an entity type parameter form.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
*/
......@@ -260,7 +247,7 @@ public function testSetRouteOptionsWithEntityFormUpcasting() {
* Tests setRouteOptions() with entity form upcasting, no create method.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
*/
......@@ -282,7 +269,7 @@ public function testSetRouteOptionsWithEntityUpcastingNoCreate() {
* Tests setRouteOptions() with an form parameter without interface.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
*/
......@@ -303,7 +290,7 @@ public function testSetRouteOptionsWithEntityFormNoUpcasting() {
* Tests setRouteOptions() with an _entity_view route.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
* @covers ::setParametersFromEntityInformation()
......@@ -335,7 +322,7 @@ public function testSetRouteOptionsWithEntityViewRouteAndManualParameters() {
* Tests setRouteOptions() with an _entity_view route.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
* @covers ::setParametersFromEntityInformation()
......@@ -357,7 +344,7 @@ public function testSetRouteOptionsWithEntityViewRoute() {
* Tests setRouteOptions() with an _entity_list route.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
* @covers ::setParametersFromEntityInformation()
......@@ -379,7 +366,7 @@ public function testSetRouteOptionsWithEntityListRoute() {
* Tests setRouteOptions() with an _entity_form route.
*
* @covers ::setRouteOptions()
* @covers ::getController()
* @covers ::getControllerClass()
* @covers ::getEntityTypes()
* @covers ::setParametersFromReflection()
* @covers ::setParametersFromEntityInformation()
......@@ -397,29 +384,6 @@ public function testSetRouteOptionsWithEntityFormRoute() {
$this->assertEquals(array('entity_test' => array('type' => 'entity:entity_test')), $parameters);
}
/**
* Setups the controller resolver to return the given controller definition.
*
* @param string $controller_definition
* The definition of a controller
*/
protected function setupControllerResolver($controller_definition) {
$controller = $controller_definition;
if (strpos($controller, '::')) {
list($class, $method) = explode('::', $controller);
$expected = array(new $class(), $method);
}
else {
$expected = $controller;
}
$this->controllerResolver->expects($this->atLeastOnce())
->method('getControllerFromDefinition')
->with($controller_definition)
->will($this->returnValue($expected));
}
/**
* Creates the entity manager mock returning entity type objects.
*/
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment