From 8ac8b3afe00c74856fe0c87539e97b332c60c2bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 14 Oct 2024 13:22:04 +0200 Subject: [PATCH 1/5] Fix dropping search index (#2689) Error "Cannot use object of type stdClass as array" --- lib/Doctrine/ODM/MongoDB/SchemaManager.php | 8 ++++++-- psalm-baseline.xml | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/SchemaManager.php b/lib/Doctrine/ODM/MongoDB/SchemaManager.php index b142f1902..160be2328 100644 --- a/lib/Doctrine/ODM/MongoDB/SchemaManager.php +++ b/lib/Doctrine/ODM/MongoDB/SchemaManager.php @@ -413,7 +413,9 @@ public function updateDocumentSearchIndexes(string $documentName): void $definedNames = array_column($searchIndexes, 'name'); try { - $existingNames = array_column(iterator_to_array($collection->listSearchIndexes()), 'name'); + /* The typeMap option can be removed when bug is fixed in the minimum required version. + * https://jira.mongodb.org/browse/PHPLIB-1548 */ + $existingNames = array_column(iterator_to_array($collection->listSearchIndexes(['typeMap' => ['root' => 'array']])), 'name'); } catch (CommandException $e) { /* If $listSearchIndexes doesn't exist, only throw if search indexes have been defined. * If no search indexes are defined and the server doesn't support search indexes, there's @@ -465,7 +467,9 @@ public function deleteDocumentSearchIndexes(string $documentName): void $collection = $this->dm->getDocumentCollection($class->name); try { - $searchIndexes = $collection->listSearchIndexes(); + /* The typeMap option can be removed when bug is fixed in the minimum required version. + * https://jira.mongodb.org/browse/PHPLIB-1548 */ + $searchIndexes = $collection->listSearchIndexes(['typeMap' => ['root' => 'array']]); } catch (CommandException $e) { // If the server does not support search indexes, there are no indexes to remove in any case if ($this->isSearchIndexCommandException($e)) { diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 110172f14..221a7a95f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -184,6 +184,12 @@ + + + ['root' => 'array']]]]> + ['root' => 'array']]]]> + + From e4c971f63e60a1e3274a9f852e1ee2028558d1ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 22 Oct 2024 10:41:37 +0200 Subject: [PATCH 2/5] Remove usage of deprecated ReadPreference::RP_* constants (#2693) --- .../Tests/Functional/ReadPreferenceTest.php | 18 +++++++++--------- .../Tests/Functional/ReferencePrimerTest.php | 2 +- .../ODM/MongoDB/Tests/Query/BuilderTest.php | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReadPreferenceTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReadPreferenceTest.php index 8135cce28..1ad64db3a 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReadPreferenceTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReadPreferenceTest.php @@ -46,7 +46,7 @@ public function testHintIsNotSetByDefault(): void /** @psalm-param ReadPreferenceTagShape[] $tags */ #[DataProvider('provideReadPreferenceHints')] - public function testHintIsSetOnQuery(int $readPreference, array $tags = []): void + public function testHintIsSetOnQuery(string $readPreference, array $tags = []): void { $this->skipTestIfSharded(User::class); @@ -68,9 +68,9 @@ public function testHintIsSetOnQuery(int $readPreference, array $tags = []): voi public static function provideReadPreferenceHints(): array { return [ - [ReadPreference::RP_PRIMARY, []], - [ReadPreference::RP_SECONDARY_PREFERRED, []], - [ReadPreference::RP_SECONDARY, [['dc' => 'east'], []]], + [ReadPreference::PRIMARY, []], + [ReadPreference::SECONDARY_PREFERRED, []], + [ReadPreference::SECONDARY, [['dc' => 'east'], []]], ]; } @@ -78,7 +78,7 @@ public function testDocumentLevelReadPreferenceIsSetInCollection(): void { $coll = $this->dm->getDocumentCollection(DocumentWithReadPreference::class); - self::assertSame(ReadPreference::RP_NEAREST, $coll->getReadPreference()->getMode()); + self::assertSame(ReadPreference::NEAREST, $coll->getReadPreference()->getModeString()); self::assertSame([['dc' => 'east']], $coll->getReadPreference()->getTagSets()); } @@ -88,7 +88,7 @@ public function testDocumentLevelReadPreferenceIsAppliedInQueryBuilder(): void ->createQueryBuilder() ->getQuery(); - $this->assertReadPreferenceHint(ReadPreference::RP_NEAREST, $query->getQuery()['readPreference'], [['dc' => 'east']]); + $this->assertReadPreferenceHint(ReadPreference::NEAREST, $query->getQuery()['readPreference'], [['dc' => 'east']]); } public function testDocumentLevelReadPreferenceCanBeOverriddenInQueryBuilder(): void @@ -98,14 +98,14 @@ public function testDocumentLevelReadPreferenceCanBeOverriddenInQueryBuilder(): ->setReadPreference(new ReadPreference('secondary', [])) ->getQuery(); - $this->assertReadPreferenceHint(ReadPreference::RP_SECONDARY, $query->getQuery()['readPreference']); + $this->assertReadPreferenceHint(ReadPreference::SECONDARY, $query->getQuery()['readPreference']); } /** @psalm-param ReadPreferenceTagShape[] $tags */ - private function assertReadPreferenceHint(int $mode, ReadPreference $readPreference, array $tags = []): void + private function assertReadPreferenceHint(string $mode, ReadPreference $readPreference, array $tags = []): void { self::assertInstanceOf(ReadPreference::class, $readPreference); - self::assertEquals($mode, $readPreference->getMode()); + self::assertEquals($mode, $readPreference->getModeString()); self::assertEquals($tags, $readPreference->getTagSets()); } } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php index 44c2c5c40..00468ea54 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php @@ -391,7 +391,7 @@ public function testPrimeReferencesInvokesPrimer(): void // Note: using a secondary read preference here can cause issues when using transactions // Using a primaryPreferred works just as well to check if the hint is passed on to the primer - $readPreference = new ReadPreference(ReadPreference::RP_PRIMARY_PREFERRED); + $readPreference = new ReadPreference(ReadPreference::PRIMARY_PREFERRED); $this->dm->createQueryBuilder(User::class) ->field('account')->prime($primer) ->field('groups')->prime($primer) diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php index 77252d76d..0f7d67308 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php @@ -675,7 +675,7 @@ public function testSetReadPreference(): void $readPreference = $qb->debug('readPreference'); self::assertInstanceOf(ReadPreference::class, $readPreference); - self::assertEquals(ReadPreference::RP_SECONDARY, $readPreference->getMode()); + self::assertEquals(ReadPreference::SECONDARY, $readPreference->getModeString()); self::assertEquals([['dc' => 'east']], $readPreference->getTagSets()); } From 27483495e170e5af1e36eecf250e91de4903d294 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 18 Nov 2024 13:08:43 +0100 Subject: [PATCH 3/5] Fix handling of unmapped properties in proxy objects (#2698) --- .../Proxy/Factory/StaticProxyFactory.php | 14 +++++++++++--- .../Proxy/Factory/StaticProxyFactoryTest.php | 19 +++++++++++++------ .../DocumentWithUnmappedProperties.php | 17 +++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 tests/Documents/DocumentWithUnmappedProperties.php diff --git a/lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php b/lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php index 737795fe4..5d7afba23 100644 --- a/lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php +++ b/lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php @@ -145,13 +145,21 @@ private function createInitializer( */ private function skippedFieldsFqns(ClassMetadata $metadata): array { - $idFieldFqcns = []; + $skippedFieldsFqns = []; foreach ($metadata->getIdentifierFieldNames() as $idField) { - $idFieldFqcns[] = $this->propertyFqcn($metadata->getReflectionProperty($idField)); + $skippedFieldsFqns[] = $this->propertyFqcn($metadata->getReflectionProperty($idField)); } - return $idFieldFqcns; + foreach ($metadata->getReflectionClass()->getProperties() as $property) { + if ($metadata->hasField($property->getName())) { + continue; + } + + $skippedFieldsFqns[] = $this->propertyFqcn($property); + } + + return $skippedFieldsFqns; } private function propertyFqcn(ReflectionProperty $property): string diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Proxy/Factory/StaticProxyFactoryTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Proxy/Factory/StaticProxyFactoryTest.php index 2be5e3fc1..5a1db88dc 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Proxy/Factory/StaticProxyFactoryTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Proxy/Factory/StaticProxyFactoryTest.php @@ -11,6 +11,7 @@ use Doctrine\ODM\MongoDB\LockException; use Doctrine\ODM\MongoDB\Tests\BaseTestCase; use Documents\Cart; +use Documents\DocumentWithUnmappedProperties; use MongoDB\Client; use MongoDB\Collection; use MongoDB\Database; @@ -22,15 +23,10 @@ class StaticProxyFactoryTest extends BaseTestCase /** @var Client|MockObject */ private Client $client; - public function setUp(): void + public function testProxyInitializeWithException(): void { - parent::setUp(); - $this->dm = $this->createMockedDocumentManager(); - } - public function testProxyInitializeWithException(): void - { $collection = $this->createMock(Collection::class); $database = $this->createMock(Database::class); @@ -81,6 +77,17 @@ private function createMockedDocumentManager(): DocumentManager return DocumentManager::create($this->client, $config); } + + public function testCreateProxyForDocumentWithUnmappedProperties(): void + { + $proxy = $this->dm->getReference(DocumentWithUnmappedProperties::class, '123'); + self::assertInstanceOf(GhostObjectInterface::class, $proxy); + + // Disable initialiser so we can access properties without initialising the object + $proxy->setProxyInitializer(null); + + self::assertSame('bar', $proxy->foo); + } } class DocumentNotFoundListener diff --git a/tests/Documents/DocumentWithUnmappedProperties.php b/tests/Documents/DocumentWithUnmappedProperties.php new file mode 100644 index 000000000..06339e79b --- /dev/null +++ b/tests/Documents/DocumentWithUnmappedProperties.php @@ -0,0 +1,17 @@ + Date: Mon, 16 Dec 2024 11:41:27 +0000 Subject: [PATCH 4/5] Update BSON PHP Link (#2708) The page no longer exists for the original link, so updated it to the new URL. --- docs/en/reference/basic-mapping.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/reference/basic-mapping.rst b/docs/en/reference/basic-mapping.rst index fbbe373de..82000d9c3 100644 --- a/docs/en/reference/basic-mapping.rst +++ b/docs/en/reference/basic-mapping.rst @@ -151,7 +151,7 @@ Here is a quick overview of the built-in mapping types: - ``string`` - ``timestamp`` -You can read more about the available MongoDB types on `php.net `_. +You can read more about the available MongoDB types on `php.net `_. .. note:: From ad7827c7915b8b867cb28f5252d99b08ec67ac78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 19 Dec 2024 10:54:22 +0100 Subject: [PATCH 5/5] Replace `wrapTransversable` generators to prevent memory leaks (#2709) The generator hold a circular reference to the iterator instance. --- .../ODM/MongoDB/Iterator/CachingIterator.php | 49 +++++---------- .../MongoDB/Iterator/HydratingIterator.php | 25 +++----- .../MongoDB/Iterator/UnrewindableIterator.php | 61 ++++++++----------- .../Iterator/CachingIteratorTest.php | 13 ++++ .../Iterator/UnrewindableIteratorTest.php | 9 +++ 5 files changed, 69 insertions(+), 88 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php index d72671201..8d80284fa 100644 --- a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php +++ b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php @@ -5,7 +5,8 @@ namespace Doctrine\ODM\MongoDB\Iterator; use Countable; -use Generator; +use Iterator as SPLIterator; +use IteratorIterator; use ReturnTypeWillChange; use RuntimeException; use Traversable; @@ -33,13 +34,11 @@ final class CachingIterator implements Countable, Iterator /** @var array */ private array $items = []; - /** @var Generator|null */ - private ?Generator $iterator; + /** @var SPLIterator|null */ + private ?SPLIterator $iterator; private bool $iteratorAdvanced = false; - private bool $iteratorExhausted = false; - /** * Initialize the iterator and stores the first item in the cache. This * effectively rewinds the Traversable and the wrapping Generator, which @@ -51,7 +50,8 @@ final class CachingIterator implements Countable, Iterator */ public function __construct(Traversable $iterator) { - $this->iterator = $this->wrapTraversable($iterator); + $this->iterator = new IteratorIterator($iterator); + $this->iterator->rewind(); $this->storeCurrentItem(); } @@ -94,9 +94,10 @@ public function key() /** @see http://php.net/iterator.next */ public function next(): void { - if (! $this->iteratorExhausted) { - $this->getIterator()->next(); + if ($this->iterator !== null) { + $this->iterator->next(); $this->storeCurrentItem(); + $this->iteratorAdvanced = true; } next($this->items); @@ -126,15 +127,13 @@ public function valid(): bool */ private function exhaustIterator(): void { - while (! $this->iteratorExhausted) { + while ($this->iterator !== null) { $this->next(); } - - $this->iterator = null; } - /** @return Generator */ - private function getIterator(): Generator + /** @return SPLIterator */ + private function getIterator(): SPLIterator { if ($this->iterator === null) { throw new RuntimeException('Iterator has already been destroyed'); @@ -148,28 +147,12 @@ private function getIterator(): Generator */ private function storeCurrentItem(): void { - $key = $this->getIterator()->key(); + $key = $this->iterator->key(); if ($key === null) { - return; + $this->iterator = null; + } else { + $this->items[$key] = $this->getIterator()->current(); } - - $this->items[$key] = $this->getIterator()->current(); - } - - /** - * @param Traversable $traversable - * - * @return Generator - */ - private function wrapTraversable(Traversable $traversable): Generator - { - foreach ($traversable as $key => $value) { - yield $key => $value; - - $this->iteratorAdvanced = true; - } - - $this->iteratorExhausted = true; } } diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php index 94f5f219f..1e1433cdb 100644 --- a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php +++ b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php @@ -6,8 +6,8 @@ use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\UnitOfWork; -use Generator; use Iterator; +use IteratorIterator; use ReturnTypeWillChange; use RuntimeException; use Traversable; @@ -24,8 +24,8 @@ */ final class HydratingIterator implements Iterator { - /** @var Generator>|null */ - private ?Generator $iterator; + /** @var Iterator>|null */ + private ?Iterator $iterator; /** * @param Traversable> $traversable @@ -34,7 +34,8 @@ final class HydratingIterator implements Iterator */ public function __construct(Traversable $traversable, private UnitOfWork $unitOfWork, private ClassMetadata $class, private array $unitOfWorkHints = []) { - $this->iterator = $this->wrapTraversable($traversable); + $this->iterator = new IteratorIterator($traversable); + $this->iterator->rewind(); } public function __destruct() @@ -74,8 +75,8 @@ public function valid(): bool return $this->key() !== null; } - /** @return Generator> */ - private function getIterator(): Generator + /** @return Iterator> */ + private function getIterator(): Iterator { if ($this->iterator === null) { throw new RuntimeException('Iterator has already been destroyed'); @@ -93,16 +94,4 @@ private function hydrate(?array $document): ?object { return $document !== null ? $this->unitOfWork->getOrCreateDocument($this->class->name, $document, $this->unitOfWorkHints) : null; } - - /** - * @param Traversable> $traversable - * - * @return Generator> - */ - private function wrapTraversable(Traversable $traversable): Generator - { - foreach ($traversable as $key => $value) { - yield $key => $value; - } - } } diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php index f13baed35..c0352c050 100644 --- a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php +++ b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php @@ -4,7 +4,8 @@ namespace Doctrine\ODM\MongoDB\Iterator; -use Generator; +use Iterator as SPLIterator; +use IteratorIterator; use LogicException; use ReturnTypeWillChange; use RuntimeException; @@ -23,39 +24,34 @@ */ final class UnrewindableIterator implements Iterator { - /** @var Generator|null */ - private ?Generator $iterator; + /** @var SPLIterator|null */ + private ?SPLIterator $iterator; private bool $iteratorAdvanced = false; /** - * Initialize the iterator. This effectively rewinds the Traversable and - * the wrapping Generator, which will execute up to its first yield statement. - * Additionally, this mimics behavior of the SPL iterators and allows users - * to omit an explicit call to rewind() before using the other methods. + * Initialize the iterator. This effectively rewinds the Traversable. + * This mimics behavior of the SPL iterators and allows users to omit an + * explicit call to rewind() before using the other methods. * * @param Traversable $iterator */ public function __construct(Traversable $iterator) { - $this->iterator = $this->wrapTraversable($iterator); - $this->iterator->key(); + $this->iterator = new IteratorIterator($iterator); + $this->iterator->rewind(); } public function toArray(): array { $this->preventRewinding(__METHOD__); - $toArray = function () { - if (! $this->valid()) { - return; - } - - yield $this->key() => $this->current(); - yield from $this->getIterator(); - }; - - return iterator_to_array($toArray()); + try { + return iterator_to_array($this->getIterator()); + } finally { + $this->iteratorAdvanced = true; + $this->iterator = null; + } } /** @return TValue|null */ @@ -84,6 +80,13 @@ public function next(): void } $this->iterator->next(); + $this->iteratorAdvanced = true; + + if ($this->iterator->valid()) { + return; + } + + $this->iterator = null; } /** @see http://php.net/iterator.rewind */ @@ -108,8 +111,8 @@ private function preventRewinding(string $method): void } } - /** @return Generator */ - private function getIterator(): Generator + /** @return SPLIterator */ + private function getIterator(): SPLIterator { if ($this->iterator === null) { throw new RuntimeException('Iterator has already been destroyed'); @@ -117,20 +120,4 @@ private function getIterator(): Generator return $this->iterator; } - - /** - * @param Traversable $traversable - * - * @return Generator - */ - private function wrapTraversable(Traversable $traversable): Generator - { - foreach ($traversable as $key => $value) { - yield $key => $value; - - $this->iteratorAdvanced = true; - } - - $this->iterator = null; - } } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php index 52b90c1de..dc8373f11 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php @@ -59,6 +59,19 @@ public function testIterationWithEmptySet(): void self::assertFalse($iterator->valid()); } + public function testIterationWithInvalidIterator(): void + { + $mock = $this->createMock(Iterator::class); + // The method next() should not be called on a dead cursor. + $mock->expects(self::never())->method('next'); + // The method valid() return false on a dead cursor. + $mock->expects(self::once())->method('valid')->willReturn(false); + + $iterator = new CachingIterator($mock); + + $this->assertEquals([], $iterator->toArray()); + } + public function testPartialIterationDoesNotExhaust(): void { $traversable = $this->getTraversableThatThrows([1, 2, new Exception()]); diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php index 304724c2a..128420425 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php @@ -100,6 +100,15 @@ public function testRewindAfterPartialIteration(): void iterator_to_array($iterator); } + public function testRewindAfterToArray(): void + { + $iterator = new UnrewindableIterator($this->getTraversable([1, 2, 3])); + + $iterator->toArray(); + $this->expectException(LogicException::class); + $iterator->rewind(); + } + public function testToArray(): void { $iterator = new UnrewindableIterator($this->getTraversable([1, 2, 3]));