From 355199c5d1acbb1d6d6e866b0f4d132d937f5e06 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Wed, 15 Jan 2020 20:23:55 +0000 Subject: [PATCH] Fix ORMPurger generating table names with unnecessary quotes Change in 1.4.1 (#334) caused ORMPurger to generate quotes inside table names with mocked schemas leaving them invalid (e.g. test_schema__"group") becuase quoting strategy was ignored. Reverted changes to ORMPurger::getTableName and added new fucntion to mimic behaviour of SchemaTool in doctrine/orm when generating delete purge SQL. Unit tests included to check for correct SQL generation for tables both with and without schema. Fixes #338. --- .../Common/DataFixtures/Purger/ORMPurger.php | 28 +++++------ .../DataFixtures/Purger/ORMPurgerTest.php | 33 +++++++++--- .../TestEntity/GroupWithSchema.php | 50 +++++++++++++++++++ 3 files changed, 88 insertions(+), 23 deletions(-) create mode 100644 tests/Doctrine/Tests/Common/DataFixtures/TestEntity/GroupWithSchema.php diff --git a/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php b/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php index 1be4d48c..e7f503d1 100644 --- a/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php +++ b/lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php @@ -12,7 +12,6 @@ use function array_reverse; use function array_search; use function count; -use function implode; use function is_callable; use function method_exists; use function preg_match; @@ -152,7 +151,7 @@ public function purge() } if ($this->purgeMode === self::PURGE_MODE_DELETE) { - $connection->executeUpdate('DELETE FROM ' . $tbl); + $connection->executeUpdate($this->getDeleteFromTableSQL($tbl, $platform)); } else { $connection->executeUpdate($platform->getTruncateTableSQL($tbl, true)); } @@ -240,23 +239,13 @@ private function getAssociationTables(array $classes, AbstractPlatform $platform return $associationTables; } - /** - * @param ClassMetadata $class - * @param AbstractPlatform $platform - * - * @return string - */ - private function getTableName($class, $platform) + private function getTableName(ClassMetadata $class, AbstractPlatform $platform) : string { - if (method_exists($class, 'getSchemaName')) { - $identifier[] = $class->getSchemaName(); - } elseif (isset($class->table['schema'])) { - $identifier[] = $class->table['schema']; + if (isset($class->table['schema']) && ! method_exists($class, 'getSchemaName')) { + return $class->table['schema'] . '.' . $this->em->getConfiguration()->getQuoteStrategy()->getTableName($class, $platform); } - $identifier[] = $class->getTableName(); - - return (new Identifier(implode('.', $identifier)))->getQuotedName($platform); + return $this->em->getConfiguration()->getQuoteStrategy()->getTableName($class, $platform); } /** @@ -274,4 +263,11 @@ private function getJoinTableName($assoc, $class, $platform) return $this->em->getConfiguration()->getQuoteStrategy()->getJoinTableName($assoc, $class, $platform); } + + private function getDeleteFromTableSQL(string $tableName, AbstractPlatform $platform) : string + { + $tableIdentifier = new Identifier($tableName); + + return 'DELETE FROM ' . $tableIdentifier->getQuotedName($platform); + } } diff --git a/tests/Doctrine/Tests/Common/DataFixtures/Purger/ORMPurgerTest.php b/tests/Doctrine/Tests/Common/DataFixtures/Purger/ORMPurgerTest.php index 257d6fc2..c87b2f9d 100644 --- a/tests/Doctrine/Tests/Common/DataFixtures/Purger/ORMPurgerTest.php +++ b/tests/Doctrine/Tests/Common/DataFixtures/Purger/ORMPurgerTest.php @@ -12,10 +12,11 @@ */ class ORMPurgerTest extends BaseTest { - public const TEST_ENTITY_USER = TestEntity\User::class; - public const TEST_ENTITY_USER_WITH_SCHEMA = TestEntity\UserWithSchema::class; - public const TEST_ENTITY_QUOTED = TestEntity\Quoted::class; - public const TEST_ENTITY_GROUP = TestEntity\Group::class; + public const TEST_ENTITY_USER = TestEntity\User::class; + public const TEST_ENTITY_USER_WITH_SCHEMA = TestEntity\UserWithSchema::class; + public const TEST_ENTITY_QUOTED = TestEntity\Quoted::class; + public const TEST_ENTITY_GROUP = TestEntity\Group::class; + public const TEST_ENTITY_GROUP_WITH_SCHEMA = TestEntity\GroupWithSchema::class; public function testGetAssociationTables() { @@ -56,7 +57,7 @@ public function testTableNameWithSchema() $this->assertStringStartsWith('test_schema', $tableName); } - public function testGetTableNameQuoted() : void + public function testGetDeleteFromTableSQL() : void { $em = $this->getMockAnnotationReaderEntityManager(); $metadata = $em->getClassMetadata(self::TEST_ENTITY_GROUP); @@ -66,7 +67,25 @@ public function testGetTableNameQuoted() : void $method = $class->getMethod('getTableName'); $method->setAccessible(true); $tableName = $method->invokeArgs($purger, [$metadata, $platform]); - $this->assertStringStartsWith('"', $tableName); - $this->assertStringEndsWith('"', $tableName); + $method = $class->getMethod('getDeleteFromTableSQL'); + $method->setAccessible(true); + $sql = $method->invokeArgs($purger, [$tableName, $platform]); + $this->assertEquals('DELETE FROM "Group"', $sql); + } + + public function testGetDeleteFromTableSQLWithSchema() : void + { + $em = $this->getMockAnnotationReaderEntityManager(); + $metadata = $em->getClassMetadata(self::TEST_ENTITY_GROUP_WITH_SCHEMA); + $platform = $em->getConnection()->getDatabasePlatform(); + $purger = new ORMPurger($em); + $class = new ReflectionClass(ORMPurger::class); + $method = $class->getMethod('getTableName'); + $method->setAccessible(true); + $tableName = $method->invokeArgs($purger, [$metadata, $platform]); + $method = $class->getMethod('getDeleteFromTableSQL'); + $method->setAccessible(true); + $sql = $method->invokeArgs($purger, [$tableName, $platform]); + $this->assertEquals('DELETE FROM test_schema__group', $sql); } } diff --git a/tests/Doctrine/Tests/Common/DataFixtures/TestEntity/GroupWithSchema.php b/tests/Doctrine/Tests/Common/DataFixtures/TestEntity/GroupWithSchema.php new file mode 100644 index 00000000..826e0e16 --- /dev/null +++ b/tests/Doctrine/Tests/Common/DataFixtures/TestEntity/GroupWithSchema.php @@ -0,0 +1,50 @@ +id = $id; + } + + public function getId() : ?int + { + return $this->id; + } + + public function setCode($code) : void + { + $this->code = $code; + } + + public function getCode() : ?string + { + return $this->code; + } +}