From b92439f50c5d33fb7b03095dbe01cea948867eec Mon Sep 17 00:00:00 2001 From: Ener-Getick Date: Mon, 14 Mar 2016 20:21:02 +0100 Subject: [PATCH 1/5] [DependencyInjection] Fix a limitation of the PhpDumper --- .../DependencyInjection/Dumper/PhpDumper.php | 15 ++++++++++++--- .../Tests/Dumper/PhpDumperTest.php | 13 +++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index e0a763dfe8d5b..b7187252f0228 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -58,6 +58,7 @@ class PhpDumper extends Dumper private $targetDirRegex; private $targetDirMaxMatches; private $docStar; + private $existingNames = array(); /** * @var \Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface @@ -1349,13 +1350,21 @@ private function getServiceCall($id, Reference $reference = null) */ private function camelize($id) { + if (isset($this->existingNames[$id])) { + return $this->existingNames[$id]; + } + $name = Container::camelize($id); + $uniqueName = $name = preg_replace('/[^a-zA-Z0-9_\x7f-\xff]/', '', $name); + $prefix = 1; - if (!preg_match('/^[a-zA-Z0-9_\x7f-\xff]+$/', $name)) { - throw new InvalidArgumentException(sprintf('Service id "%s" cannot be converted to a valid PHP method name.', $id)); + while (in_array($uniqueName, $this->existingNames)) { + ++$prefix; + $uniqueName = $name.$prefix; } + $this->existingNames[$id] = $uniqueName; - return $name; + return $uniqueName; } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 81b633dcbb91b..c82002e1072fa 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -132,16 +132,17 @@ public function testServicesWithAnonymousFactories() $this->assertStringEqualsFile(self::$fixturesPath.'/php/services19.php', $dumper->dump(), '->dump() dumps services with anonymous factories'); } - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Service id "bar$" cannot be converted to a valid PHP method name. - */ - public function testAddServiceInvalidServiceId() + public function testAddServiceIdWithUnsupportedCharacters() { $container = new ContainerBuilder(); $container->register('bar$', 'FooClass'); + $container->register('bar$!', 'FooClass'); $dumper = new PhpDumper($container); - $dumper->dump(); + eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Unsupported_Characters'))); + + $container = new \Symfony_DI_PhpDumper_Test_Unsupported_Characters(); + $this->assertTrue(method_exists($container, 'getBarService')); + $this->assertTrue(method_exists($container, 'getBar2Service')); } /** From 67dcb92118daba1bfcbca991f2c5adbfddc0d664 Mon Sep 17 00:00:00 2001 From: Ener-Getick Date: Tue, 15 Mar 2016 17:31:51 +0100 Subject: [PATCH 2/5] Improve performance --- .../DependencyInjection/Dumper/PhpDumper.php | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index b7187252f0228..738fe069ba905 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -58,7 +58,8 @@ class PhpDumper extends Dumper private $targetDirRegex; private $targetDirMaxMatches; private $docStar; - private $existingNames = array(); + private $serviceIdToMethodNameMap = array(); + private $usedMethodNames = array(); /** * @var \Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface @@ -1350,21 +1351,22 @@ private function getServiceCall($id, Reference $reference = null) */ private function camelize($id) { - if (isset($this->existingNames[$id])) { - return $this->existingNames[$id]; + if (isset($this->serviceIdToMethodNameMap[$id])) { + return $this->serviceIdToMethodNameMap[$id]; } $name = Container::camelize($id); - $uniqueName = $name = preg_replace('/[^a-zA-Z0-9_\x7f-\xff]/', '', $name); - $prefix = 1; + $methodName = $name = preg_replace('/[^a-zA-Z0-9_\x7f-\xff]/', '', $name); + $suffix = 1; - while (in_array($uniqueName, $this->existingNames)) { - ++$prefix; - $uniqueName = $name.$prefix; + while (isset($this->usedMethodNames[$methodName])) { + ++$suffix; + $methodName = $name.$suffix; } - $this->existingNames[$id] = $uniqueName; + $this->serviceIdToMethodNameMap[$id] = $methodName; + $this->usedMethodNames[$methodName] = true; - return $uniqueName; + return $methodName; } /** From ae853876c738ec15f59022b18d8438076b30349a Mon Sep 17 00:00:00 2001 From: Ener-Getick Date: Wed, 16 Mar 2016 16:59:07 +0100 Subject: [PATCH 3/5] [PhpDumper] Fix method names conflicts --- .../DependencyInjection/Dumper/PhpDumper.php | 16 +++++++------- .../Tests/Dumper/PhpDumperTest.php | 21 +++++++++++++++---- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 738fe069ba905..130bf6f53c27d 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -634,7 +634,7 @@ private function addService($id, $definition) *$lazyInitializationDoc * $return */ - {$visibility} function get{$this->camelize($id)}Service($lazyInitialization) + {$visibility} function {$this->generateMethodName($id)}($lazyInitialization) { EOF; @@ -866,7 +866,7 @@ private function addMethodMap() $code = " \$this->methodMap = array(\n"; ksort($definitions); foreach ($definitions as $id => $definition) { - $code .= ' '.var_export($id, true).' => '.var_export('get'.$this->camelize($id).'Service', true).",\n"; + $code .= ' '.var_export($id, true).' => '.var_export($this->generateMethodName($id), true).",\n"; } return $code." );\n"; @@ -1349,22 +1349,24 @@ private function getServiceCall($id, Reference $reference = null) * * @throws InvalidArgumentException */ - private function camelize($id) + private function generateMethodName($id) { if (isset($this->serviceIdToMethodNameMap[$id])) { return $this->serviceIdToMethodNameMap[$id]; } $name = Container::camelize($id); - $methodName = $name = preg_replace('/[^a-zA-Z0-9_\x7f-\xff]/', '', $name); + $name = preg_replace('/[^a-zA-Z0-9_\x7f-\xff]/', '', $name); + $methodName = 'get'.$name.'Service'; $suffix = 1; - while (isset($this->usedMethodNames[$methodName])) { + while (isset($this->usedMethodNames[strtolower($methodName)])) { ++$suffix; - $methodName = $name.$suffix; + $methodName = 'get'.$name.$suffix.'Service'; } + $this->serviceIdToMethodNameMap[$id] = $methodName; - $this->usedMethodNames[$methodName] = true; + $this->usedMethodNames[strtolower($methodName)] = true; return $methodName; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index c82002e1072fa..f802bba25bbd1 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -134,15 +134,28 @@ public function testServicesWithAnonymousFactories() public function testAddServiceIdWithUnsupportedCharacters() { + $class = 'Symfony_DI_PhpDumper_Test_Unsupported_Characters'; $container = new ContainerBuilder(); $container->register('bar$', 'FooClass'); $container->register('bar$!', 'FooClass'); $dumper = new PhpDumper($container); - eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Unsupported_Characters'))); + eval('?>'.$dumper->dump(array('class' => $class))); - $container = new \Symfony_DI_PhpDumper_Test_Unsupported_Characters(); - $this->assertTrue(method_exists($container, 'getBarService')); - $this->assertTrue(method_exists($container, 'getBar2Service')); + $this->assertTrue(method_exists($class, 'getBarService')); + $this->assertTrue(method_exists($class, 'getBar2Service')); + } + + public function testConflictingServiceIds() + { + $class = 'Symfony_DI_PhpDumper_Test_Conflicting_Service_Ids'; + $container = new ContainerBuilder(); + $container->register('foo_bar', 'FooClass'); + $container->register('foobar', 'FooClass'); + $dumper = new PhpDumper($container); + eval('?>'.$dumper->dump(array('class' => $class))); + + $this->assertTrue(method_exists($class, 'getFooBarService')); + $this->assertTrue(method_exists($class, 'getFoobar2Service')); } /** From 5efed7b428d7851b6a439db1b2ac8371b40a71be Mon Sep 17 00:00:00 2001 From: Ener-Getick Date: Wed, 16 Mar 2016 17:14:04 +0100 Subject: [PATCH 4/5] Fix method conflicts with parent class --- .../DependencyInjection/Dumper/PhpDumper.php | 26 +++++++++++++++++-- .../Tests/Dumper/PhpDumperTest.php | 16 ++++++++++++ .../Fixtures/containers/CustomContainer.php | 17 ++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/CustomContainer.php diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 130bf6f53c27d..e7d871e07bfb1 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -58,8 +58,8 @@ class PhpDumper extends Dumper private $targetDirRegex; private $targetDirMaxMatches; private $docStar; - private $serviceIdToMethodNameMap = array(); - private $usedMethodNames = array(); + private $serviceIdToMethodNameMap; + private $usedMethodNames; /** * @var \Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface @@ -108,6 +108,9 @@ public function dump(array $options = array()) 'namespace' => '', 'debug' => true, ), $options); + + $this->initializeMethodNamesMap($options['base_class']); + $this->docStar = $options['debug'] ? '*' : ''; if (!empty($options['file']) && is_dir($dir = dirname($options['file']))) { @@ -1340,6 +1343,25 @@ private function getServiceCall($id, Reference $reference = null) } } + /** + * Initializes the method names map to avoid conflicts with the Container methods. + * + * @param string $class the container base class + */ + private function initializeMethodNamesMap($class) + { + $this->serviceIdToMethodNameMap = array(); + $this->usedMethodNames = array(); + + try { + $reflectionClass = new \ReflectionClass($class); + foreach ($reflectionClass->getMethods() as $method) { + $this->usedMethodNames[strtolower($method->getName())] = true; + } + } catch (\ReflectionException $e) { + } + } + /** * Convert a service id to a valid PHP method name. * diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index f802bba25bbd1..ae0c82e405d12 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -158,6 +158,22 @@ public function testConflictingServiceIds() $this->assertTrue(method_exists($class, 'getFoobar2Service')); } + public function testConflictingMethodsWithParent() + { + $class = 'Symfony_DI_PhpDumper_Test_Conflicting_Method_With_Parent'; + $container = new ContainerBuilder(); + $container->register('bar', 'FooClass'); + $container->register('foo_bar', 'FooClass'); + $dumper = new PhpDumper($container); + eval('?>'.$dumper->dump(array( + 'class' => $class, + 'base_class' => 'Symfony\Component\DependencyInjection\Tests\Fixtures\containers\CustomContainer', + ))); + + $this->assertTrue(method_exists($class, 'getBar2Service')); + $this->assertTrue(method_exists($class, 'getFoobar2Service')); + } + /** * @dataProvider provideInvalidFactories * @expectedException Symfony\Component\DependencyInjection\Exception\RuntimeException diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/CustomContainer.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/CustomContainer.php new file mode 100644 index 0000000000000..2251435324b38 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/CustomContainer.php @@ -0,0 +1,17 @@ + Date: Wed, 16 Mar 2016 18:40:39 +0100 Subject: [PATCH 5/5] Fix the lazy PhpDumper --- .../LazyProxy/PhpDumper/ProxyDumper.php | 7 ++++++- .../LazyProxy/PhpDumper/ProxyDumperTest.php | 21 +++++++++++++++++++ .../DependencyInjection/Dumper/PhpDumper.php | 5 +++-- .../LazyProxy/PhpDumper/DumperInterface.php | 3 ++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php b/src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php index cf2520a371549..447549b970442 100644 --- a/src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php +++ b/src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php @@ -71,7 +71,12 @@ public function getProxyFactoryCode(Definition $definition, $id) $instantiation .= " \$this->services['$id'] ="; } - $methodName = 'get'.Container::camelize($id).'Service'; + if (func_num_args() >= 3) { + $methodName = func_get_arg(2); + } else { + @trigger_error(sprintf('You must use the third argument of %s to define the method to call to construct your service since version 3.1, not using it won\'t be supported in 4.0.', __METHOD__), E_USER_DEPRECATED); + $methodName = 'get'.Container::camelize($id).'Service'; + } $proxyClass = $this->getProxyClassName($definition); $generatedClass = $this->generateProxyClass($definition); diff --git a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php index de7f9ea221a7e..cd3af44a81f16 100644 --- a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php +++ b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php @@ -60,6 +60,27 @@ public function testGetProxyCode() ); } + public function testGetProxyFactoryCodeWithCustomMethod() + { + $definition = new Definition(__CLASS__); + + $definition->setLazy(true); + + $code = $this->dumper->getProxyFactoryCode($definition, 'foo', 'getFoo2Service'); + + $this->assertStringMatchesFormat( + '%wif ($lazyLoad) {%wreturn $this->services[\'foo\'] =%s' + .'SymfonyBridgeProxyManagerTestsLazyProxyPhpDumperProxyDumperTest_%s(%wfunction ' + .'(&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {' + .'%w$wrappedInstance = $this->getFoo2Service(false);%w$proxy->setProxyInitializer(null);' + .'%wreturn true;%w}%w);%w}%w', + $code + ); + } + + /** + * @group legacy + */ public function testGetProxyFactoryCode() { $definition = new Definition(__CLASS__); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index e7d871e07bfb1..a8894f39e2f15 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -630,6 +630,7 @@ private function addService($id, $definition) // with proxies, for 5.3.3 compatibility, the getter must be public to be accessible to the initializer $isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition); $visibility = $isProxyCandidate ? 'public' : 'protected'; + $methodName = $this->generateMethodName($id); $code = <<docStar} @@ -637,12 +638,12 @@ private function addService($id, $definition) *$lazyInitializationDoc * $return */ - {$visibility} function {$this->generateMethodName($id)}($lazyInitialization) + {$visibility} function {$methodName}($lazyInitialization) { EOF; - $code .= $isProxyCandidate ? $this->getProxyDumper()->getProxyFactoryCode($definition, $id) : ''; + $code .= $isProxyCandidate ? $this->getProxyDumper()->getProxyFactoryCode($definition, $id, $methodName) : ''; if ($definition->isSynthetic()) { $code .= sprintf(" throw new RuntimeException('You have requested a synthetic service (\"%s\"). The DIC does not know how to construct this service.');\n }\n", $id); diff --git a/src/Symfony/Component/DependencyInjection/LazyProxy/PhpDumper/DumperInterface.php b/src/Symfony/Component/DependencyInjection/LazyProxy/PhpDumper/DumperInterface.php index 878d965b1c39d..3ff2e01c13582 100644 --- a/src/Symfony/Component/DependencyInjection/LazyProxy/PhpDumper/DumperInterface.php +++ b/src/Symfony/Component/DependencyInjection/LazyProxy/PhpDumper/DumperInterface.php @@ -34,10 +34,11 @@ public function isProxyCandidate(Definition $definition); * * @param Definition $definition * @param string $id service identifier + * @param string $methodName the method name to get the service, will be added to the interface in 4.0. * * @return string */ - public function getProxyFactoryCode(Definition $definition, $id); + public function getProxyFactoryCode(Definition $definition, $id/**, $methodName = null */); /** * Generates the code for the lazy proxy.