makinacorpus
Symfony - `storage_filename_strategy` configuration not evaluated
The Symfony integration as of version 2.0 of this bundle does not evaluate the storage_filename_strategy and connections.<connection_name>.storage.filename_strategy configuration.
The service db_tools.storage will be configured with strategies resolved from the deprecated configuration path db_tools.storage.filename_strategy at https://github.com/makinacorpus/DbToolsBundle/blob/d0cb6322305a9a050917b70a2392fe31f4d83142/src/Bridge/Symfony/DependencyInjection/DbToolsExtension.php#L88-L109
Due to the initialization of the configuration object at https://github.com/makinacorpus/DbToolsBundle/blob/d0cb6322305a9a050917b70a2392fe31f4d83142/src/Bridge/Symfony/DependencyInjection/DbToolsExtension.php#L115 its not possible anymore to use this configuration path.
The configuration at db_tools.storage_filename_strategy and db_tools.connections.<connection_name>.storage_filename_strategy does not seem to be taken into account at all.
For anyone facing the same issue, as temporary a workaround, it is possible to update the definition of the service `db_tools.storage` through a custom compiler pass.
<?php
declare(strict_types=1);
namespace App\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Reference;
class DbToolsCompilerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
{
/**
* @var array<int, array{
* storage_filename_strategy?: 'default'|'datetime'|string,
* connections: array<string, array{
* storage_filename_strategy?: 'default'|'datetime'|string,
* }
* } $dbToolsConfigurations
*/
$dbToolsConfigurations = $container->getExtensionConfig('db_tools');
if (count($dbToolsConfigurations) > 1) {
throw new LogicException('More than one db tool configuration defined.');
}
$dbToolsConfiguration = $dbToolsConfigurations[0];
if ([] === $dbToolsConfiguration) {
return;
}
$storageFilenameStrategies = [];
$defaultStorageFilenameStrategy = $this->retrieveStorageFilenameStrategyFromConfiguration(
$dbToolsConfiguration,
);
if (null !== $defaultStorageFilenameStrategy) {
$storageFilenameStrategies['default'] = $defaultStorageFilenameStrategy;
}
if (array_key_exists('connections', $dbToolsConfiguration)) {
foreach ($dbToolsConfiguration['connections'] as $connectionName => $connectionConfiguration) {
$storageFilenameStrategy = $this->retrieveStorageFilenameStrategyFromConfiguration(
$connectionConfiguration,
);
if (null === $storageFilenameStrategy) {
continue;
}
$storageFilenameStrategies[$connectionName] = $storageFilenameStrategy;
}
}
if ($storageFilenameStrategies === []) {
return;
}
$dbToolsStorageDefinition = $container->getDefinition('db_tools.storage');
$dbToolsStorageDefinition->replaceArgument(1, $storageFilenameStrategies);
}
/**
* @param array{storage_filename_strategy?: 'default'|'datetime'|string} $configuration
*
* @return Reference|null
*/
private function retrieveStorageFilenameStrategyFromConfiguration(array $configuration): ?Reference
{
if (!array_key_exists('storage_filename_strategy', $configuration)
|| in_array($configuration['storage_filename_strategy'], ['default', 'datetime'])
) {
return null;
}
return new Reference($configuration['storage_filename_strategy']);
}
}
Fixed and shipped in version 2.0.1.
@MaSpeng I let you confirm this is ok for you close this issue.
Just out of curiosity, what filename strategy do you implement? Do you think its generic enough to be added to the lib? or it's too specific to your use case? Could your use case have been covered by a generic pattern-based filename strategy?
@SimonMellerin Thanks for the new release, the new version is partially working:
The following configuration is working now: ✅
db_tools:
connections:
default:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
Whats still not working as expected is: ❌
db_tools:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
I would expect the top level configuration to be used for all available connections until i specify it explicitly for a connection, but it does not have any effect.
I would also expect that this configuration: ❌
db_tools:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
connections:
not_default_connection:
storage_filename_strategy: App\DbTools\Storage\AnotherCustomFilenameStrategy
would use the top most configured storage file name strategy for all connections except the override for the not_default_connection.
Our custom file name strategy is rather simple, we just need the backup to be stored as {filename}.{extension}, we actually want this backup to be overwritten by the next execution.
A pattern based filename strategy would solve this by using the available arguments of the \MakinaCorpus\DbToolsBundle\Storage\FilenameStrategyInterface::generateFilename function, for example {connectionName}.{extension}.
A more flexible approach using a set of predefined placeholder in conjunction with a pattern to be able to use sub directories would be more flexible indeed.
Thank you for your vigilance! Should be good with version 2.0.1
We may consider to add this kind of pattern based filename strategy. Seems a nice way to easily implement a simple custom strategy.
cheers
@SimonMellerin Thanks for the new release, the new version is partially working:
The following configuration is working, the connection specific configured strategy is used for the connection: ✅
db_tools:
connections:
default:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
The following configuration is also working, the connection specific configured strategy is used for the connection, the top level configured strategy is used for all other configured connections: ✅
db_tools:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
connections:
default:
...
not_default_connection:
storage_filename_strategy: App\DbTools\Storage\AnotherCustomFilenameStrategy
The following configuration is not working as expected: ❌
db_tools:
storage_filename_strategy: App\DbTools\Storage\CustomFilenameStrategy
I would expect this storage filename strategy to be applied for all auto discovered connections (default in most cases), but it is required to add a connection configuration to make it work as seen in the second working configuration example above.
This is happening as the fallback to the top most configured strategy is done inside the foreach loop at: https://github.com/makinacorpus/DbToolsBundle/blob/3b8d30549fa9b30d7a5bbf762361eac36731cf1f/src/Bridge/Symfony/DependencyInjection/DbToolsExtension.php#L91
As a result, when the developer is relying on the automatic discovery of the connections based on the configured doctrine connections, the top level strategy will not be applied to any of those.