Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Correct DQL INSTANCE OF to filter all possible child classes #6392

Merged
merged 29 commits into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e798bfe
[QUERY] "INSTANCE OF" now behaves correctly with subclasses
taueres Jun 29, 2015
4eb4465
Fix as per review
Jean85 Apr 18, 2017
3219fe5
Fix small CS issues as per review
Jean85 May 3, 2017
11c84c7
Split SqlWalker::walkInstanceOfExpression method
Jean85 May 3, 2017
21e12ef
Move tests to ticket namespace (and rename them)
Jean85 May 3, 2017
96bcee4
Fix test
Jean85 May 3, 2017
30256e7
Refactor a bit the SqlWalker modifications
Jean85 Jun 21, 2017
11c54b7
Apply additional fixes to the SqlWalker to fix tests
Jean85 Jun 21, 2017
8b9c297
Put all tests classes in a single namespace
Jean85 Jun 21, 2017
ba69cc8
Simplify stubs used in tests
Jean85 Jun 21, 2017
d4cdc6e
Adding a failing test case for parameter binding using metadata with …
Jean85 Jun 22, 2017
7d98135
[QUERY] "INSTANCE OF" now behaves correctly with subclasses
taueres Jun 29, 2015
04acde6
Fix as per review
Jean85 Apr 18, 2017
aa233f8
Fix small CS issues as per review
Jean85 May 3, 2017
0e88f1b
Split SqlWalker::walkInstanceOfExpression method
Jean85 May 3, 2017
bd47ec9
Move tests to ticket namespace (and rename them)
Jean85 May 3, 2017
31d2d84
Fix test
Jean85 May 3, 2017
5181eae
Refactor a bit the SqlWalker modifications
Jean85 Jun 21, 2017
167dfde
Apply additional fixes to the SqlWalker to fix tests
Jean85 Jun 21, 2017
d2f7514
Put all tests classes in a single namespace
Jean85 Jun 21, 2017
2fd8752
Simplify stubs used in tests
Jean85 Jun 21, 2017
b1f7c59
Adding a failing test case for parameter binding using metadata with …
Jean85 Jun 22, 2017
e91dcf8
Fix discriminator resolution when using parameters
Jun 24, 2017
c7ef908
Merge additional fix (and master changes) from taueres/fix-instance-o…
Jean85 Jun 26, 2017
d4db126
Remove code duplication of the getAllDiscriminators method
Jean85 Aug 18, 2017
5224a89
Apply various and CS fixes as per review
Jean85 Aug 18, 2017
9864a5a
Add unit test for HierarchyDiscriminatorResolverTest
Jean85 Aug 18, 2017
19bc499
Add more CS fixes
Jean85 Aug 18, 2017
c799c6d
Add new functional test to check usage of INSTANCEOF with multiple pa…
Jean85 Aug 18, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions lib/Doctrine/ORM/Query/SqlWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -2038,8 +2038,10 @@ public function walkInstanceOfExpression($instanceOfExpr)

$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');
Expand All @@ -2049,21 +2051,31 @@ public function walkInstanceOfExpression($instanceOfExpr)
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);
// Check parameter is really in the hierarchy
if ($parameter !== $discrClass->name && ! array_key_exists($parameter, $knownSubclasses)) {
throw QueryException::instanceOfUnrelatedClass($parameter, $discrClass->name);
}

if ( ! isset($discrMap[$entityClassName])) {
throw QueryException::instanceOfUnrelatedClass($entityClassName, $class->rootEntityName);
}
// Include discriminators for parameter class and its subclass
$metadata = $this->em->getClassMetadata($parameter);
$hierarchyClasses = $metadata->subClasses;
$hierarchyClasses[] = $metadata->name;

$discriminatorValue = $discrMap[$entityClassName];
foreach ($hierarchyClasses as $class) {
$currentMetadata = $this->em->getClassMetadata($class);
$currentDiscriminator = $currentMetadata->discriminatorValue;

if (is_string($currentDiscriminator) && ! array_key_exists($currentDiscriminator, $discriminators)) {
$discriminators[$currentDiscriminator] = true;
}
}
}

$sqlParameterList[] = $this->conn->quote($discriminatorValue);
foreach (array_keys($discriminators) as $dis) {
$sqlParameterList[] = $this->conn->quote($dis);
}

$sql .= '(' . implode(', ', $sqlParameterList) . ')';
Expand Down
113 changes: 113 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/InstanceOfAbstractTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php

namespace Doctrine\Tests\ORM\Functional {

use Doctrine\Tests\OrmFunctionalTestCase;

class InstanceOfAbstractTest extends OrmFunctionalTestCase
{
protected function setUp()
{
parent::setUp();

$this->_schemaTool->createSchema(array(
Copy link
Member

Choose a reason for hiding this comment

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

Please use short array syntax

$this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Person'),
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

$this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfAbstractTest\Employee'),
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

));
}

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);
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use ::class constant instead of hardcoding the QCN here.

$this->assertInstanceOf('Doctrine\Tests\ORM\Functional\InstanceOfAbstractTest\Employee', $r);
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

$this->assertEquals('bar', $r->getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use assertSame() instead.

}
}

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"
Copy link
Member

Choose a reason for hiding this comment

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

You can use "employee": Employee::class

* })
*/
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;
}
}

}
153 changes: 153 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/InstanceOfMultiLevelTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
<?php

namespace Doctrine\Tests\ORM\Functional {

use Doctrine\Tests\OrmFunctionalTestCase;

class InstanceOfMultiLevelTest extends OrmFunctionalTestCase
{
protected function setUp()
{
parent::setUp();

$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Person'),
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

$this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Employee'),
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

$this->_em->getClassMetadata(__NAMESPACE__ . '\InstanceOfMultiLevelTest\Engineer'),
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

));
}

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);
Copy link
Member

Choose a reason for hiding this comment

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

Please use ::class syntax

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;
}
}
}
Loading