From 71b17c779077fffb595325f439c84d6a53dee784 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 2 Jan 2017 14:43:40 +0100 Subject: [PATCH 1/2] [DI] Optional class for named services --- .../Compiler/FactoryReturnTypePass.php | 24 +++++++-- .../Compiler/PassConfig.php | 3 +- .../Compiler/ResolveClassPass.php | 52 +++++++++++++++++++ .../DependencyInjection/ContainerBuilder.php | 37 +++++++++++-- .../Loader/XmlFileLoader.php | 8 +-- .../Compiler/FactoryReturnTypePassTest.php | 11 ++-- 6 files changed, 118 insertions(+), 17 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/ResolveClassPass.php diff --git a/src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php b/src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php index e4e64db41bb9e..e772036152726 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php @@ -17,9 +17,21 @@ /** * @author Guilhem N. + * + * @deprecated since version 3.3, to be removed in 4.0. */ class FactoryReturnTypePass implements CompilerPassInterface { + private $resolveClassPass; + + public function __construct(ResolveClassPass $resolveClassPass = null) + { + if (null === $resolveClassPass) { + @trigger_error('The '.__CLASS__.' class is deprecated since version 3.3 and will be removed in 4.0.', E_USER_DEPRECATED); + } + $this->resolveClassPass = $resolveClassPass; + } + /** * {@inheritdoc} */ @@ -29,13 +41,14 @@ public function process(ContainerBuilder $container) if (!method_exists(\ReflectionMethod::class, 'getReturnType')) { return; } + $resolveClassPassChanges = null !== $this->resolveClassPass ? $this->resolveClassPass->getChanges() : array(); foreach ($container->getDefinitions() as $id => $definition) { - $this->updateDefinition($container, $id, $definition); + $this->updateDefinition($container, $id, $definition, $resolveClassPassChanges); } } - private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $previous = array()) + private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $resolveClassPassChanges, array $previous = array()) { // circular reference if (isset($previous[$id])) { @@ -43,7 +56,7 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $ } $factory = $definition->getFactory(); - if (null === $factory || null !== $definition->getClass()) { + if (null === $factory || (!isset($resolveClassPassChanges[$id]) && null !== $definition->getClass())) { return; } @@ -58,7 +71,7 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $ if ($factory[0] instanceof Reference) { $previous[$id] = true; $factoryDefinition = $container->findDefinition((string) $factory[0]); - $this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $previous); + $this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $resolveClassPassChanges, $previous); $class = $factoryDefinition->getClass(); } else { $class = $factory[0]; @@ -83,6 +96,9 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $ } } + if (null !== $returnType && (!isset($resolveClassPassChanges[$id]) || $returnType !== $resolveClassPassChanges[$id])) { + @trigger_error(sprintf('Relying on its factory\'s return-type to define the class of service "%s" is deprecated since Symfony 3.3 and won\'t work in 4.0. Set the "class" attribute to "%s" on the service definition instead.', $id, $returnType), E_USER_DEPRECATED); + } $definition->setClass($returnType); } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index b95a41482983d..d3a40db6ce562 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -41,10 +41,11 @@ public function __construct() $this->optimizationPasses = array(array( new ExtensionCompilerPass(), + $resolveClassPass = new ResolveClassPass(), new ResolveDefinitionTemplatesPass(), new DecoratorServicePass(), new ResolveParameterPlaceHoldersPass(), - new FactoryReturnTypePass(), + new FactoryReturnTypePass($resolveClassPass), new CheckDefinitionValidityPass(), new ResolveReferencesToAliasesPass(), new ResolveInvalidReferencesPass(), diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveClassPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveClassPass.php new file mode 100644 index 0000000000000..a33fc2d76ddd8 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveClassPass.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ChildDefinition; + +/** + * @author Nicolas Grekas + */ +class ResolveClassPass implements CompilerPassInterface +{ + private $changes = array(); + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + foreach ($container->getDefinitions() as $id => $definition) { + if ($definition instanceof ChildDefinition || $definition->isSynthetic() || null !== $definition->getClass()) { + continue; + } + if (preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $id)) { + $this->changes[$id] = $container->getCaseSensitiveId($id); + $definition->setClass($this->changes[$id]); + } + } + } + + /** + * @internal + * + * @deprecated since 3.3, to be removed in 4.0. + */ + public function getChanges() + { + $changes = $this->changes; + $this->changes = array(); + + return $changes; + } +} diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 981adf6f46231..cea457858fe46 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -103,6 +103,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ private $envCounters = array(); + /** + * @var array a map of case less to case sensitive ids + */ + private $caseSensitiveIds = array(); + /** * Sets the track resources flag. * @@ -367,7 +372,8 @@ public function getCompiler() */ public function set($id, $service) { - $id = strtolower($id); + $caseSensitiveId = $id; + $id = strtolower($caseSensitiveId); if ($this->isFrozen() && (isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())) { // setting a synthetic service on a frozen container is alright @@ -375,6 +381,9 @@ public function set($id, $service) } unset($this->definitions[$id], $this->aliasDefinitions[$id]); + if ($id !== $caseSensitiveId) { + $this->caseSensitiveIds[$id] = $caseSensitiveId; + } parent::set($id, $service); } @@ -628,7 +637,8 @@ public function setAliases(array $aliases) */ public function setAlias($alias, $id) { - $alias = strtolower($alias); + $caseSensitiveAlias = $alias; + $alias = strtolower($caseSensitiveAlias); if (is_string($id)) { $id = new Alias($id); @@ -641,6 +651,9 @@ public function setAlias($alias, $id) } unset($this->definitions[$alias]); + if ($alias !== $caseSensitiveAlias) { + $this->caseSensitiveIds[$alias] = $caseSensitiveAlias; + } $this->aliasDefinitions[$alias] = $id; } @@ -778,9 +791,13 @@ public function setDefinition($id, Definition $definition) throw new BadMethodCallException('Adding definition to a frozen container is not allowed'); } - $id = strtolower($id); + $caseSensitiveId = $id; + $id = strtolower($caseSensitiveId); unset($this->aliasDefinitions[$id]); + if ($id !== $caseSensitiveId) { + $this->caseSensitiveIds[$id] = $caseSensitiveId; + } return $this->definitions[$id] = $definition; } @@ -839,6 +856,20 @@ public function findDefinition($id) return $this->getDefinition($id); } + /** + * Returns the case sensitive id used at registration time. + * + * @param string $id + * + * @return string + */ + public function getCaseSensitiveId($id) + { + $id = strtolower($id); + + return isset($this->caseSensitiveIds[$id]) ? $this->caseSensitiveIds[$id] : $id; + } + /** * Creates a service for a service definition. * diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index d90777a9cfb28..ab06a0c0af18c 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -303,7 +303,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file) if (false !== $nodes = $xpath->query('//container:argument[@type="service"][not(@id)]|//container:property[@type="service"][not(@id)]')) { foreach ($nodes as $node) { // give it a unique name - $id = sprintf('%s_%d', hash('sha256', $file), ++$count); + $id = sprintf('%d_%s', ++$count, hash('sha256', $file)); $node->setAttribute('id', $id); if ($services = $this->getChildren($node, 'service')) { @@ -321,15 +321,15 @@ private function processAnonymousServices(\DOMDocument $xml, $file) if (false !== $nodes = $xpath->query('//container:services/container:service[not(@id)]')) { foreach ($nodes as $node) { // give it a unique name - $id = sprintf('%s_%d', hash('sha256', $file), ++$count); + $id = sprintf('%d_%s', ++$count, hash('sha256', $file)); $node->setAttribute('id', $id); $definitions[$id] = array($node, $file, true); } } // resolve definitions - krsort($definitions); - foreach ($definitions as $id => list($domElement, $file, $wild)) { + uksort($definitions, 'strnatcmp'); + foreach (array_reverse($definitions) as $id => list($domElement, $file, $wild)) { if (null !== $definition = $this->parseDefinition($domElement, $file)) { $this->container->setDefinition($id, $definition); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php index ecee63799c36b..2c95ecdbd22d8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php @@ -20,6 +20,8 @@ /** * @author Guilhem N. + * + * @group legacy */ class FactoryReturnTypePassTest extends \PHPUnit_Framework_TestCase { @@ -103,17 +105,16 @@ public function testCircularReference() $this->assertNull($factory2->getClass()); } + /** + * @requires function ReflectionMethod::getReturnType + * @expectedDeprecation Relying on its factory's return-type to define the class of service "factory" is deprecated since Symfony 3.3 and won't work in 4.0. Set the "class" attribute to "Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy" on the service definition instead. + */ public function testCompile() { $container = new ContainerBuilder(); $factory = $container->register('factory'); $factory->setFactory(array(FactoryDummy::class, 'createFactory')); - - if (!method_exists(\ReflectionMethod::class, 'getReturnType')) { - $this->setExpectedException(\RuntimeException::class, 'Please add the class to service "factory" even if it is constructed by a factory since we might need to add method calls based on compile-time checks.'); - } - $container->compile(); $this->assertEquals(FactoryDummy::class, $container->getDefinition('factory')->getClass()); From a18c4b6ab2acbcb8e621815af2114f8b2e425e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haso=C5=88?= Date: Fri, 21 Oct 2016 12:29:35 +0200 Subject: [PATCH 2/2] [DI] Add tests for class named services --- .../Tests/ContainerBuilderTest.php | 39 +++++++++++++++++++ .../Tests/Fixtures/CaseSensitiveClass.php | 16 ++++++++ .../Tests/Fixtures/xml/class_from_id.xml | 6 +++ .../Tests/Fixtures/yaml/class_from_id.yml | 3 ++ .../Tests/Loader/XmlFileLoaderTest.php | 11 ++++++ .../Tests/Loader/YamlFileLoaderTest.php | 11 ++++++ 6 files changed, 86 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/class_from_id.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/class_from_id.yml diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 3fa87ff9c9f8d..e9284b24a6cf7 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -32,6 +32,7 @@ use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\ExpressionLanguage\Expression; class ContainerBuilderTest extends \PHPUnit_Framework_TestCase @@ -926,6 +927,44 @@ public function testClosureProxyOnInvalidException() $container->get('foo'); } + + public function testClassFromId() + { + $container = new ContainerBuilder(); + + $unknown = $container->register('unknown_class'); + $class = $container->register(\stdClass::class); + $autoloadClass = $container->register(CaseSensitiveClass::class); + $container->compile(); + + $this->assertSame('unknown_class', $unknown->getClass()); + $this->assertEquals(\stdClass::class, $class->getClass()); + $this->assertEquals(CaseSensitiveClass::class, $autoloadClass->getClass()); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage The definition for "123_abc" has no class. + */ + public function testNoClassFromNonClassId() + { + $container = new ContainerBuilder(); + + $definition = $container->register('123_abc'); + $container->compile(); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage The definition for "\foo" has no class. + */ + public function testNoClassFromNsSeparatorId() + { + $container = new ContainerBuilder(); + + $definition = $container->register('\\foo'); + $container->compile(); + } } class FooClass diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php new file mode 100644 index 0000000000000..87b258429e6a3 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php @@ -0,0 +1,16 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Tests\Fixtures; + +class CaseSensitiveClass +{ +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/class_from_id.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/class_from_id.xml new file mode 100644 index 0000000000000..45415cce472f0 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/class_from_id.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/class_from_id.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/class_from_id.yml new file mode 100644 index 0000000000000..33f0b2ea6821f --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/class_from_id.yml @@ -0,0 +1,3 @@ +services: + Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass: + autowire: true diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 5c687d6e7b1ad..7fda551371a27 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -20,6 +20,7 @@ use Symfony\Component\DependencyInjection\Loader\IniFileLoader; use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\ExpressionLanguage\Expression; class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase @@ -582,6 +583,16 @@ public function testAutowireAttributeAndTag() $loader->load('services28.xml'); } + public function testClassFromId() + { + $container = new ContainerBuilder(); + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('class_from_id.xml'); + $container->compile(); + + $this->assertEquals(CaseSensitiveClass::class, $container->getDefinition(CaseSensitiveClass::class)->getClass()); + } + /** * @group legacy * @expectedDeprecation Using the attribute "class" is deprecated for the service "bar" which is defined as an alias %s. diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 8ce3a9f9f68aa..d0390ce314f57 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -20,6 +20,7 @@ use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\ExpressionLanguage\Expression; class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase @@ -341,6 +342,16 @@ public function testAutowire() $this->assertEquals(array('set*', 'bar'), $container->getDefinition('autowire_array')->getAutowiredMethods()); } + public function testClassFromId() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('class_from_id.yml'); + $container->compile(); + + $this->assertEquals(CaseSensitiveClass::class, $container->getDefinition(CaseSensitiveClass::class)->getClass()); + } + /** * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException * @expectedExceptionMessage The value of the "decorates" option for the "bar" service must be the id of the service without the "@" prefix (replace "@foo" with "foo").