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

Whilelist existing assets we know about from metadata in SchemaTool::getUpdateSchemaSql() #7875

Merged
merged 3 commits into from
Dec 16, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ Method `Doctrine\ORM\AbstractQuery#useResultCache()` which could be used for bot
To optimize DB interaction, `Doctrine\ORM\Tools\Pagination\Paginator` no longer fetches identifiers to be able to
perform the pagination with join collections when max results isn't set in the query.

## Minor BC BREAK: tables filtered with `schema_filter` are no longer created

When generating schema diffs, if a source table is filtered out by a `schema_filter` expression, then a `CREATE TABLE` was
always generated, even if the table already existed. This has been changed in this release and the table will no longer
be created.

## Deprecated number unaware `Doctrine\ORM\Mapping\UnderscoreNamingStrategy`

In the last patch of the `v2.6.x` series, we fixed a bug that was not converting names properly when they had numbers
Expand Down
38 changes: 34 additions & 4 deletions lib/Doctrine/ORM/Tools/SchemaTool.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

namespace Doctrine\ORM\Tools;

use Doctrine\ORM\ORMException;
use Doctrine\DBAL\Schema\AbstractAsset;
use Doctrine\DBAL\Schema\Comparator;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Schema;
Expand All @@ -28,6 +28,7 @@
use Doctrine\DBAL\Schema\Visitor\RemoveNamespacedAssets;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\ORMException;
use Doctrine\ORM\Tools\Event\GenerateSchemaTableEventArgs;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;

Expand Down Expand Up @@ -891,10 +892,8 @@ public function updateSchema(array $classes, $saveMode = false)
*/
public function getUpdateSchemaSql(array $classes, $saveMode = false)
{
$sm = $this->em->getConnection()->getSchemaManager();

$fromSchema = $sm->createSchema();
$toSchema = $this->getSchemaFromMetadata($classes);
$fromSchema = $this->createSchemaForComparison($toSchema);

$comparator = new Comparator();
$schemaDiff = $comparator->compare($fromSchema, $toSchema);
Expand All @@ -905,4 +904,35 @@ public function getUpdateSchemaSql(array $classes, $saveMode = false)

return $schemaDiff->toSql($this->platform);
}

/**
* Creates the schema from the database, ensuring tables from the target schema are whitelisted for comparison.
*/
private function createSchemaForComparison(Schema $toSchema) : Schema
{
$connection = $this->em->getConnection();
$schemaManager = $connection->getSchemaManager();

// backup schema assets filter
$config = $connection->getConfiguration();
$previousFilter = $config->getSchemaAssetsFilter();

if ($previousFilter === null) {
return $schemaManager->createSchema();
}

// whitelist assets we already know about in $toSchema, use the existing filter otherwise
$config->setSchemaAssetsFilter(static function ($asset) use ($previousFilter, $toSchema) : bool {
$assetName = $asset instanceof AbstractAsset ? $asset->getName() : $asset;

return $toSchema->hasTable($assetName) || $toSchema->hasSequence($assetName) || $previousFilter($asset);
});

try {
return $schemaManager->createSchema();
} finally {
// restore schema assets filter
$config->setSchemaAssetsFilter($previousFilter);
}
}
}
136 changes: 136 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH7875Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
use Doctrine\ORM\Tools\SchemaTool;
use Doctrine\Tests\OrmFunctionalTestCase;
use function array_filter;
use function current;
use function method_exists;
use function sprintf;
use function strpos;

/** @group GH7875 */
final class GH7875Test extends OrmFunctionalTestCase
{
/** @after */
public function cleanUpSchema() : void
{
$connection = $this->_em->getConnection();

$connection->exec('DROP TABLE IF EXISTS gh7875_my_entity');
$connection->exec('DROP TABLE IF EXISTS gh7875_my_other_entity');

if ($connection->getDatabasePlatform() instanceof PostgreSqlPlatform) {
$connection->exec('DROP SEQUENCE IF EXISTS gh7875_my_entity_id_seq');
$connection->exec('DROP SEQUENCE IF EXISTS gh7875_my_other_entity_id_seq');
}
}

/**
* @param string[] $sqls
*
* @return string[]
*/
private function filterCreateTable(array $sqls, string $tableName) : array
{
return array_filter($sqls, static function (string $sql) use ($tableName) : bool {
return strpos($sql, sprintf('CREATE TABLE %s (', $tableName)) === 0;
});
}

public function testUpdateSchemaSql() : void
{
$classes = [$this->_em->getClassMetadata(GH7875MyEntity::class)];

$tool = new SchemaTool($this->_em);
$sqls = $this->filterCreateTable($tool->getUpdateSchemaSql($classes), 'gh7875_my_entity');

self::assertCount(1, $sqls);

$this->_em->getConnection()->exec(current($sqls));

$sqls = array_filter($tool->getUpdateSchemaSql($classes), static function (string $sql) : bool {
return strpos($sql, ' gh7875_my_entity ') !== false;
});

self::assertSame([], $sqls);

$classes[] = $this->_em->getClassMetadata(GH7875MyOtherEntity::class);

$sqls = $tool->getUpdateSchemaSql($classes);

self::assertCount(0, $this->filterCreateTable($sqls, 'gh7875_my_entity'));
self::assertCount(1, $this->filterCreateTable($sqls, 'gh7875_my_other_entity'));
}

/**
* @return array<array<string|callable|null>>
*/
public function provideUpdateSchemaSqlWithSchemaAssetFilter() : array
{
return [
['/^(?!my_enti)/', null],
[
null,
static function ($assetName) : bool {
return $assetName !== 'gh7875_my_entity';
},
],
];
}

/** @dataProvider provideUpdateSchemaSqlWithSchemaAssetFilter */
public function testUpdateSchemaSqlWithSchemaAssetFilter(?string $filterRegex, ?callable $filterCallback) : void
{
if ($filterRegex && ! method_exists(Configuration::class, 'setFilterSchemaAssetsExpression')) {
self::markTestSkipped(sprintf('Test require %s::setFilterSchemaAssetsExpression method', Configuration::class));
}

$classes = [$this->_em->getClassMetadata(GH7875MyEntity::class)];

$tool = new SchemaTool($this->_em);
$tool->createSchema($classes);

$config = $this->_em->getConnection()->getConfiguration();
if ($filterRegex) {
$config->setFilterSchemaAssetsExpression($filterRegex);
} else {
$config->setSchemaAssetsFilter($filterCallback);
}

$previousFilter = $config->getSchemaAssetsFilter();

$sqls = $tool->getUpdateSchemaSql($classes);
$sqls = array_filter($sqls, static function (string $sql) : bool {
return strpos($sql, ' gh7875_my_entity ') !== false;
});

self::assertCount(0, $sqls);
self::assertSame($previousFilter, $config->getSchemaAssetsFilter());
}
}

/**
* @Entity
* @Table(name="gh7875_my_entity")
*/
class GH7875MyEntity
{
/** @Id @Column(type="integer") @GeneratedValue(strategy="AUTO") */
public $id;
}

/**
* @Entity
* @Table(name="gh7875_my_other_entity")
*/
class GH7875MyOtherEntity
{
/** @Id @Column(type="integer") @GeneratedValue(strategy="AUTO") */
public $id;
}