Skip to content

Commit 152a906

Browse files
buffcodebuffcode
committed
fix: Don't process subclass indexes with SingleCollection inheritance
Fixes #2561
1 parent fa10f36 commit 152a906

File tree

7 files changed

+217
-40
lines changed

7 files changed

+217
-40
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
139139
$class->setIdGeneratorType($parent->generatorType);
140140
$this->addInheritedFields($class, $parent);
141141
$this->addInheritedRelations($class, $parent);
142-
$this->addInheritedIndexes($class, $parent);
142+
if ($parent->isMappedSuperclass) {
143+
// only inherit indexes if parent won't apply them itself
144+
$this->addInheritedIndexes($class, $parent);
145+
}
146+
143147
$this->setInheritedShardKey($class, $parent);
144148
$class->setIdentifier($parent->identifier);
145149
$class->setVersioned($parent->isVersioned);

‎lib/Doctrine/ODM/MongoDB/SchemaManager.php

Lines changed: 56 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ public function updateIndexes(?int $maxTimeMs = null, ?WriteConcern $writeConcer
9292
continue;
9393
}
9494

95+
if ($class->isInheritanceTypeSingleCollection() && count($class->parentClasses) > 0) {
96+
// Skip document nodes that use the same collection as one of their parents.
97+
// Indexes will be added by the parent document.
98+
continue;
99+
}
100+
95101
$this->updateDocumentIndexes($class->name, $maxTimeMs, $writeConcern);
96102
}
97103
}
@@ -172,56 +178,68 @@ private function doGetDocumentIndexes(string $documentName, array &$visited): ar
172178
return [];
173179
}
174180

175-
$visited[$documentName] = true;
176-
177-
$class = $this->dm->getClassMetadata($documentName);
178-
$indexes = $this->prepareIndexes($class);
179-
$embeddedDocumentIndexes = [];
181+
$class = $this->dm->getClassMetadata($documentName);
182+
$processClasses = [$class];
180183

181-
// Add indexes from embedded & referenced documents
182-
foreach ($class->fieldMappings as $fieldMapping) {
183-
if (isset($fieldMapping['embedded'])) {
184-
if (isset($fieldMapping['targetDocument'])) {
185-
$possibleEmbeds = [$fieldMapping['targetDocument']];
186-
} elseif (isset($fieldMapping['discriminatorMap'])) {
187-
$possibleEmbeds = array_unique($fieldMapping['discriminatorMap']);
188-
} else {
189-
continue;
190-
}
184+
if ($class->isInheritanceTypeSingleCollection()) {
185+
// process all subclasses as well
186+
foreach ($class->subClasses as $subClassName) {
187+
$processClasses[] = $this->metadataFactory->getMetadataFor($subClassName);
188+
}
189+
}
191190

192-
foreach ($possibleEmbeds as $embed) {
193-
if (isset($embeddedDocumentIndexes[$embed])) {
194-
$embeddedIndexes = $embeddedDocumentIndexes[$embed];
191+
$indexes = [];
192+
$embeddedDocumentIndexes = [];
193+
foreach ($processClasses as $class) {
194+
$visited[$class->name] = true;
195+
196+
$indexes = array_merge($indexes, $this->prepareIndexes($class));
197+
198+
// Add indexes from embedded & referenced documents
199+
foreach ($class->fieldMappings as $fieldMapping) {
200+
if (isset($fieldMapping['embedded'])) {
201+
if (isset($fieldMapping['targetDocument'])) {
202+
$possibleEmbeds = [$fieldMapping['targetDocument']];
203+
} elseif (isset($fieldMapping['discriminatorMap'])) {
204+
$possibleEmbeds = array_unique($fieldMapping['discriminatorMap']);
195205
} else {
196-
$embeddedIndexes = $this->doGetDocumentIndexes($embed, $visited);
197-
$embeddedDocumentIndexes[$embed] = $embeddedIndexes;
206+
continue;
198207
}
199208

200-
foreach ($embeddedIndexes as $embeddedIndex) {
201-
foreach ($embeddedIndex['keys'] as $key => $value) {
202-
$embeddedIndex['keys'][$fieldMapping['name'] . '.' . $key] = $value;
203-
unset($embeddedIndex['keys'][$key]);
209+
foreach ($possibleEmbeds as $embed) {
210+
if (isset($embeddedDocumentIndexes[$embed])) {
211+
$embeddedIndexes = $embeddedDocumentIndexes[$embed];
212+
} else {
213+
$embeddedIndexes = $this->doGetDocumentIndexes($embed, $visited);
214+
$embeddedDocumentIndexes[$embed] = $embeddedIndexes;
204215
}
205216

206-
if (isset($embeddedIndex['options']['name'])) {
207-
$embeddedIndex['options']['name'] = sprintf('%s_%s', $fieldMapping['name'], $embeddedIndex['options']['name']);
208-
}
217+
foreach ($embeddedIndexes as $embeddedIndex) {
218+
foreach ($embeddedIndex['keys'] as $key => $value) {
219+
$embeddedIndex['keys'][$fieldMapping['name'] . '.' . $key] = $value;
220+
unset($embeddedIndex['keys'][$key]);
221+
}
209222

210-
$indexes[] = $embeddedIndex;
223+
if (isset($embeddedIndex['options']['name'])) {
224+
$embeddedIndex['options']['name'] = sprintf('%s_%s', $fieldMapping['name'], $embeddedIndex['options']['name']);
225+
}
226+
227+
$indexes[] = $embeddedIndex;
228+
}
211229
}
212-
}
213-
} elseif (isset($fieldMapping['reference']) && isset($fieldMapping['targetDocument'])) {
214-
foreach ($indexes as $idx => $index) {
215-
$newKeys = [];
216-
foreach ($index['keys'] as $key => $v) {
217-
if ($key === $fieldMapping['name']) {
218-
$key = ClassMetadata::getReferenceFieldName($fieldMapping['storeAs'], $key);
230+
} elseif (isset($fieldMapping['reference']) && isset($fieldMapping['targetDocument'])) {
231+
foreach ($indexes as $idx => $index) {
232+
$newKeys = [];
233+
foreach ($index['keys'] as $key => $v) {
234+
if ($key === $fieldMapping['name']) {
235+
$key = ClassMetadata::getReferenceFieldName($fieldMapping['storeAs'], $key);
236+
}
237+
238+
$newKeys[$key] = $v;
219239
}
220240

221-
$newKeys[$key] = $v;
241+
$indexes[$idx]['keys'] = $newKeys;
222242
}
223-
224-
$indexes[$idx]['keys'] = $newKeys;
225243
}
226244
}
227245
}

‎tests/Doctrine/ODM/MongoDB/Tests/Functional/IndexesTest.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use Doctrine\ODM\MongoDB\Tests\BaseTestCase;
1010
use MongoDB\Driver\Exception\BulkWriteException;
1111

12+
use function key;
13+
1214
class IndexesTest extends BaseTestCase
1315
{
1416
/** @param class-string $class */
@@ -239,6 +241,30 @@ public function testPartialIndexCreation(): void
239241
self::assertSame(['counter' => ['$gt' => 5]], $indexes[0]['options']['partialFilterExpression']);
240242
self::assertTrue($indexes[0]['options']['unique']);
241243
}
244+
245+
public function testSubclassDoesNotInheritParentIndexes(): void
246+
{
247+
$baseIndexes = $this->dm->getSchemaManager()->getDocumentIndexes(QueryableBaseDocument::class);
248+
$extendedIndexes = $this->dm->getSchemaManager()->getDocumentIndexes(ExtendedSubDocument::class);
249+
250+
self::assertCount(1, $baseIndexes);
251+
self::assertSame('foo', key($baseIndexes[0]['keys']));
252+
253+
self::assertCount(1, $extendedIndexes);
254+
self::assertSame('bar', key($extendedIndexes[0]['keys']));
255+
}
256+
257+
public function testSubclassInheritsSuperclassIndexes(): void
258+
{
259+
$baseIndexes = $this->dm->getSchemaManager()->getDocumentIndexes(NonQueryableBaseDocument::class);
260+
$extendedIndexes = $this->dm->getSchemaManager()->getDocumentIndexes(ExtendedInheritedDocument::class);
261+
262+
self::assertCount(0, $baseIndexes);
263+
264+
self::assertCount(2, $extendedIndexes);
265+
self::assertSame('bar', key($extendedIndexes[0]['keys']));
266+
self::assertSame('foo', key($extendedIndexes[1]['keys']));
267+
}
242268
}
243269

244270
/** @ODM\Document */
@@ -657,3 +683,66 @@ class DocumentWithIndexInDiscriminatedEmbeds
657683
*/
658684
public $embedded;
659685
}
686+
687+
/** @ODM\Document */
688+
class QueryableBaseDocument
689+
{
690+
/**
691+
* @ODM\Id
692+
*
693+
* @var string|null
694+
*/
695+
public $id;
696+
697+
/**
698+
* @ODM\Field(type="string")
699+
* @ODM\Index
700+
*
701+
* @var string|null
702+
*/
703+
public $foo;
704+
}
705+
706+
/** @ODM\Document */
707+
class ExtendedSubDocument extends QueryableBaseDocument
708+
{
709+
/**
710+
* @ODM\Field(type="string")
711+
* @ODM\Index
712+
*
713+
* @var string|null
714+
*/
715+
public $bar;
716+
}
717+
718+
719+
/** @ODM\MappedSuperclass */
720+
class NonQueryableBaseDocument
721+
{
722+
/**
723+
* @ODM\Id
724+
*
725+
* @var string|null
726+
*/
727+
public $id;
728+
729+
/**
730+
* @ODM\Field(type="string")
731+
* @ODM\Index
732+
*
733+
* @var string|null
734+
*/
735+
public $foo;
736+
}
737+
738+
/** @ODM\Document */
739+
class ExtendedInheritedDocument extends NonQueryableBaseDocument
740+
{
741+
/**
742+
* @ODM\Field(type="string")
743+
* @ODM\Index
744+
*
745+
* @var string|null
746+
*/
747+
public $bar;
748+
}

‎tests/Doctrine/ODM/MongoDB/Tests/SchemaManagerTest.php

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Documents\CmsProduct;
1717
use Documents\Comment;
1818
use Documents\File;
19+
use Documents\Project;
1920
use Documents\SchemaValidated;
2021
use Documents\Sharded\ShardedOne;
2122
use Documents\Sharded\ShardedOneWithDifferentKey;
@@ -41,6 +42,7 @@
4142
use function array_map;
4243
use function assert;
4344
use function in_array;
45+
use function key;
4446
use function MongoDB\BSON\fromJSON;
4547
use function MongoDB\BSON\toPHP;
4648

@@ -60,6 +62,7 @@ class SchemaManagerTest extends BaseTestCase
6062
SimpleReferenceUser::class,
6163
ShardedOne::class,
6264
ShardedOneWithDifferentKey::class,
65+
Project::class,
6366
];
6467

6568
/** @psalm-var list<class-string> */
@@ -285,7 +288,7 @@ public function testEnsureDocumentIndexesWithTwoLevelInheritance(array $expected
285288
$collectionName = $this->dm->getClassMetadata(CmsProduct::class)->getCollection();
286289
$collection = $this->documentCollections[$collectionName];
287290
$collection
288-
->expects($this->once())
291+
->expects($this->exactly(2))
289292
->method('createIndex')
290293
->with($this->anything(), $this->writeOptions($expectedWriteOptions));
291294

@@ -317,6 +320,52 @@ public function testUpdateDocumentIndexesShouldCreateMappedIndexes(array $expect
317320
$this->schemaManager->updateDocumentIndexes(CmsArticle::class, $maxTimeMs, $writeConcern);
318321
}
319322

323+
/** @psalm-param IndexOptions $expectedWriteOptions */
324+
public function testUpdateDocumentIndexesShouldCreateIndexesFromMappedSuperclass(): void
325+
{
326+
$collectionName = $this->dm->getClassMetadata(CmsProduct::class)->getCollection();
327+
$collection = $this->documentCollections[$collectionName];
328+
$collection
329+
->expects($this->once())
330+
->method('listIndexes')
331+
->willReturn(new IndexInfoIteratorIterator(new ArrayIterator([])));
332+
$collection
333+
->expects($this->exactly(2))
334+
->method('createIndex')
335+
->with($this->anything(), $this->anything());
336+
$collection
337+
->expects($this->never())
338+
->method('dropIndex')
339+
->with($this->anything());
340+
341+
$this->schemaManager->updateDocumentIndexes(CmsProduct::class);
342+
}
343+
344+
/** @psalm-param IndexOptions $expectedWriteOptions */
345+
public function testUpdateDocumentIndexesShouldCreateIndexFromSubclasses(): void
346+
{
347+
$collectionName = $this->dm->getClassMetadata(Project::class)->getCollection();
348+
$collection = $this->documentCollections[$collectionName];
349+
$collection
350+
->expects($this->once())
351+
->method('listIndexes')
352+
->willReturn(new IndexInfoIteratorIterator(new ArrayIterator([])));
353+
354+
$matcher = $this->exactly(2);
355+
$collection
356+
->expects($matcher)
357+
->method('createIndex')
358+
->willReturnCallback(static function ($key, $value) use ($matcher) {
359+
self::assertSame(['name', 'externalId'][$matcher->numberOfInvocations() - 1], key($key));
360+
});
361+
$collection
362+
->expects($this->never())
363+
->method('dropIndex')
364+
->with($this->anything());
365+
366+
$this->schemaManager->updateDocumentIndexes(Project::class);
367+
}
368+
320369
/**
321370
* @psalm-param IndexOptions $expectedWriteOptions
322371
*

‎tests/Documents/CmsProduct.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,12 @@ class CmsProduct extends CmsContent
1515
* @var string|null
1616
*/
1717
public $name;
18+
19+
/**
20+
* @ODM\Field(type="string")
21+
* @ODM\Index
22+
*
23+
* @var string|null
24+
*/
25+
public $productId;
1826
}

‎tests/Documents/Project.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class Project
2525

2626
/**
2727
* @ODM\Field(type="string")
28+
* @ODM\UniqueIndex
2829
*
2930
* @var string|null
3031
*/

‎tests/Documents/SubProject.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@
1010
/** @ODM\Document */
1111
class SubProject extends Project
1212
{
13+
/**
14+
* @ODM\Field(type="string")
15+
* @ODM\Index
16+
*
17+
* @var string|null
18+
*/
19+
private $externalId;
20+
1321
/**
1422
* @ODM\EmbedMany(targetDocument=Issue::class)
1523
*

0 commit comments

Comments
 (0)