Fix - Symfony - load custom strategies in db_tools.storage service (#216)

Author: SimonMellerin

docs/content/configuration/reference.md

@@ -623,19 +623,12 @@ db_tools:
 ```
 @@@
 @@@ standalone docker
-```yaml
-# Default value, `default` is an alias of `datetime`.
-storage_filename_strategy: default
-
-# Explicit default.
-storage_filename_strategy: datetime
-
-# Using a service name.
-storage_filename_strategy: app.my_filename_strategy
+:::warning
+There is as of now no way to implement a custom filename strategy when using
+*DbToolsBundle* as a standalone CLI tool or with the Docker image.
 
-# Using a class name.
-storage_filename_strategy: App\DbTools\Storage\MyCustomFilenameStrategy
-```
+If you need this feature, please let us know by [creating an issue](https://github.com/makinacorpus/DbToolsBundle/issues).
+:::
 @@@
 
 :::tip

src/Bridge/Laravel/DbToolsServiceProvider.php

@@ -110,7 +110,7 @@ public function register(): void
             foreach ($connections as $connectionName => $connection) {
                 $strategyId = $connection->getStorageFilenameStrategy();
 
-                if ($strategyId === null || $strategyId === 'default' || $strategyId === 'datetime') {
+                if ($strategyId === 'default' || $strategyId === 'datetime') {
                     $strategy = new DefaultFilenameStrategy();
                 } elseif ($app->bound($strategyId)) {
                     $strategy = $app->make($strategyId);

src/Bridge/Symfony/DependencyInjection/DbToolsExtension.php

@@ -85,27 +85,28 @@ public function load(array $configs, ContainerBuilder $container): void
         $container->setParameter('db_tools.anonymization.anonymizer.paths', $anonymizerPaths);
 
         // Register filename strategies.
-        $strategies = [];
-        foreach (($config['storage']['filename_strategy'] ?? []) as $connectionName => $strategyId) {
+        $strategyServices = [];
+        foreach ($config['connections'] as $connectionName => $connectionConfig) {
             // Default is handled directly by the storage service.
+            $strategyId = $connectionConfig['storage_filename_strategy'] ?? null;
             if ($strategyId !== null && $strategyId !== 'default' && $strategyId !== 'datetime') {
                 if ($container->hasDefinition($strategyId)) {
-                    $strategies[$connectionName] = new Reference($strategyId);
+                    $strategyServices[$connectionName] = new Reference($strategyId);
                 } elseif (\class_exists($strategyId)) {
                     if (!\is_subclass_of($strategyId, FilenameStrategyInterface::class)) {
                         throw new InvalidArgumentException(\sprintf('"db_tools.connections.%s.filename_strategy": class "%s" does not implement "%s"', $connectionName, $strategyId, FilenameStrategyInterface::class));
                     }
                     $serviceId = '.db_tools.filename_strategy.' . \sha1($strategyId);
                     $container->setDefinition($serviceId, (new Definition())->setClass($strategyId));
-                    $strategies[$connectionName] = new Reference($serviceId);
+                    $strategyServices[$connectionName] = new Reference($serviceId);
                 } else {
                     throw new InvalidArgumentException(\sprintf('"db_tools.connections.%s.filename_strategy": class or service "%s" does not exist or is not registered in container', $connectionName, $strategyId));
                 }
                 break;
             }
         }
-        if ($strategies) {
-            $container->getDefinition('db_tools.storage')->setArgument(1, $strategies);
+        if ($strategyServices) {
+            $container->getDefinition('db_tools.storage')->setArgument(1, $strategyServices);
         }
     }
 

src/Configuration/ConfigurationRegistry.php

@@ -31,6 +31,9 @@ public function getDefaultConnection(): ?string
         return null;
     }
 
+    /**
+     * @return array<string,Configuration>
+     */
     public function getConnectionConfigAll(): array
     {
         return $this->connections;

tests/Resources/FilenameStrategy/CustomFilenameStrategy.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace MakinaCorpus\DbToolsBundle\Tests\Resources\FilenameStrategy;
+
+use MakinaCorpus\DbToolsBundle\Storage\AbstractFilenameStrategy;
+
+class CustomFilenameStrategy extends AbstractFilenameStrategy
+{
+    #[\Override]
+    public function generateFilename(
+        string $connectionName = 'default',
+        string $extension = 'sql',
+        bool $anonymized = false
+    ): string {
+        $now = new \DateTimeImmutable();
+
+        return \sprintf(
+            '%s/custom%s-%s.%s',
+            $connectionName,
+            $anonymized ? '-anonymized' : '',
+            $now->format('YmdHis'),
+            $extension
+        );
+    }
+}

tests/Unit/Bridge/Symfony/DependencyInjection/DbToolsExtensionTest.php

@@ -4,13 +4,14 @@
 
 namespace MakinaCorpus\DbToolsBundle\Tests\Unit\Bridge\Symfony\DependencyInjection;
 
-use MakinaCorpus\DbToolsBundle\Anonymization\AnonymizatorFactory;
 use MakinaCorpus\DbToolsBundle\Bridge\Symfony\DependencyInjection\DbToolsExtension;
 use MakinaCorpus\DbToolsBundle\Configuration\ConfigurationRegistry;
+use MakinaCorpus\DbToolsBundle\Tests\Resources\FilenameStrategy\CustomFilenameStrategy;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Framework\Attributes\After;
 use PHPUnit\Framework\Attributes\DependsExternal;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
 
 class DbToolsExtensionTest extends TestCase
@@ -31,10 +32,11 @@ private function getContainer(array $parameters = [], array $bundles = []): Cont
         return $container;
     }
 
-    private function testExtension(array $config): void
+    private function testExtension(array $config, ContainerBuilder $container = null): void
     {
+        $container ??= $this->getContainer();
         $extension = new DbToolsExtension();
-        $extension->load([$config], $container = $this->getContainer());
+        $extension->load([$config], $container);
 
         // No need to test them all, simply validate the config was loaded.
         self::assertTrue($container->hasDefinition('db_tools.storage'));
@@ -95,7 +97,9 @@ public function testEnvVarAreOverridenByConfiguration(array $config): void
         $this->setAllDbToolsEnv();
 
         $extension = new DbToolsExtension();
-        $extension->load([$config], $container = $this->getContainer());
+        $container = $this->getContainer();
+        $container->setDefinition('one_strategy', (new Definition())->setClass(CustomFilenameStrategy::class));
+        $extension->load([$config], $container);
         $container->getDefinition('db_tools.configuration.registry')->setPublic(true);
         $container->compile(true);
 
@@ -147,7 +151,9 @@ public function testConnectionResolveParentForNonSetValues(array $config): void
         $this->setAllDbToolsEnv();
 
         $extension = new DbToolsExtension();
-        $extension->load([$config], $container = $this->getContainer());
+        $container = $this->getContainer();
+        $container->setDefinition('one_strategy', (new Definition())->setClass(CustomFilenameStrategy::class));
+        $extension->load([$config], $container);
         $container->getDefinition('db_tools.configuration.registry')->setPublic(true);
         $container->compile(true);
 
@@ -215,7 +221,9 @@ public function testExtensionWithMinimalConfig(array $config): void
     #[DependsExternal(DbToolsConfigurationTest::class, 'testConfigurationFull')]
     public function testExtensionWithFullConfig(array $config): void
     {
-        $this->testExtension($config);
+        $container = $this->getContainer();
+        $container->setDefinition('one_strategy', (new Definition())->setClass(CustomFilenameStrategy::class));
+        $this->testExtension($config, $container);
     }
 
     #[DependsExternal(DbToolsConfigurationTest::class, 'testConfigurationFull')]