Fixup #53 (#55)

teamneusta · Nov 12, 2023 · 0c597ab · 0c597ab
1 parent 8fdac78
commit 0c597ab
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 93 deletions.
diff --git a/config/packages/neusta_converter.yaml b/config/packages/neusta_converter.yaml
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -5,7 +5,7 @@
 namespace Neusta\ConverterBundle\DependencyInjection;
 
 use Neusta\ConverterBundle\Converter\GenericConverter;
-use Neusta\ConverterBundle\Populator;
+use Neusta\ConverterBundle\Populator\ConvertingPopulator;
 use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -92,15 +92,15 @@ private function addPopulatorSection(ArrayNodeDefinition $rootNode): void
     {
         $rootNode
             ->children()
-                ->arrayNode('populators')
+                ->arrayNode('populator')
                     ->info('Populator configuration')
                     ->normalizeKeys(false)
                     ->useAttributeAsKey('name')
                     ->arrayPrototype()
                         ->children()
-                            ->scalarNode('class')
+                            ->scalarNode('populator')
                                 ->info('class of the "Populator" implementation')
-                                ->defaultValue(Populator\ConvertingPopulator::class)
+                                ->defaultValue(ConvertingPopulator::class)
                             ->end()
                             ->scalarNode('converter')
                                 ->info('Service id of the internal "Converter"')
@@ -112,13 +112,12 @@ private function addPopulatorSection(ArrayNodeDefinition $rootNode): void
                                 ->normalizeKeys(false)
                                 ->useAttributeAsKey('target')
                                 ->prototype('scalar')
-                                ->isRequired()
                             ->end()
                         ->end()
                     ->end()
                     ->validate()
-                        ->ifTrue(fn (array $c) => empty($c['converter']) && empty($c['property']))
-                        ->thenInvalid('A "converter" and "property" must be defined.')
+                        ->ifTrue(fn (array $c) => empty($c['property']))
+                        ->thenInvalid('At least one "property" must be defined.')
                     ->end()
                 ->end()
             ->end()

diff --git a/src/DependencyInjection/NeustaConverterExtension.php b/src/DependencyInjection/NeustaConverterExtension.php
@@ -30,7 +30,7 @@ public function loadInternal(array $mergedConfig, ContainerBuilder $container):
             $this->registerConverterConfiguration($converterId, $converter, $container);
         }
 
-        foreach ($mergedConfig['populators'] as $populatorId => $populator) {
+        foreach ($mergedConfig['populator'] as $populatorId => $populator) {
             $this->registerPopulatorConfiguration($populatorId, $populator, $container);
         }
     }
@@ -80,56 +80,30 @@ private function registerConverterConfiguration(string $id, array $config, Conta
      */
     private function registerPopulatorConfiguration(string $id, array $config, ContainerBuilder $container): void
     {
-        $arguments = [];
-        if (empty($config['class']) || ConvertingPopulator::class === $config['class']) {
-            $arguments = $this->buildArgumentsForConvertingPopulator($config);
-        } elseif (ArrayConvertingPopulator::class === $config['class']) {
-            $arguments = $this->buildArgumentsForArrayConvertingPopulator($config);
-        }
-
-        $container->register($id, $config['class'])
-            ->setPublic(true)
-            ->setArguments($arguments);
-    }
-
-    /**
-     * @param array<string, mixed> $config
-     *
-     * @return array<string, string>
-     */
-    private function buildArgumentsForConvertingPopulator(array $config): array
-    {
-        $targetProperty = array_key_first($config['property']);
-        $sourceProperty = $config['property'][$targetProperty];
-        $sourceProperty = $sourceProperty ?? $targetProperty;
-
-        return
-            [
-                '$converter' => new TypedReference($config['converter'], Converter::class),
-                '$sourcePropertyName' => $sourceProperty,
-                '$targetPropertyName' => $targetProperty,
-                '$accessor' => new Reference('property_accessor'),
-            ];
-    }
-
-    /**
-     * @param array<string, mixed> $config
-     *
-     * @return array<string, string>
-     */
-    private function buildArgumentsForArrayConvertingPopulator(array $config): array
-    {
-        $innerPropertyArgument = [];
-        $innerProperty = $config['property']['itemProperty'];
-        $innerPropertyArgument['$sourceArrayItemPropertyName'] = $innerProperty;
+        $itemProperty = $config['property']['itemProperty'] ?? null;
         unset($config['property']['itemProperty']);
 
-        $arguments = array_merge(
-            $innerPropertyArgument,
-            $this->buildArgumentsForConvertingPopulator($config),
-        );
+        $targetProperty = array_key_first($config['property']);
+        $sourceProperty = $config['property'][$targetProperty] ?? $targetProperty;
 
-        return $arguments;
+        $container->register($id, $config['populator'])
+            ->setPublic(true)
+            ->setArguments(match ($config['populator']) {
+                ConvertingPopulator::class => [
+                    '$converter' => new TypedReference($config['converter'], Converter::class),
+                    '$sourcePropertyName' => $sourceProperty,
+                    '$targetPropertyName' => $targetProperty,
+                    '$accessor' => new Reference('property_accessor'),
+                ],
+                ArrayConvertingPopulator::class => [
+                    '$converter' => new TypedReference($config['converter'], Converter::class),
+                    '$sourceArrayPropertyName' => $sourceProperty,
+                    '$targetPropertyName' => $targetProperty,
+                    '$sourceArrayItemPropertyName' => $itemProperty,
+                    '$accessor' => new Reference('property_accessor'),
+                ],
+                default => [],
+            });
     }
 
     private function appendSuffix(string $value, string $suffix): string

diff --git a/tests/DependencyInjection/NeustaConverterExtensionTest.php b/tests/DependencyInjection/NeustaConverterExtensionTest.php
@@ -139,7 +139,7 @@ public function test_with_mapped_context(): void
     public function test_with_converting_populator(): void
     {
         $container = $this->buildContainer([
-            'populators' => [
+            'populator' => [
                 'foobar' => [
                     'converter' => GenericConverter::class,
                     'property' => [
@@ -155,15 +155,17 @@ public function test_with_converting_populator(): void
         self::assertSame(ConvertingPopulator::class, $populator->getClass());
         self::assertTrue($populator->isPublic());
         self::assertInstanceOf(TypedReference::class, $populator->getArgument('$converter'));
+        self::assertSame(GenericConverter::class, (string) $populator->getArgument('$converter'));
         self::assertSame('targetTest', $populator->getArgument('$targetPropertyName'));
         self::assertSame('sourceTest', $populator->getArgument('$sourcePropertyName'));
     }
 
     public function test_with_array_converting_populator(): void
     {
         $container = $this->buildContainer([
-            'populators' => [
+            'populator' => [
                 'foobar' => [
+                    'populator' => ArrayConvertingPopulator::class,
                     'converter' => GenericConverter::class,
                     'property' => [
                         'targetTest' => 'sourceTest',
@@ -175,19 +177,20 @@ public function test_with_array_converting_populator(): void
         // converter
         $populator = $container->getDefinition('foobar');
 
-        self::assertSame(ConvertingPopulator::class, $populator->getClass());
+        self::assertSame(ArrayConvertingPopulator::class, $populator->getClass());
         self::assertTrue($populator->isPublic());
         self::assertInstanceOf(TypedReference::class, $populator->getArgument('$converter'));
+        self::assertSame(GenericConverter::class, (string) $populator->getArgument('$converter'));
         self::assertSame('targetTest', $populator->getArgument('$targetPropertyName'));
-        self::assertSame('sourceTest', $populator->getArgument('$sourcePropertyName'));
+        self::assertSame('sourceTest', $populator->getArgument('$sourceArrayPropertyName'));
     }
 
     public function test_with_array_converting_populator_with_inner_property(): void
     {
         $container = $this->buildContainer([
-            'populators' => [
+            'populator' => [
                 'foobar' => [
-                    'class' => ArrayConvertingPopulator::class,
+                    'populator' => ArrayConvertingPopulator::class,
                     'converter' => GenericConverter::class,
                     'property' => [
                         'targetTest' => 'sourceTest',
@@ -207,9 +210,9 @@ public function test_with_array_converting_populator_with_inner_property(): void
     public function test_with_array_converting_populator_with_inner_property_same_name(): void
     {
         $container = $this->buildContainer([
-            'populators' => [
+            'populator' => [
                 'foobar' => [
-                    'class' => ArrayConvertingPopulator::class,
+                    'populator' => ArrayConvertingPopulator::class,
                     'converter' => GenericConverter::class,
                     'property' => [
                         'test' => null, // in yaml one will write ~
@@ -224,16 +227,16 @@ public function test_with_array_converting_populator_with_inner_property_same_na
 
         self::assertSame(ArrayConvertingPopulator::class, $populator->getClass());
         self::assertSame('test', $populator->getArgument('$targetPropertyName'));
-        self::assertSame('test', $populator->getArgument('$sourcePropertyName'));
+        self::assertSame('test', $populator->getArgument('$sourceArrayPropertyName'));
         self::assertSame('value', $populator->getArgument('$sourceArrayItemPropertyName'));
     }
 
     public function test_with_array_converting_populator_with_inner_property_first(): void
     {
         $container = $this->buildContainer([
-            'populators' => [
+            'populator' => [
                 'foobar' => [
-                    'class' => ArrayConvertingPopulator::class,
+                    'populator' => ArrayConvertingPopulator::class,
                     'converter' => GenericConverter::class,
                     'property' => [
                         'itemProperty' => 'value',
@@ -248,7 +251,7 @@ public function test_with_array_converting_populator_with_inner_property_first()
 
         self::assertSame(ArrayConvertingPopulator::class, $populator->getClass());
         self::assertSame('targetTest', $populator->getArgument('$targetPropertyName'));
-        self::assertSame('sourceTest', $populator->getArgument('$sourcePropertyName'));
+        self::assertSame('sourceTest', $populator->getArgument('$sourceArrayPropertyName'));
         self::assertSame('value', $populator->getArgument('$sourceArrayItemPropertyName'));
     }
 

diff --git a/tests/app/config/packages/neusta_converter.yaml b/tests/app/config/packages/neusta_converter.yaml
@@ -17,7 +17,7 @@ neusta_converter:
       properties:
         phoneNumber: number
 
-  populators:
+  populator:
     test.person.address.populator:
       converter: test.address.converter
       property: