Skip to content

Commit 89c4da8

Browse files
malarzmmalarzm
authored
Merge pull request #2025 from alcaeus/fix-wrong-discriminator-inheritance
Fix wrong usage of discriminator map in complex document inheritance chains
2 parents 51093a4 + e7d240c commit 89c4da8

File tree

5 files changed

+208
-18
lines changed

5 files changed

+208
-18
lines changed

‎UPGRADE-2.0.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
use ODM 2.0 directly.
1818
* `doctrine/mongodb` is no longer used by ODM. If you've been relying on its
1919
functionality, please update accordingly. Most utility classes from
20-
`doctrine/mongodb` have been merged into their ODM counterparts. Classes
20+
`doctrine/mongodb` have been merged into their ODM counterparts. Classes
2121
handling connections to MongoDB servers are being replaced by the MongoDB
2222
library (`mongodb/mongodb`).
2323
* The constructor signature of `Doctrine\ODM\MongoDB\DocumentManager` as well as
@@ -261,6 +261,9 @@ ocramius. If you are checking for proxies, the following changed:
261261
cursor.
262262
* The `eagerCursor` helper in `Doctrine\ODM\MongoDB\Query\Builder` and its logic
263263
have been removed entirely without replacement.
264+
* Querying for a mapped superclass in a complex inheritance chain will now only
265+
return children of that specific class instead of all classes in the
266+
inheritance tree.
264267

265268
## Schema manager
266269

‎lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,10 @@ public function setDiscriminatorMap(array $map) : void
784784
throw MappingException::discriminatorNotAllowedForGridFS($this->name);
785785
}
786786

787+
$this->subClasses = [];
788+
$this->discriminatorMap = [];
789+
$this->discriminatorValue = null;
790+
787791
foreach ($map as $value => $className) {
788792
$this->discriminatorMap[$value] = $className;
789793
if ($this->name === $className) {

‎lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public function __construct(
124124
$this->class = $class;
125125
$this->cp = $this->uow->getCollectionPersister();
126126

127-
if ($class->isMappedSuperclass || $class->isEmbeddedDocument || $class->isQueryResultDocument) {
127+
if ($class->isEmbeddedDocument || $class->isQueryResultDocument) {
128128
return;
129129
}
130130

@@ -918,22 +918,31 @@ public function prepareFieldName(string $fieldName) : string
918918
/**
919919
* Adds discriminator criteria to an already-prepared query.
920920
*
921+
* If the class we're querying has a discriminator field set, we add all
922+
* possible discriminator values to the query. The list of possible
923+
* discriminator values is based on the discriminatorValue of the class
924+
* itself as well as those of all its subclasses.
925+
*
921926
* This method should be used once for query criteria and not be used for
922927
* nested expressions. It should be called before
923928
* {@link DocumentPerister::addFilterToPreparedQuery()}.
924929
*/
925930
public function addDiscriminatorToPreparedQuery(array $preparedQuery) : array
926931
{
927-
/* If the class has a discriminator field, which is not already in the
928-
* criteria, inject it now. The field/values need no preparation.
929-
*/
930-
if ($this->class->hasDiscriminator() && ! isset($preparedQuery[$this->class->discriminatorField])) {
931-
$discriminatorValues = $this->getClassDiscriminatorValues($this->class);
932-
if (count($discriminatorValues) === 1) {
933-
$preparedQuery[$this->class->discriminatorField] = $discriminatorValues[0];
934-
} else {
935-
$preparedQuery[$this->class->discriminatorField] = ['$in' => $discriminatorValues];
936-
}
932+
if (isset($preparedQuery[$this->class->discriminatorField]) || $this->class->discriminatorField === null) {
933+
return $preparedQuery;
934+
}
935+
936+
$discriminatorValues = $this->getClassDiscriminatorValues($this->class);
937+
938+
if ($discriminatorValues === []) {
939+
return $preparedQuery;
940+
}
941+
942+
if (count($discriminatorValues) === 1) {
943+
$preparedQuery[$this->class->discriminatorField] = $discriminatorValues[0];
944+
} else {
945+
$preparedQuery[$this->class->discriminatorField] = ['$in' => $discriminatorValues];
937946
}
938947

939948
return $preparedQuery;
@@ -1281,11 +1290,16 @@ private function hasQueryOperators($value) : bool
12811290
}
12821291

12831292
/**
1284-
* Gets the array of discriminator values for the given ClassMetadata
1293+
* Returns the list of discriminator values for the given ClassMetadata
12851294
*/
12861295
private function getClassDiscriminatorValues(ClassMetadata $metadata) : array
12871296
{
1288-
$discriminatorValues = [$metadata->discriminatorValue];
1297+
$discriminatorValues = [];
1298+
1299+
if ($metadata->discriminatorValue !== null) {
1300+
$discriminatorValues[] = $metadata->discriminatorValue;
1301+
}
1302+
12891303
foreach ($metadata->subClasses as $className) {
12901304
$key = array_search($className, $metadata->discriminatorMap);
12911305
if (! $key) {

‎tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1962Test.php‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;
6+
7+
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
8+
use Doctrine\ODM\MongoDB\Tests\BaseTest;
9+
10+
class GH1962Test extends BaseTest
11+
{
12+
public function testDiscriminatorMaps()
13+
{
14+
$metadata = $this->dm->getClassMetadata(GH1962Superclass::class);
15+
self::assertCount(3, $metadata->discriminatorMap);
16+
17+
$metadata = $this->dm->getClassMetadata(GH1962BarSuperclass::class);
18+
self::assertCount(2, $metadata->discriminatorMap);
19+
}
20+
21+
public function testFetchingDiscriminatedDocuments()
22+
{
23+
$foo = new GH1962FooDocument();
24+
$bar = new GH1962BarDocument();
25+
$baz = new GH1962BazDocument();
26+
27+
$this->dm->persist($foo);
28+
$this->dm->persist($bar);
29+
$this->dm->persist($baz);
30+
31+
$this->dm->flush();
32+
$this->dm->clear();
33+
34+
self::assertCount(3, $this->dm->getRepository(GH1962Superclass::class)->findAll());
35+
self::assertCount(2, $this->dm->getRepository(GH1962BarSuperclass::class)->findAll());
36+
37+
self::assertCount(1, $this->dm->getRepository(GH1962FooDocument::class)->findAll());
38+
self::assertCount(1, $this->dm->getRepository(GH1962BarDocument::class)->findAll());
39+
self::assertCount(1, $this->dm->getRepository(GH1962BazDocument::class)->findAll());
40+
}
41+
42+
public function testFetchingDiscriminatedDocumentsWithoutDiscriminatorMap()
43+
{
44+
$foo = new GH1962FooDocumentWithoutDiscriminatorMap();
45+
$bar = new GH1962BarDocumentWithoutDiscriminatorMap();
46+
$baz = new GH1962BazDocumentWithoutDiscriminatorMap();
47+
48+
$this->dm->persist($foo);
49+
$this->dm->persist($bar);
50+
$this->dm->persist($baz);
51+
52+
$this->dm->flush();
53+
$this->dm->clear();
54+
55+
// Since the extending superclass does not know about its child classes,
56+
// it will yield the same results as querying for the parent class
57+
// Without discriminator maps, only leafs in the inheritance tree will
58+
// use a discriminator value in the query
59+
self::assertCount(3, $this->dm->getRepository(GH1962SuperclassWithoutDiscriminatorMap::class)->findAll());
60+
self::assertCount(3, $this->dm->getRepository(GH1962BarSuperclassWithoutDiscriminatorMap::class)->findAll());
61+
62+
self::assertCount(3, $this->dm->getRepository(GH1962FooDocumentWithoutDiscriminatorMap::class)->findAll());
63+
self::assertCount(3, $this->dm->getRepository(GH1962BarDocumentWithoutDiscriminatorMap::class)->findAll());
64+
self::assertCount(1, $this->dm->getRepository(GH1962BazDocumentWithoutDiscriminatorMap::class)->findAll());
65+
}
66+
}
67+
68+
/**
69+
* @ODM\MappedSuperclass()
70+
* @ODM\DiscriminatorField("type")
71+
* @ODM\InheritanceType("SINGLE_COLLECTION")
72+
* @ODM\DiscriminatorMap({
73+
* "foo"=GH1962FooDocument::class,
74+
* "bar"=GH1962BarDocument::class,
75+
* "baz"=GH1962BazDocument::class
76+
* })
77+
*/
78+
class GH1962Superclass
79+
{
80+
/** @ODM\Id */
81+
public $id;
82+
}
83+
84+
/** @ODM\Document */
85+
class GH1962FooDocument extends GH1962Superclass
86+
{
87+
}
88+
89+
/**
90+
* @ODM\MappedSuperclass()
91+
* @ODM\DiscriminatorMap({
92+
* "bar"=GH1962BarDocument::class,
93+
* "baz"=GH1962BazDocument::class
94+
* })
95+
*/
96+
class GH1962BarSuperclass extends GH1962Superclass
97+
{
98+
}
99+
100+
/** @ODM\Document */
101+
class GH1962BarDocument extends GH1962BarSuperclass
102+
{
103+
}
104+
105+
/** @ODM\Document */
106+
class GH1962BazDocument extends GH1962BarSuperclass
107+
{
108+
}
109+
/**
110+
* @ODM\MappedSuperclass()
111+
* @ODM\DiscriminatorField("type")
112+
* @ODM\InheritanceType("SINGLE_COLLECTION")
113+
*/
114+
class GH1962SuperclassWithoutDiscriminatorMap
115+
{
116+
/** @ODM\Id */
117+
public $id;
118+
}
119+
120+
/** @ODM\Document */
121+
class GH1962FooDocumentWithoutDiscriminatorMap extends GH1962SuperclassWithoutDiscriminatorMap
122+
{
123+
}
124+
125+
/** @ODM\MappedSuperclass() */
126+
class GH1962BarSuperclassWithoutDiscriminatorMap extends GH1962SuperclassWithoutDiscriminatorMap
127+
{
128+
}
129+
130+
/** @ODM\Document */
131+
class GH1962BarDocumentWithoutDiscriminatorMap extends GH1962BarSuperclassWithoutDiscriminatorMap
132+
{
133+
}
134+
135+
/**
136+
* @ODM\Document
137+
* @ODM\DiscriminatorValue(GH1962BazDocumentWithoutDiscriminatorMap::class)
138+
*/
139+
class GH1962BazDocumentWithoutDiscriminatorMap extends GH1962BarSuperclassWithoutDiscriminatorMap
140+
{
141+
}

‎tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,25 @@ public function testReferencesGoesThroughDiscriminatorMap()
3939
->field('featureFull')->references($f)
4040
->getQuery()->debug();
4141

42-
$this->assertEquals([ 'featureFull.$id' => new ObjectId($f->id) ], $q1['query']);
42+
$this->assertEquals(
43+
[
44+
'featureFull.$id' => new ObjectId($f->id),
45+
'type' => ['$in' => ['ca', 'cb', 'cc']],
46+
],
47+
$q1['query']
48+
);
4349

4450
$q2 = $this->dm->createQueryBuilder(ParentClass::class)
4551
->field('featureSimple')->references($f)
4652
->getQuery()->debug();
4753

48-
$this->assertEquals([ 'featureSimple' => new ObjectId($f->id) ], $q2['query']);
54+
$this->assertEquals(
55+
[
56+
'featureSimple' => new ObjectId($f->id),
57+
'type' => ['$in' => ['ca', 'cb', 'cc']],
58+
],
59+
$q2['query']
60+
);
4961

5062
$q3 = $this->dm->createQueryBuilder(ParentClass::class)
5163
->field('featurePartial')->references($f)
@@ -55,6 +67,7 @@ public function testReferencesGoesThroughDiscriminatorMap()
5567
[
5668
'featurePartial.$id' => new ObjectId($f->id),
5769
'featurePartial.$ref' => 'Feature',
70+
'type' => ['$in' => ['ca', 'cb', 'cc']],
5871
],
5972
$q3['query']
6073
);
@@ -97,13 +110,27 @@ public function testIncludesReferenceToGoesThroughDiscriminatorMap()
97110
->field('featureFullMany')->includesReferenceTo($f)
98111
->getQuery()->debug();
99112

100-
$this->assertEquals([ 'featureFullMany' => [ '$elemMatch' => [ '$id' => new ObjectId($f->id) ] ] ], $q1['query']);
113+
$this->assertEquals(
114+
[
115+
'featureFullMany' => [
116+
'$elemMatch' => ['$id' => new ObjectId($f->id)],
117+
],
118+
'type' => ['$in' => ['ca', 'cb', 'cc']],
119+
],
120+
$q1['query']
121+
);
101122

102123
$q2 = $this->dm->createQueryBuilder(ParentClass::class)
103124
->field('featureSimpleMany')->includesReferenceTo($f)
104125
->getQuery()->debug();
105126

106-
$this->assertEquals([ 'featureSimpleMany' => new ObjectId($f->id) ], $q2['query']);
127+
$this->assertEquals(
128+
[
129+
'featureSimpleMany' => new ObjectId($f->id),
130+
'type' => ['$in' => ['ca', 'cb', 'cc']],
131+
],
132+
$q2['query']
133+
);
107134

108135
$q3 = $this->dm->createQueryBuilder(ParentClass::class)
109136
->field('featurePartialMany')->includesReferenceTo($f)
@@ -117,6 +144,7 @@ public function testIncludesReferenceToGoesThroughDiscriminatorMap()
117144
'$ref' => 'Feature',
118145
],
119146
],
147+
'type' => ['$in' => ['ca', 'cb', 'cc']],
120148
],
121149
$q3['query']
122150
);

0 commit comments

Comments
 (0)