Add GenericTargetFactory and related configuration (#33)

Co-authored-by: Jacob Dreesen <j.dreesen@neusta.de>
Co-authored-by: Michael Albrecht <m.albrecht@neusta.de>

docs/usage.md

@@ -1,7 +1,6 @@
 ## Usage
 
-After the bundle is activated, you can directly use it by implementing a factory and a populator for your target and
-source types.
+After the bundle is activated, you can directly use it by implementing populators for your target and source types.
 
 Imagine your source type is `User`:
 
@@ -41,31 +40,6 @@ Separation of Concerns.
 
 You should use the Converter-and-Populator-pattern. But how?!
 
-Implement the following three artifacts:
-
-### Factory
-
-Implement a comfortable factory for your target type:
-
-```php
-use Neusta\ConverterBundle\Converter\Context\GenericContext;
-use Neusta\ConverterBundle\TargetFactory;
-
-/**
- * @implements TargetFactory<Person, GenericContext>
- */
-class PersonFactory implements TargetFactory
-{
-    public function create(?object $ctx = null): Person
-    {
-        return new Person();
-    }
-}
-```
-
-Skip thinking about the converter context at the moment. It will help you...
-maybe not now but in a few weeks. You will see.
-
 ### Populators
 
 Implement one or several populators:
@@ -91,14 +65,37 @@ As you can see, implementation here is quite simple - just concatenation of two
 But however transformation will become more and more complex, it should be done in a testable,
 separated Populator or in several of them.
 
+Skip thinking about the converter context at the moment. It will help you...
+maybe not now but in a few weeks. You will see.
+
 ### Configuration
 
-To put things together, register the factory and populator as services:
+First register the populator as a service:
+
+```yaml
+# config/services.yaml
+services:
+  YourNamespace\PersonNamePopulator: ~
+```
+
+Then declare the following converter in your package config:
+
+```yaml
+# config/packages/neusta_converter.yaml
+neusta_converter:
+  converter:
+    person.converter:
+      target: YourNamespace\Person
+      populators:
+        - YourNamespace\PersonNamePopulator
+        # additional populators may follow
+```
+
+To put things together, register the populator as services:
 
 ```yaml
 # config/services.yaml
 services:
-  YourNamespace\PersonFactory: ~
   YourNamespace\PersonNamePopulator: ~
 ```
 
@@ -109,7 +106,7 @@ And then declare the following converter in your package config:
 neusta_converter:
   converter:
     person.converter:
-      target_factory: YourNamespace\PersonFactory
+      target: YourNamespace\Person
       populators:
         - YourNamespace\PersonNamePopulator
         # additional populators may follow
@@ -118,6 +115,9 @@ neusta_converter:
 > Note: You can use a custom implementation of the `Converter` interface via the `converter` keyword.
 > Its constructor must contain the two parameters `TargetFactory $factory` and `array $populators`.
 
+> Note: You can use a custom implementation of the `TargetTypeFactory` interface via the `target_factory` keyword,
+> if you have special needs when creating the target object.
+
 #### Mapping properties
 
 If you just want to map a single property from the source to the target without transforming it in between, you don't
@@ -131,7 +131,7 @@ You can use it in your converter config via the `properties` keyword:
 neusta_converter:
   converter:
     person.converter:
-      # ...
+      target: YourNamespace\Person
       properties:
         email: ~
         phoneNumber: phone
@@ -159,7 +159,7 @@ neusta_converter:
     converter:
       person.converter:
         properties:
-          # ...
+          target: YourNamespace\Person
           phoneNumber:
             source: phone
             default: '0123456789'
@@ -181,7 +181,7 @@ You can use it in your converter config via the `context` keyword:
 neusta_converter:
   converter:
     person.converter:
-      # ...
+      target: YourNamespace\Person
       context:
         group: ~
         locale: language

src/DependencyInjection/Configuration.php

@@ -40,10 +40,15 @@ private function addConverterSection(ArrayNodeDefinition $rootNode): void
                                 ->info('Class name of the "Converter" implementation')
                                 ->defaultValue(GenericConverter::class)
                             ->end()
+                            ->scalarNode('target')
+                                ->info('Class name of the target')
+                                ->validate()
+                                    ->ifTrue(fn ($v) => !class_exists($v))
+                                    ->thenInvalid('The target type %s does not exist.')
+                                ->end()
+                            ->end()
                             ->scalarNode('target_factory')
                                 ->info('Service id of the "TargetFactory"')
-                                ->isRequired()
-                                ->cannotBeEmpty()
                             ->end()
                             ->arrayNode('populators')
                                 ->info('Service ids of the "Populator"s')
@@ -82,6 +87,14 @@ private function addConverterSection(ArrayNodeDefinition $rootNode): void
                                 ->prototype('scalar')->end()
                             ->end()
                         ->end()
+                        ->validate()
+                            ->ifTrue(fn (array $c) => !isset($c['target']) && !isset($c['target_factory']))
+                            ->thenInvalid('Either "target" or "target_factory" must be defined.')
+                        ->end()
+                        ->validate()
+                            ->ifTrue(fn (array $c) => isset($c['target'], $c['target_factory']))
+                            ->thenInvalid('Either "target" or "target_factory" must be defined, but not both.')
+                        ->end()
                         ->validate()
                             ->ifTrue(fn (array $c) => empty($c['populators']) && empty($c['properties']) && empty($c['context']))
                             ->thenInvalid('At least one "populator", "property" or "context" must be defined.')

src/DependencyInjection/NeustaConverterExtension.php

@@ -9,6 +9,7 @@
 use Neusta\ConverterBundle\Populator\ContextMappingPopulator;
 use Neusta\ConverterBundle\Populator\ConvertingPopulator;
 use Neusta\ConverterBundle\Populator\PropertyMappingPopulator;
+use Neusta\ConverterBundle\Target\GenericTargetFactory;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -41,6 +42,12 @@ public function loadInternal(array $mergedConfig, ContainerBuilder $container):
      */
     private function registerConverterConfiguration(string $id, array $config, ContainerBuilder $container): void
     {
+        $targetFactoryId = $config['target_factory'] ?? "{$id}.target_factory";
+        if (!isset($config['target_factory'])) {
+            $container->register($targetFactoryId, GenericTargetFactory::class)
+                ->setArgument('$type', $config['target']);
+        }
+
         foreach ($config['properties'] ?? [] as $targetProperty => $sourceConfig) {
             $skipNull = false;
             if (str_ends_with($targetProperty, '?')) {
@@ -74,7 +81,7 @@ private function registerConverterConfiguration(string $id, array $config, Conta
         $container->register($id, $config['converter'])
             ->setPublic(true)
             ->setArguments([
-                '$factory' => new Reference($config['target_factory']),
+                '$factory' => new Reference($targetFactoryId),
                 '$populators' => array_map(
                     static fn (string $populator) => new Reference($populator),
                     $config['populators'],

src/Target/GenericTargetFactory.php

@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Neusta\ConverterBundle\Target;
+
+use Neusta\ConverterBundle\TargetFactory;
+
+/**
+ * @template T of object
+ *
+ * @implements TargetFactory<T, object|null>
+ */
+final class GenericTargetFactory implements TargetFactory
+{
+    /** @var \ReflectionClass<T> */
+    private \ReflectionClass $type;
+
+    /**
+     * @param class-string<T> $type
+     *
+     * @throws \ReflectionException
+     * @throws \InvalidArgumentException
+     */
+    public function __construct(string $type)
+    {
+        $this->type = new \ReflectionClass($type);
+
+        if (!$this->type->isInstantiable()) {
+            throw new \InvalidArgumentException(sprintf('Target class "%s" is not instantiable.', $type));
+        }
+
+        if ($this->type->getConstructor()?->getNumberOfRequiredParameters()) {
+            throw new \InvalidArgumentException(sprintf('Target class "%s" has required constructor parameters.', $type));
+        }
+    }
+
+    /**
+     * @throws \LogicException
+     */
+    public function create(?object $ctx = null): object
+    {
+        try {
+            return $this->type->newInstance();
+        } catch (\ReflectionException $e) {
+            throw new \LogicException(sprintf('Cannot create new instance of "%s" because: %s', $this->type->getName(), $e->getMessage()), 0, $e);
+        }
+    }
+}

tests/DependencyInjection/NeustaConverterExtensionTest.php

@@ -12,8 +12,11 @@
 use Neusta\ConverterBundle\Populator\ContextMappingPopulator;
 use Neusta\ConverterBundle\Populator\ConvertingPopulator;
 use Neusta\ConverterBundle\Populator\PropertyMappingPopulator;
+use Neusta\ConverterBundle\Target\GenericTargetFactory;
 use Neusta\ConverterBundle\Tests\Fixtures\Model\Target\Factory\PersonFactory;
+use Neusta\ConverterBundle\Tests\Fixtures\Model\Target\Person;
 use Neusta\ConverterBundle\Tests\Fixtures\Populator\PersonNamePopulator;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\TypedReference;
 
@@ -45,6 +48,77 @@ public function test_with_generic_converter(): void
         $this->assertContainerBuilderHasServiceDefinitionWithArgument('foobar', '$populators', [new Reference(PersonNamePopulator::class)]);
     }
 
+    public function test_with_generic_target_factory(): void
+    {
+        $this->load([
+            'converter' => [
+                'foobar' => [
+                    'target' => Person::class,
+                    'populators' => [
+                        PersonNamePopulator::class,
+                    ],
+                ],
+            ],
+        ]);
+
+        // converter
+        $this->assertContainerBuilderHasPublicService('foobar', GenericConverter::class);
+        $this->assertContainerBuilderHasService('foobar.target_factory', GenericTargetFactory::class);
+        $this->assertContainerBuilderHasServiceDefinitionWithArgument('foobar', '$factory', new Reference('foobar.target_factory'));
+        $this->assertContainerBuilderHasServiceDefinitionWithArgument('foobar.target_factory', '$type', Person::class);
+    }
+
+    public function test_with_generic_target_factory_for_unknown_type(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('The target type "UnknownClass" does not exist.');
+
+        $this->load([
+            'converter' => [
+                'foobar' => [
+                    'target' => 'UnknownClass',
+                    'populators' => [
+                        PersonNamePopulator::class,
+                    ],
+                ],
+            ],
+        ]);
+    }
+
+    public function test_without_target_and_target_factory(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('Either "target" or "target_factory" must be defined.');
+
+        $this->load([
+            'converter' => [
+                'foobar' => [
+                    'populators' => [
+                        PersonNamePopulator::class,
+                    ],
+                ],
+            ],
+        ]);
+    }
+
+    public function test_with_target_and_target_factory(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('Either "target" or "target_factory" must be defined, but not both.');
+
+        $this->load([
+            'converter' => [
+                'foobar' => [
+                    'target' => Person::class,
+                    'target_factory' => PersonFactory::class,
+                    'populators' => [
+                        PersonNamePopulator::class,
+                    ],
+                ],
+            ],
+        ]);
+    }
+
     public function test_with_mapped_properties(): void
     {
         $this->load([

tests/Fixtures/Config/person.yaml

@@ -1,15 +1,15 @@
 neusta_converter:
   converter:
     test.person.converter:
-      converter: Neusta\ConverterBundle\Converter\GenericConverter
-      target_factory: Neusta\ConverterBundle\Tests\Fixtures\Model\Target\Factory\PersonFactory
+      target: Neusta\ConverterBundle\Tests\Fixtures\Model\Target\Person
       context:
         group: ~ # same property name
         locale: language # different property names
       populators:
         - Neusta\ConverterBundle\Tests\Fixtures\Populator\PersonNamePopulator
 
     test.person.converter.extended:
+      converter: Neusta\ConverterBundle\Converter\GenericConverter
       target_factory: Neusta\ConverterBundle\Tests\Fixtures\Model\Target\Factory\PersonWithDefaultsFactory
       properties:
         fullName: