From e798bfe34a4ec2b685567717f18b747508a47b5c Mon Sep 17 00:00:00 2001 From: Sergio Santoro Date: Mon, 29 Jun 2015 22:05:26 +0200 Subject: [PATCH 01/28] [QUERY] "INSTANCE OF" now behaves correctly with subclasses There was a bug in the "INSTANCE OF" operator as described in https://groups.google.com/forum/#!topic/doctrine-user/B8raq8CNMgg "INSTANCE OF" was not taking into account subclasses. It was merely translating the class to its discriminator. This is not correct since the class can have subtypes and those are, indeed, still instance of the superclass. Also, classes may not have a discriminator (e.g. abstract classes). This commit also provides useful tests to avoid regression. --- lib/Doctrine/ORM/Query/SqlWalker.php | 38 +++-- .../ORM/Functional/InstanceOfAbstractTest.php | 113 +++++++++++++ .../Functional/InstanceOfMultiLevelTest.php | 153 ++++++++++++++++++ .../Tests/ORM/Functional/InstanceOfTest.php | 119 ++++++++++++++ .../ORM/Query/SelectSqlGenerationTest.php | 6 +- 5 files changed, 413 insertions(+), 16 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 57287c7cb..62350c389 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2038,8 +2038,10 @@ class SqlWalker implements TreeWalker $sql .= $class->discriminatorColumn['name'] . ($instanceOfExpr->not ? ' NOT IN ' : ' IN '); - $sqlParameterList = []; + $knownSubclasses = array_flip($discrClass->subClasses); + $sqlParameterList = []; + $discriminators = []; foreach ($instanceOfExpr->value as $parameter) { if ($parameter instanceof AST\InputParameter) { $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); @@ -2049,21 +2051,31 @@ class SqlWalker implements TreeWalker continue; } - // Get name from ClassMetadata to resolve aliases. - $entityClassName = $this->em->getClassMetadata($parameter)->name; - $discriminatorValue = $class->discriminatorValue; + // Trim first backslash + $parameter = ltrim($parameter, '\\'); - if ($entityClassName !== $class->name) { - $discrMap = array_flip($class->discriminatorMap); - - if ( ! isset($discrMap[$entityClassName])) { - throw QueryException::instanceOfUnrelatedClass($entityClassName, $class->rootEntityName); - } - - $discriminatorValue = $discrMap[$entityClassName]; + // Check parameter is really in the hierarchy + if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } - $sqlParameterList[] = $this->conn->quote($discriminatorValue); + // Include discriminators for parameter class and its subclass + $metadata = $this->em->getClassMetadata($parameter); + $hierarchyClasses = $metadata->subClasses; + $hierarchyClasses[] = $metadata->name; + + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->em->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = true; + } + } + } + + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); } $sql .= '(' . implode(', ', $sqlParameterList) . ')'; diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php new file mode 100644 index 000000000..a84643f84 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -0,0 +1,113 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Employee'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(1, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person', $r); + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee', $r); + $this->assertEquals('bar', $r->getName()); + } + } + + private function loadData() + { + $employee = new InstanceOfAbstractTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $this->_em->persist($employee); + + $this->_em->flush($employee); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { + + /** + * @Entity() + * @Table(name="instance_of_abstract_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee" + * }) + */ + abstract class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_abstract_test_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php new file mode 100644 index 000000000..c4b5bbcd2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php @@ -0,0 +1,153 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Employee'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Engineer'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(3, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person', $r); + if ($r instanceof InstanceOfMultiLevelTest\Engineer) { + $this->assertEquals('foobar', $r->getName()); + $this->assertEquals('doctrine', $r->getSpecialization()); + } elseif ($r instanceof InstanceOfMultiLevelTest\Employee) { + $this->assertEquals('bar', $r->getName()); + $this->assertEquals('qux', $r->getDepartement()); + } else { + $this->assertEquals('foo', $r->getName()); + } + } + } + + private function loadData() + { + $person = new InstanceOfMultiLevelTest\Person(); + $person->setName('foo'); + + $employee = new InstanceOfMultiLevelTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $engineer = new InstanceOfMultiLevelTest\Engineer(); + $engineer->setName('foobar'); + $engineer->setDepartement('dep'); + $engineer->setSpecialization('doctrine'); + + $this->_em->persist($person); + $this->_em->persist($employee); + $this->_em->persist($engineer); + + $this->_em->flush(array($person, $employee, $engineer)); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest { + /** + * @Entity() + * @Table(name="instance_of_multi_level_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person", + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee", + * "engineer": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer", + * }) + */ + class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_multi_level_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } + + /** + * @Entity() + * @Table(name="instance_of_multi_level_engineer") + */ + class Engineer extends Employee + { + /** + * @Column(type="string") + */ + private $specialization; + + public function getSpecialization() + { + return $this->specialization; + } + + public function setSpecialization($specialization) + { + $this->specialization = $specialization; + } + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php new file mode 100644 index 000000000..7f593c3f4 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php @@ -0,0 +1,119 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(2, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfTest\Person', $r); + if ($r instanceof InstanceOfTest\Employee) { + $this->assertEquals('bar', $r->getName()); + } else { + $this->assertEquals('foo', $r->getName()); + } + } + } + + private function loadData() + { + $person = new InstanceOfTest\Person(); + $person->setName('foo'); + + $employee = new InstanceOfTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $this->_em->persist($person); + $this->_em->persist($employee); + + $this->_em->flush(array($person, $employee)); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfTest { + /** + * @Entity() + * @Table(name="instance_of_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Person", + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Employee" + * }) + */ + class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_test_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } +} diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index b6bdce323..f9fb0b820 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -502,7 +502,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF Doctrine\Tests\Models\Company\CompanyEmployee", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } @@ -511,7 +511,7 @@ class SelectSqlGenerationTest extends OrmTestCase // This also uses FQCNs starting with or without a backslash in the INSTANCE OF parameter $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF (Doctrine\Tests\Models\Company\CompanyEmployee, \Doctrine\Tests\Models\Company\CompanyManager)", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee', 'manager')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } @@ -522,7 +522,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF \Doctrine\Tests\Models\Company\CompanyEmployee", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } From 4eb4465169fb386c5bcdb0b25b316ad9f477562a Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 18 Apr 2017 11:57:49 +0200 Subject: [PATCH 02/28] Fix as per review --- .../Tests/ORM/Functional/InstanceOfAbstractTest.php | 7 +++---- tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php index a84643f84..3a52d93f9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -28,9 +28,9 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(1, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person', $r); - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee', $r); - $this->assertEquals('bar', $r->getName()); + $this->assertInstanceOf(InstanceOfAbstractTest\Person::class, $r); + $this->assertInstanceOf(InstanceOfAbstractTest\Employee::class, $r); + $this->assertSame('bar', $r->getName()); } } @@ -109,5 +109,4 @@ namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { $this->departement = $departement; } } - } diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php index 7f593c3f4..fce0075b6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php @@ -10,10 +10,10 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( + $this->_schemaTool->createSchema([ $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), - )); + ]); } public function testInstanceOf() @@ -28,7 +28,7 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(2, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfTest\Person', $r); + $this->assertInstanceOf(InstanceOfTest\Person::class, $r); if ($r instanceof InstanceOfTest\Employee) { $this->assertEquals('bar', $r->getName()); } else { From 3219fe53160a81a8dd31bd799a65a2b6f9d6d620 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 10:46:24 +0200 Subject: [PATCH 03/28] Fix small CS issues as per review --- .../ORM/Functional/InstanceOfAbstractTest.php | 12 ++++++---- .../Functional/InstanceOfMultiLevelTest.php | 23 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php index 3a52d93f9..1b826a461 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -2,6 +2,8 @@ namespace Doctrine\Tests\ORM\Functional { + use Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee; + use Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person; use Doctrine\Tests\OrmFunctionalTestCase; class InstanceOfAbstractTest extends OrmFunctionalTestCase @@ -10,10 +12,10 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Employee'), - )); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), + ]); } public function testInstanceOf() @@ -55,7 +57,7 @@ namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { * @InheritanceType(value="JOINED") * @DiscriminatorColumn(name="kind", type="string") * @DiscriminatorMap(value={ - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee" + * "employee": Employee::class * }) */ abstract class Person diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php index c4b5bbcd2..131f053a2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php @@ -2,6 +2,9 @@ namespace Doctrine\Tests\ORM\Functional { + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee; + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer; + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person; use Doctrine\Tests\OrmFunctionalTestCase; class InstanceOfMultiLevelTest extends OrmFunctionalTestCase @@ -10,11 +13,11 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Employee'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Engineer'), - )); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), + $this->_em->getClassMetadata(Engineer::class), + ]); } public function testInstanceOf() @@ -29,11 +32,11 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(3, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person', $r); + $this->assertInstanceOf(Person::class, $r); if ($r instanceof InstanceOfMultiLevelTest\Engineer) { $this->assertEquals('foobar', $r->getName()); $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof InstanceOfMultiLevelTest\Employee) { + } elseif ($r instanceof Employee) { $this->assertEquals('bar', $r->getName()); $this->assertEquals('qux', $r->getDepartement()); } else { @@ -44,14 +47,14 @@ namespace Doctrine\Tests\ORM\Functional { private function loadData() { - $person = new InstanceOfMultiLevelTest\Person(); + $person = new Person(); $person->setName('foo'); - $employee = new InstanceOfMultiLevelTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); - $engineer = new InstanceOfMultiLevelTest\Engineer(); + $engineer = new Engineer(); $engineer->setName('foobar'); $engineer->setDepartement('dep'); $engineer->setSpecialization('doctrine'); From 11c84c7b20ac9c327a3843ac53a56795c79c01f6 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:00:26 +0200 Subject: [PATCH 04/28] Split SqlWalker::walkInstanceOfExpression method --- lib/Doctrine/ORM/Query/SqlWalker.php | 94 +++++++++++++++------------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 62350c389..e308b7de7 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -37,7 +37,6 @@ use Doctrine\ORM\Utility\PersisterHelper; * @author Alexander * @author Fabio B. Silva * @since 2.0 - * @todo Rename: SQLWalker */ class SqlWalker implements TreeWalker { @@ -2020,6 +2019,7 @@ class SqlWalker implements TreeWalker /** * {@inheritdoc} + * @throws \Doctrine\ORM\Query\QueryException */ public function walkInstanceOfExpression($instanceOfExpr) { @@ -2037,48 +2037,7 @@ class SqlWalker implements TreeWalker } $sql .= $class->discriminatorColumn['name'] . ($instanceOfExpr->not ? ' NOT IN ' : ' IN '); - - $knownSubclasses = array_flip($discrClass->subClasses); - - $sqlParameterList = []; - $discriminators = []; - foreach ($instanceOfExpr->value as $parameter) { - if ($parameter instanceof AST\InputParameter) { - $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); - - $sqlParameterList[] = $this->walkInputParameter($parameter); - - continue; - } - - // Trim first backslash - $parameter = ltrim($parameter, '\\'); - - // Check parameter is really in the hierarchy - if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { - throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); - } - - // Include discriminators for parameter class and its subclass - $metadata = $this->em->getClassMetadata($parameter); - $hierarchyClasses = $metadata->subClasses; - $hierarchyClasses[] = $metadata->name; - - foreach ($hierarchyClasses as $class) { - $currentMetadata = $this->em->getClassMetadata($class); - $currentDiscriminator = $currentMetadata->discriminatorValue; - - if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { - $discriminators[$currentDiscriminator] = true; - } - } - } - - foreach (array_keys($discriminators) as $dis) { - $sqlParameterList[] = $this->conn->quote($dis); - } - - $sql .= '(' . implode(', ', $sqlParameterList) . ')'; + $sql .= $this->getChildDiscriminatorsFromClassMetadata($discrClass, $instanceOfExpr); return $sql; } @@ -2312,4 +2271,53 @@ class SqlWalker implements TreeWalker return $resultAlias; } + + /** + * @param ClassMetadataInfo $discrClass + * @param AST\InstanceOfExpression $instanceOfExpr + * @return string The list in parentheses of valid child discriminators from the given class + * @throws QueryException + */ + private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $discrClass, AST\InstanceOfExpression $instanceOfExpr) + { + $knownSubclasses = array_flip($discrClass->subClasses); + $sqlParameterList = []; + $discriminators = []; + foreach ($instanceOfExpr->value as $parameter) { + if ($parameter instanceof AST\InputParameter) { + $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); + + $sqlParameterList[] = $this->walkInputParameter($parameter); + + continue; + } + + // Trim first backslash + $parameter = ltrim($parameter, '\\'); + + if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); + } + + // Include discriminators for parameter class and its subclass + $metadata = $this->em->getClassMetadata($parameter); + $hierarchyClasses = $metadata->subClasses; + $hierarchyClasses[] = $metadata->name; + + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->em->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = true; + } + } + } + + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); + } + + return '(' . implode(', ', $sqlParameterList) . ')'; + } } From 21e12ef4a93a32fad5e15f33ec4803890bea59f6 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:06:43 +0200 Subject: [PATCH 05/28] Move tests to ticket namespace (and rename them) --- .../Ticket4646InstanceOfAbstractTest.php} | 10 +++++----- .../Ticket4646InstanceOfMultiLevelTest.php} | 6 +++--- .../Ticket4646InstanceOfTest.php} | 14 ++++++++------ 3 files changed, 16 insertions(+), 14 deletions(-) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfAbstractTest.php => Ticket/Ticket4646InstanceOfAbstractTest.php} (88%) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfMultiLevelTest.php => Ticket/Ticket4646InstanceOfMultiLevelTest.php} (95%) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfTest.php => Ticket/Ticket4646InstanceOfTest.php} (86%) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php similarity index 88% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index 1b826a461..b17ca2c2d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -1,12 +1,12 @@ assertCount(1, $result); foreach ($result as $r) { - $this->assertInstanceOf(InstanceOfAbstractTest\Person::class, $r); - $this->assertInstanceOf(InstanceOfAbstractTest\Employee::class, $r); + $this->assertInstanceOf(Person::class, $r); + $this->assertInstanceOf(Employee::class, $r); $this->assertSame('bar', $r->getName()); } } private function loadData() { - $employee = new InstanceOfAbstractTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php similarity index 95% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 131f053a2..79f0d5363 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -1,13 +1,13 @@ assertInstanceOf(Person::class, $r); - if ($r instanceof InstanceOfMultiLevelTest\Engineer) { + if ($r instanceof Engineer) { $this->assertEquals('foobar', $r->getName()); $this->assertEquals('doctrine', $r->getSpecialization()); } elseif ($r instanceof Employee) { diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php similarity index 86% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index fce0075b6..326b83393 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -1,10 +1,12 @@ assertCount(2, $result); foreach ($result as $r) { - $this->assertInstanceOf(InstanceOfTest\Person::class, $r); - if ($r instanceof InstanceOfTest\Employee) { + $this->assertInstanceOf(Person::class, $r); + if ($r instanceof Employee) { $this->assertEquals('bar', $r->getName()); } else { $this->assertEquals('foo', $r->getName()); @@ -39,10 +41,10 @@ namespace Doctrine\Tests\ORM\Functional { private function loadData() { - $person = new InstanceOfTest\Person(); + $person = new Person(); $person->setName('foo'); - $employee = new InstanceOfTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); From 96bcee4fa94921bde88b0551c6d4e5ffb6e38563 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:42:51 +0200 Subject: [PATCH 06/28] Fix test --- .../Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index 326b83393..35d4afdac 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -13,8 +13,8 @@ namespace Doctrine\Tests\ORM\Functional\Ticket { parent::setUp(); $this->_schemaTool->createSchema([ - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), ]); } From 30256e7a085c4681cbc8a6b49e9caa791ca9f4a9 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 09:34:16 +0200 Subject: [PATCH 07/28] Refactor a bit the SqlWalker modifications --- lib/Doctrine/ORM/Query/SqlWalker.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index e308b7de7..0557faf31 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2280,7 +2280,6 @@ class SqlWalker implements TreeWalker */ private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $discrClass, AST\InstanceOfExpression $instanceOfExpr) { - $knownSubclasses = array_flip($discrClass->subClasses); $sqlParameterList = []; $discriminators = []; foreach ($instanceOfExpr->value as $parameter) { @@ -2292,15 +2291,13 @@ class SqlWalker implements TreeWalker continue; } - // Trim first backslash - $parameter = ltrim($parameter, '\\'); + $metadata = $this->em->getClassMetadata($parameter); - if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + if ($metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } // Include discriminators for parameter class and its subclass - $metadata = $this->em->getClassMetadata($parameter); $hierarchyClasses = $metadata->subClasses; $hierarchyClasses[] = $metadata->name; @@ -2308,15 +2305,13 @@ class SqlWalker implements TreeWalker $currentMetadata = $this->em->getClassMetadata($class); $currentDiscriminator = $currentMetadata->discriminatorValue; - if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { - $discriminators[$currentDiscriminator] = true; + if (null !== $currentDiscriminator && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = null; } } } - foreach (array_keys($discriminators) as $dis) { - $sqlParameterList[] = $this->conn->quote($dis); - } + $sqlParameterList = array_map([$this->conn, 'quote'], array_keys($discriminators)); return '(' . implode(', ', $sqlParameterList) . ')'; } From 11c54b7715ced7cffbf44f50e4833ea3b6a8ff19 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 10:11:31 +0200 Subject: [PATCH 08/28] Apply additional fixes to the SqlWalker to fix tests --- lib/Doctrine/ORM/Query/SqlWalker.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 0557faf31..d94f8d9c6 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2293,7 +2293,7 @@ class SqlWalker implements TreeWalker $metadata = $this->em->getClassMetadata($parameter); - if ($metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { + if (! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } @@ -2305,13 +2305,15 @@ class SqlWalker implements TreeWalker $currentMetadata = $this->em->getClassMetadata($class); $currentDiscriminator = $currentMetadata->discriminatorValue; - if (null !== $currentDiscriminator && ! array_key_exists($currentDiscriminator, $discriminators)) { + if (null !== $currentDiscriminator) { $discriminators[$currentDiscriminator] = null; } } } - $sqlParameterList = array_map([$this->conn, 'quote'], array_keys($discriminators)); + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); + } return '(' . implode(', ', $sqlParameterList) . ')'; } From 8b9c29738d22729019c95fa75cf2c272cab7f75c Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 10:26:31 +0200 Subject: [PATCH 09/28] Put all tests classes in a single namespace --- lib/Doctrine/ORM/Query/SqlWalker.php | 2 +- .../Ticket4646InstanceOfAbstractTest.php | 176 ++++++------- .../Ticket4646InstanceOfMultiLevelTest.php | 244 +++++++++--------- .../Ticket/Ticket4646InstanceOfTest.php | 181 +++++++------ 4 files changed, 293 insertions(+), 310 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index d94f8d9c6..0bf999ed5 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2293,7 +2293,7 @@ class SqlWalker implements TreeWalker $metadata = $this->em->getClassMetadata($parameter); - if (! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { + if ($metadata->getName() !== $discrClass->name && ! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index b17ca2c2d..090a172a9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -1,114 +1,108 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - ]); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646Abstract::class), + $this->_em->getClassMetadata(EmployeeTicket4646Abstract::class), + ]); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(1, $result); + + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646Abstract::class, $r); + $this->assertInstanceOf(EmployeeTicket4646Abstract::class, $r); + $this->assertSame('bar', $r->getName()); } + } - public function testInstanceOf() - { - $this->loadData(); + private function loadData() + { + $employee = new EmployeeTicket4646Abstract(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $this->_em->persist($employee); - $this->assertCount(1, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - $this->assertInstanceOf(Employee::class, $r); - $this->assertSame('bar', $r->getName()); - } - } - - private function loadData() - { - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($employee); - - $this->_em->flush($employee); - } + $this->_em->flush($employee); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { +/** + * @Entity() + * @Table(name="instance_of_abstract_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "employee": EmployeeTicket4646Abstract::class + * }) + */ +abstract class PersonTicket4646Abstract +{ + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; /** - * @Entity() - * @Table(name="instance_of_abstract_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "employee": Employee::class - * }) + * @Column(type="string") */ - abstract class Person + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_abstract_test_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; + return $this->name; + } - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + public function setName($name) + { + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_abstract_test_employee") + */ +class EmployeeTicket4646Abstract extends PersonTicket4646Abstract +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 79f0d5363..649de27bb 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -1,156 +1,150 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - $this->_em->getClassMetadata(Engineer::class), - ]); - } + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646MultiLevel::class), + $this->_em->getClassMetadata(EmployeeTicket4646MultiLevel::class), + $this->_em->getClassMetadata(EngineerTicket4646MultiLevel::class), + ]); + } - public function testInstanceOf() - { - $this->loadData(); + public function testInstanceOf() + { + $this->loadData(); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); - $this->assertCount(3, $result); + $this->assertCount(3, $result); - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - if ($r instanceof Engineer) { - $this->assertEquals('foobar', $r->getName()); - $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof Employee) { - $this->assertEquals('bar', $r->getName()); - $this->assertEquals('qux', $r->getDepartement()); - } else { - $this->assertEquals('foo', $r->getName()); - } + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646MultiLevel::class, $r); + if ($r instanceof EngineerTicket4646MultiLevel) { + $this->assertEquals('foobar', $r->getName()); + $this->assertEquals('doctrine', $r->getSpecialization()); + } elseif ($r instanceof EmployeeTicket4646MultiLevel) { + $this->assertEquals('bar', $r->getName()); + $this->assertEquals('qux', $r->getDepartement()); + } else { + $this->assertEquals('foo', $r->getName()); } } + } - private function loadData() - { - $person = new Person(); - $person->setName('foo'); + private function loadData() + { + $person = new PersonTicket4646MultiLevel(); + $person->setName('foo'); - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); + $employee = new EmployeeTicket4646MultiLevel(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $engineer = new Engineer(); - $engineer->setName('foobar'); - $engineer->setDepartement('dep'); - $engineer->setSpecialization('doctrine'); + $engineer = new EngineerTicket4646MultiLevel(); + $engineer->setName('foobar'); + $engineer->setDepartement('dep'); + $engineer->setSpecialization('doctrine'); - $this->_em->persist($person); - $this->_em->persist($employee); - $this->_em->persist($engineer); + $this->_em->persist($person); + $this->_em->persist($employee); + $this->_em->persist($engineer); - $this->_em->flush(array($person, $employee, $engineer)); - } + $this->_em->flush(array($person, $employee, $engineer)); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest { +/** + * @Entity() + * @Table(name="instance_of_multi_level_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646MultiLevel", + * "engineer": "Doctrine\Tests\ORM\Functional\Ticket\EngineerTicket4646MultiLevel", + * }) + */ +class PersonTicket4646MultiLevel +{ /** - * @Entity() - * @Table(name="instance_of_multi_level_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "person": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person", - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee", - * "engineer": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer", - * }) + * @Id() + * @GeneratedValue() + * @Column(type="integer") */ - class Person + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_multi_level_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + return $this->name; } - /** - * @Entity() - * @Table(name="instance_of_multi_level_engineer") - */ - class Engineer extends Employee + public function setName($name) { - /** - * @Column(type="string") - */ - private $specialization; - - public function getSpecialization() - { - return $this->specialization; - } - - public function setSpecialization($specialization) - { - $this->specialization = $specialization; - } + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_multi_level_employee") + */ +class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } +} + +/** + * @Entity() + * @Table(name="instance_of_multi_level_engineer") + */ +class EngineerTicket4646MultiLevel extends EmployeeTicket4646MultiLevel +{ + /** + * @Column(type="string") + */ + private $specialization; + + public function getSpecialization() + { + return $this->specialization; + } + + public function setSpecialization($specialization) + { + $this->specialization = $specialization; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index 35d4afdac..a83e6e733 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -1,121 +1,116 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - ]); - } + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646::class), + $this->_em->getClassMetadata(EmployeeTicket4646::class), + ]); + } - public function testInstanceOf() - { - $this->loadData(); + public function testInstanceOf() + { + $this->loadData(); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646 p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); - $this->assertCount(2, $result); + $this->assertCount(2, $result); - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - if ($r instanceof Employee) { - $this->assertEquals('bar', $r->getName()); - } else { - $this->assertEquals('foo', $r->getName()); - } + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646::class, $r); + if ($r instanceof EmployeeTicket4646) { + $this->assertEquals('bar', $r->getName()); + } else { + $this->assertEquals('foo', $r->getName()); } } + } - private function loadData() - { - $person = new Person(); - $person->setName('foo'); + private function loadData() + { + $person = new PersonTicket4646(); + $person->setName('foo'); - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); + $employee = new EmployeeTicket4646(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $this->_em->persist($person); - $this->_em->persist($employee); + $this->_em->persist($person); + $this->_em->persist($employee); - $this->_em->flush(array($person, $employee)); - } + $this->_em->flush(array($person, $employee)); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfTest { +/** + * @Entity() + * @Table(name="instance_of_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646" + * }) + */ +class PersonTicket4646 +{ /** - * @Entity() - * @Table(name="instance_of_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "person": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Person", - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Employee" - * }) + * @Id() + * @GeneratedValue() + * @Column(type="integer") */ - class Person + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_test_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; + return $this->name; + } - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + public function setName($name) + { + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_test_employee") + */ +class EmployeeTicket4646 extends PersonTicket4646 +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; } } From ba69cc8f7a0786539b1ead150fa6179ff37aea57 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 13:25:31 +0200 Subject: [PATCH 10/28] Simplify stubs used in tests --- .../Ticket4646InstanceOfAbstractTest.php | 50 +---------- .../Ticket4646InstanceOfMultiLevelTest.php | 83 ++----------------- .../Ticket/Ticket4646InstanceOfTest.php | 58 +------------ 3 files changed, 12 insertions(+), 179 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index 090a172a9..f0ea53fe8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -18,7 +18,8 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new EmployeeTicket4646Abstract()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract'; @@ -26,23 +27,7 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(1, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646Abstract::class, $r); - $this->assertInstanceOf(EmployeeTicket4646Abstract::class, $r); - $this->assertSame('bar', $r->getName()); - } - } - - private function loadData() - { - $employee = new EmployeeTicket4646Abstract(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($employee); - - $this->_em->flush($employee); + $this->assertContainsOnlyInstancesOf(PersonTicket4646Abstract::class, $result); } } @@ -64,25 +49,10 @@ abstract class PersonTicket4646Abstract */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -91,18 +61,4 @@ abstract class PersonTicket4646Abstract */ class EmployeeTicket4646Abstract extends PersonTicket4646Abstract { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 649de27bb..6e4160b24 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -19,7 +19,10 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new PersonTicket4646MultiLevel()); + $this->_em->persist(new EmployeeTicket4646MultiLevel()); + $this->_em->persist(new EngineerTicket4646MultiLevel()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel'; @@ -27,40 +30,7 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(3, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646MultiLevel::class, $r); - if ($r instanceof EngineerTicket4646MultiLevel) { - $this->assertEquals('foobar', $r->getName()); - $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof EmployeeTicket4646MultiLevel) { - $this->assertEquals('bar', $r->getName()); - $this->assertEquals('qux', $r->getDepartement()); - } else { - $this->assertEquals('foo', $r->getName()); - } - } - } - - private function loadData() - { - $person = new PersonTicket4646MultiLevel(); - $person->setName('foo'); - - $employee = new EmployeeTicket4646MultiLevel(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $engineer = new EngineerTicket4646MultiLevel(); - $engineer->setName('foobar'); - $engineer->setDepartement('dep'); - $engineer->setSpecialization('doctrine'); - - $this->_em->persist($person); - $this->_em->persist($employee); - $this->_em->persist($engineer); - - $this->_em->flush(array($person, $employee, $engineer)); + $this->assertContainsOnlyInstancesOf(PersonTicket4646MultiLevel::class, $result); } } @@ -84,25 +54,10 @@ class PersonTicket4646MultiLevel */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -111,20 +66,6 @@ class PersonTicket4646MultiLevel */ class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } /** @@ -133,18 +74,4 @@ class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel */ class EngineerTicket4646MultiLevel extends EmployeeTicket4646MultiLevel { - /** - * @Column(type="string") - */ - private $specialization; - - public function getSpecialization() - { - return $this->specialization; - } - - public function setSpecialization($specialization) - { - $this->specialization = $specialization; - } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index a83e6e733..363611d4d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -18,7 +18,9 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new PersonTicket4646()); + $this->_em->persist(new EmployeeTicket4646()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646 p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646'; @@ -26,30 +28,7 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(2, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646::class, $r); - if ($r instanceof EmployeeTicket4646) { - $this->assertEquals('bar', $r->getName()); - } else { - $this->assertEquals('foo', $r->getName()); - } - } - } - - private function loadData() - { - $person = new PersonTicket4646(); - $person->setName('foo'); - - $employee = new EmployeeTicket4646(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($person); - $this->_em->persist($employee); - - $this->_em->flush(array($person, $employee)); + $this->assertContainsOnlyInstancesOf(PersonTicket4646::class, $result); } } @@ -72,25 +51,10 @@ class PersonTicket4646 */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -99,18 +63,4 @@ class PersonTicket4646 */ class EmployeeTicket4646 extends PersonTicket4646 { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } From d4cdc6e1febf1cfb80e054b8324ef2c2e4e762b4 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 22 Jun 2017 09:50:53 +0200 Subject: [PATCH 11/28] Adding a failing test case for parameter binding using metadata with INSTANCE OF --- .../Ticket4646InstanceOfParametricTest.php | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php new file mode 100644 index 000000000..302934e8c --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php @@ -0,0 +1,67 @@ +_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646Parametric::class), + $this->_em->getClassMetadata(EmployeeTicket4646Parametric::class), + ]); + } + + public function testInstanceOf() + { + $this->_em->persist(new PersonTicket4646Parametric()); + $this->_em->persist(new EmployeeTicket4646Parametric()); + $this->_em->flush(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric p + WHERE p INSTANCE OF :parameter'; + $query = $this->_em->createQuery($dql); + $query->setParameter( + 'parameter', + $this->_em->getClassMetadata(PersonTicket4646Parametric::class) + ); + $result = $query->getResult(); + $this->assertCount(2, $result); + $this->assertContainsOnlyInstancesOf(PersonTicket4646Parametric::class, $result); + } +} + +/** + * @Entity() + * @Table(name="instance_of_parametric_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646Parametric" + * }) + */ +class PersonTicket4646Parametric +{ + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + public function getId() + { + return $this->id; + } +} + +/** + * @Entity() + * @Table(name="instance_of_parametric_employee") + */ +class EmployeeTicket4646Parametric extends PersonTicket4646Parametric +{ +} From 7d981350846118513fc0df3db350cc4f9968fdf6 Mon Sep 17 00:00:00 2001 From: Sergio Santoro Date: Mon, 29 Jun 2015 22:05:26 +0200 Subject: [PATCH 12/28] [QUERY] "INSTANCE OF" now behaves correctly with subclasses There was a bug in the "INSTANCE OF" operator as described in https://groups.google.com/forum/#!topic/doctrine-user/B8raq8CNMgg "INSTANCE OF" was not taking into account subclasses. It was merely translating the class to its discriminator. This is not correct since the class can have subtypes and those are, indeed, still instance of the superclass. Also, classes may not have a discriminator (e.g. abstract classes). This commit also provides useful tests to avoid regression. --- lib/Doctrine/ORM/Query/SqlWalker.php | 38 +++-- .../ORM/Functional/InstanceOfAbstractTest.php | 113 +++++++++++++ .../Functional/InstanceOfMultiLevelTest.php | 153 ++++++++++++++++++ .../Tests/ORM/Functional/InstanceOfTest.php | 119 ++++++++++++++ .../ORM/Query/SelectSqlGenerationTest.php | 6 +- 5 files changed, 413 insertions(+), 16 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 98f58ad8c..645a6c21f 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2035,8 +2035,10 @@ class SqlWalker implements TreeWalker $sql .= $class->discriminatorColumn['name'] . ($instanceOfExpr->not ? ' NOT IN ' : ' IN '); - $sqlParameterList = []; + $knownSubclasses = array_flip($discrClass->subClasses); + $sqlParameterList = []; + $discriminators = []; foreach ($instanceOfExpr->value as $parameter) { if ($parameter instanceof AST\InputParameter) { $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); @@ -2046,21 +2048,31 @@ class SqlWalker implements TreeWalker continue; } - // Get name from ClassMetadata to resolve aliases. - $entityClassName = $this->em->getClassMetadata($parameter)->name; - $discriminatorValue = $class->discriminatorValue; + // Trim first backslash + $parameter = ltrim($parameter, '\\'); - if ($entityClassName !== $class->name) { - $discrMap = array_flip($class->discriminatorMap); - - if ( ! isset($discrMap[$entityClassName])) { - throw QueryException::instanceOfUnrelatedClass($entityClassName, $class->rootEntityName); - } - - $discriminatorValue = $discrMap[$entityClassName]; + // Check parameter is really in the hierarchy + if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } - $sqlParameterList[] = $this->conn->quote($discriminatorValue); + // Include discriminators for parameter class and its subclass + $metadata = $this->em->getClassMetadata($parameter); + $hierarchyClasses = $metadata->subClasses; + $hierarchyClasses[] = $metadata->name; + + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->em->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = true; + } + } + } + + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); } $sql .= '(' . implode(', ', $sqlParameterList) . ')'; diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php new file mode 100644 index 000000000..a84643f84 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -0,0 +1,113 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Employee'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(1, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person', $r); + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee', $r); + $this->assertEquals('bar', $r->getName()); + } + } + + private function loadData() + { + $employee = new InstanceOfAbstractTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $this->_em->persist($employee); + + $this->_em->flush($employee); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { + + /** + * @Entity() + * @Table(name="instance_of_abstract_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee" + * }) + */ + abstract class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_abstract_test_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php new file mode 100644 index 000000000..c4b5bbcd2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php @@ -0,0 +1,153 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Employee'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Engineer'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(3, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person', $r); + if ($r instanceof InstanceOfMultiLevelTest\Engineer) { + $this->assertEquals('foobar', $r->getName()); + $this->assertEquals('doctrine', $r->getSpecialization()); + } elseif ($r instanceof InstanceOfMultiLevelTest\Employee) { + $this->assertEquals('bar', $r->getName()); + $this->assertEquals('qux', $r->getDepartement()); + } else { + $this->assertEquals('foo', $r->getName()); + } + } + } + + private function loadData() + { + $person = new InstanceOfMultiLevelTest\Person(); + $person->setName('foo'); + + $employee = new InstanceOfMultiLevelTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $engineer = new InstanceOfMultiLevelTest\Engineer(); + $engineer->setName('foobar'); + $engineer->setDepartement('dep'); + $engineer->setSpecialization('doctrine'); + + $this->_em->persist($person); + $this->_em->persist($employee); + $this->_em->persist($engineer); + + $this->_em->flush(array($person, $employee, $engineer)); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest { + /** + * @Entity() + * @Table(name="instance_of_multi_level_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person", + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee", + * "engineer": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer", + * }) + */ + class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_multi_level_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } + + /** + * @Entity() + * @Table(name="instance_of_multi_level_engineer") + */ + class Engineer extends Employee + { + /** + * @Column(type="string") + */ + private $specialization; + + public function getSpecialization() + { + return $this->specialization; + } + + public function setSpecialization($specialization) + { + $this->specialization = $specialization; + } + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php new file mode 100644 index 000000000..7f593c3f4 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php @@ -0,0 +1,119 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), + )); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfTest\Person p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfTest\Person'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(2, $result); + + foreach ($result as $r) { + $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfTest\Person', $r); + if ($r instanceof InstanceOfTest\Employee) { + $this->assertEquals('bar', $r->getName()); + } else { + $this->assertEquals('foo', $r->getName()); + } + } + } + + private function loadData() + { + $person = new InstanceOfTest\Person(); + $person->setName('foo'); + + $employee = new InstanceOfTest\Employee(); + $employee->setName('bar'); + $employee->setDepartement('qux'); + + $this->_em->persist($person); + $this->_em->persist($employee); + + $this->_em->flush(array($person, $employee)); + } + } +} + +namespace Doctrine\Tests\ORM\Functional\InstanceOfTest { + /** + * @Entity() + * @Table(name="instance_of_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Person", + * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Employee" + * }) + */ + class Person + { + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } + } + + /** + * @Entity() + * @Table(name="instance_of_test_employee") + */ + class Employee extends Person + { + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } + } +} diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index c10afbb89..82facdb49 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -500,7 +500,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF Doctrine\Tests\Models\Company\CompanyEmployee", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } @@ -509,7 +509,7 @@ class SelectSqlGenerationTest extends OrmTestCase // This also uses FQCNs starting with or without a backslash in the INSTANCE OF parameter $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF (Doctrine\Tests\Models\Company\CompanyEmployee, \Doctrine\Tests\Models\Company\CompanyManager)", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee', 'manager')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } @@ -520,7 +520,7 @@ class SelectSqlGenerationTest extends OrmTestCase { $this->assertSqlGeneration( "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF \Doctrine\Tests\Models\Company\CompanyEmployee", - "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('employee')" + "SELECT c0_.id AS id_0, c0_.name AS name_1, c0_.discr AS discr_2 FROM company_persons c0_ WHERE c0_.discr IN ('manager', 'employee')" ); } From 04acde667a517db81ed5798d58ccdf39bbb82d06 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 18 Apr 2017 11:57:49 +0200 Subject: [PATCH 13/28] Fix as per review --- .../Tests/ORM/Functional/InstanceOfAbstractTest.php | 7 +++---- tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php index a84643f84..3a52d93f9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -28,9 +28,9 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(1, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person', $r); - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee', $r); - $this->assertEquals('bar', $r->getName()); + $this->assertInstanceOf(InstanceOfAbstractTest\Person::class, $r); + $this->assertInstanceOf(InstanceOfAbstractTest\Employee::class, $r); + $this->assertSame('bar', $r->getName()); } } @@ -109,5 +109,4 @@ namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { $this->departement = $departement; } } - } diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php index 7f593c3f4..fce0075b6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php @@ -10,10 +10,10 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( + $this->_schemaTool->createSchema([ $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), - )); + ]); } public function testInstanceOf() @@ -28,7 +28,7 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(2, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfTest\Person', $r); + $this->assertInstanceOf(InstanceOfTest\Person::class, $r); if ($r instanceof InstanceOfTest\Employee) { $this->assertEquals('bar', $r->getName()); } else { From aa233f8e57f8ae313eb7119174ce26377ec63cd5 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 10:46:24 +0200 Subject: [PATCH 14/28] Fix small CS issues as per review --- .../ORM/Functional/InstanceOfAbstractTest.php | 12 ++++++---- .../Functional/InstanceOfMultiLevelTest.php | 23 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php index 3a52d93f9..1b826a461 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php @@ -2,6 +2,8 @@ namespace Doctrine\Tests\ORM\Functional { + use Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee; + use Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person; use Doctrine\Tests\OrmFunctionalTestCase; class InstanceOfAbstractTest extends OrmFunctionalTestCase @@ -10,10 +12,10 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Employee'), - )); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), + ]); } public function testInstanceOf() @@ -55,7 +57,7 @@ namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { * @InheritanceType(value="JOINED") * @DiscriminatorColumn(name="kind", type="string") * @DiscriminatorMap(value={ - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee" + * "employee": Employee::class * }) */ abstract class Person diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php index c4b5bbcd2..131f053a2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php @@ -2,6 +2,9 @@ namespace Doctrine\Tests\ORM\Functional { + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee; + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer; + use Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person; use Doctrine\Tests\OrmFunctionalTestCase; class InstanceOfMultiLevelTest extends OrmFunctionalTestCase @@ -10,11 +13,11 @@ namespace Doctrine\Tests\ORM\Functional { { parent::setUp(); - $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Employee'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Engineer'), - )); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), + $this->_em->getClassMetadata(Engineer::class), + ]); } public function testInstanceOf() @@ -29,11 +32,11 @@ namespace Doctrine\Tests\ORM\Functional { $this->assertCount(3, $result); foreach ($result as $r) { - $this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person', $r); + $this->assertInstanceOf(Person::class, $r); if ($r instanceof InstanceOfMultiLevelTest\Engineer) { $this->assertEquals('foobar', $r->getName()); $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof InstanceOfMultiLevelTest\Employee) { + } elseif ($r instanceof Employee) { $this->assertEquals('bar', $r->getName()); $this->assertEquals('qux', $r->getDepartement()); } else { @@ -44,14 +47,14 @@ namespace Doctrine\Tests\ORM\Functional { private function loadData() { - $person = new InstanceOfMultiLevelTest\Person(); + $person = new Person(); $person->setName('foo'); - $employee = new InstanceOfMultiLevelTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); - $engineer = new InstanceOfMultiLevelTest\Engineer(); + $engineer = new Engineer(); $engineer->setName('foobar'); $engineer->setDepartement('dep'); $engineer->setSpecialization('doctrine'); From 0e88f1b654f963c1e7623ab77d11d9d3c21226e8 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:00:26 +0200 Subject: [PATCH 15/28] Split SqlWalker::walkInstanceOfExpression method --- lib/Doctrine/ORM/Query/SqlWalker.php | 94 +++++++++++++++------------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 645a6c21f..baf05ffca 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -37,7 +37,6 @@ use Doctrine\ORM\Utility\PersisterHelper; * @author Alexander * @author Fabio B. Silva * @since 2.0 - * @todo Rename: SQLWalker */ class SqlWalker implements TreeWalker { @@ -2017,6 +2016,7 @@ class SqlWalker implements TreeWalker /** * {@inheritdoc} + * @throws \Doctrine\ORM\Query\QueryException */ public function walkInstanceOfExpression($instanceOfExpr) { @@ -2034,48 +2034,7 @@ class SqlWalker implements TreeWalker } $sql .= $class->discriminatorColumn['name'] . ($instanceOfExpr->not ? ' NOT IN ' : ' IN '); - - $knownSubclasses = array_flip($discrClass->subClasses); - - $sqlParameterList = []; - $discriminators = []; - foreach ($instanceOfExpr->value as $parameter) { - if ($parameter instanceof AST\InputParameter) { - $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); - - $sqlParameterList[] = $this->walkInputParameter($parameter); - - continue; - } - - // Trim first backslash - $parameter = ltrim($parameter, '\\'); - - // Check parameter is really in the hierarchy - if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { - throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); - } - - // Include discriminators for parameter class and its subclass - $metadata = $this->em->getClassMetadata($parameter); - $hierarchyClasses = $metadata->subClasses; - $hierarchyClasses[] = $metadata->name; - - foreach ($hierarchyClasses as $class) { - $currentMetadata = $this->em->getClassMetadata($class); - $currentDiscriminator = $currentMetadata->discriminatorValue; - - if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { - $discriminators[$currentDiscriminator] = true; - } - } - } - - foreach (array_keys($discriminators) as $dis) { - $sqlParameterList[] = $this->conn->quote($dis); - } - - $sql .= '(' . implode(', ', $sqlParameterList) . ')'; + $sql .= $this->getChildDiscriminatorsFromClassMetadata($discrClass, $instanceOfExpr); return $sql; } @@ -2309,4 +2268,53 @@ class SqlWalker implements TreeWalker return $resultAlias; } + + /** + * @param ClassMetadataInfo $discrClass + * @param AST\InstanceOfExpression $instanceOfExpr + * @return string The list in parentheses of valid child discriminators from the given class + * @throws QueryException + */ + private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $discrClass, AST\InstanceOfExpression $instanceOfExpr) + { + $knownSubclasses = array_flip($discrClass->subClasses); + $sqlParameterList = []; + $discriminators = []; + foreach ($instanceOfExpr->value as $parameter) { + if ($parameter instanceof AST\InputParameter) { + $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); + + $sqlParameterList[] = $this->walkInputParameter($parameter); + + continue; + } + + // Trim first backslash + $parameter = ltrim($parameter, '\\'); + + if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); + } + + // Include discriminators for parameter class and its subclass + $metadata = $this->em->getClassMetadata($parameter); + $hierarchyClasses = $metadata->subClasses; + $hierarchyClasses[] = $metadata->name; + + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->em->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = true; + } + } + } + + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); + } + + return '(' . implode(', ', $sqlParameterList) . ')'; + } } From bd47ec95a10d0d5f358a3de299ba8fd0e9f114a5 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:06:43 +0200 Subject: [PATCH 16/28] Move tests to ticket namespace (and rename them) --- .../Ticket4646InstanceOfAbstractTest.php} | 10 +++++----- .../Ticket4646InstanceOfMultiLevelTest.php} | 6 +++--- .../Ticket4646InstanceOfTest.php} | 14 ++++++++------ 3 files changed, 16 insertions(+), 14 deletions(-) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfAbstractTest.php => Ticket/Ticket4646InstanceOfAbstractTest.php} (88%) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfMultiLevelTest.php => Ticket/Ticket4646InstanceOfMultiLevelTest.php} (95%) rename tests/Doctrine/Tests/ORM/Functional/{InstanceOfTest.php => Ticket/Ticket4646InstanceOfTest.php} (86%) diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php similarity index 88% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index 1b826a461..b17ca2c2d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -1,12 +1,12 @@ assertCount(1, $result); foreach ($result as $r) { - $this->assertInstanceOf(InstanceOfAbstractTest\Person::class, $r); - $this->assertInstanceOf(InstanceOfAbstractTest\Employee::class, $r); + $this->assertInstanceOf(Person::class, $r); + $this->assertInstanceOf(Employee::class, $r); $this->assertSame('bar', $r->getName()); } } private function loadData() { - $employee = new InstanceOfAbstractTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php similarity index 95% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 131f053a2..79f0d5363 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -1,13 +1,13 @@ assertInstanceOf(Person::class, $r); - if ($r instanceof InstanceOfMultiLevelTest\Engineer) { + if ($r instanceof Engineer) { $this->assertEquals('foobar', $r->getName()); $this->assertEquals('doctrine', $r->getSpecialization()); } elseif ($r instanceof Employee) { diff --git a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php similarity index 86% rename from tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php rename to tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index fce0075b6..326b83393 100644 --- a/tests/Doctrine/Tests/ORM/Functional/InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -1,10 +1,12 @@ assertCount(2, $result); foreach ($result as $r) { - $this->assertInstanceOf(InstanceOfTest\Person::class, $r); - if ($r instanceof InstanceOfTest\Employee) { + $this->assertInstanceOf(Person::class, $r); + if ($r instanceof Employee) { $this->assertEquals('bar', $r->getName()); } else { $this->assertEquals('foo', $r->getName()); @@ -39,10 +41,10 @@ namespace Doctrine\Tests\ORM\Functional { private function loadData() { - $person = new InstanceOfTest\Person(); + $person = new Person(); $person->setName('foo'); - $employee = new InstanceOfTest\Employee(); + $employee = new Employee(); $employee->setName('bar'); $employee->setDepartement('qux'); From 31d2d841601c64d686385ac58cc63778d24c8751 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 3 May 2017 11:42:51 +0200 Subject: [PATCH 17/28] Fix test --- .../Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index 326b83393..35d4afdac 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -13,8 +13,8 @@ namespace Doctrine\Tests\ORM\Functional\Ticket { parent::setUp(); $this->_schemaTool->createSchema([ - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Person'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfTest\Employee'), + $this->_em->getClassMetadata(Person::class), + $this->_em->getClassMetadata(Employee::class), ]); } From 5181eae8d683ffc0367229df40d52da9895568a6 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 09:34:16 +0200 Subject: [PATCH 18/28] Refactor a bit the SqlWalker modifications --- lib/Doctrine/ORM/Query/SqlWalker.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index baf05ffca..33ee45d19 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2277,7 +2277,6 @@ class SqlWalker implements TreeWalker */ private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $discrClass, AST\InstanceOfExpression $instanceOfExpr) { - $knownSubclasses = array_flip($discrClass->subClasses); $sqlParameterList = []; $discriminators = []; foreach ($instanceOfExpr->value as $parameter) { @@ -2289,15 +2288,13 @@ class SqlWalker implements TreeWalker continue; } - // Trim first backslash - $parameter = ltrim($parameter, '\\'); + $metadata = $this->em->getClassMetadata($parameter); - if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) { + if ($metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } // Include discriminators for parameter class and its subclass - $metadata = $this->em->getClassMetadata($parameter); $hierarchyClasses = $metadata->subClasses; $hierarchyClasses[] = $metadata->name; @@ -2305,15 +2302,13 @@ class SqlWalker implements TreeWalker $currentMetadata = $this->em->getClassMetadata($class); $currentDiscriminator = $currentMetadata->discriminatorValue; - if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) { - $discriminators[$currentDiscriminator] = true; + if (null !== $currentDiscriminator && ! array_key_exists($currentDiscriminator, $discriminators)) { + $discriminators[$currentDiscriminator] = null; } } } - foreach (array_keys($discriminators) as $dis) { - $sqlParameterList[] = $this->conn->quote($dis); - } + $sqlParameterList = array_map([$this->conn, 'quote'], array_keys($discriminators)); return '(' . implode(', ', $sqlParameterList) . ')'; } From 167dfde00f047a99495e1098db114f96d4f892f3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 10:11:31 +0200 Subject: [PATCH 19/28] Apply additional fixes to the SqlWalker to fix tests --- lib/Doctrine/ORM/Query/SqlWalker.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 33ee45d19..78179d367 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2290,7 +2290,7 @@ class SqlWalker implements TreeWalker $metadata = $this->em->getClassMetadata($parameter); - if ($metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { + if (! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } @@ -2302,13 +2302,15 @@ class SqlWalker implements TreeWalker $currentMetadata = $this->em->getClassMetadata($class); $currentDiscriminator = $currentMetadata->discriminatorValue; - if (null !== $currentDiscriminator && ! array_key_exists($currentDiscriminator, $discriminators)) { + if (null !== $currentDiscriminator) { $discriminators[$currentDiscriminator] = null; } } } - $sqlParameterList = array_map([$this->conn, 'quote'], array_keys($discriminators)); + foreach (array_keys($discriminators) as $dis) { + $sqlParameterList[] = $this->conn->quote($dis); + } return '(' . implode(', ', $sqlParameterList) . ')'; } From d2f75142486ef8e7fe60e87692ab82f765086b03 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 10:26:31 +0200 Subject: [PATCH 20/28] Put all tests classes in a single namespace --- lib/Doctrine/ORM/Query/SqlWalker.php | 2 +- .../Ticket4646InstanceOfAbstractTest.php | 176 ++++++------- .../Ticket4646InstanceOfMultiLevelTest.php | 244 +++++++++--------- .../Ticket/Ticket4646InstanceOfTest.php | 181 +++++++------ 4 files changed, 293 insertions(+), 310 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 78179d367..8c8ef30d1 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2290,7 +2290,7 @@ class SqlWalker implements TreeWalker $metadata = $this->em->getClassMetadata($parameter); - if (! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { + if ($metadata->getName() !== $discrClass->name && ! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index b17ca2c2d..090a172a9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -1,114 +1,108 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - ]); + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646Abstract::class), + $this->_em->getClassMetadata(EmployeeTicket4646Abstract::class), + ]); + } + + public function testInstanceOf() + { + $this->loadData(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + $this->assertCount(1, $result); + + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646Abstract::class, $r); + $this->assertInstanceOf(EmployeeTicket4646Abstract::class, $r); + $this->assertSame('bar', $r->getName()); } + } - public function testInstanceOf() - { - $this->loadData(); + private function loadData() + { + $employee = new EmployeeTicket4646Abstract(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $this->_em->persist($employee); - $this->assertCount(1, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - $this->assertInstanceOf(Employee::class, $r); - $this->assertSame('bar', $r->getName()); - } - } - - private function loadData() - { - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($employee); - - $this->_em->flush($employee); - } + $this->_em->flush($employee); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest { +/** + * @Entity() + * @Table(name="instance_of_abstract_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "employee": EmployeeTicket4646Abstract::class + * }) + */ +abstract class PersonTicket4646Abstract +{ + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; /** - * @Entity() - * @Table(name="instance_of_abstract_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "employee": Employee::class - * }) + * @Column(type="string") */ - abstract class Person + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_abstract_test_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; + return $this->name; + } - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + public function setName($name) + { + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_abstract_test_employee") + */ +class EmployeeTicket4646Abstract extends PersonTicket4646Abstract +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 79f0d5363..649de27bb 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -1,156 +1,150 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - $this->_em->getClassMetadata(Engineer::class), - ]); - } + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646MultiLevel::class), + $this->_em->getClassMetadata(EmployeeTicket4646MultiLevel::class), + $this->_em->getClassMetadata(EngineerTicket4646MultiLevel::class), + ]); + } - public function testInstanceOf() - { - $this->loadData(); + public function testInstanceOf() + { + $this->loadData(); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); - $this->assertCount(3, $result); + $this->assertCount(3, $result); - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - if ($r instanceof Engineer) { - $this->assertEquals('foobar', $r->getName()); - $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof Employee) { - $this->assertEquals('bar', $r->getName()); - $this->assertEquals('qux', $r->getDepartement()); - } else { - $this->assertEquals('foo', $r->getName()); - } + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646MultiLevel::class, $r); + if ($r instanceof EngineerTicket4646MultiLevel) { + $this->assertEquals('foobar', $r->getName()); + $this->assertEquals('doctrine', $r->getSpecialization()); + } elseif ($r instanceof EmployeeTicket4646MultiLevel) { + $this->assertEquals('bar', $r->getName()); + $this->assertEquals('qux', $r->getDepartement()); + } else { + $this->assertEquals('foo', $r->getName()); } } + } - private function loadData() - { - $person = new Person(); - $person->setName('foo'); + private function loadData() + { + $person = new PersonTicket4646MultiLevel(); + $person->setName('foo'); - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); + $employee = new EmployeeTicket4646MultiLevel(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $engineer = new Engineer(); - $engineer->setName('foobar'); - $engineer->setDepartement('dep'); - $engineer->setSpecialization('doctrine'); + $engineer = new EngineerTicket4646MultiLevel(); + $engineer->setName('foobar'); + $engineer->setDepartement('dep'); + $engineer->setSpecialization('doctrine'); - $this->_em->persist($person); - $this->_em->persist($employee); - $this->_em->persist($engineer); + $this->_em->persist($person); + $this->_em->persist($employee); + $this->_em->persist($engineer); - $this->_em->flush(array($person, $employee, $engineer)); - } + $this->_em->flush(array($person, $employee, $engineer)); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest { +/** + * @Entity() + * @Table(name="instance_of_multi_level_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646MultiLevel", + * "engineer": "Doctrine\Tests\ORM\Functional\Ticket\EngineerTicket4646MultiLevel", + * }) + */ +class PersonTicket4646MultiLevel +{ /** - * @Entity() - * @Table(name="instance_of_multi_level_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "person": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Person", - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Employee", - * "engineer": "Doctrine\Tests\ORM\Functional\InstanceOfMultiLevelTest\Engineer", - * }) + * @Id() + * @GeneratedValue() + * @Column(type="integer") */ - class Person + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_multi_level_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + return $this->name; } - /** - * @Entity() - * @Table(name="instance_of_multi_level_engineer") - */ - class Engineer extends Employee + public function setName($name) { - /** - * @Column(type="string") - */ - private $specialization; - - public function getSpecialization() - { - return $this->specialization; - } - - public function setSpecialization($specialization) - { - $this->specialization = $specialization; - } + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_multi_level_employee") + */ +class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; + } +} + +/** + * @Entity() + * @Table(name="instance_of_multi_level_engineer") + */ +class EngineerTicket4646MultiLevel extends EmployeeTicket4646MultiLevel +{ + /** + * @Column(type="string") + */ + private $specialization; + + public function getSpecialization() + { + return $this->specialization; + } + + public function setSpecialization($specialization) + { + $this->specialization = $specialization; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index 35d4afdac..a83e6e733 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -1,121 +1,116 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Person::class), - $this->_em->getClassMetadata(Employee::class), - ]); - } + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646::class), + $this->_em->getClassMetadata(EmployeeTicket4646::class), + ]); + } - public function testInstanceOf() - { - $this->loadData(); + public function testInstanceOf() + { + $this->loadData(); - $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\InstanceOfTest\Person p - WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\InstanceOfTest\Person'; - $query = $this->_em->createQuery($dql); - $result = $query->getResult(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646 p + WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); - $this->assertCount(2, $result); + $this->assertCount(2, $result); - foreach ($result as $r) { - $this->assertInstanceOf(Person::class, $r); - if ($r instanceof Employee) { - $this->assertEquals('bar', $r->getName()); - } else { - $this->assertEquals('foo', $r->getName()); - } + foreach ($result as $r) { + $this->assertInstanceOf(PersonTicket4646::class, $r); + if ($r instanceof EmployeeTicket4646) { + $this->assertEquals('bar', $r->getName()); + } else { + $this->assertEquals('foo', $r->getName()); } } + } - private function loadData() - { - $person = new Person(); - $person->setName('foo'); + private function loadData() + { + $person = new PersonTicket4646(); + $person->setName('foo'); - $employee = new Employee(); - $employee->setName('bar'); - $employee->setDepartement('qux'); + $employee = new EmployeeTicket4646(); + $employee->setName('bar'); + $employee->setDepartement('qux'); - $this->_em->persist($person); - $this->_em->persist($employee); + $this->_em->persist($person); + $this->_em->persist($employee); - $this->_em->flush(array($person, $employee)); - } + $this->_em->flush(array($person, $employee)); } } -namespace Doctrine\Tests\ORM\Functional\InstanceOfTest { +/** + * @Entity() + * @Table(name="instance_of_test_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646" + * }) + */ +class PersonTicket4646 +{ /** - * @Entity() - * @Table(name="instance_of_test_person") - * @InheritanceType(value="JOINED") - * @DiscriminatorColumn(name="kind", type="string") - * @DiscriminatorMap(value={ - * "person": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Person", - * "employee": "Doctrine\Tests\ORM\Functional\InstanceOfTest\Employee" - * }) + * @Id() + * @GeneratedValue() + * @Column(type="integer") */ - class Person + private $id; + + /** + * @Column(type="string") + */ + private $name; + + public function getId() { - /** - * @Id() - * @GeneratedValue() - * @Column(type="integer") - */ - private $id; - - /** - * @Column(type="string") - */ - private $name; - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } + return $this->id; } - /** - * @Entity() - * @Table(name="instance_of_test_employee") - */ - class Employee extends Person + public function getName() { - /** - * @Column(type="string") - */ - private $departement; + return $this->name; + } - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } + public function setName($name) + { + $this->name = $name; + } +} + +/** + * @Entity() + * @Table(name="instance_of_test_employee") + */ +class EmployeeTicket4646 extends PersonTicket4646 +{ + /** + * @Column(type="string") + */ + private $departement; + + public function getDepartement() + { + return $this->departement; + } + + public function setDepartement($departement) + { + $this->departement = $departement; } } From 2fd875281809ce1de4fa33070c7dc11895312f0e Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Jun 2017 13:25:31 +0200 Subject: [PATCH 21/28] Simplify stubs used in tests --- .../Ticket4646InstanceOfAbstractTest.php | 50 +---------- .../Ticket4646InstanceOfMultiLevelTest.php | 83 ++----------------- .../Ticket/Ticket4646InstanceOfTest.php | 58 +------------ 3 files changed, 12 insertions(+), 179 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index 090a172a9..f0ea53fe8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -18,7 +18,8 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new EmployeeTicket4646Abstract()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Abstract'; @@ -26,23 +27,7 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(1, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646Abstract::class, $r); - $this->assertInstanceOf(EmployeeTicket4646Abstract::class, $r); - $this->assertSame('bar', $r->getName()); - } - } - - private function loadData() - { - $employee = new EmployeeTicket4646Abstract(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($employee); - - $this->_em->flush($employee); + $this->assertContainsOnlyInstancesOf(PersonTicket4646Abstract::class, $result); } } @@ -64,25 +49,10 @@ abstract class PersonTicket4646Abstract */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -91,18 +61,4 @@ abstract class PersonTicket4646Abstract */ class EmployeeTicket4646Abstract extends PersonTicket4646Abstract { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 649de27bb..6e4160b24 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -19,7 +19,10 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new PersonTicket4646MultiLevel()); + $this->_em->persist(new EmployeeTicket4646MultiLevel()); + $this->_em->persist(new EngineerTicket4646MultiLevel()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646MultiLevel'; @@ -27,40 +30,7 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(3, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646MultiLevel::class, $r); - if ($r instanceof EngineerTicket4646MultiLevel) { - $this->assertEquals('foobar', $r->getName()); - $this->assertEquals('doctrine', $r->getSpecialization()); - } elseif ($r instanceof EmployeeTicket4646MultiLevel) { - $this->assertEquals('bar', $r->getName()); - $this->assertEquals('qux', $r->getDepartement()); - } else { - $this->assertEquals('foo', $r->getName()); - } - } - } - - private function loadData() - { - $person = new PersonTicket4646MultiLevel(); - $person->setName('foo'); - - $employee = new EmployeeTicket4646MultiLevel(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $engineer = new EngineerTicket4646MultiLevel(); - $engineer->setName('foobar'); - $engineer->setDepartement('dep'); - $engineer->setSpecialization('doctrine'); - - $this->_em->persist($person); - $this->_em->persist($employee); - $this->_em->persist($engineer); - - $this->_em->flush(array($person, $employee, $engineer)); + $this->assertContainsOnlyInstancesOf(PersonTicket4646MultiLevel::class, $result); } } @@ -84,25 +54,10 @@ class PersonTicket4646MultiLevel */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -111,20 +66,6 @@ class PersonTicket4646MultiLevel */ class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } /** @@ -133,18 +74,4 @@ class EmployeeTicket4646MultiLevel extends PersonTicket4646MultiLevel */ class EngineerTicket4646MultiLevel extends EmployeeTicket4646MultiLevel { - /** - * @Column(type="string") - */ - private $specialization; - - public function getSpecialization() - { - return $this->specialization; - } - - public function setSpecialization($specialization) - { - $this->specialization = $specialization; - } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index a83e6e733..363611d4d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -18,7 +18,9 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase public function testInstanceOf() { - $this->loadData(); + $this->_em->persist(new PersonTicket4646()); + $this->_em->persist(new EmployeeTicket4646()); + $this->_em->flush(); $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646 p WHERE p INSTANCE OF Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646'; @@ -26,30 +28,7 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase $result = $query->getResult(); $this->assertCount(2, $result); - - foreach ($result as $r) { - $this->assertInstanceOf(PersonTicket4646::class, $r); - if ($r instanceof EmployeeTicket4646) { - $this->assertEquals('bar', $r->getName()); - } else { - $this->assertEquals('foo', $r->getName()); - } - } - } - - private function loadData() - { - $person = new PersonTicket4646(); - $person->setName('foo'); - - $employee = new EmployeeTicket4646(); - $employee->setName('bar'); - $employee->setDepartement('qux'); - - $this->_em->persist($person); - $this->_em->persist($employee); - - $this->_em->flush(array($person, $employee)); + $this->assertContainsOnlyInstancesOf(PersonTicket4646::class, $result); } } @@ -72,25 +51,10 @@ class PersonTicket4646 */ private $id; - /** - * @Column(type="string") - */ - private $name; - public function getId() { return $this->id; } - - public function getName() - { - return $this->name; - } - - public function setName($name) - { - $this->name = $name; - } } /** @@ -99,18 +63,4 @@ class PersonTicket4646 */ class EmployeeTicket4646 extends PersonTicket4646 { - /** - * @Column(type="string") - */ - private $departement; - - public function getDepartement() - { - return $this->departement; - } - - public function setDepartement($departement) - { - $this->departement = $departement; - } } From b1f7c59ed560af7770d2ff649d7461605253d1cd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 22 Jun 2017 09:50:53 +0200 Subject: [PATCH 22/28] Adding a failing test case for parameter binding using metadata with INSTANCE OF --- .../Ticket4646InstanceOfParametricTest.php | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php new file mode 100644 index 000000000..302934e8c --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php @@ -0,0 +1,67 @@ +_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646Parametric::class), + $this->_em->getClassMetadata(EmployeeTicket4646Parametric::class), + ]); + } + + public function testInstanceOf() + { + $this->_em->persist(new PersonTicket4646Parametric()); + $this->_em->persist(new EmployeeTicket4646Parametric()); + $this->_em->flush(); + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric p + WHERE p INSTANCE OF :parameter'; + $query = $this->_em->createQuery($dql); + $query->setParameter( + 'parameter', + $this->_em->getClassMetadata(PersonTicket4646Parametric::class) + ); + $result = $query->getResult(); + $this->assertCount(2, $result); + $this->assertContainsOnlyInstancesOf(PersonTicket4646Parametric::class, $result); + } +} + +/** + * @Entity() + * @Table(name="instance_of_parametric_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Parametric", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646Parametric" + * }) + */ +class PersonTicket4646Parametric +{ + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + public function getId() + { + return $this->id; + } +} + +/** + * @Entity() + * @Table(name="instance_of_parametric_employee") + */ +class EmployeeTicket4646Parametric extends PersonTicket4646Parametric +{ +} From e91dcf8fb44c2e15afdcdbf403ef62af88547b45 Mon Sep 17 00:00:00 2001 From: Sergio Santoro Date: Sat, 24 Jun 2017 13:35:44 +0200 Subject: [PATCH 23/28] Fix discriminator resolution when using parameters --- lib/Doctrine/ORM/Query.php | 23 ++++++++++ lib/Doctrine/ORM/Query/ResultSetMapping.php | 7 +++ lib/Doctrine/ORM/Query/SqlWalker.php | 46 +++++++++++-------- .../ORM/Functional/Ticket/DDC1995Test.php | 5 +- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 385b8c8ea..a86cf6201 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -371,6 +371,25 @@ final class Query extends AbstractQuery $this->_em->getCache()->evictEntityRegion($className); } + private function getAllDiscriminators(ClassMetadata $classMetadata) + { + // FIXME: this code is copied from SqlWalker->getAllDiscriminators() + $hierarchyClasses = $classMetadata->subClasses; + $hierarchyClasses[] = $classMetadata->name; + + $discriminators = []; + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->getEntityManager()->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (null !== $currentDiscriminator) { + $discriminators[$currentDiscriminator] = null; + } + } + + return $discriminators; + } + /** * Processes query parameter mappings. * @@ -398,6 +417,10 @@ final class Query extends AbstractQuery $value = $value->getMetadataValue($rsm->metadataParameterMapping[$key]); } + if (isset($rsm->discriminatorParameters[$key]) && $value instanceof ClassMetadata) { + $value = array_keys($this->getAllDiscriminators($value)); + } + $value = $this->processParameterValue($value); $type = ($parameter->getValue() === $value) ? $parameter->getType() diff --git a/lib/Doctrine/ORM/Query/ResultSetMapping.php b/lib/Doctrine/ORM/Query/ResultSetMapping.php index 5c4550980..058ae8d4c 100644 --- a/lib/Doctrine/ORM/Query/ResultSetMapping.php +++ b/lib/Doctrine/ORM/Query/ResultSetMapping.php @@ -168,6 +168,13 @@ class ResultSetMapping */ public $metadataParameterMapping = []; + /** + * Contains query parameter names to be resolved as discriminator values + * + * @var array + */ + public $discriminatorParameters = []; + /** * Adds an entity result to this ResultSetMapping. * diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 8c8ef30d1..3407b0a8b 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2270,42 +2270,29 @@ class SqlWalker implements TreeWalker } /** - * @param ClassMetadataInfo $discrClass + * @param ClassMetadataInfo $rootClass * @param AST\InstanceOfExpression $instanceOfExpr * @return string The list in parentheses of valid child discriminators from the given class * @throws QueryException */ - private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $discrClass, AST\InstanceOfExpression $instanceOfExpr) + private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $rootClass, AST\InstanceOfExpression $instanceOfExpr) { $sqlParameterList = []; $discriminators = []; foreach ($instanceOfExpr->value as $parameter) { if ($parameter instanceof AST\InputParameter) { - $this->rsm->addMetadataParameterMapping($parameter->name, 'discriminatorValue'); - - $sqlParameterList[] = $this->walkInputParameter($parameter); - + $this->rsm->discriminatorParameters[$parameter->name] = $parameter->name; + $sqlParameterList[] = $this->walkInParameter($parameter); continue; } $metadata = $this->em->getClassMetadata($parameter); - if ($metadata->getName() !== $discrClass->name && ! $metadata->getReflectionClass()->isSubclassOf($discrClass->name)) { - throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name); + if ($metadata->getName() !== $rootClass->name && ! $metadata->getReflectionClass()->isSubclassOf($rootClass->name)) { + throw QueryException::instanceOfUnrelatedClass($parameter, $rootClass->name); } - // Include discriminators for parameter class and its subclass - $hierarchyClasses = $metadata->subClasses; - $hierarchyClasses[] = $metadata->name; - - foreach ($hierarchyClasses as $class) { - $currentMetadata = $this->em->getClassMetadata($class); - $currentDiscriminator = $currentMetadata->discriminatorValue; - - if (null !== $currentDiscriminator) { - $discriminators[$currentDiscriminator] = null; - } - } + $discriminators = $discriminators + $this->getAllDiscriminators($metadata); } foreach (array_keys($discriminators) as $dis) { @@ -2314,4 +2301,23 @@ class SqlWalker implements TreeWalker return '(' . implode(', ', $sqlParameterList) . ')'; } + + private function getAllDiscriminators(ClassMetadata $classMetadata) + { + // FIXME: this code is identical to Query->getAllDiscriminators() + $hierarchyClasses = $classMetadata->subClasses; + $hierarchyClasses[] = $classMetadata->name; + + $discriminators = []; + foreach ($hierarchyClasses as $class) { + $currentMetadata = $this->em->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (null !== $currentDiscriminator) { + $discriminators[$currentDiscriminator] = null; + } + } + + return $discriminators; + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1995Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1995Test.php index fb32b99fe..8bb975a05 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1995Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1995Test.php @@ -72,10 +72,9 @@ class DDC1995Test extends \Doctrine\Tests\OrmFunctionalTestCase ->getResult(); $this->assertCount(1, $result1); - $this->assertCount(1, $result2); + $this->assertCount(2, $result2); $this->assertInstanceOf(CompanyEmployee::class, $result1[0]); - $this->assertInstanceOf(CompanyPerson::class, $result2[0]); - $this->assertNotInstanceOf(CompanyEmployee::class, $result2[0]); + $this->assertContainsOnlyInstancesOf(CompanyPerson::class, $result2); } } From d4db126bb07558408ff2d1e112ef0831191dd539 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 18 Aug 2017 10:17:52 +0200 Subject: [PATCH 24/28] Remove code duplication of the getAllDiscriminators method --- lib/Doctrine/ORM/Query.php | 22 ++---------- lib/Doctrine/ORM/Query/SqlWalker.php | 22 ++---------- .../HierarchyDiscriminatorResolver.php | 34 +++++++++++++++++++ 3 files changed, 38 insertions(+), 40 deletions(-) create mode 100644 lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index a86cf6201..222586a1f 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -27,6 +27,7 @@ use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Query\ParameterTypeInferer; use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver; /** * A Query object represents a DQL query. @@ -371,25 +372,6 @@ final class Query extends AbstractQuery $this->_em->getCache()->evictEntityRegion($className); } - private function getAllDiscriminators(ClassMetadata $classMetadata) - { - // FIXME: this code is copied from SqlWalker->getAllDiscriminators() - $hierarchyClasses = $classMetadata->subClasses; - $hierarchyClasses[] = $classMetadata->name; - - $discriminators = []; - foreach ($hierarchyClasses as $class) { - $currentMetadata = $this->getEntityManager()->getClassMetadata($class); - $currentDiscriminator = $currentMetadata->discriminatorValue; - - if (null !== $currentDiscriminator) { - $discriminators[$currentDiscriminator] = null; - } - } - - return $discriminators; - } - /** * Processes query parameter mappings. * @@ -418,7 +400,7 @@ final class Query extends AbstractQuery } if (isset($rsm->discriminatorParameters[$key]) && $value instanceof ClassMetadata) { - $value = array_keys($this->getAllDiscriminators($value)); + $value = array_keys(HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($value, $this->_em)); } $value = $this->processParameterValue($value); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 3407b0a8b..186187025 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -25,6 +25,7 @@ use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; +use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver; use Doctrine\ORM\Utility\PersisterHelper; /** @@ -2292,7 +2293,7 @@ class SqlWalker implements TreeWalker throw QueryException::instanceOfUnrelatedClass($parameter, $rootClass->name); } - $discriminators = $discriminators + $this->getAllDiscriminators($metadata); + $discriminators += HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($metadata, $this->em); } foreach (array_keys($discriminators) as $dis) { @@ -2301,23 +2302,4 @@ class SqlWalker implements TreeWalker return '(' . implode(', ', $sqlParameterList) . ')'; } - - private function getAllDiscriminators(ClassMetadata $classMetadata) - { - // FIXME: this code is identical to Query->getAllDiscriminators() - $hierarchyClasses = $classMetadata->subClasses; - $hierarchyClasses[] = $classMetadata->name; - - $discriminators = []; - foreach ($hierarchyClasses as $class) { - $currentMetadata = $this->em->getClassMetadata($class); - $currentDiscriminator = $currentMetadata->discriminatorValue; - - if (null !== $currentDiscriminator) { - $discriminators[$currentDiscriminator] = null; - } - } - - return $discriminators; - } } diff --git a/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php b/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php new file mode 100644 index 000000000..a8ac100a5 --- /dev/null +++ b/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php @@ -0,0 +1,34 @@ +subClasses; + $hierarchyClasses[] = $rootClassMetadata->name; + + $discriminators = []; + foreach ($hierarchyClasses as $class) { + $currentMetadata = $entityManager->getClassMetadata($class); + $currentDiscriminator = $currentMetadata->discriminatorValue; + + if (null !== $currentDiscriminator) { + $discriminators[$currentDiscriminator] = null; + } + } + + return $discriminators; + } +} From 5224a89549453a8a021fbfd47d93829efecc0f5b Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 18 Aug 2017 11:02:28 +0200 Subject: [PATCH 25/28] Apply various and CS fixes as per review --- .../Utility/HierarchyDiscriminatorResolver.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php b/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php index a8ac100a5..9b08b7f17 100644 --- a/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php +++ b/lib/Doctrine/ORM/Utility/HierarchyDiscriminatorResolver.php @@ -6,20 +6,27 @@ use Doctrine\Common\Persistence\Mapping\ClassMetadata; use Doctrine\ORM\EntityManagerInterface; /** - * Class HierarchyDiscriminatorResolver - * @package Doctrine\ORM\Utility * @internal This class exists only to avoid code duplication, do not reuse it externally */ -class HierarchyDiscriminatorResolver +final class HierarchyDiscriminatorResolver { + private function __construct() + { + } + + /** + * This method is needed to make INSTANCEOF work correctly with inheritance: if the class at hand has inheritance, + * it extracts all the discriminators from the child classes and returns them + */ public static function resolveDiscriminatorsForClass( ClassMetadata $rootClassMetadata, EntityManagerInterface $entityManager - ) { + ): array { $hierarchyClasses = $rootClassMetadata->subClasses; $hierarchyClasses[] = $rootClassMetadata->name; $discriminators = []; + foreach ($hierarchyClasses as $class) { $currentMetadata = $entityManager->getClassMetadata($class); $currentDiscriminator = $currentMetadata->discriminatorValue; From 9864a5a9b9c85abe50a3ce6a897ad347b42f317c Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 18 Aug 2017 12:11:09 +0200 Subject: [PATCH 26/28] Add unit test for HierarchyDiscriminatorResolverTest --- .../HierarchyDiscriminatorResolverTest.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Utility/HierarchyDiscriminatorResolverTest.php diff --git a/tests/Doctrine/Tests/ORM/Utility/HierarchyDiscriminatorResolverTest.php b/tests/Doctrine/Tests/ORM/Utility/HierarchyDiscriminatorResolverTest.php new file mode 100644 index 000000000..cffb8cd3b --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Utility/HierarchyDiscriminatorResolverTest.php @@ -0,0 +1,56 @@ +name = 'Some\Class\Child\Name'; + $childClassMetadata->discriminatorValue = 'child-discriminator'; + + $classMetadata = new ClassMetadata('Entity'); + $classMetadata->subClasses = [$childClassMetadata->name]; + $classMetadata->name = 'Some\Class\Name'; + $classMetadata->discriminatorValue = 'discriminator'; + + $em = $this->prophesize(EntityManagerInterface::class); + $em->getClassMetadata($classMetadata->name) + ->shouldBeCalled() + ->willReturn($classMetadata); + $em->getClassMetadata($childClassMetadata->name) + ->shouldBeCalled() + ->willReturn($childClassMetadata); + + $discriminators = HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($classMetadata, $em->reveal()); + + $this->assertCount(2, $discriminators); + $this->assertArrayHasKey($classMetadata->discriminatorValue, $discriminators); + $this->assertArrayHasKey($childClassMetadata->discriminatorValue, $discriminators); + } + + public function testResolveDiscriminatorsForClassWithNoSubclasses() + { + $classMetadata = new ClassMetadata('Entity'); + $classMetadata->subClasses = []; + $classMetadata->name = 'Some\Class\Name'; + $classMetadata->discriminatorValue = 'discriminator'; + + $em = $this->prophesize(EntityManagerInterface::class); + $em->getClassMetadata($classMetadata->name) + ->shouldBeCalled() + ->willReturn($classMetadata); + + $discriminators = HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($classMetadata, $em->reveal()); + + $this->assertCount(1, $discriminators); + $this->assertArrayHasKey($classMetadata->discriminatorValue, $discriminators); + } +} From 19bc4991aef1cc7a1be2747e937d3b0b38560ebe Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 18 Aug 2017 12:35:51 +0200 Subject: [PATCH 27/28] Add more CS fixes --- lib/Doctrine/ORM/Query/SqlWalker.php | 2 +- .../Ticket/Ticket4646InstanceOfAbstractTest.php | 10 +++++----- .../Ticket/Ticket4646InstanceOfMultiLevelTest.php | 10 +++++----- .../Ticket/Ticket4646InstanceOfParametricTest.php | 10 +++++----- .../ORM/Functional/Ticket/Ticket4646InstanceOfTest.php | 10 +++++----- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 186187025..bc86ec8f6 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -2276,7 +2276,7 @@ class SqlWalker implements TreeWalker * @return string The list in parentheses of valid child discriminators from the given class * @throws QueryException */ - private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $rootClass, AST\InstanceOfExpression $instanceOfExpr) + private function getChildDiscriminatorsFromClassMetadata(ClassMetadataInfo $rootClass, AST\InstanceOfExpression $instanceOfExpr): string { $sqlParameterList = []; $discriminators = []; diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php index f0ea53fe8..5fa5cdc86 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfAbstractTest.php @@ -6,7 +6,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -16,7 +16,7 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase ]); } - public function testInstanceOf() + public function testInstanceOf(): void { $this->_em->persist(new EmployeeTicket4646Abstract()); $this->_em->flush(); @@ -26,8 +26,8 @@ class Ticket4646InstanceOfAbstractTest extends OrmFunctionalTestCase $query = $this->_em->createQuery($dql); $result = $query->getResult(); - $this->assertCount(1, $result); - $this->assertContainsOnlyInstancesOf(PersonTicket4646Abstract::class, $result); + self::assertCount(1, $result); + self::assertContainsOnlyInstancesOf(PersonTicket4646Abstract::class, $result); } } @@ -49,7 +49,7 @@ abstract class PersonTicket4646Abstract */ private $id; - public function getId() + public function getId(): ?int { return $this->id; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php index 6e4160b24..2c111a9e6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfMultiLevelTest.php @@ -6,7 +6,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -17,7 +17,7 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase ]); } - public function testInstanceOf() + public function testInstanceOf(): void { $this->_em->persist(new PersonTicket4646MultiLevel()); $this->_em->persist(new EmployeeTicket4646MultiLevel()); @@ -29,8 +29,8 @@ class Ticket4646InstanceOfMultiLevelTest extends OrmFunctionalTestCase $query = $this->_em->createQuery($dql); $result = $query->getResult(); - $this->assertCount(3, $result); - $this->assertContainsOnlyInstancesOf(PersonTicket4646MultiLevel::class, $result); + self::assertCount(3, $result); + self::assertContainsOnlyInstancesOf(PersonTicket4646MultiLevel::class, $result); } } @@ -54,7 +54,7 @@ class PersonTicket4646MultiLevel */ private $id; - public function getId() + public function getId(): ?int { return $this->id; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php index 302934e8c..bf11c3a85 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfParametricTest.php @@ -6,7 +6,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; class Ticket4646InstanceOfParametricTest extends OrmFunctionalTestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); $this->_schemaTool->createSchema([ @@ -15,7 +15,7 @@ class Ticket4646InstanceOfParametricTest extends OrmFunctionalTestCase ]); } - public function testInstanceOf() + public function testInstanceOf(): void { $this->_em->persist(new PersonTicket4646Parametric()); $this->_em->persist(new EmployeeTicket4646Parametric()); @@ -28,8 +28,8 @@ class Ticket4646InstanceOfParametricTest extends OrmFunctionalTestCase $this->_em->getClassMetadata(PersonTicket4646Parametric::class) ); $result = $query->getResult(); - $this->assertCount(2, $result); - $this->assertContainsOnlyInstancesOf(PersonTicket4646Parametric::class, $result); + self::assertCount(2, $result); + self::assertContainsOnlyInstancesOf(PersonTicket4646Parametric::class, $result); } } @@ -52,7 +52,7 @@ class PersonTicket4646Parametric */ private $id; - public function getId() + public function getId(): ?int { return $this->id; } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php index 363611d4d..0ed97243c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfTest.php @@ -6,7 +6,7 @@ use Doctrine\Tests\OrmFunctionalTestCase; class Ticket4646InstanceOfTest extends OrmFunctionalTestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -16,7 +16,7 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase ]); } - public function testInstanceOf() + public function testInstanceOf(): void { $this->_em->persist(new PersonTicket4646()); $this->_em->persist(new EmployeeTicket4646()); @@ -27,8 +27,8 @@ class Ticket4646InstanceOfTest extends OrmFunctionalTestCase $query = $this->_em->createQuery($dql); $result = $query->getResult(); - $this->assertCount(2, $result); - $this->assertContainsOnlyInstancesOf(PersonTicket4646::class, $result); + self::assertCount(2, $result); + self::assertContainsOnlyInstancesOf(PersonTicket4646::class, $result); } } @@ -51,7 +51,7 @@ class PersonTicket4646 */ private $id; - public function getId() + public function getId(): ?int { return $this->id; } From c799c6da8be7b7d9d15ef119614cee914f51d8c3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 18 Aug 2017 13:10:04 +0200 Subject: [PATCH 28/28] Add new functional test to check usage of INSTANCEOF with multiple parameters --- ...46InstanceOfWithMultipleParametersTest.php | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfWithMultipleParametersTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfWithMultipleParametersTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfWithMultipleParametersTest.php new file mode 100644 index 000000000..c8c172ab6 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Ticket4646InstanceOfWithMultipleParametersTest.php @@ -0,0 +1,88 @@ +_schemaTool->createSchema([ + $this->_em->getClassMetadata(PersonTicket4646Multiple::class), + $this->_em->getClassMetadata(EmployeeTicket4646Multiple::class), + $this->_em->getClassMetadata(ManagerTicket4646Multiple::class), + $this->_em->getClassMetadata(InternTicket4646Multiple::class), + ]); + } + + public function testInstanceOf(): void + { + $this->_em->persist(new PersonTicket4646Multiple()); + $this->_em->persist(new EmployeeTicket4646Multiple()); + $this->_em->persist(new ManagerTicket4646Multiple()); + $this->_em->persist(new InternTicket4646Multiple()); + $this->_em->flush(); + + $dql = 'SELECT p FROM Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Multiple p + WHERE p INSTANCE OF (Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646Multiple, Doctrine\Tests\ORM\Functional\Ticket\InternTicket4646Multiple)'; + $query = $this->_em->createQuery($dql); + $result = $query->getResult(); + + self::assertCount(2, $result); + self::assertContainsOnlyInstancesOf(PersonTicket4646Multiple::class, $result); + } +} + +/** + * @Entity() + * @Table(name="instance_of_test_multiple_person") + * @InheritanceType(value="JOINED") + * @DiscriminatorColumn(name="kind", type="string") + * @DiscriminatorMap(value={ + * "person": "Doctrine\Tests\ORM\Functional\Ticket\PersonTicket4646Multiple", + * "employee": "Doctrine\Tests\ORM\Functional\Ticket\EmployeeTicket4646Multiple", + * "manager": "Doctrine\Tests\ORM\Functional\Ticket\ManagerTicket4646Multiple", + * "intern": "Doctrine\Tests\ORM\Functional\Ticket\InternTicket4646Multiple" + * }) + */ +class PersonTicket4646Multiple +{ + /** + * @Id() + * @GeneratedValue() + * @Column(type="integer") + */ + private $id; + + public function getId() + { + return $this->id; + } +} + +/** + * @Entity() + * @Table(name="instance_of_test_multiple_employee") + */ +class EmployeeTicket4646Multiple extends PersonTicket4646Multiple +{ +} + +/** + * @Entity() + * @Table(name="instance_of_test_multiple_manager") + */ +class ManagerTicket4646Multiple extends PersonTicket4646Multiple +{ +} + +/** + * @Entity() + * @Table(name="instance_of_test_multiple_intern") + */ +class InternTicket4646Multiple extends PersonTicket4646Multiple +{ +}