Merge branch '3.4' into 4.4

* 3.4: [DI] fix loading defaults when using the PHP-DSL RepeatedType should always have inner types mapped
symfony · Apr 13, 2020 · acb1060 · acb1060
2 parents 7b8a374 + f702863
commit acb1060
Show file tree

Hide file tree

Showing 20 changed files with 99 additions and 27 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
@@ -355,7 +355,7 @@ protected function describeContainerDefinition(Definition $definition, array $op
 
     protected function describeContainerAlias(Alias $alias, array $options = [], ContainerBuilder $builder = null)
     {
-        if ($alias->isPublic()) {
+        if ($alias->isPublic() && !$alias->isPrivate()) {
             $options['output']->comment(sprintf('This service is a <info>public</info> alias for the service <info>%s</info>', (string) $alias));
         } else {
             $options['output']->comment(sprintf('This service is a <comment>private</comment> alias for the service <info>%s</info>', (string) $alias));

diff --git a/src/Symfony/Component/Cache/DependencyInjection/CacheCollectorPass.php b/src/Symfony/Component/Cache/DependencyInjection/CacheCollectorPass.php
@@ -55,7 +55,9 @@ public function process(ContainerBuilder $container)
 
             $recorder = new Definition(is_subclass_of($definition->getClass(), TagAwareAdapterInterface::class) ? TraceableTagAwareAdapter::class : TraceableAdapter::class);
             $recorder->setTags($definition->getTags());
-            $recorder->setPublic($definition->isPublic());
+            if (!$definition->isPublic() || !$definition->isPrivate()) {
+                $recorder->setPublic($definition->isPublic());
+            }
             $recorder->setArguments([new Reference($innerId = $id.$this->cachePoolRecorderInnerSuffix)]);
 
             $definition->setTags([]);

diff --git a/...Symfony/Component/DependencyInjection/Loader/Configurator/AbstractServiceConfigurator.php b/...Symfony/Component/DependencyInjection/Loader/Configurator/AbstractServiceConfigurator.php
@@ -42,7 +42,7 @@ public function __destruct()
     /**
      * Registers a service.
      */
-    final public function set(string $id, string $class = null): ServiceConfigurator
+    final public function set(?string $id, string $class = null): ServiceConfigurator
     {
         $this->__destruct();
 

diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/PrototypeConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/PrototypeConfigurator.php
@@ -45,10 +45,13 @@ class PrototypeConfigurator extends AbstractServiceConfigurator
     public function __construct(ServicesConfigurator $parent, PhpFileLoader $loader, Definition $defaults, string $namespace, string $resource, bool $allowParent)
     {
         $definition = new Definition();
-        $definition->setPublic($defaults->isPublic());
+        if (!$defaults->isPublic() || !$defaults->isPrivate()) {
+            $definition->setPublic($defaults->isPublic());
+        }
         $definition->setAutowired($defaults->isAutowired());
         $definition->setAutoconfigured($defaults->isAutoconfigured());
-        $definition->setBindings($defaults->getBindings());
+        // deep clone, to avoid multiple process of the same instance in the passes
+        $definition->setBindings(unserialize(serialize($defaults->getBindings())));
         $definition->setChanges([]);
 
         $this->loader = $loader;

diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php
@@ -83,13 +83,14 @@ final public function set(?string $id, string $class = null): ServiceConfigurato
 
             $id = sprintf('.%d_%s', ++$this->anonymousCount, preg_replace('/^.*\\\\/', '', $class).'~'.$this->anonymousHash);
             $definition->setPublic(false);
-        } else {
-            $definition->setPublic($defaults->isPublic());
+        } elseif (!$defaults->isPublic() || !$defaults->isPrivate()) {
+            $definition->setPublic($defaults->isPublic() && !$defaults->isPrivate());
         }
 
         $definition->setAutowired($defaults->isAutowired());
         $definition->setAutoconfigured($defaults->isAutoconfigured());
-        $definition->setBindings($defaults->getBindings());
+        // deep clone, to avoid multiple process of the same instance in the passes
+        $definition->setBindings(unserialize(serialize($defaults->getBindings())));
         $definition->setChanges([]);
 
         $configurator = new ServiceConfigurator($this->container, $this->instanceof, $allowParent, $this, $definition, $id, $defaults->getTags(), $this->path);
@@ -103,7 +104,10 @@ final public function set(?string $id, string $class = null): ServiceConfigurato
     final public function alias(string $id, string $referencedId): AliasConfigurator
     {
         $ref = static::processValue($referencedId, true);
-        $alias = new Alias((string) $ref, $this->defaults->isPublic());
+        $alias = new Alias((string) $ref);
+        if (!$this->defaults->isPublic() || !$this->defaults->isPrivate()) {
+            $alias->setPublic($this->defaults->isPublic());
+        }
         $this->container->setAlias($id, $alias);
 
         return new AliasConfigurator($this, $alias);

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.php
@@ -7,7 +7,7 @@
 use Symfony\Component\DependencyInjection\Tests\Fixtures\StdClassDecorator;
 
 return function (ContainerConfigurator $c) {
-    $s = $c->services();
+    $s = $c->services()->defaults()->public();
 
     $s->set('decorated', stdClass::class);
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/basic.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/basic.php
@@ -5,7 +5,7 @@
 use App\BarService;
 
 return function (ContainerConfigurator $c) {
-    $s = $c->services();
+    $s = $c->services()->defaults()->public();
     $s->set(BarService::class)
         ->args([inline('FooClass')]);
 };
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.php
@@ -6,12 +6,14 @@
 
 return function (ContainerConfigurator $c) {
     $c->services()
-        ->set('bar', 'Class1')
+        ->set('bar', 'Class1')->public()
         ->set(BarService::class)
+            ->public()
             ->abstract(true)
             ->lazy()
         ->set('foo')
             ->parent(BarService::class)
+            ->public()
             ->decorate('bar', 'b', 1)
             ->args([ref('b')])
             ->class('Class2')

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml
@@ -18,7 +18,7 @@ services:
         arguments: ['@bar']
     bar:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
-        public: false
+        public: true
         tags:
             - { name: t, a: b }
         autowire: true

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.php
@@ -14,7 +14,7 @@
         ->autowire()
         ->tag('t', ['a' => 'b'])
         ->bind(Foo::class, ref('bar'))
-        ->private();
+        ->public();
 
     $s->set(Foo::class)->args([ref('bar')])->public();
     $s->set('bar', Foo::class)->call('setFoo')->autoconfigure(false);

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.php
@@ -6,7 +6,7 @@
 use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype;
 
 return function (ContainerConfigurator $c) {
-    $s = $c->services();
+    $s = $c->services()->defaults()->public();
     $s->instanceof(Prototype\Foo::class)
         ->property('p', 0)
         ->call('setFoo', [ref('foo')])

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.php
@@ -3,6 +3,6 @@
 namespace Symfony\Component\DependencyInjection\Loader\Configurator;
 
 return function (ContainerConfigurator $c) {
-    $di = $c->services();
+    $di = $c->services()->defaults()->public();
     $di->set('foo', 'stdClass')->lazy('SomeInterface');
 };
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/object.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/object.php
@@ -7,7 +7,7 @@
 return new class() {
     public function __invoke(ContainerConfigurator $c)
     {
-        $s = $c->services();
+        $s = $c->services()->defaults()->public();
         $s->set(BarService::class)
             ->args([inline('FooClass')]);
     }

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.php
@@ -9,7 +9,7 @@
         ('foo', 'Foo')
         ('bar', 'Bar')
     ;
-    $c->services()
+    $c->services()->defaults()->public()
         (Foo::class)
             ->arg('$bar', ref('bar'))
             ->public()

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.php
@@ -8,6 +8,7 @@
     $di = $c->services()->defaults()
         ->tag('baz');
     $di->load(Prototype::class.'\\', '../Prototype')
+        ->public()
         ->autoconfigure()
         ->exclude('../Prototype/{OtherDir,BadClasses,SinglyImplementedInterface}')
         ->factory('f')
@@ -17,6 +18,6 @@
         ->autoconfigure(false)
         ->tag('foo')
         ->parent('foo');
-    $di->set('foo')->lazy()->abstract();
+    $di->set('foo')->lazy()->abstract()->public();
     $di->get(Prototype\Foo::class)->lazy(false);
 };
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php
@@ -8,6 +8,7 @@
     $di = $c->services()->defaults()
         ->tag('baz');
     $di->load(Prototype::class.'\\', '../Prototype')
+        ->public()
         ->autoconfigure()
         ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses', '../Prototype/SinglyImplementedInterface'])
         ->factory('f')
@@ -17,6 +18,6 @@
         ->autoconfigure(false)
         ->tag('foo')
         ->parent('foo');
-    $di->set('foo')->lazy()->abstract();
+    $di->set('foo')->lazy()->abstract()->public();
     $di->get(Prototype\Foo::class)->lazy(false);
 };
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php
@@ -16,8 +16,7 @@
     $p->set('foo_class', FooClass::class)
       ->set('foo', 'bar');
 
-    $s = $c->services()->defaults()
-        ->public();
+    $s = $c->services()->defaults()->public();
     $s->set('foo')
         ->args(['foo', ref('foo.baz'), ['%foo%' => 'foo is %foo%', 'foobar' => '%foo%'], true, ref('service_container')])
         ->class(FooClass::class)
@@ -127,12 +126,10 @@
         ->tag('foo');
 
     $s->set('tagged_iterator', 'Bar')
-        ->public()
         ->args([tagged_iterator('foo')]);
 
     $s->set('runtime_error', 'stdClass')
-        ->args([new Reference('errored_definition', ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE)])
-        ->public();
+        ->args([new Reference('errored_definition', ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE)]);
     $s->set('errored_definition', 'stdClass')->private();
 
     $s->alias('alias_for_foo', 'foo')->private()->public();

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php b/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php
@@ -31,13 +31,16 @@ public function buildForm(FormBuilderInterface $builder, array $options)
             $options['options']['error_bubbling'] = $options['error_bubbling'];
         }
 
+        // children fields must always be mapped
+        $defaultOptions = ['mapped' => true];
+
         $builder
             ->addViewTransformer(new ValueToDuplicatesTransformer([
                 $options['first_name'],
                 $options['second_name'],
             ]))
-            ->add($options['first_name'], $options['type'], array_merge($options['options'], $options['first_options']))
-            ->add($options['second_name'], $options['type'], array_merge($options['options'], $options['second_options']))
+            ->add($options['first_name'], $options['type'], array_merge($options['options'], $options['first_options'], $defaultOptions))
+            ->add($options['second_name'], $options['type'], array_merge($options['options'], $options['second_options'], $defaultOptions))
         ;
     }
 

diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/RepeatedTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/RepeatedTypeTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Form\Tests\Extension\Core\Type;
 
 use Symfony\Component\Form\Form;
+use Symfony\Component\Form\Tests\Fixtures\NotMappedType;
 
 class RepeatedTypeTest extends BaseTypeTest
 {
@@ -78,6 +79,41 @@ public function testSetRequired()
         $this->assertFalse($form['second']->isRequired());
     }
 
+    public function testMappedOverridesDefault()
+    {
+        $form = $this->factory->create(NotMappedType::class);
+        $this->assertFalse($form->getConfig()->getMapped());
+
+        $form = $this->factory->create(static::TESTED_TYPE, null, [
+            'type' => NotMappedType::class,
+        ]);
+
+        $this->assertTrue($form['first']->getConfig()->getMapped());
+        $this->assertTrue($form['second']->getConfig()->getMapped());
+    }
+
+    /**
+     * @dataProvider notMappedConfigurationKeys
+     */
+    public function testNotMappedInnerIsOverridden($configurationKey)
+    {
+        $form = $this->factory->create(static::TESTED_TYPE, null, [
+            'type' => TextTypeTest::TESTED_TYPE,
+            $configurationKey => ['mapped' => false],
+        ]);
+
+        $this->assertTrue($form['first']->getConfig()->getMapped());
+        $this->assertTrue($form['second']->getConfig()->getMapped());
+    }
+
+    public function notMappedConfigurationKeys()
+    {
+        return [
+            ['first_options'],
+            ['second_options'],
+        ];
+    }
+
     public function testSetInvalidOptions()
     {
         $this->expectException('Symfony\Component\OptionsResolver\Exception\InvalidOptionsException');

diff --git a/src/Symfony/Component/Form/Tests/Fixtures/NotMappedType.php b/src/Symfony/Component/Form/Tests/Fixtures/NotMappedType.php
@@ -0,0 +1,23 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Form\Tests\Fixtures;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+class NotMappedType extends AbstractType
+{
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefault('mapped', false);
+    }
+}