Repository: pheature-flags/pheature-flags (php)

Issue:
[Symfony-toggle] Split configuration in different methods for better readability
**Epic:** https://github.com/pheature-flags/pheature-flags/issues/97 Symfony Toggle
**Package:** https://github.com/pheature-flags/symfony-toggle 

**Description:**

Split configuration in different methods based on different config sections. See the example in  https://github.com/doctrine/DoctrineMongoDBBundle/blob/4.4.x/DependencyInjection/Configuration.php

Pull request description:
[#176] [Symfony-toggle] Split configuration in different methods for better readability
It closes #176 .

You'll see I've added unit tests to check valid and invalid user configurations. I've noticed that we're not setting the fields to define a `strategy_type` or a `segment_type` and also for the toggles' definition mandatory fields. i.e:

```yaml
pheature_flags:
      strategy_types:
           - { type: 'a_type' } # <--- Here this config needs the `factory_id` in order to instantiate the Strategy so the config should fail

```

We could add it using [this nomenclature](https://symfony.com/doc/current/components/config/definition.html#default-and-required-values). What do you think?

Patch:
diff --git a/packages/symfony-toggle/src/DependencyInjection/Configuration.php b/packages/symfony-toggle/src/DependencyInjection/Configuration.php
index 95d5f14..53dce19 100644
--- a/packages/symfony-toggle/src/DependencyInjection/Configuration.php
+++ b/packages/symfony-toggle/src/DependencyInjection/Configuration.php
@@ -26,38 +26,67 @@ final class Configuration implements ConfigurationInterface
             ->scalarNode('prefix')
                 ->defaultValue('')
             ->end()
-            ->arrayNode('strategy_types')
-                ->arrayPrototype()
-                    ->children()
-                        ->scalarNode('type')->end()
-                        ->scalarNode('factory_id')->end()
+        ->end();
+
+        $this->addStrategyTypes($rootNode);
+        $this->addSegmentTypes($rootNode);
+        $this->addToggles($rootNode);
+
+        return $treeBuilder;
+    }
+
+    private function addStrategyTypes(ArrayNodeDefinition $rootNode): void
+    {
+        /** @phpstan-ignore-next-line */
+        $rootNode
+            ->children()
+                ->arrayNode('strategy_types')
+                    ->arrayPrototype()
+                        ->children()
+                            ->scalarNode('type')->end()
+                            ->scalarNode('factory_id')->end()
+                        ->end()
                     ->end()
-                ->end()
-            ->end()
-            ->arrayNode('segment_types')
-                ->arrayPrototype()
-                    ->children()
-                        ->scalarNode('type')->end()
-                        ->scalarNode('factory_id')->end()
+                ->end();
+    }
+
+    private function addSegmentTypes(ArrayNodeDefinition $rootNode): void
+    {
+        /** @phpstan-ignore-next-line */
+        $rootNode
+            ->children()
+                ->arrayNode('segment_types')
+                    ->arrayPrototype()
+                        ->children()
+                            ->scalarNode('type')->end()
+                            ->scalarNode('factory_id')->end()
+                        ->end()
                     ->end()
-                ->end()
-            ->end()
-            ->arrayNode('toggles')
-                ->arrayPrototype()
-                    ->children()
-                        ->scalarNode('id')->end()
-                        ->scalarNode('enabled')->end()
-                        ->arrayNode('strategies')
-                            ->arrayPrototype()
-                                ->children()
-                                    ->scalarNode('strategy_id')->end()
-                                    ->scalarNode('strategy_type')->end()
-                                    ->arrayNode('segments')
-                                        ->arrayPrototype()
-                                            ->children()
-                                                ->scalarNode('segment_id')->end()
-                                                ->scalarNode('segment_type')->end()
-                                                ->variableNode('criteria')->end()
+                ->end();
+    }
+
+    private function addToggles(ArrayNodeDefinition $rootNode): void
+    {
+        /** @phpstan-ignore-next-line */
+        $rootNode
+            ->children()
+                ->arrayNode('toggles')
+                    ->arrayPrototype()
+                        ->children()
+                            ->scalarNode('id')->end()
+                            ->scalarNode('enabled')->end()
+                            ->arrayNode('strategies')
+                                ->arrayPrototype()
+                                    ->children()
+                                        ->scalarNode('strategy_id')->end()
+                                        ->scalarNode('strategy_type')->end()
+                                        ->arrayNode('segments')
+                                            ->arrayPrototype()
+                                                ->children()
+                                                    ->scalarNode('segment_id')->end()
+                                                    ->scalarNode('segment_type')->end()
+                                                    ->variableNode('criteria')->end()
+                                                ->end()
                                             ->end()
                                         ->end()
                                     ->end()
@@ -65,10 +94,6 @@ final class Configuration implements ConfigurationInterface
                             ->end()
                         ->end()
                     ->end()
-                ->end()
-            ->end()
-        ->end();
-
-        return $treeBuilder;
+                ->end();
     }
 }


Test patch:
diff --git a/packages/symfony-toggle/test/DependencyInjection/ConfigurationTest.php b/packages/symfony-toggle/test/DependencyInjection/ConfigurationTest.php
new file mode 100644
index 0000000..1f6c8b4
--- /dev/null
+++ b/packages/symfony-toggle/test/DependencyInjection/ConfigurationTest.php
@@ -0,0 +1,261 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pheature\Test\Community\Symfony\DependencyInjection;
+
+use Generator;
+use Pheature\Community\Symfony\DependencyInjection\Configuration;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
+use Symfony\Component\Config\Definition\Processor;
+
+class ConfigurationTest extends TestCase
+{
+    /** @dataProvider validConfigurations */
+    public function testItShouldReturnAConfiguration(array $actualConfig, array $expectedConfig): void
+    {
+        $processor = new Processor();
+        $configuration = new Configuration();
+        $actualConfiguration = $processor->processConfiguration($configuration, $actualConfig);
+
+        self::assertEquals($expectedConfig, $actualConfiguration);
+    }
+
+    /** @dataProvider invalidConfigurations */
+    public function testItShouldThrowAnExceptionForAnInvalidConfiguration(array $actualConfig): void
+    {
+        $processor = new Processor();
+        $configuration = new Configuration();
+
+        $this->expectException(InvalidConfigurationException::class);
+        $processor->processConfiguration($configuration, $actualConfig);
+    }
+
+    public function validConfigurations(): Generator
+    {
+        yield 'user does not define any config' => [
+            'user config' => [],
+            'expected config' => [
+                'prefix' => '',
+                'strategy_types' => [],
+                'segment_types' => [],
+                'toggles' => [],
+            ]
+        ];
+
+        yield 'user defines only the prefix' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'prefix' => 'myapp',
+                ],
+            ],
+            'expected config' => [
+                'prefix' => 'myapp',
+                'strategy_types' => [],
+                'segment_types' => [],
+                'toggles' => [],
+            ]
+        ];
+
+        yield 'user defines a driver' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'driver' => 'dbal',
+                ],
+            ],
+            'expected config' => [
+                'prefix' => '',
+                'driver' => 'dbal',
+                'strategy_types' => [],
+                'segment_types' => [],
+                'toggles' => [],
+            ]
+        ];
+
+        yield 'user define a strategy type' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'strategy_types' => [
+                        [
+                            'type' => 'my_strategy_type',
+                            'factory_id' => "\A\Factory\Id",
+                        ]
+                    ],
+                ],
+            ],
+            'expected config' => [
+                'prefix' => '',
+                'strategy_types' => [
+                    [
+                        'type' => 'my_strategy_type',
+                        'factory_id' => "\A\Factory\Id",
+                    ]
+                ],
+                'segment_types' => [],
+                'toggles' => [],
+            ]
+        ];
+
+        yield 'user defines a segment type' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'segment_types' => [
+                        [
+                            'type' => 'my_segment_type',
+                            'factory_id' => "\A\Factory\Id",
+                        ]
+                    ],
+                ],
+            ],
+            'expected config' => [
+                'prefix' => '',
+                'strategy_types' => [],
+                'segment_types' => [
+                    [
+                        'type' => 'my_segment_type',
+                        'factory_id' => "\A\Factory\Id",
+                    ]
+                ],
+                'toggles' => [],
+            ]
+        ];
+
+        yield 'user defines a complete toggle' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'toggles' => [
+                        [
+                            'id' => 'my_feature',
+                            'enabled' => true,
+                            'strategies' => [
+                                [
+                                    'strategy_id' => 'a_strategy_id',
+                                    'strategy_type' => 'a_strategy_type',
+                                    'segments' => [
+                                        [
+                                            'segment_id' => 'a_segment_id',
+                                            'segment_type' => 'a_segment_type',
+                                            'criteria' => [
+                                                'some' => 'criterion'
+                                            ]
+                                        ]
+                                    ]
+                                ]
+                            ]
+                        ]
+                    ],
+                ],
+            ],
+            'expected config' => [
+                'prefix' => '',
+                'strategy_types' => [],
+                'segment_types' => [],
+                'toggles' => [
+                    [
+                        'id' => 'my_feature',
+                        'enabled' => true,
+                        'strategies' => [
+                            [
+                                'strategy_id' => 'a_strategy_id',
+                                'strategy_type' => 'a_strategy_type',
+                                'segments' => [
+                                    [
+                                        'segment_id' => 'a_segment_id',
+                                        'segment_type' => 'a_segment_type',
+                                        'criteria' => [
+                                            'some' => 'criterion'
+                                        ]
+                                    ]
+                                ]
+                            ]
+                        ]
+                    ]
+                ],
+            ]
+        ];
+    }
+
+    public function invalidConfigurations(): Generator
+    {
+        yield 'user defines an invalid driver' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'driver' => 'invalid_driver'
+                ],
+            ]
+        ];
+
+        yield 'user defines a strategy_type with a wrong field of the strategy' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'strategy_types' => [
+                        [
+                            'wrong_key' => 'a_factory_id',
+                        ]
+                    ],
+                ],
+            ]
+        ];
+
+        yield 'user defines a segment_types with a wrong field of the segment' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'segment_types' => [
+                        [
+                            'wrong_key' => 'a_factory_id',
+                        ]
+                    ],
+                ],
+            ]
+        ];
+
+        yield 'user defines a toggle with a wrong field in the base toggle config' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'toggles' => [
+                        [
+                            'wrong field' => 'a_value'
+                        ]
+                    ],
+                ],
+            ]
+        ];
+
+        yield 'user defines a toggle with a wrong field in the toggle\'s strategy' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'toggles' => [
+                        [
+                            'strategies' => [
+                                [
+                                    'wrong field' => 'a_value'
+                                ]
+                            ]
+                        ]
+                    ],
+                ],
+            ]
+        ];
+
+        yield 'user defines a toggle with a wrong field in toggle\'s strategy segment' => [
+            'user config' => [
+                'pheature_flags' => [
+                    'toggles' => [
+                        [
+                            'strategies' => [
+                                [
+                                    'segments' => [
+                                        [
+                                            'wrong field' => 'a_value'
+                                        ]
+                                    ]
+                                ]
+                            ]
+                        ]
+                    ],
+                ],
+            ]
+        ];
+    }
+}