From 0a2343187fa5d2d679b583274351aef69df9db07 Mon Sep 17 00:00:00 2001 From: Kyle Cannon Date: Mon, 26 Nov 2012 12:05:44 -0800 Subject: [PATCH 1/4] Fix issue causing service classes to not be found with long XML parameter wrapped to multiple lines. --- .../DependencyInjection/ContainerBuilder.php | 5 ++-- .../Tests/ContainerBuilderTest.php | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 0fd333aedcf23..123931e96afee 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -759,8 +759,9 @@ private function createService(Definition $definition, $id) $service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments); } else { - $r = new \ReflectionClass($parameterBag->resolveValue($definition->getClass())); - + $classDefinition = preg_replace('~[\r\n]+~', '', trim($parameterBag->resolveValue($definition->getClass()))); + $r = new \ReflectionClass($classDefinition); + $service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index c9e6b07847dc0..214762012edea 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -319,6 +319,35 @@ public function testCreateServiceConfigurator() } } + /** + * @covers Symfony\Component\DependencyInjection\ContainerBuilder::createService + */ + public function testCreateServiceWithNewLineWrapping() + { + $builder = new ContainerBuilder(); + $builder->setParameter('foo1class',"\nFooClass\n"); + $builder->register('foo1','%foo1class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); + $this->assertInstanceOf('\FooClass',$builder->get('foo1'), 'New Line Wrapping \n in Parameter should be stripped out.'); + $builder->setParameter('foo2class',"\rFooClass\r"); + $builder->register('foo2','%foo2class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); + $this->assertInstanceOf('\FooClass',$builder->get('foo2'), 'New Line Wrapping \r in Parameter should be stripped out.'); + } + + /** + * @covers Symfony\Component\DependencyInjection\ContainerBuilder::createService + */ + public function testCreateServiceWithNewLineWrappingWithWhiteSpace() + { + $builder = new ContainerBuilder(); + $builder->setParameter('foo1class',"\n FooClass \n"); + $builder->register('foo1','%foo1class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); + $this->assertInstanceOf('\FooClass',$builder->get('foo1'),'New Line Wrapping \n in Parameter should be stripped out along with whitespace.'); + $builder = new ContainerBuilder(); + $builder->setParameter('foo2class',"\r FooClass \r"); + $builder->register('foo2','%foo2class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); + $this->assertInstanceOf('\FooClass',$builder->get('foo2'),'New Line Wrapping \r in Parameter should be stripped out along with whitespace.'); + } + /** * @covers Symfony\Component\DependencyInjection\ContainerBuilder::resolveServices */ From 48bc27a8e28852b85978476b150172f65f602a2e Mon Sep 17 00:00:00 2001 From: Kyle Cannon Date: Mon, 26 Nov 2012 13:07:30 -0800 Subject: [PATCH 2/4] Add try/catch to Unit Tests --- .../Tests/ContainerBuilderTest.php | 61 +++++++++++++++---- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 214762012edea..77c3b84d0525b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -325,12 +325,28 @@ public function testCreateServiceConfigurator() public function testCreateServiceWithNewLineWrapping() { $builder = new ContainerBuilder(); - $builder->setParameter('foo1class',"\nFooClass\n"); - $builder->register('foo1','%foo1class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); - $this->assertInstanceOf('\FooClass',$builder->get('foo1'), 'New Line Wrapping \n in Parameter should be stripped out.'); - $builder->setParameter('foo2class',"\rFooClass\r"); - $builder->register('foo2','%foo2class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); - $this->assertInstanceOf('\FooClass',$builder->get('foo2'), 'New Line Wrapping \r in Parameter should be stripped out.'); + $builder->setParameter('foo1class', "\nFooClass\n"); + $builder->register('foo1', '%foo1class%')->setFile(__DIR__ . '/Fixtures/includes/foo.php'); + try { + $this->assertInstanceOf( + '\FooClass', + $builder->get('foo1'), + 'New Line Wrapping \n in Parameter should be stripped out.' + ); + } catch (\ReflectionException $e) { + $this->fail('FooClass is not callable due to the new lines not being stripped'); + } + $builder->setParameter('foo2class', "\rFooClass\r"); + $builder->register('foo2', '%foo2class%')->setFile(__DIR__ . '/Fixtures/includes/foo.php'); + try { + $this->assertInstanceOf( + '\FooClass', + $builder->get('foo2'), + 'New Line Wrapping \r in Parameter should be stripped out.' + ); + } catch (\ReflectionException $e) { + $this->fail('FooClass is not callable due to the new lines not being stripped'); + } } /** @@ -339,13 +355,32 @@ public function testCreateServiceWithNewLineWrapping() public function testCreateServiceWithNewLineWrappingWithWhiteSpace() { $builder = new ContainerBuilder(); - $builder->setParameter('foo1class',"\n FooClass \n"); - $builder->register('foo1','%foo1class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); - $this->assertInstanceOf('\FooClass',$builder->get('foo1'),'New Line Wrapping \n in Parameter should be stripped out along with whitespace.'); - $builder = new ContainerBuilder(); - $builder->setParameter('foo2class',"\r FooClass \r"); - $builder->register('foo2','%foo2class%')->setFile(__DIR__.'/Fixtures/includes/foo.php'); - $this->assertInstanceOf('\FooClass',$builder->get('foo2'),'New Line Wrapping \r in Parameter should be stripped out along with whitespace.'); + $builder->setParameter('foo1class', "\n FooClass \n"); + $builder->register('foo1', '%foo1class%')->setFile(__DIR__ . '/Fixtures/includes/foo.php'); + try { + $this->assertInstanceOf( + '\FooClass', + $builder->get('foo1'), + 'New Line Wrapping \n in Parameter should be stripped out along with whitespace.' + ); + } catch (\ReflectionException $e) { + $this->fail( + 'FooClass is not callable due to the new lines not being stripped and/or whitespacing not being trimmed' + ); + } + $builder->setParameter('foo2class', "\r FooClass \r"); + $builder->register('foo2', '%foo2class%')->setFile(__DIR__ . '/Fixtures/includes/foo.php'); + try { + $this->assertInstanceOf( + '\FooClass', + $builder->get('foo2'), + 'New Line Wrapping \r in Parameter should be stripped out along with whitespace.' + ); + } catch (\ReflectionException $e) { + $this->fail( + 'FooClass is not callable due to the new lines not being stripped and/or whitespacing not being trimmed.' + ); + } } /** From ddde8561aa34fb41862ed4cb5d031521efcab8ce Mon Sep 17 00:00:00 2001 From: Kyle Cannon Date: Mon, 26 Nov 2012 15:28:44 -0800 Subject: [PATCH 3/4] Remove preg_replace and only use trim as it already handles removing the extra lines --- src/Symfony/Component/DependencyInjection/ContainerBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 123931e96afee..34cb3fceafc4a 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -759,7 +759,7 @@ private function createService(Definition $definition, $id) $service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments); } else { - $classDefinition = preg_replace('~[\r\n]+~', '', trim($parameterBag->resolveValue($definition->getClass()))); + $classDefinition = trim($parameterBag->resolveValue($definition->getClass())); $r = new \ReflectionClass($classDefinition); $service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments); From 71623fd57b7cb23854b0ebd4d9c029d1fc94cf32 Mon Sep 17 00:00:00 2001 From: Kyle Cannon Date: Wed, 28 Nov 2012 20:34:57 -0800 Subject: [PATCH 4/4] Remove temporary variable. --- src/Symfony/Component/DependencyInjection/ContainerBuilder.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 34cb3fceafc4a..f09850e928401 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -759,8 +759,7 @@ private function createService(Definition $definition, $id) $service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments); } else { - $classDefinition = trim($parameterBag->resolveValue($definition->getClass())); - $r = new \ReflectionClass($classDefinition); + $r = new \ReflectionClass(trim($parameterBag->resolveValue($definition->getClass()))); $service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments); }