From 728cd66a1317c159fb2c0722af548f9d2d8173d3 Mon Sep 17 00:00:00 2001 From: Artem Lopata Date: Fri, 10 Apr 2020 13:43:00 +0200 Subject: [PATCH 1/2] RepeatedType should always have inner types mapped --- .../Form/Extension/Core/Type/RepeatedType.php | 7 ++-- .../Extension/Core/Type/RepeatedTypeTest.php | 36 +++++++++++++++++++ .../Form/Tests/Fixtures/NotMappedType.php | 23 ++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Form/Tests/Fixtures/NotMappedType.php diff --git a/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php b/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php index ffb7520a58..6ed403523c 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/RepeatedType.php @@ -31,13 +31,16 @@ class RepeatedType extends AbstractType $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 index 8b4666d6fa..3d5544c05d 100644 --- 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 @@ class RepeatedTypeTest extends BaseTypeTest $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 new file mode 100644 index 0000000000..14c340b891 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/NotMappedType.php @@ -0,0 +1,23 @@ + + * + * 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); + } +} From 51e0d3792caa7c53860b2c54bf2c949ddae8c7cd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 13 Apr 2020 11:14:49 +0200 Subject: [PATCH 2/2] [DI] fix loading defaults when using the PHP-DSL --- .../Compiler/CacheCollectorPass.php | 4 +++- .../Loader/Configurator/PrototypeConfigurator.php | 7 +++++-- .../Loader/Configurator/ServicesConfigurator.php | 12 +++++++++--- .../Tests/Fixtures/config/basic.expected.yml | 1 - .../Tests/Fixtures/config/child.expected.yml | 5 +---- .../Tests/Fixtures/config/defaults.expected.yml | 1 - .../Tests/Fixtures/config/instanceof.expected.yml | 2 -- .../Tests/Fixtures/config/php7.expected.yml | 1 - .../Tests/Fixtures/config/prototype.expected.yml | 2 -- .../Tests/Fixtures/config/services9.php | 3 +-- 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CacheCollectorPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CacheCollectorPass.php index 7f8494d98e..6bb614489e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CacheCollectorPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CacheCollectorPass.php @@ -44,7 +44,9 @@ class CacheCollectorPass implements CompilerPassInterface $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.'.recorder_inner')]); $definition->setTags([]); diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/PrototypeConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/PrototypeConfigurator.php index d80c8b1338..3d844798d4 100644 --- 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, $namespace, $resource, $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 index e7677eb5e6..b6ccbc63b4 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php @@ -79,10 +79,13 @@ class ServicesConfigurator extends AbstractConfigurator $allowParent = !$defaults->getChanges() && empty($this->instanceof); $definition = new Definition(); - $definition->setPublic($defaults->isPublic()); + if (!$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()); @@ -101,7 +104,10 @@ class ServicesConfigurator extends AbstractConfigurator final public function alias($id, $referencedId) { $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/basic.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/basic.expected.yml index 1137961ade..39a3b631b9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/basic.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/basic.expected.yml @@ -6,5 +6,4 @@ services: synthetic: true App\BarService: class: App\BarService - public: true arguments: [!service { class: FooClass }] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml index aaab7131c4..f60d6bb5b7 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml @@ -6,10 +6,7 @@ services: synthetic: true foo: class: Class2 - public: true file: file.php lazy: true arguments: [!service { class: Class1, public: false }] - bar: - alias: foo - public: true + bar: '@foo' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml index a534f7267a..3f01b1099f 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml @@ -6,7 +6,6 @@ services: synthetic: true App\BarService: class: App\BarService - public: true arguments: [!service { class: FooClass }] Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml index b12a304221..1238a7bda4 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml @@ -6,7 +6,6 @@ services: synthetic: true Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo - public: true tags: - { name: tag, k: v } lazy: true @@ -18,4 +17,3 @@ services: configurator: c foo: class: App\FooService - public: true diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.expected.yml index 7c5b714ffb..7cec320b2e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/php7.expected.yml @@ -13,7 +13,6 @@ services: arguments: ['@bar'] bar: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo - public: true calls: - [setFoo, { }] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml index ebfe087d77..24a7940153 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml @@ -6,7 +6,6 @@ services: synthetic: true Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo - public: true tags: - { name: foo } - { name: baz } @@ -15,7 +14,6 @@ services: factory: f Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar: class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar - public: true tags: - { name: foo } - { name: baz } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php index d9373a2a6f..ef39093768 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php @@ -14,7 +14,7 @@ return function (ContainerConfigurator $c) { $p->set('foo_class', FooClass::class) ->set('foo', 'bar'); - $s = $c->services(); + $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) @@ -120,7 +120,6 @@ return function (ContainerConfigurator $c) { ->tag('foo'); $s->set('tagged_iterator', 'Bar') - ->public() ->args([tagged('foo')]); $s->alias('alias_for_foo', 'foo')->private()->public();