From c9c18ac7f317e269e82b2289169287142321c436 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 23 Nov 2017 09:59:26 +0100 Subject: [PATCH 1/2] [DI] Fix handling of inlined definitions by ContainerBuilder --- .../DependencyInjection/ContainerBuilder.php | 50 ++++++++++++------- .../Tests/ContainerBuilderTest.php | 22 ++++++++ .../Tests/Fixtures/includes/classes.php | 2 +- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 65ea1c842c..59f8cc7d0d 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -441,7 +441,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface $this->loading[$id] = true; try { - $service = $this->createService($definition, $id); + $service = $this->createService($definition, new \SplObjectStorage(), $id); } catch (\Exception $e) { unset($this->loading[$id]); @@ -827,8 +827,12 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * * @internal this method is public because of PHP 5.3 limitations, do not use it explicitly in your code */ - public function createService(Definition $definition, $id, $tryProxy = true) + public function createService(Definition $definition, \SplObjectStorage $inlinedDefinitions, $id = null, $tryProxy = true) { + if (null === $id && isset($inlinedDefinitions[$definition])) { + return $inlinedDefinitions[$definition]; + } + if ($definition instanceof DefinitionDecorator) { throw new RuntimeException(sprintf('Constructing service "%s" from a parent definition is not supported at build time.', $id)); } @@ -845,11 +849,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface ->instantiateProxy( $container, $definition, - $id, function () use ($definition, $id, $container) { - return $container->createService($definition, $id, false); + $id, function () use ($definition, $inlinedDefinitions, $id, $container) { + return $container->createService($definition, $inlinedDefinitions, $id, false); } ); - $this->shareService($definition, $proxy, $id); + $this->shareService($definition, $proxy, $id, $inlinedDefinitions); return $proxy; } @@ -860,11 +864,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface require_once $parameterBag->resolveValue($definition->getFile()); } - $arguments = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments()))); + $arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlinedDefinitions); if (null !== $factory = $definition->getFactory()) { if (is_array($factory)) { - $factory = array($this->resolveServices($parameterBag->resolveValue($factory[0])), $factory[1]); + $factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlinedDefinitions), $factory[1]); } elseif (!is_string($factory)) { throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id)); } @@ -888,16 +892,16 @@ class ContainerBuilder extends Container implements TaggedContainerInterface if ($tryProxy || !$definition->isLazy()) { // share only if proxying failed, or if not a proxy - $this->shareService($definition, $service, $id); + $this->shareService($definition, $service, $id, $inlinedDefinitions); } - $properties = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties()))); + $properties = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())), $inlinedDefinitions); foreach ($properties as $name => $value) { $service->$name = $value; } foreach ($definition->getMethodCalls() as $call) { - $this->callMethod($service, $call); + $this->callMethod($service, $call, $inlinedDefinitions); } if ($callable = $definition->getConfigurator()) { @@ -907,7 +911,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface if ($callable[0] instanceof Reference) { $callable[0] = $this->get((string) $callable[0], $callable[0]->getInvalidBehavior()); } elseif ($callable[0] instanceof Definition) { - $callable[0] = $this->createService($callable[0], null); + $callable[0] = $this->createService($callable[0], $inlinedDefinitions); } } @@ -930,15 +934,20 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * the real service instances and all expressions evaluated */ public function resolveServices($value) + { + return $this->doResolveServices($value, new \SplObjectStorage()); + } + + private function doResolveServices($value, \SplObjectStorage $inlinedDefinitions) { if (is_array($value)) { foreach ($value as $k => $v) { - $value[$k] = $this->resolveServices($v); + $value[$k] = $this->doResolveServices($v, $inlinedDefinitions); } } elseif ($value instanceof Reference) { $value = $this->get((string) $value, $value->getInvalidBehavior()); } elseif ($value instanceof Definition) { - $value = $this->createService($value, null); + $value = $this->createService($value, $inlinedDefinitions); } elseif ($value instanceof Expression) { $value = $this->getExpressionLanguage()->evaluate($value, array('container' => $this)); } @@ -1065,14 +1074,14 @@ class ContainerBuilder extends Container implements TaggedContainerInterface foreach ($definition->getMethodCalls() as $call) { foreach ($call[1] as $argument) { if ($argument instanceof Reference && $id == (string) $argument) { - $this->callMethod($this->get($definitionId), $call); + $this->callMethod($this->get($definitionId), $call, new \SplObjectStorage()); } } } } } - private function callMethod($service, $call) + private function callMethod($service, $call, \SplObjectStorage $inlinedDefinitions) { $services = self::getServiceConditionals($call[1]); @@ -1082,7 +1091,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface } } - call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])))); + call_user_func_array(array($service, $call[0]), $this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlinedDefinitions)); } /** @@ -1094,9 +1103,14 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * * @throws InactiveScopeException */ - private function shareService(Definition $definition, $service, $id) + private function shareService(Definition $definition, $service, $id, \SplObjectStorage $inlinedDefinitions) { - if (null !== $id && self::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) { + if (self::SCOPE_PROTOTYPE === $scope = $definition->getScope()) { + return; + } + if (null === $id) { + $inlinedDefinitions[$definition] = $service; + } else { if (self::SCOPE_CONTAINER !== $scope && !isset($this->scopedServices[$scope])) { throw new InactiveScopeException($id, $scope); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index f4274b80a4..32e1b99adb 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -827,6 +827,28 @@ class ContainerBuilderTest extends TestCase $this->assertTrue($classInList); } + public function testInlinedDefinitions() + { + $container = new ContainerBuilder(); + + $definition = new Definition('BarClass'); + + $container->register('bar_user', 'BarUserClass') + ->addArgument($definition) + ->setProperty('foo', $definition); + + $container->register('bar', 'BarClass') + ->setProperty('foo', $definition) + ->addMethodCall('setBaz', array($definition)); + + $barUser = $container->get('bar_user'); + $bar = $container->get('bar'); + + $this->assertSame($barUser->foo, $barUser->bar); + $this->assertSame($bar->foo, $bar->getBaz()); + $this->assertNotSame($bar->foo, $barUser->foo); + } + public function testInitializePropertiesBeforeMethodCalls() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php index 0ecdea3fbf..92db8f3c5e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php @@ -8,7 +8,7 @@ function sc_configure($instance) $instance->configure(); } -class BarClass +class BarClass extends BazClass { protected $baz; public $foo = 'foo'; From 1b408e692ddc6d353c89e9684afd96f51ce6d417 Mon Sep 17 00:00:00 2001 From: chihiro-adachi Date: Wed, 22 Nov 2017 19:00:37 +0900 Subject: [PATCH 2/2] [Form] Fixed ContextErrorException in FileType --- .../Form/Extension/Core/Type/FileType.php | 7 ++++++- .../Tests/Extension/Core/Type/FileTypeTest.php | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php index fc5b1aae68..cff7adf1bb 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php @@ -34,8 +34,13 @@ class FileType extends AbstractType if ($options['multiple']) { $data = array(); + $files = $event->getData(); - foreach ($event->getData() as $file) { + if (!is_array($files)) { + $files = array(null); + } + + foreach ($files as $file) { if ($requestHandler->isFileUpload($file)) { $data[] = $file; } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php index 5d6fe1e20d..646470ff5a 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php @@ -158,6 +158,24 @@ class FileTypeTest extends BaseTypeTest $this->assertCount(1, $form->getData()); } + /** + * @dataProvider requestHandlerProvider + */ + public function testSubmitNonArrayValueWhenMultiple(RequestHandlerInterface $requestHandler) + { + $form = $this->factory + ->createBuilder(static::TESTED_TYPE, null, array( + 'multiple' => true, + )) + ->setRequestHandler($requestHandler) + ->getForm(); + $form->submit(null); + + $this->assertSame(array(), $form->getData()); + $this->assertSame(array(), $form->getNormData()); + $this->assertSame(array(), $form->getViewData()); + } + public function requestHandlerProvider() { return array(