From 888a4a8eff54ff762ca46535c049a054cc0865b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Mon, 11 Jul 2022 09:20:46 +0200 Subject: [PATCH] Avoid SQL assertions doctrine/dbal is the component responsible for generating the queries. Let us make the test suite more robust by asserting that things work from a functional point of view. --- .../ORM/Functional/EntityRepositoryTest.php | 77 ++++++++++++------- .../Functional/OneToOneEagerLoadingTest.php | 26 +++---- .../ORM/Functional/Ticket/GH6823Test.php | 31 ++++++-- .../ORM/Functional/Ticket/GH7012Test.php | 23 +----- 4 files changed, 88 insertions(+), 69 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 3cfec064d23..a40b0be099c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -8,10 +8,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Criteria; use Doctrine\Common\Persistence\PersistentObject; -use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; -use Doctrine\DBAL\Logging\Middleware as LoggingMiddleware; -use Doctrine\DBAL\ParameterType; use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Exception\NotSupported; @@ -34,7 +31,6 @@ use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository; use Doctrine\Tests\OrmFunctionalTestCase; -use function array_fill; use function array_values; use function class_exists; use function reset; @@ -721,28 +717,57 @@ public function testInvalidOrientation(): void */ public function testFindByAssociationArray(): void { - $repo = $this->_em->getRepository(CmsAddress::class); - $repo->findBy(['user' => [1, 2, 3]]); - - if (! class_exists(LoggingMiddleware::class)) { - // DBAL 2 logs queries before resolving parameter positions - self::assertSame( - [ - 'sql' => 'SELECT t0.id AS id_1, t0.country AS country_2, t0.zip AS zip_3, t0.city AS city_4, t0.user_id AS user_id_5 FROM cms_addresses t0 WHERE t0.user_id IN (?)', - 'params' => [[1, 2, 3]], - 'types' => [Connection::PARAM_INT_ARRAY], - ], - $this->getLastLoggedQuery() - ); - } else { - self::assertSame( - [ - 'sql' => 'SELECT t0.id AS id_1, t0.country AS country_2, t0.zip AS zip_3, t0.city AS city_4, t0.user_id AS user_id_5 FROM cms_addresses t0 WHERE t0.user_id IN (?, ?, ?)', - 'params' => [1 => 1, 2 => 2, 3 => 3], - 'types' => array_fill(1, 3, ParameterType::INTEGER), - ], - $this->getLastLoggedQuery() - ); + $address1 = new CmsAddress(); + $address1->country = 'Germany'; + $address1->zip = '12345'; + $address1->city = 'Berlin'; + $user1 = new CmsUser(); + $user1->username = 'nifnif'; + $user1->name = 'Nif-Nif'; + $user1->setAddress($address1); + + $address2 = new CmsAddress(); + $address2->country = 'France'; + $address2->zip = '54321'; + $address2->city = 'Paris'; + $user2 = new CmsUser(); + $user2->username = 'noufnouf'; + $user2->name = 'Nouf-Nouf'; + $user2->setAddress($address2); + + $address3 = new CmsAddress(); + $address3->country = 'Spain'; + $address3->zip = '98765'; + $address3->city = 'Madrid'; + $user3 = new CmsUser(); + $user3->username = 'nafnaf'; + $user3->name = 'Naf-Naf'; + $user3->setAddress($address3); + + $address4 = new CmsAddress(); + $address4->country = 'United Kingdom'; + $address4->zip = '32145'; + $address4->city = 'London'; + $user4 = new CmsUser(); + $user4->username = 'wolf'; + $user4->name = 'Wolf'; + $user4->setAddress($address4); + + $this->_em->persist($user1); + $this->_em->persist($user2); + $this->_em->persist($user3); + $this->_em->persist($user4); + $this->_em->flush(); + + $ids = [$user1->id, $user2->id, $user3->id]; + $this->_em->clear(); + + $repo = $this->_em->getRepository(CmsAddress::class); + $addresses = $repo->findBy(['user' => $ids]); + + self::assertCount(3, $addresses); + foreach ($addresses as $address) { + self::assertContains($address->zip, ['12345', '54321', '98765']); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php index 6473b02ff99..5b774fc2a0e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php @@ -146,24 +146,22 @@ public function testEagerLoadWithNullableColumnsGeneratesLeftJoinOnBothSides(): { $train = new Train(new TrainOwner('Alexander')); $driver = new TrainDriver('Benjamin'); - $train->setDriver($driver); $this->_em->persist($train); + $this->_em->persist($driver); $this->_em->flush(); + $trainId = $train->id; + $driverId = $driver->id; $this->_em->clear(); $train = $this->_em->find(get_class($train), $train->id); - $this->assertSQLEquals( - 'SELECT t0.id AS id_1, t0.driver_id AS driver_id_2, t3.id AS id_4, t3.name AS name_5, t0.owner_id AS owner_id_6, t7.id AS id_8, t7.name AS name_9 FROM Train t0 LEFT JOIN TrainDriver t3 ON t0.driver_id = t3.id INNER JOIN TrainOwner t7 ON t0.owner_id = t7.id WHERE t0.id = ?', - $this->getLastLoggedQuery()['sql'] - ); + self::assertNotNull($train, 'It should be possible to find the train even though it has no driver'); + self::assertSame($trainId, $train->id); $this->_em->clear(); $driver = $this->_em->find(get_class($driver), $driver->id); - $this->assertSQLEquals( - 'SELECT t0.id AS id_1, t0.name AS name_2, t3.id AS id_4, t3.driver_id AS driver_id_5, t3.owner_id AS owner_id_6 FROM TrainOwner t0 LEFT JOIN Train t3 ON t3.owner_id = t0.id WHERE t0.id IN (?)', - $this->getLastLoggedQuery()['sql'] - ); + self::assertNotNull($driver, 'It should be possible to find the driver even though they drive no train'); + self::assertSame($driverId, $driver->id); } /** @@ -202,16 +200,12 @@ public function testEagerLoadWithNonNullableColumnsGeneratesInnerJoinOnOwningSid public function testEagerLoadWithNonNullableColumnsGeneratesLeftJoinOnNonOwningSide(): void { $owner = new TrainOwner('Alexander'); - $train = new Train($owner); - $this->_em->persist($train); + $this->_em->persist($owner); $this->_em->flush(); $this->_em->clear(); - $waggon = $this->_em->find(get_class($owner), $owner->id); - $this->assertSQLEquals( - 'SELECT t0.id AS id_1, t0.name AS name_2, t3.id AS id_4, t3.driver_id AS driver_id_5, t3.owner_id AS owner_id_6 FROM TrainOwner t0 LEFT JOIN Train t3 ON t3.owner_id = t0.id WHERE t0.id = ?', - $this->getLastLoggedQuery()['sql'] - ); + $owner = $this->_em->find(get_class($owner), $owner->id); + self::assertNotNull($owner, 'An owner without a train should be able to exist.'); } /** diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6823Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6823Test.php index b2ea93e2511..ae11ab913ec 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6823Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH6823Test.php @@ -37,13 +37,30 @@ public function testCharsetCollationWhenCreatingForeignRelations(): void GH6823Status::class ); - self::assertEquals('CREATE TABLE gh6823_user (id VARCHAR(255) NOT NULL, group_id VARCHAR(255) CHARACTER SET ascii DEFAULT NULL COLLATE `ascii_general_ci`, status_id VARCHAR(255) CHARACTER SET latin1 DEFAULT NULL COLLATE `latin1_bin`, INDEX IDX_70DD1774FE54D947 (group_id), INDEX IDX_70DD17746BF700BD (status_id), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_bin` ENGINE = InnoDB', $this->getLastLoggedQuery(6)['sql']); - self::assertEquals('CREATE TABLE gh6823_user_tags (user_id VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_bin`, tag_id VARCHAR(255) CHARACTER SET latin1 NOT NULL COLLATE `latin1_bin`, INDEX IDX_596B1281A76ED395 (user_id), INDEX IDX_596B1281BAD26311 (tag_id), PRIMARY KEY(user_id, tag_id)) DEFAULT CHARACTER SET ascii COLLATE `ascii_general_ci` ENGINE = InnoDB', $this->getLastLoggedQuery(5)['sql']); - self::assertEquals('CREATE TABLE gh6823_group (id VARCHAR(255) NOT NULL, PRIMARY KEY(id)) DEFAULT CHARACTER SET ascii COLLATE `ascii_general_ci` ENGINE = InnoDB', $this->getLastLoggedQuery(4)['sql']); - self::assertEquals('CREATE TABLE gh6823_status (id VARCHAR(255) CHARACTER SET latin1 NOT NULL COLLATE `latin1_bin`, PRIMARY KEY(id)) DEFAULT CHARACTER SET koi8r COLLATE `koi8r_bin` ENGINE = InnoDB', $this->getLastLoggedQuery(3)['sql']); - self::assertEquals('ALTER TABLE gh6823_user ADD CONSTRAINT FK_70DD1774FE54D947 FOREIGN KEY (group_id) REFERENCES gh6823_group (id)', $this->getLastLoggedQuery(2)['sql']); - self::assertEquals('ALTER TABLE gh6823_user ADD CONSTRAINT FK_70DD17746BF700BD FOREIGN KEY (status_id) REFERENCES gh6823_status (id)', $this->getLastLoggedQuery(1)['sql']); - self::assertEquals('ALTER TABLE gh6823_user_tags ADD CONSTRAINT FK_596B1281A76ED395 FOREIGN KEY (user_id) REFERENCES gh6823_user (id)', $this->getLastLoggedQuery(0)['sql']); + $schemaManager = $this->createSchemaManager(); + /* gh6823_user.group_id should use charset ascii and collation + * ascii_general_ci, because that is what gh6823_group.id falls back to */ + $userGroupIdOptions = $schemaManager->listTableDetails('gh6823_user')->getColumn('group_id')->toArray(); + self::assertSame('ascii', $userGroupIdOptions['charset']); + self::assertSame('ascii_general_ci', $userGroupIdOptions['collation']); + + /* gh6823_user.status_id should use charset latin1 and collation + * latin1_bin, because that is what gh6823_status.id uses */ + $userStatusIdOptions = $schemaManager->listTableDetails('gh6823_user')->getColumn('status_id')->toArray(); + self::assertSame('latin1', $userStatusIdOptions['charset']); + self::assertSame('latin1_bin', $userStatusIdOptions['collation']); + + /* gh6823_user_tags.user_id should use charset utf8mb4 and collation + * utf8mb4_bin, because that is what gh6823_user.id falls back to */ + $userTagsUserIdOptions = $schemaManager->listTableDetails('gh6823_user_tags')->getColumn('user_id')->toArray(); + self::assertSame('utf8mb4', $userTagsUserIdOptions['charset']); + self::assertSame('utf8mb4_bin', $userTagsUserIdOptions['collation']); + + /* gh6823_user_tags.tag_id should use charset latin1 and collation + * latin1_bin, because that is what gh6823_tag.id falls back to */ + $userTagsTagIdOption = $schemaManager->listTableDetails('gh6823_user_tags')->getColumn('tag_id')->toArray(); + self::assertSame('latin1', $userTagsTagIdOption['charset']); + self::assertSame('latin1_bin', $userTagsTagIdOption['collation']); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7012Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7012Test.php index cb682151770..cf869eb83ad 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7012Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7012Test.php @@ -13,8 +13,6 @@ use Doctrine\Tests\Models\Quote\User as QuotedUser; use Doctrine\Tests\OrmFunctionalTestCase; -use function sprintf; - final class GH7012Test extends OrmFunctionalTestCase { protected function setUp(): void @@ -45,24 +43,9 @@ public function testUpdateEntityWithIdentifierAssociationWithQuotedJoinColumn(): $userData->name = '4321'; $this->_em->flush(); - $platform = $this->_em->getConnection()->getDatabasePlatform(); - $quotedTableName = $platform->quoteIdentifier('quote-user-data'); - $quotedColumn = $platform->quoteIdentifier('name'); - $quotedIdentifier = $platform->quoteIdentifier('user-id'); - - self::assertNotEquals('quote-user-data', $quotedTableName); - self::assertNotEquals('name', $quotedColumn); - self::assertNotEquals('user-id', $quotedIdentifier); - - $lastLoggedQuery = $this->getLastLoggedQuery()['sql']; - // DBAL 2 logs a commit as last query. - if ($lastLoggedQuery === '"COMMIT"') { - $lastLoggedQuery = $this->getLastLoggedQuery(1)['sql']; - } - - $this->assertSQLEquals( - sprintf('UPDATE %s SET %s = ? WHERE %s = ?', $quotedTableName, $quotedColumn, $quotedIdentifier), - $lastLoggedQuery + self::assertSame( + '4321', + $this->_em->getRepository(GH7012UserData::class)->findOneBy(['user' => $user])->name ); } }