From e569b6df1553ada7a5c4b2d428958de1039c9372 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Sun, 6 Jun 2021 15:35:22 +0200 Subject: [PATCH 1/2] Fix ClassMetadata FieldMapping --- lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php index f068be824a..0a37293af8 100644 --- a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php +++ b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php @@ -85,7 +85,8 @@ * version?: bool, * lock?: bool, * inherited?: string, - * declared?: class-string + * declared?: class-string, + * prime?: list * } * @psalm-type FieldMapping = array{ * type: string, @@ -99,7 +100,6 @@ * isOwningSide: bool, * isInverseSide: bool, * strategy?: string, - * notSaved?: bool, * association?: int, * id?: bool, * collectionClass?: class-string, @@ -124,7 +124,8 @@ * lock?: bool, * notSaved?: bool, * inherited?: string, - * declared?: class-string + * declared?: class-string, + * prime?: list * } * @psalm-type AssociationFieldMapping = array{ * type: string, @@ -162,7 +163,8 @@ * lock?: bool, * notSaved?: bool, * inherited?: string, - * declared?: class-string + * declared?: class-string, + * prime?: list * } * @psalm-type IndexKeys = array * @psalm-type IndexOptions = array From 5e57d0e033660b3bebe17e718348c8e454639066 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Sun, 6 Jun 2021 15:36:12 +0200 Subject: [PATCH 2/2] Make PersistentCollectionInterface generic --- .../ODM/MongoDB/PersistentCollection.php | 7 +++- .../PersistentCollectionInterface.php | 32 +++++++++++++++- .../PersistentCollectionTrait.php | 37 ++++++++++--------- phpstan-baseline.neon | 24 ++++++++++++ .../Tests/PersistentCollectionTest.php | 27 ++++++++++---- 5 files changed, 100 insertions(+), 27 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/PersistentCollection.php b/lib/Doctrine/ODM/MongoDB/PersistentCollection.php index 74265c5190..60852b35da 100644 --- a/lib/Doctrine/ODM/MongoDB/PersistentCollection.php +++ b/lib/Doctrine/ODM/MongoDB/PersistentCollection.php @@ -10,13 +10,18 @@ /** * A PersistentCollection represents a collection of elements that have persistent state. + * + * @template TKey of array-key + * @template T of object + * @template-implements PersistentCollectionInterface */ final class PersistentCollection implements PersistentCollectionInterface { + /** @use PersistentCollectionTrait */ use PersistentCollectionTrait; /** - * @param BaseCollection $coll + * @param BaseCollection $coll */ public function __construct(BaseCollection $coll, DocumentManager $dm, UnitOfWork $uow) { diff --git a/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionInterface.php b/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionInterface.php index 9f1268e71e..d600466a5a 100644 --- a/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionInterface.php +++ b/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionInterface.php @@ -13,11 +13,19 @@ * Interface for persistent collection classes. * * @internal + * + * @psalm-import-type FieldMapping from \Doctrine\ODM\MongoDB\Mapping\ClassMetadata + * + * @template TKey of array-key + * @template T of object + * @template-extends Collection */ interface PersistentCollectionInterface extends Collection { /** * Sets the document manager and unit of work (used during merge operations). + * + * @return void */ public function setDocumentManager(DocumentManager $dm); @@ -25,6 +33,8 @@ public function setDocumentManager(DocumentManager $dm); * Sets the array of raw mongo data that will be used to initialize this collection. * * @param array $mongoData + * + * @return void */ public function setMongoData(array $mongoData); @@ -39,6 +49,8 @@ public function getMongoData(); * Set hints to account for during reconstitution/lookup of the documents. * * @param array $hints + * + * @return void */ public function setHints(array $hints); @@ -52,6 +64,8 @@ public function getHints(); /** * Initializes the collection by loading its contents from the database * if the collection is not yet initialized. + * + * @return void */ public function initialize(); @@ -67,12 +81,18 @@ public function isDirty(); * Sets a boolean flag, indicating whether this collection is dirty. * * @param bool $dirty Whether the collection should be marked dirty or not. + * + * @return void */ public function setDirty($dirty); /** - * Sets the collection's owning entity together with the AssociationMapping that + * Sets the collection's owning document together with the AssociationMapping that * describes the association between the owner and the elements of the collection. + * + * @psalm-param FieldMapping $mapping + * + * @return void */ public function setOwner(object $document, array $mapping); @@ -81,12 +101,16 @@ public function setOwner(object $document, array $mapping); * itself numerically if using save strategy that is enforcing BSON array. * Reindexing is safe as snapshot is taken only after synchronizing collection * with database or clearing it. + * + * @return void */ public function takeSnapshot(); /** * Clears the internal snapshot information and sets isDirty to true if the collection * has elements. + * + * @return void */ public function clearSnapshot(); @@ -128,11 +152,13 @@ public function getOwner(): ?object; /** * @return array + * @psalm-return FieldMapping */ public function getMapping(); /** * @return ClassMetadata + * @psalm-return ClassMetadata * * @throws MongoDBException */ @@ -142,6 +168,8 @@ public function getTypeClass(); * Sets the initialized flag of the collection, forcing it into that state. * * @param bool $bool + * + * @return void */ public function setInitialized($bool); @@ -155,7 +183,7 @@ public function isInitialized(); /** * Returns the wrapped Collection instance. * - * @return Collection + * @return Collection */ public function unwrap(); } diff --git a/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php b/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php index 5e2a8af86d..3b3ccd2b5b 100644 --- a/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php +++ b/lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php @@ -7,6 +7,7 @@ use Closure; use Doctrine\Common\Collections\Collection as BaseCollection; use Doctrine\ODM\MongoDB\DocumentManager; +use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\MongoDBException; use Doctrine\ODM\MongoDB\UnitOfWork; use Doctrine\ODM\MongoDB\Utility\CollectionHelper; @@ -21,6 +22,10 @@ /** * Trait with methods needed to implement PersistentCollectionInterface. + * + * @psalm-import-type FieldMapping from ClassMetadata + * @template TKey of array-key + * @template T of object */ trait PersistentCollectionTrait { @@ -28,18 +33,21 @@ trait PersistentCollectionTrait * A snapshot of the collection at the moment it was fetched from the database. * This is used to create a diff of the collection at commit time. * - * @var array + * @var array */ private $snapshot = []; /** - * Collection's owning entity + * Collection's owning document * * @var object|null */ private $owner; - /** @var array|null */ + /** + * @var array|null + * @psalm-var FieldMapping|null + */ private $mapping; /** @@ -60,7 +68,7 @@ trait PersistentCollectionTrait /** * The wrapped Collection instance. * - * @var BaseCollection + * @var BaseCollection */ private $coll; @@ -130,6 +138,7 @@ public function initialize() return; } + /** @psalm-var array $newObjects */ $newObjects = []; if ($this->isDirty) { @@ -164,7 +173,7 @@ public function initialize() /** * Marks this collection as changed/dirty. */ - private function changed() + private function changed(): void { if ($this->isDirty) { return; @@ -681,7 +690,7 @@ public function unwrap() * 2. New collection is not dirty, if reused on other document nothing * changes. * 3. Snapshot leads to invalid diffs being generated. - * 4. Lazy loading grabs entities from old owner object. + * 4. Lazy loading grabs documents from old owner object. * 5. New collection is connected to old owner and leads to duplicate keys. */ public function __clone() @@ -730,11 +739,10 @@ private function doAdd($value, $arrayAccess) * Actual logic for removing element by its key. * * @param mixed $offset - * @param bool $arrayAccess * - * @return bool + * @return bool|T|null */ - private function doRemove($offset, $arrayAccess) + private function doRemove($offset, bool $arrayAccess) { $this->initialize(); if ($arrayAccess) { @@ -758,9 +766,8 @@ private function doRemove($offset, $arrayAccess) * * @param mixed $offset * @param mixed $value - * @param bool $arrayAccess */ - private function doSet($offset, $value, $arrayAccess) + private function doSet($offset, $value, bool $arrayAccess): void { $arrayAccess ? $this->coll->offsetSet($offset, $value) : $this->coll->set($offset, $value); @@ -777,10 +784,8 @@ private function doSet($offset, $value, $arrayAccess) * * Embedded documents are automatically considered as "orphan removal enabled" because they might have references * that require to trigger cascade remove operations. - * - * @return bool */ - private function isOrphanRemovalEnabled() + private function isOrphanRemovalEnabled(): bool { if ($this->mapping === null) { return false; @@ -795,10 +800,8 @@ private function isOrphanRemovalEnabled() /** * Checks whether collection owner needs to be scheduled for dirty change in case the collection is modified. - * - * @return bool */ - private function needsSchedulingForSynchronization() + private function needsSchedulingForSynchronization(): bool { return $this->owner && $this->dm && ! empty($this->mapping['isOwningSide']) && $this->dm->getClassMetadata(get_class($this->owner))->isChangeTrackingNotify(); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4243184cee..abcf0fb8c9 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -228,3 +228,27 @@ parameters: message: "#^Method Doctrine\\\\ODM\\\\MongoDB\\\\Tests\\\\SchemaManagerTest\\:\\:writeOptions\\(\\) should return PHPUnit\\\\Framework\\\\Constraint\\\\Constraint but returns PHPUnit\\\\Framework\\\\Constraint\\\\ArraySubset\\.$#" count: 1 path: tests/Doctrine/ODM/MongoDB/Tests/SchemaManagerTest.php + + # import type in PHPStan does not work, see https://github.com/phpstan/phpstan/issues/5091 + - + message: "#^Access to offset '.+' on an unknown class Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\\\FieldMapping\\.$#" + count: 12 + path: lib/Doctrine/ODM/MongoDB/PersistentCollection.php + + # import type in PHPStan does not work, see https://github.com/phpstan/phpstan/issues/5091 + - + message: "#^Method Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\:\\:getMapping\\(\\) should return array\\('type' \\=\\> string, 'fieldName' \\=\\> string, 'name' \\=\\> string, 'isCascadeRemove' \\=\\> bool, 'isCascadePersist' \\=\\> bool, 'isCascadeRefresh' \\=\\> bool, 'isCascadeMerge' \\=\\> bool, 'isCascadeDetach' \\=\\> bool, \\.\\.\\.\\) but returns Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\\\FieldMapping\\|null\\.$#" + count: 1 + path: lib/Doctrine/ODM/MongoDB/PersistentCollection.php + + # import type in PHPStan does not work, see https://github.com/phpstan/phpstan/issues/5091 + - + message: "#^Property Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\:\\:\\$mapping has unknown class Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\\\FieldMapping as its type\\.$#" + count: 1 + path: lib/Doctrine/ODM/MongoDB/PersistentCollection.php + + # import type in PHPStan does not work, see https://github.com/phpstan/phpstan/issues/5091 + - + message: "#^Property Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\\\:\\:\\$mapping \\(Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\\\FieldMapping\\|null\\) does not accept array\\\\|bool\\|int\\|string\\|null\\>\\.$#" + count: 1 + path: lib/Doctrine/ODM/MongoDB/PersistentCollection.php diff --git a/tests/Doctrine/ODM/MongoDB/Tests/PersistentCollectionTest.php b/tests/Doctrine/ODM/MongoDB/Tests/PersistentCollectionTest.php index df90b7e88b..72e10420e0 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/PersistentCollectionTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/PersistentCollectionTest.php @@ -42,7 +42,19 @@ public function testExceptionForGetTypeClassWithoutDocumentManager() $unserialized = unserialize($serialized); assert($unserialized instanceof PersistentCollection); - $unserialized->setOwner($owner, ['targetDocument' => stdClass::class]); + $unserialized->setOwner($owner, [ + 'type' => ClassMetadata::ONE, + 'name' => 'name', + 'fieldName' => 'fieldName', + 'isCascadeRemove' => false, + 'isCascadePersist' => false, + 'isCascadeRefresh' => false, + 'isCascadeMerge' => false, + 'isCascadeDetach' => false, + 'isOwningSide' => false, + 'isInverseSide' => false, + 'targetDocument' => stdClass::class, + ]); $this->expectException(MongoDBException::class); $this->expectExceptionMessage( 'No DocumentManager is associated with this PersistentCollection, ' . @@ -274,9 +286,10 @@ public function testOffsetExistsIsForwarded() public function testOffsetGetIsForwarded() { $collection = $this->getMockCollection(); - $collection->expects($this->once())->method('offsetGet')->willReturn(2); + $object = new stdClass(); + $collection->expects($this->once())->method('offsetGet')->willReturn($object); $pcoll = new PersistentCollection($collection, $this->dm, $this->uow); - $this->assertSame(2, $pcoll[0]); + $this->assertSame($object, $pcoll[0]); } public function testOffsetUnsetIsForwarded() @@ -302,12 +315,12 @@ public function testOffsetSetIsForwarded() $collection = $this->getMockCollection(); $collection->expects($this->exactly(2))->method('offsetSet'); $pcoll = new PersistentCollection($collection, $this->dm, $this->uow); - $pcoll[] = 1; - $pcoll[1] = 2; + $pcoll[] = new stdClass(); + $pcoll[1] = new stdClass(); $collection->expects($this->once())->method('add'); - $pcoll->add(3); + $pcoll->add(new stdClass()); $collection->expects($this->once())->method('set'); - $pcoll->set(3, 4); + $pcoll->set(3, new stdClass()); } public function testIsEmptyIsForwardedWhenCollectionIsInitialized()