Commit 978b47c7 authored by alexpott's avatar alexpott

Issue #2250239 by dawehner | sun: Remove needless ContainerAware dependency...

Issue #2250239 by dawehner | sun: Remove needless ContainerAware dependency from ParamConverterManager.
parent 35bc4a2c
......@@ -427,8 +427,8 @@ services:
- { name: route_filter }
paramconverter_manager:
class: Drupal\Core\ParamConverter\ParamConverterManager
calls:
- [setContainer, ['@service_container']]
tags:
- { name: service_collector, tag: paramconverter, call: addConverter }
paramconverter_subscriber:
class: Drupal\Core\EventSubscriber\ParamConverterSubscriber
tags:
......
......@@ -15,7 +15,6 @@
use Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterAccessChecksPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterParamConvertersPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterServicesForDestructionPass;
use Drupal\Core\DependencyInjection\Compiler\RegisterAuthenticationPass;
use Drupal\Core\Plugin\PluginManagerPass;
......@@ -63,9 +62,6 @@ public function register(ContainerBuilder $container) {
$container->addCompilerPass(new RegisterAccessChecksPass());
// Add a compiler pass for upcasting route parameters.
$container->addCompilerPass(new RegisterParamConvertersPass());
// Add a compiler pass for registering services needing destruction.
$container->addCompilerPass(new RegisterServicesForDestructionPass());
......
......@@ -85,8 +85,22 @@ public function process(ContainerBuilder $container) {
$consumer = $container->getDefinition($consumer_id);
$method = new \ReflectionMethod($consumer->getClass(), $method_name);
$params = $method->getParameters();
$interface = $params[0]->getClass();
$accepts_priority = isset($params[1]) && $params[1]->getName() === 'priority';
$interface_pos = 0;
$id_pos = NULL;
$priority_pos = NULL;
foreach ($params as $pos => $param) {
if ($param->getClass()) {
$interface = $param->getClass();
}
if ($param->getName() == 'id') {
$id_pos = $pos;
}
if ($param->getName() == 'priority') {
$priority_pos = $pos;
}
}
// Determine the ID.
if (!isset($interface)) {
throw new LogicException(vsprintf("Service consumer '%s' class method %s::%s() has to type-hint an interface.", array(
......@@ -116,12 +130,17 @@ public function process(ContainerBuilder $container) {
// Add a method call for each handler to the consumer service
// definition.
foreach ($handlers as $id => $priority) {
if ($accepts_priority) {
$consumer->addMethodCall($method_name, array(new Reference($id), $priority));
$arguments = array();
$arguments[$interface_pos] = new Reference($id);
if (isset($priority_pos)) {
$arguments[$priority_pos] = $priority;
}
else {
$consumer->addMethodCall($method_name, array(new Reference($id)));
if (isset($id_pos)) {
$arguments[$id_pos] = $id;
}
// Sort the arguments by position.
ksort($arguments);
$consumer->addMethodCall($method_name, $arguments);
}
}
}
......
......@@ -19,23 +19,7 @@
* A typical use case for this would be upcasting (converting) a node id to a
* node entity.
*/
class ParamConverterManager implements ParamConverterManagerInterface, ContainerAwareInterface {
use ContainerAwareTrait;
/**
* An array of registered converter service ids.
*
* @var array
*/
protected $converterIds = array();
/**
* Array of registered converter service ids sorted by their priority.
*
* @var array
*/
protected $sortedConverterIds;
class ParamConverterManager implements ParamConverterManagerInterface {
/**
* Array of loaded converter services keyed by their ids.
......@@ -47,29 +31,11 @@ class ParamConverterManager implements ParamConverterManagerInterface, Container
/**
* {@inheritdoc}
*/
public function addConverter($converter, $priority = 0) {
if (empty($this->converterIds[$priority])) {
$this->converterIds[$priority] = array();
}
$this->converterIds[$priority][] = $converter;
unset($this->sortedConverterIds);
public function addConverter(ParamConverterInterface $param_converter, $id) {
$this->converters[$id] = $param_converter;
return $this;
}
/**
* {@inheritdoc}
*/
public function getConverterIds() {
if (!isset($this->sortedConverterIds)) {
krsort($this->converterIds);
$this->sortedConverterIds = array();
foreach ($this->converterIds as $resolvers) {
$this->sortedConverterIds = array_merge($this->sortedConverterIds, $resolvers);
}
}
return $this->sortedConverterIds;
}
/**
* {@inheritdoc}
*/
......@@ -77,10 +43,9 @@ public function getConverter($converter) {
if (isset($this->converters[$converter])) {
return $this->converters[$converter];
}
if (!in_array($converter, $this->getConverterIds())) {
else {
throw new \InvalidArgumentException(sprintf('No converter has been registered for %s', $converter));
}
return $this->converters[$converter] = $this->container->get($converter);
}
/**
......@@ -100,7 +65,7 @@ public function setRouteParameterConverters(RouteCollection $routes) {
continue;
}
foreach ($this->getConverterIds() as $converter) {
foreach (array_keys($this->converters) as $converter) {
if ($this->getConverter($converter)->applies($definition, $name, $route)) {
$definition['converter'] = $converter;
break;
......
......@@ -18,27 +18,19 @@ interface ParamConverterManagerInterface {
/**
* Registers a parameter converter with the manager.
*
* @param string $converter
* @param \Drupal\Core\ParamConverter\ParamConverterInterface $param_converter
* The added param converter instance.
* @param string $id
* The parameter converter service id to register.
* @param int $priority
* (optional) The priority of the converter. Defaults to 0.
*
* @return $this
*/
public function addConverter($converter, $priority = 0);
/**
* Sorts the converter service ids and flattens them.
*
* @return array
* The sorted parameter converter service ids.
*/
public function getConverterIds();
public function addConverter(ParamConverterInterface $param_converter, $id);
/**
* Lazy-loads converter services.
*
* @param string $converter
* @param string $id
* The service id of converter service to load.
*
* @return \Drupal\Core\ParamConverter\ParamConverterInterface
......@@ -47,7 +39,7 @@ public function getConverterIds();
* @throws \InvalidArgumentException
* If the given service id is not a registered converter.
*/
public function getConverter($converter);
public function getConverter($id);
/**
* Saves a list of applicable converters to each route.
......
......@@ -162,6 +162,41 @@ public function testProcessNoPriorityParam() {
$this->assertCount(1, $method_calls[0][1]);
}
/**
* Tests consumer method with an ID parameter.
*
* @covers ::process
*/
public function testProcessWithIdParameter() {
$container = $this->buildContainer();
$container
->register('consumer_id', __NAMESPACE__ . '\ValidConsumer')
->addTag('service_collector', array(
'call' => 'addWithId',
));
$container
->register('handler1', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id');
$container
->register('handler2', __NAMESPACE__ . '\ValidHandler')
->addTag('consumer_id', array(
'priority' => 10,
));
$handler_pass = new TaggedHandlersPass();
$handler_pass->process($container);
$method_calls = $container->getDefinition('consumer_id')->getMethodCalls();
$this->assertCount(2, $method_calls);
$this->assertEquals(new Reference('handler2'), $method_calls[0][1][0]);
$this->assertEquals('handler2', $method_calls[0][1][1]);
$this->assertEquals(10, $method_calls[0][1][2]);
$this->assertEquals(new Reference('handler1'), $method_calls[1][1][0]);
$this->assertEquals('handler1', $method_calls[1][1][1]);
$this->assertEquals(0, $method_calls[1][1][2]);
}
/**
* Tests interface validation in non-production environment.
*
......@@ -196,6 +231,8 @@ public function addHandler(HandlerInterface $instance, $priority = 0) {
}
public function addNoPriority(HandlerInterface $instance) {
}
public function addWithId(HandlerInterface $instance, $id, $priority = 0) {
}
}
class InvalidConsumer {
public function addHandler($instance, $priority = 0) {
......
......@@ -21,11 +21,6 @@
*/
class ParamConverterManagerTest extends UnitTestCase {
/**
* @var \Symfony\Component\DependencyInjection\ContainerBuilder|\PHPUnit_Framework_MockObject_MockObject
*/
protected $container;
/**
* @var \Drupal\Core\ParamConverter\ParamConverterManager
*/
......@@ -48,29 +43,7 @@ public static function getInfo() {
public function setUp() {
parent::setUp();
$this->container = $this->getMock('Drupal\Core\DependencyInjection\Container');
$this->manager = new ParamConverterManager();
$this->manager->setContainer($this->container);
}
/**
* Tests \Drupal\Core\ParamConverter\ParamConverterManager::addConverter().
*
* @dataProvider providerTestAddConverter
*
* @covers ::addConverter()
* @covers ::getConverterIds()
*/
public function testAddConverter($unsorted, $sorted) {
foreach ($unsorted as $data) {
$this->manager->addConverter($data['name'], $data['priority']);
}
// Test that ResolverManager::getTypedDataResolvers() returns the resolvers
// in the expected order.
foreach ($this->manager->getConverterIds() as $key => $converter) {
$this->assertEquals($sorted[$key], $converter);
}
}
/**
......@@ -80,16 +53,12 @@ public function testAddConverter($unsorted, $sorted) {
*
* @covers ::getConverter()
*/
public function testGetConverter($name, $priority, $class) {
public function testGetConverter($name, $class) {
$converter = $this->getMockBuilder('Drupal\Core\ParamConverter\ParamConverterInterface')
->setMockClassName($class)
->getMock();
$this->manager->addConverter($name, $priority);
$this->container->expects($this->once())
->method('get')
->with($name)
->will($this->returnValue($converter));
$this->manager->addConverter($converter, $name);
$this->assertInstanceOf($class, $this->manager->getConverter($name));
// Assert that a second call to getConverter() does not use the container.
......@@ -118,13 +87,13 @@ public function testGetConverterException() {
*/
public function providerTestAddConverter() {
$converters[0]['unsorted'] = array(
array('name' => 'raspberry', 'priority' => 10),
array('name' => 'pear', 'priority' => 5),
array('name' => 'strawberry', 'priority' => 20),
array('name' => 'pineapple', 'priority' => 0),
array('name' => 'banana', 'priority' => -10),
array('name' => 'apple', 'priority' => -10),
array('name' => 'peach', 'priority' => 5),
array('name' => 'strawberry'),
array('name' => 'raspberry'),
array('name' => 'pear'),
array('name' => 'peach'),
array('name' => 'pineapple'),
array('name' => 'banana'),
array('name' => 'apple'),
);
$converters[0]['sorted'] = array(
......@@ -133,13 +102,13 @@ public function providerTestAddConverter() {
);
$converters[1]['unsorted'] = array(
array('name' => 'ape', 'priority' => 0),
array('name' => 'cat', 'priority' => -5),
array('name' => 'puppy', 'priority' => -10),
array('name' => 'llama', 'priority' => -15),
array('name' => 'giraffe', 'priority' => 10),
array('name' => 'zebra', 'priority' => 10),
array('name' => 'eagle', 'priority' => 5),
array('name' => 'giraffe'),
array('name' => 'zebra'),
array('name' => 'eagle'),
array('name' => 'ape'),
array('name' => 'cat'),
array('name' => 'puppy'),
array('name' => 'llama'),
);
$converters[1]['sorted'] = array(
......@@ -161,13 +130,13 @@ public function providerTestAddConverter() {
*/
public function providerTestGetConverter() {
return array(
array('ape', 0, 'ApeConverterClass'),
array('cat', -5, 'CatConverterClass'),
array('puppy', -10, 'PuppyConverterClass'),
array('llama', -15, 'LlamaConverterClass'),
array('giraffe', 10, 'GiraffeConverterClass'),
array('zebra', 10, 'ZebraConverterClass'),
array('eagle', 5, 'EagleConverterClass'),
array('ape', 'ApeConverterClass'),
array('cat', 'CatConverterClass'),
array('puppy', 'PuppyConverterClass'),
array('llama', 'LlamaConverterClass'),
array('giraffe', 'GiraffeConverterClass'),
array('zebra', 'ZebraConverterClass'),
array('eagle', 'EagleConverterClass'),
);
}
......@@ -182,11 +151,7 @@ public function testSetRouteParameterConverters($path, $parameters = NULL, $expe
->method('applies')
->with($this->anything(), 'id', $this->anything())
->will($this->returnValue(TRUE));
$this->manager->addConverter('applied');
$this->container->expects($this->any())
->method('get')
->with('applied')
->will($this->returnValue($converter));
$this->manager->addConverter($converter, 'applied');
$route = new Route($path);
if ($parameters) {
......@@ -248,11 +213,7 @@ public function testConvert() {
->method('convert')
->with(1, $this->isType('array'), 'id', $this->isType('array'), $this->isInstanceOf('Symfony\Component\HttpFoundation\Request'))
->will($this->returnValue('something_better!'));
$this->manager->addConverter('test_convert');
$this->container->expects($this->once())
->method('get')
->with('test_convert')
->will($this->returnValue($converter));
$this->manager->addConverter($converter, 'test_convert');
$result = $this->manager->convert($defaults, new Request());
......@@ -301,11 +262,7 @@ public function testConvertMissingParam() {
->method('convert')
->with(1, $this->isType('array'), 'id', $this->isType('array'), $this->isInstanceOf('Symfony\Component\HttpFoundation\Request'))
->will($this->returnValue(NULL));
$this->manager->addConverter('test_convert');
$this->container->expects($this->once())
->method('get')
->with('test_convert')
->will($this->returnValue($converter));
$this->manager->addConverter($converter, 'test_convert');
$this->manager->convert($defaults, new Request());
}
......
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