Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing deprecated call to Configuration::setSchemaAssetsFilter() + extendable #935

Merged
merged 1 commit into from Apr 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions Dbal/RegexSchemaAssetFilter.php
@@ -0,0 +1,25 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Dbal;

use Doctrine\DBAL\Schema\AbstractAsset;

class RegexSchemaAssetFilter
{
/** @var string */
private $filterExpression;
weaverryan marked this conversation as resolved.
Show resolved Hide resolved

public function __construct(string $filterExpression)
{
$this->filterExpression = $filterExpression;
}

public function __invoke($assetName) : bool
{
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}

return preg_match($this->filterExpression, $assetName);
}
}
31 changes: 31 additions & 0 deletions Dbal/SchemaAssetsFilterManager.php
@@ -0,0 +1,31 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Dbal;

/**
* Manages schema filters passed to Connection::setSchemaAssetsFilter()
*/
class SchemaAssetsFilterManager
{
/** @var callable[] */
private $schemaAssetFilters;

/**
* @param callable[] $schemaAssetFilters
*/
public function __construct(array $schemaAssetFilters)
{
$this->schemaAssetFilters = $schemaAssetFilters;
}

public function __invoke($assetName) : bool
{
foreach ($this->schemaAssetFilters as $schemaAssetFilter) {
if ($schemaAssetFilter($assetName) === false) {
return false;
}
}

return true;
}
}
57 changes: 57 additions & 0 deletions DependencyInjection/Compiler/DbalSchemaFilterPass.php
@@ -0,0 +1,57 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler;

use Doctrine\DBAL\Configuration;
use LogicException;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;

/**
* Processes the doctrine.dbal.schema_filter
*/
class DbalSchemaFilterPass implements CompilerPassInterface
{
/**
* {@inheritDoc}
*/
public function process(ContainerBuilder $container)
{
$filters = $container->findTaggedServiceIds('doctrine.dbal.schema_filter');

if (count($filters) > 0 && ! method_exists(Configuration::class, 'setSchemaAssetsFilter')) {
throw new LogicException('The doctrine.dbal.schema_filter tag is only supported when using doctrine/dbal 2.9 or higher.');
}

$connectionFilters = [];
foreach ($filters as $id => $tagAttributes) {
foreach ($tagAttributes as $attributes) {
$name = isset($attributes['connection']) ? $attributes['connection'] : $container->getParameter('doctrine.default_connection');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should ignoring the name apply it to the default connection, or to all connections ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about this too. Making it apply to all connections is actually more useful - as, for example, Symfony's session system could (in theory) say: "Hey, ignore a session table that exits in any connection... I'm not sure which connection is used for the PDO instance I'm using".

But, that also seems like a possible, legitimate edge-case: I want to ignore session on connection 1, but I really DO have a session table on connection2 that I'm managing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Ryan here: users should know which connection handles which tables and can build their ignore lists accordingly.


if (! isset($connectionFilters[$name])) {
$connectionFilters[$name] = [];
}

$connectionFilters[$name][] = new Reference($id);
}
}

foreach ($connectionFilters as $name => $references) {
$configurationId = sprintf('doctrine.dbal.%s_connection.configuration', $name);

if (! $container->hasDefinition($configurationId)) {
continue;
}

$definition = new ChildDefinition('doctrine.dbal.schema_asset_filter_manager');
$definition->setArgument(0, $references);

$id = sprintf('doctrine.dbal.%s_schema_asset_filter_manager', $name);
$container->setDefinition($id, $definition);
$container->findDefinition($configurationId)
->addMethodCall('setSchemaAssetsFilter', [new Reference($id)]);
}
}
}
10 changes: 9 additions & 1 deletion DependencyInjection/DoctrineExtension.php
Expand Up @@ -2,6 +2,7 @@

namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection;

use Doctrine\Bundle\DoctrineBundle\Dbal\RegexSchemaAssetFilter;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\ServiceRepositoryCompilerPass;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepositoryInterface;
use Doctrine\Bundle\DoctrineCacheBundle\DependencyInjection\CacheProviderLoader;
Expand Down Expand Up @@ -150,7 +151,14 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder
unset($connection['auto_commit']);

if (isset($connection['schema_filter']) && $connection['schema_filter']) {
$configuration->addMethodCall('setFilterSchemaAssetsExpression', [$connection['schema_filter']]);
if (method_exists(\Doctrine\DBAL\Configuration::class, 'setSchemaAssetsFilter')) {
$definition = new Definition(RegexSchemaAssetFilter::class, [$connection['schema_filter']]);
$definition->addTag('doctrine.dbal.schema_filter', ['connection' => $name]);
$container->setDefinition(sprintf('doctrine.dbal.%s_regex_schema_filter', $name), $definition);
} else {
// backwards compatibility with dbal < 2.9
$configuration->addMethodCall('setFilterSchemaAssetsExpression', [$connection['schema_filter']]);
}
}

unset($connection['schema_filter']);
Expand Down
4 changes: 4 additions & 0 deletions Resources/config/dbal.xml
Expand Up @@ -70,6 +70,10 @@
<tag name="twig.extension" />
</service>

<service id="doctrine.dbal.schema_asset_filter_manager" class="Doctrine\Bundle\DoctrineBundle\Dbal\SchemaAssetsFilterManager" public="false" abstract="true">
<!-- schema assets filters -->
</service>

<!-- commands -->
<service id="doctrine.database_create_command" class="Doctrine\Bundle\DoctrineBundle\Command\CreateDatabaseDoctrineCommand">
<argument type="service" id="doctrine" />
Expand Down
17 changes: 17 additions & 0 deletions Tests/Dbal/RegexSchemaAssetFilterTest.php
@@ -0,0 +1,17 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Tests\Dbal;

use Doctrine\Bundle\DoctrineBundle\Dbal\RegexSchemaAssetFilter;
use PHPUnit\Framework\TestCase;

class RegexSchemaAssetFilterTest extends TestCase
{
public function testShouldIncludeAsset()
{
$filter = new RegexSchemaAssetFilter('~^(?!t_)~');

$this->assertTrue($filter('do_not_t_ignore_me'));
$this->assertFalse($filter('t_ignore_me'));
}
}
23 changes: 23 additions & 0 deletions Tests/Dbal/SchemaAssetsFilterManagerTest.php
@@ -0,0 +1,23 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\Tests\Dbal;

use Doctrine\Bundle\DoctrineBundle\Dbal\RegexSchemaAssetFilter;
use Doctrine\Bundle\DoctrineBundle\Dbal\SchemaAssetsFilterManager;
use PHPUnit\Framework\TestCase;

class SchemaAssetsFilterManagerTest extends TestCase
{
public function testInvoke()
{
$filterA = new RegexSchemaAssetFilter('~^(?!t_)~');
$filterB = new RegexSchemaAssetFilter('~^(?!s_)~');

$manager = new SchemaAssetsFilterManager([$filterA, $filterB]);
$tables = ['do_not_filter', 't_filter_me', 's_filter_me_too'];
$this->assertSame(
['do_not_filter'],
array_values(array_filter($tables, $manager))
);
}
}
77 changes: 75 additions & 2 deletions Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
Expand Up @@ -2,8 +2,11 @@

namespace Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection;

use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\DbalSchemaFilterPass;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\EntityListenerPass;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\DoctrineExtension;
use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Schema\AbstractAsset;
use Doctrine\ORM\EntityManager;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
Expand Down Expand Up @@ -761,10 +764,60 @@ public function testDbalOracleInstancename()

public function testDbalSchemaFilter()
{
if (method_exists(Configuration::class, 'setSchemaAssetsFilter')) {
$this->markTestSkipped('Test only applies to doctrine/dbal 2.8 or lower');
}

$container = $this->loadContainer('dbal_schema_filter');

$definition = $container->getDefinition('doctrine.dbal.default_connection.configuration');
$this->assertDICDefinitionMethodCallOnce($definition, 'setFilterSchemaAssetsExpression', ['^sf2_']);
$definition = $container->getDefinition('doctrine.dbal.connection1_connection.configuration');
$this->assertDICDefinitionMethodCallOnce($definition, 'setFilterSchemaAssetsExpression', ['~^(?!t_)~']);
}

public function testDbalSchemaFilterNewConfig()
{
if (! method_exists(Configuration::class, 'setSchemaAssetsFilter')) {
$this->markTestSkipped('Test requires doctrine/dbal 2.9 or higher');
}

$container = $this->getContainer([]);
$loader = new DoctrineExtension();
$container->registerExtension($loader);
$container->addCompilerPass(new DbalSchemaFilterPass());

// ignore table1 table on "default" connection
$container->register('dummy_filter1', DummySchemaAssetsFilter::class)
->setArguments(['table1'])
->addTag('doctrine.dbal.schema_filter');

// ignore table2 table on "connection2" connection
$container->register('dummy_filter2', DummySchemaAssetsFilter::class)
->setArguments(['table2'])
->addTag('doctrine.dbal.schema_filter', ['connection' => 'connection2']);

$this->loadFromFile($container, 'dbal_schema_filter');

$assetNames = ['table1', 'table2', 'table3', 't_ignored'];
$expectedConnectionAssets = [
// ignores table1 + schema_filter applies
'connection1' => ['table2', 'table3'],
// ignores table2, no schema_filter applies
'connection2' => ['table1', 'table3', 't_ignored'],
// connection3 has no ignores, handled separately
];

$this->compileContainer($container);

$getConfiguration = static function (string $connectionName) use ($container) : Configuration {
return $container->get(sprintf('doctrine.dbal.%s_connection', $connectionName))->getConfiguration();
};

foreach ($expectedConnectionAssets as $connectionName => $expectedTables) {
$connConfig = $getConfiguration($connectionName);
$this->assertSame($expectedTables, array_values(array_filter($assetNames, $connConfig->getSchemaAssetsFilter())), sprintf('Filtering for connection "%s"', $connectionName));
}

$this->assertNull($connConfig = $getConfiguration('connection3')->getSchemaAssetsFilter());
}

public function testEntityListenerResolver()
Expand Down Expand Up @@ -1048,3 +1101,23 @@ private function compileContainer(ContainerBuilder $container)
$container->compile();
}
}

class DummySchemaAssetsFilter
{
/** @var string */
private $tableToIgnore;

public function __construct(string $tableToIgnore)
{
$this->tableToIgnore = $tableToIgnore;
}

public function __invoke($assetName) : bool
{
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}

return $assetName !== $this->tableToIgnore;
}
}
Expand Up @@ -7,6 +7,10 @@
http://symfony.com/schema/dic/doctrine http://symfony.com/schema/dic/doctrine/doctrine-1.0.xsd">

<config>
<dbal schema-filter="^sf2_" />
<dbal default-connection="connection1">
<connection name="connection1" schema-filter="~^(?!t_)~" />
<connection name="connection2" />
<connection name="connection3" />
</dbal>
</config>
</srv:container>
@@ -1,3 +1,8 @@
doctrine:
dbal:
schema_filter: ^sf2_
default_connection: connection1
connections:
connection1:
schema_filter: ~^(?!t_)~
connection2: []
connection3: []