diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index b40b805bb5..b1d2979dd1 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -17,7 +17,7 @@ use ODM 2.0 directly. * `doctrine/mongodb` is no longer used by ODM. If you've been relying on its functionality, please update accordingly. Most utility classes from - `doctrine/mongodb` have been merged into their ODM counterparts. Classes + `doctrine/mongodb` have been merged into their ODM counterparts. Classes handling connections to MongoDB servers are being replaced by the MongoDB library (`mongodb/mongodb`). * 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: cursor. * The `eagerCursor` helper in `Doctrine\ODM\MongoDB\Query\Builder` and its logic have been removed entirely without replacement. +* Querying for a mapped superclass in a complex inheritance chain will now only + return children of that specific class instead of all classes in the + inheritance tree. ## Schema manager diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php index 90a2d7f11f..c96c1246fc 100644 --- a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php +++ b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php @@ -784,6 +784,10 @@ public function setDiscriminatorMap(array $map) : void throw MappingException::discriminatorNotAllowedForGridFS($this->name); } + $this->subClasses = []; + $this->discriminatorMap = []; + $this->discriminatorValue = null; + foreach ($map as $value => $className) { $this->discriminatorMap[$value] = $className; if ($this->name === $className) { diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 4377d001cd..f2c0b05c52 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -124,7 +124,7 @@ public function __construct( $this->class = $class; $this->cp = $this->uow->getCollectionPersister(); - if ($class->isMappedSuperclass || $class->isEmbeddedDocument || $class->isQueryResultDocument) { + if ($class->isEmbeddedDocument || $class->isQueryResultDocument) { return; } @@ -918,22 +918,31 @@ public function prepareFieldName(string $fieldName) : string /** * Adds discriminator criteria to an already-prepared query. * + * If the class we're querying has a discriminator field set, we add all + * possible discriminator values to the query. The list of possible + * discriminator values is based on the discriminatorValue of the class + * itself as well as those of all its subclasses. + * * This method should be used once for query criteria and not be used for * nested expressions. It should be called before * {@link DocumentPerister::addFilterToPreparedQuery()}. */ public function addDiscriminatorToPreparedQuery(array $preparedQuery) : array { - /* If the class has a discriminator field, which is not already in the - * criteria, inject it now. The field/values need no preparation. - */ - if ($this->class->hasDiscriminator() && ! isset($preparedQuery[$this->class->discriminatorField])) { - $discriminatorValues = $this->getClassDiscriminatorValues($this->class); - if (count($discriminatorValues) === 1) { - $preparedQuery[$this->class->discriminatorField] = $discriminatorValues[0]; - } else { - $preparedQuery[$this->class->discriminatorField] = ['$in' => $discriminatorValues]; - } + if (isset($preparedQuery[$this->class->discriminatorField]) || $this->class->discriminatorField === null) { + return $preparedQuery; + } + + $discriminatorValues = $this->getClassDiscriminatorValues($this->class); + + if ($discriminatorValues === []) { + return $preparedQuery; + } + + if (count($discriminatorValues) === 1) { + $preparedQuery[$this->class->discriminatorField] = $discriminatorValues[0]; + } else { + $preparedQuery[$this->class->discriminatorField] = ['$in' => $discriminatorValues]; } return $preparedQuery; @@ -1281,11 +1290,16 @@ private function hasQueryOperators($value) : bool } /** - * Gets the array of discriminator values for the given ClassMetadata + * Returns the list of discriminator values for the given ClassMetadata */ private function getClassDiscriminatorValues(ClassMetadata $metadata) : array { - $discriminatorValues = [$metadata->discriminatorValue]; + $discriminatorValues = []; + + if ($metadata->discriminatorValue !== null) { + $discriminatorValues[] = $metadata->discriminatorValue; + } + foreach ($metadata->subClasses as $className) { $key = array_search($className, $metadata->discriminatorMap); if (! $key) { diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1962Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1962Test.php new file mode 100644 index 0000000000..0150c95ccd --- /dev/null +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1962Test.php @@ -0,0 +1,141 @@ +dm->getClassMetadata(GH1962Superclass::class); + self::assertCount(3, $metadata->discriminatorMap); + + $metadata = $this->dm->getClassMetadata(GH1962BarSuperclass::class); + self::assertCount(2, $metadata->discriminatorMap); + } + + public function testFetchingDiscriminatedDocuments() + { + $foo = new GH1962FooDocument(); + $bar = new GH1962BarDocument(); + $baz = new GH1962BazDocument(); + + $this->dm->persist($foo); + $this->dm->persist($bar); + $this->dm->persist($baz); + + $this->dm->flush(); + $this->dm->clear(); + + self::assertCount(3, $this->dm->getRepository(GH1962Superclass::class)->findAll()); + self::assertCount(2, $this->dm->getRepository(GH1962BarSuperclass::class)->findAll()); + + self::assertCount(1, $this->dm->getRepository(GH1962FooDocument::class)->findAll()); + self::assertCount(1, $this->dm->getRepository(GH1962BarDocument::class)->findAll()); + self::assertCount(1, $this->dm->getRepository(GH1962BazDocument::class)->findAll()); + } + + public function testFetchingDiscriminatedDocumentsWithoutDiscriminatorMap() + { + $foo = new GH1962FooDocumentWithoutDiscriminatorMap(); + $bar = new GH1962BarDocumentWithoutDiscriminatorMap(); + $baz = new GH1962BazDocumentWithoutDiscriminatorMap(); + + $this->dm->persist($foo); + $this->dm->persist($bar); + $this->dm->persist($baz); + + $this->dm->flush(); + $this->dm->clear(); + + // Since the extending superclass does not know about its child classes, + // it will yield the same results as querying for the parent class + // Without discriminator maps, only leafs in the inheritance tree will + // use a discriminator value in the query + self::assertCount(3, $this->dm->getRepository(GH1962SuperclassWithoutDiscriminatorMap::class)->findAll()); + self::assertCount(3, $this->dm->getRepository(GH1962BarSuperclassWithoutDiscriminatorMap::class)->findAll()); + + self::assertCount(3, $this->dm->getRepository(GH1962FooDocumentWithoutDiscriminatorMap::class)->findAll()); + self::assertCount(3, $this->dm->getRepository(GH1962BarDocumentWithoutDiscriminatorMap::class)->findAll()); + self::assertCount(1, $this->dm->getRepository(GH1962BazDocumentWithoutDiscriminatorMap::class)->findAll()); + } +} + +/** + * @ODM\MappedSuperclass() + * @ODM\DiscriminatorField("type") + * @ODM\InheritanceType("SINGLE_COLLECTION") + * @ODM\DiscriminatorMap({ + * "foo"=GH1962FooDocument::class, + * "bar"=GH1962BarDocument::class, + * "baz"=GH1962BazDocument::class + * }) + */ +class GH1962Superclass +{ + /** @ODM\Id */ + public $id; +} + +/** @ODM\Document */ +class GH1962FooDocument extends GH1962Superclass +{ +} + +/** + * @ODM\MappedSuperclass() + * @ODM\DiscriminatorMap({ + * "bar"=GH1962BarDocument::class, + * "baz"=GH1962BazDocument::class + * }) + */ +class GH1962BarSuperclass extends GH1962Superclass +{ +} + +/** @ODM\Document */ +class GH1962BarDocument extends GH1962BarSuperclass +{ +} + +/** @ODM\Document */ +class GH1962BazDocument extends GH1962BarSuperclass +{ +} +/** + * @ODM\MappedSuperclass() + * @ODM\DiscriminatorField("type") + * @ODM\InheritanceType("SINGLE_COLLECTION") + */ +class GH1962SuperclassWithoutDiscriminatorMap +{ + /** @ODM\Id */ + public $id; +} + +/** @ODM\Document */ +class GH1962FooDocumentWithoutDiscriminatorMap extends GH1962SuperclassWithoutDiscriminatorMap +{ +} + +/** @ODM\MappedSuperclass() */ +class GH1962BarSuperclassWithoutDiscriminatorMap extends GH1962SuperclassWithoutDiscriminatorMap +{ +} + +/** @ODM\Document */ +class GH1962BarDocumentWithoutDiscriminatorMap extends GH1962BarSuperclassWithoutDiscriminatorMap +{ +} + +/** + * @ODM\Document + * @ODM\DiscriminatorValue(GH1962BazDocumentWithoutDiscriminatorMap::class) + */ +class GH1962BazDocumentWithoutDiscriminatorMap extends GH1962BarSuperclassWithoutDiscriminatorMap +{ +} diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php index 0eb5f14441..420bf1dbee 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php @@ -39,13 +39,25 @@ public function testReferencesGoesThroughDiscriminatorMap() ->field('featureFull')->references($f) ->getQuery()->debug(); - $this->assertEquals([ 'featureFull.$id' => new ObjectId($f->id) ], $q1['query']); + $this->assertEquals( + [ + 'featureFull.$id' => new ObjectId($f->id), + 'type' => ['$in' => ['ca', 'cb', 'cc']], + ], + $q1['query'] + ); $q2 = $this->dm->createQueryBuilder(ParentClass::class) ->field('featureSimple')->references($f) ->getQuery()->debug(); - $this->assertEquals([ 'featureSimple' => new ObjectId($f->id) ], $q2['query']); + $this->assertEquals( + [ + 'featureSimple' => new ObjectId($f->id), + 'type' => ['$in' => ['ca', 'cb', 'cc']], + ], + $q2['query'] + ); $q3 = $this->dm->createQueryBuilder(ParentClass::class) ->field('featurePartial')->references($f) @@ -55,6 +67,7 @@ public function testReferencesGoesThroughDiscriminatorMap() [ 'featurePartial.$id' => new ObjectId($f->id), 'featurePartial.$ref' => 'Feature', + 'type' => ['$in' => ['ca', 'cb', 'cc']], ], $q3['query'] ); @@ -97,13 +110,27 @@ public function testIncludesReferenceToGoesThroughDiscriminatorMap() ->field('featureFullMany')->includesReferenceTo($f) ->getQuery()->debug(); - $this->assertEquals([ 'featureFullMany' => [ '$elemMatch' => [ '$id' => new ObjectId($f->id) ] ] ], $q1['query']); + $this->assertEquals( + [ + 'featureFullMany' => [ + '$elemMatch' => ['$id' => new ObjectId($f->id)], + ], + 'type' => ['$in' => ['ca', 'cb', 'cc']], + ], + $q1['query'] + ); $q2 = $this->dm->createQueryBuilder(ParentClass::class) ->field('featureSimpleMany')->includesReferenceTo($f) ->getQuery()->debug(); - $this->assertEquals([ 'featureSimpleMany' => new ObjectId($f->id) ], $q2['query']); + $this->assertEquals( + [ + 'featureSimpleMany' => new ObjectId($f->id), + 'type' => ['$in' => ['ca', 'cb', 'cc']], + ], + $q2['query'] + ); $q3 = $this->dm->createQueryBuilder(ParentClass::class) ->field('featurePartialMany')->includesReferenceTo($f) @@ -117,6 +144,7 @@ public function testIncludesReferenceToGoesThroughDiscriminatorMap() '$ref' => 'Feature', ], ], + 'type' => ['$in' => ['ca', 'cb', 'cc']], ], $q3['query'] );