From 13e5ca2df242f81363a34f8d9371f43cd0d85e9c Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 24 Nov 2023 09:29:31 +0100 Subject: [PATCH] Refactor commit logic (#2580) * Add tests for commit consistency showing wrong behaviour * Clear scheduled document changes at the end of a commit operation * Rename private variables to communicate intent * Remove obsolete comment * Use different error code * Use single mongos in consistency tests This ensures that the fail points are created on the same server that the write operations take place on, which can't be guaranteed in a sharded cluster with multiple mongoses. * Rename test methods for clarity * Explain reasoning for index error * Extract helper method to create failpoint --- lib/Doctrine/ODM/MongoDB/UnitOfWork.php | 219 ++++----- .../ODM/MongoDB/Tests/BaseTestCase.php | 48 +- .../Tests/UnitOfWorkCommitConsistencyTest.php | 458 ++++++++++++++++++ 3 files changed, 610 insertions(+), 115 deletions(-) create mode 100644 tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkCommitConsistencyTest.php diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php index e29b883a00..96ba4c92a8 100644 --- a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php +++ b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php @@ -162,42 +162,42 @@ final class UnitOfWork implements PropertyChangedListener * * @var array */ - private array $documentInsertions = []; + private array $scheduledDocumentInsertions = []; /** * A list of all pending document updates. * * @var array */ - private array $documentUpdates = []; + private array $scheduledDocumentUpdates = []; /** * A list of all pending document upserts. * * @var array */ - private array $documentUpserts = []; + private array $scheduledDocumentUpserts = []; /** * A list of all pending document deletions. * * @var array */ - private array $documentDeletions = []; + private array $scheduledDocumentDeletions = []; /** * All pending collection deletions. * * @psalm-var array> */ - private array $collectionDeletions = []; + private array $scheduledCollectionDeletions = []; /** * All pending collection updates. * * @psalm-var array> */ - private array $collectionUpdates = []; + private array $scheduledCollectionUpdates = []; /** * A list of documents related to collections scheduled for update or deletion @@ -418,12 +418,12 @@ public function commit(array $options = []): void $this->computeChangeSets(); if ( - ! ($this->documentInsertions || - $this->documentUpserts || - $this->documentDeletions || - $this->documentUpdates || - $this->collectionUpdates || - $this->collectionDeletions || + ! ($this->scheduledDocumentInsertions || + $this->scheduledDocumentUpserts || + $this->scheduledDocumentDeletions || + $this->scheduledDocumentUpdates || + $this->scheduledCollectionUpdates || + $this->scheduledCollectionDeletions || $this->orphanRemovals) ) { return; // Nothing to do. @@ -444,22 +444,22 @@ public function commit(array $options = []): void // Raise onFlush $this->evm->dispatchEvent(Events::onFlush, new Event\OnFlushEventArgs($this->dm)); - foreach ($this->getClassesForCommitAction($this->documentUpserts) as $classAndDocuments) { + foreach ($this->getClassesForCommitAction($this->scheduledDocumentUpserts) as $classAndDocuments) { [$class, $documents] = $classAndDocuments; $this->executeUpserts($class, $documents, $options); } - foreach ($this->getClassesForCommitAction($this->documentInsertions) as $classAndDocuments) { + foreach ($this->getClassesForCommitAction($this->scheduledDocumentInsertions) as $classAndDocuments) { [$class, $documents] = $classAndDocuments; $this->executeInserts($class, $documents, $options); } - foreach ($this->getClassesForCommitAction($this->documentUpdates) as $classAndDocuments) { + foreach ($this->getClassesForCommitAction($this->scheduledDocumentUpdates) as $classAndDocuments) { [$class, $documents] = $classAndDocuments; $this->executeUpdates($class, $documents, $options); } - foreach ($this->getClassesForCommitAction($this->documentDeletions, true) as $classAndDocuments) { + foreach ($this->getClassesForCommitAction($this->scheduledDocumentDeletions, true) as $classAndDocuments) { [$class, $documents] = $classAndDocuments; $this->executeDeletions($class, $documents, $options); } @@ -468,17 +468,17 @@ public function commit(array $options = []): void $this->evm->dispatchEvent(Events::postFlush, new Event\PostFlushEventArgs($this->dm)); // Clear up - $this->documentInsertions = - $this->documentUpserts = - $this->documentUpdates = - $this->documentDeletions = - $this->documentChangeSets = - $this->collectionUpdates = - $this->collectionDeletions = - $this->visitedCollections = - $this->scheduledForSynchronization = - $this->orphanRemovals = - $this->hasScheduledCollections = []; + $this->scheduledDocumentInsertions = + $this->scheduledDocumentUpserts = + $this->scheduledDocumentUpdates = + $this->scheduledDocumentDeletions = + $this->documentChangeSets = + $this->scheduledCollectionUpdates = + $this->scheduledCollectionDeletions = + $this->visitedCollections = + $this->scheduledForSynchronization = + $this->orphanRemovals = + $this->hasScheduledCollections = []; } finally { $this->commitsInProgress--; } @@ -537,7 +537,7 @@ private function getClassesForCommitAction(array $documents, bool $includeEmbedd */ private function computeScheduleInsertsChangeSets(): void { - foreach ($this->documentInsertions as $document) { + foreach ($this->scheduledDocumentInsertions as $document) { $class = $this->dm->getClassMetadata($document::class); if ($class->isEmbeddedDocument || $class->isView()) { continue; @@ -554,7 +554,7 @@ private function computeScheduleInsertsChangeSets(): void */ private function computeScheduleUpsertsChangeSets(): void { - foreach ($this->documentUpserts as $document) { + foreach ($this->scheduledDocumentUpserts as $document) { $class = $this->dm->getClassMetadata($document::class); if ($class->isEmbeddedDocument || $class->isView()) { continue; @@ -647,7 +647,7 @@ public function getDocumentActualData(object $document): array * entry is the new value of the property. Changesets are used by persisters * to INSERT/UPDATE the persistent document state. * - * {@link documentUpdates} + * {@link scheduledDocumentUpdates} * If the document is already fully MANAGED (has been fetched from the database before) * and any changes to its properties are detected, then a reference to the document is stored * there to mark it for an update. @@ -930,9 +930,9 @@ public function computeChangeSets(): void // Only MANAGED documents that are NOT SCHEDULED FOR INSERTION, UPSERT OR DELETION are processed here. $oid = spl_object_hash($document); if ( - isset($this->documentInsertions[$oid]) - || isset($this->documentUpserts[$oid]) - || isset($this->documentDeletions[$oid]) + isset($this->scheduledDocumentInsertions[$oid]) + || isset($this->scheduledDocumentUpserts[$oid]) + || isset($this->scheduledDocumentDeletions[$oid]) || ! isset($this->documentStates[$oid]) ) { continue; @@ -953,7 +953,7 @@ public function computeChangeSets(): void */ private function computeAssociationChanges(object $parentDocument, array $assoc, $value): void { - $isNewParentDocument = isset($this->documentInsertions[spl_object_hash($parentDocument)]); + $isNewParentDocument = isset($this->scheduledDocumentInsertions[spl_object_hash($parentDocument)]); $class = $this->dm->getClassMetadata($parentDocument::class); $topOrExistingDocument = ( ! $isNewParentDocument || ! $class->isEmbeddedDocument); @@ -1161,9 +1161,8 @@ private function executeInserts(ClassMetadata $class, array $documents, array $o { $persister = $this->getDocumentPersister($class->name); - foreach ($documents as $oid => $document) { + foreach ($documents as $document) { $persister->addInsert($document); - unset($this->documentInsertions[$oid]); } $persister->executeInserts($options); @@ -1186,9 +1185,8 @@ private function executeUpserts(ClassMetadata $class, array $documents, array $o { $persister = $this->getDocumentPersister($class->name); - foreach ($documents as $oid => $document) { + foreach ($documents as $document) { $persister->addUpsert($document); - unset($this->documentUpserts[$oid]); } $persister->executeUpserts($options); @@ -1223,8 +1221,6 @@ private function executeUpdates(ClassMetadata $class, array $documents, array $o $persister->update($document, $options); } - unset($this->documentUpdates[$oid]); - $this->lifecycleEventManager->postUpdate($class, $document); } } @@ -1248,7 +1244,6 @@ private function executeDeletions(ClassMetadata $class, array $documents, array } unset( - $this->documentDeletions[$oid], $this->documentIdentifiers[$oid], $this->originalDocumentData[$oid], ); @@ -1268,10 +1263,6 @@ private function executeDeletions(ClassMetadata $class, array $documents, array $value->clearSnapshot(); } - // Document with this $oid after deletion treated as NEW, even if the $oid - // is obtained by a new document because the old one went out of scope. - $this->documentStates[$oid] = self::STATE_NEW; - $this->lifecycleEventManager->postRemove($class, $document); } } @@ -1294,19 +1285,19 @@ public function scheduleForInsert(ClassMetadata $class, object $document): void { $oid = spl_object_hash($document); - if (isset($this->documentUpdates[$oid])) { + if (isset($this->scheduledDocumentUpdates[$oid])) { throw new InvalidArgumentException('Dirty document can not be scheduled for insertion.'); } - if (isset($this->documentDeletions[$oid])) { + if (isset($this->scheduledDocumentDeletions[$oid])) { throw new InvalidArgumentException('Removed document can not be scheduled for insertion.'); } - if (isset($this->documentInsertions[$oid])) { + if (isset($this->scheduledDocumentInsertions[$oid])) { throw new InvalidArgumentException('Document can not be scheduled for insertion twice.'); } - $this->documentInsertions[$oid] = $document; + $this->scheduledDocumentInsertions[$oid] = $document; if (! isset($this->documentIdentifiers[$oid])) { return; @@ -1336,20 +1327,20 @@ public function scheduleForUpsert(ClassMetadata $class, object $document): void throw new InvalidArgumentException('Embedded document can not be scheduled for upsert.'); } - if (isset($this->documentUpdates[$oid])) { + if (isset($this->scheduledDocumentUpdates[$oid])) { throw new InvalidArgumentException('Dirty document can not be scheduled for upsert.'); } - if (isset($this->documentDeletions[$oid])) { + if (isset($this->scheduledDocumentDeletions[$oid])) { throw new InvalidArgumentException('Removed document can not be scheduled for upsert.'); } - if (isset($this->documentUpserts[$oid])) { + if (isset($this->scheduledDocumentUpserts[$oid])) { throw new InvalidArgumentException('Document can not be scheduled for upsert twice.'); } - $this->documentUpserts[$oid] = $document; - $this->documentIdentifiers[$oid] = $class->getIdentifierValue($document); + $this->scheduledDocumentUpserts[$oid] = $document; + $this->documentIdentifiers[$oid] = $class->getIdentifierValue($document); $this->addToIdentityMap($document); } @@ -1358,7 +1349,7 @@ public function scheduleForUpsert(ClassMetadata $class, object $document): void */ public function isScheduledForInsert(object $document): bool { - return isset($this->documentInsertions[spl_object_hash($document)]); + return isset($this->scheduledDocumentInsertions[spl_object_hash($document)]); } /** @@ -1366,7 +1357,7 @@ public function isScheduledForInsert(object $document): bool */ public function isScheduledForUpsert(object $document): bool { - return isset($this->documentUpserts[spl_object_hash($document)]); + return isset($this->scheduledDocumentUpserts[spl_object_hash($document)]); } /** @@ -1383,19 +1374,19 @@ public function scheduleForUpdate(object $document): void throw new InvalidArgumentException('Document has no identity.'); } - if (isset($this->documentDeletions[$oid])) { + if (isset($this->scheduledDocumentDeletions[$oid])) { throw new InvalidArgumentException('Document is removed.'); } if ( - isset($this->documentUpdates[$oid]) - || isset($this->documentInsertions[$oid]) - || isset($this->documentUpserts[$oid]) + isset($this->scheduledDocumentUpdates[$oid]) + || isset($this->scheduledDocumentInsertions[$oid]) + || isset($this->scheduledDocumentUpserts[$oid]) ) { return; } - $this->documentUpdates[$oid] = $document; + $this->scheduledDocumentUpdates[$oid] = $document; } /** @@ -1405,7 +1396,7 @@ public function scheduleForUpdate(object $document): void */ public function isScheduledForUpdate(object $document): bool { - return isset($this->documentUpdates[spl_object_hash($document)]); + return isset($this->scheduledDocumentUpdates[spl_object_hash($document)]); } /** @@ -1427,12 +1418,12 @@ public function scheduleForDelete(object $document, bool $isView = false): void { $oid = spl_object_hash($document); - if (isset($this->documentInsertions[$oid])) { + if (isset($this->scheduledDocumentInsertions[$oid])) { if ($this->isInIdentityMap($document)) { $this->removeFromIdentityMap($document); } - unset($this->documentInsertions[$oid]); + unset($this->scheduledDocumentInsertions[$oid]); return; // document has not been persisted yet, so nothing more to do. } @@ -1444,15 +1435,15 @@ public function scheduleForDelete(object $document, bool $isView = false): void $this->removeFromIdentityMap($document); $this->documentStates[$oid] = self::STATE_REMOVED; - if (isset($this->documentUpdates[$oid])) { - unset($this->documentUpdates[$oid]); + if (isset($this->scheduledDocumentUpdates[$oid])) { + unset($this->scheduledDocumentUpdates[$oid]); } - if (isset($this->documentUpserts[$oid])) { - unset($this->documentUpserts[$oid]); + if (isset($this->scheduledDocumentUpserts[$oid])) { + unset($this->scheduledDocumentUpserts[$oid]); } - if (isset($this->documentDeletions[$oid])) { + if (isset($this->scheduledDocumentDeletions[$oid])) { return; } @@ -1460,7 +1451,7 @@ public function scheduleForDelete(object $document, bool $isView = false): void return; } - $this->documentDeletions[$oid] = $document; + $this->scheduledDocumentDeletions[$oid] = $document; } /** @@ -1469,7 +1460,7 @@ public function scheduleForDelete(object $document, bool $isView = false): void */ public function isScheduledForDelete(object $document): bool { - return isset($this->documentDeletions[spl_object_hash($document)]); + return isset($this->scheduledDocumentDeletions[spl_object_hash($document)]); } /** @@ -1481,10 +1472,10 @@ public function isDocumentScheduled(object $document): bool { $oid = spl_object_hash($document); - return isset($this->documentInsertions[$oid]) || - isset($this->documentUpserts[$oid]) || - isset($this->documentUpdates[$oid]) || - isset($this->documentDeletions[$oid]); + return isset($this->scheduledDocumentInsertions[$oid]) || + isset($this->scheduledDocumentUpserts[$oid]) || + isset($this->scheduledDocumentUpdates[$oid]) || + isset($this->scheduledDocumentDeletions[$oid]); } /** @@ -1781,7 +1772,7 @@ private function doPersist(object $document, array &$visited): void case self::STATE_REMOVED: // Document becomes managed again - unset($this->documentDeletions[$oid]); + unset($this->scheduledDocumentDeletions[$oid]); $this->documentStates[$oid] = self::STATE_MANAGED; break; @@ -2094,14 +2085,14 @@ private function doDetach(object $document, array &$visited): void case self::STATE_MANAGED: $this->removeFromIdentityMap($document); unset( - $this->documentInsertions[$oid], - $this->documentUpdates[$oid], - $this->documentDeletions[$oid], + $this->scheduledDocumentInsertions[$oid], + $this->scheduledDocumentUpdates[$oid], + $this->scheduledDocumentDeletions[$oid], $this->documentIdentifiers[$oid], $this->documentStates[$oid], $this->originalDocumentData[$oid], $this->parentAssociations[$oid], - $this->documentUpserts[$oid], + $this->scheduledDocumentUpserts[$oid], $this->hasScheduledCollections[$oid], $this->embeddedDocumentsRegistry[$oid], ); @@ -2392,22 +2383,22 @@ public function unlock(object $document): void public function clear(?string $documentName = null): void { if ($documentName === null) { - $this->identityMap = - $this->documentIdentifiers = - $this->originalDocumentData = - $this->documentChangeSets = - $this->documentStates = - $this->scheduledForSynchronization = - $this->documentInsertions = - $this->documentUpserts = - $this->documentUpdates = - $this->documentDeletions = - $this->collectionUpdates = - $this->collectionDeletions = - $this->parentAssociations = - $this->embeddedDocumentsRegistry = - $this->orphanRemovals = - $this->hasScheduledCollections = []; + $this->identityMap = + $this->documentIdentifiers = + $this->originalDocumentData = + $this->documentChangeSets = + $this->documentStates = + $this->scheduledForSynchronization = + $this->scheduledDocumentInsertions = + $this->scheduledDocumentUpserts = + $this->scheduledDocumentUpdates = + $this->scheduledDocumentDeletions = + $this->scheduledCollectionUpdates = + $this->scheduledCollectionDeletions = + $this->parentAssociations = + $this->embeddedDocumentsRegistry = + $this->orphanRemovals = + $this->hasScheduledCollections = []; $event = new Event\OnClearEventArgs($this->dm); } else { @@ -2497,12 +2488,12 @@ private function fixPersistentCollectionOwnership(PersistentCollectionInterface public function scheduleCollectionDeletion(PersistentCollectionInterface $coll): void { $oid = spl_object_hash($coll); - unset($this->collectionUpdates[$oid]); - if (isset($this->collectionDeletions[$oid])) { + unset($this->scheduledCollectionUpdates[$oid]); + if (isset($this->scheduledCollectionDeletions[$oid])) { return; } - $this->collectionDeletions[$oid] = $coll; + $this->scheduledCollectionDeletions[$oid] = $coll; $this->scheduleCollectionOwner($coll); } @@ -2515,7 +2506,7 @@ public function scheduleCollectionDeletion(PersistentCollectionInterface $coll): */ public function isCollectionScheduledForDeletion(PersistentCollectionInterface $coll): bool { - return isset($this->collectionDeletions[spl_object_hash($coll)]); + return isset($this->scheduledCollectionDeletions[spl_object_hash($coll)]); } /** @@ -2532,12 +2523,12 @@ public function unscheduleCollectionDeletion(PersistentCollectionInterface $coll } $oid = spl_object_hash($coll); - if (! isset($this->collectionDeletions[$oid])) { + if (! isset($this->scheduledCollectionDeletions[$oid])) { return; } $topmostOwner = $this->getOwningDocument($coll->getOwner()); - unset($this->collectionDeletions[$oid]); + unset($this->scheduledCollectionDeletions[$oid]); unset($this->hasScheduledCollections[spl_object_hash($topmostOwner)][$oid]); } @@ -2559,11 +2550,11 @@ public function scheduleCollectionUpdate(PersistentCollectionInterface $coll): v } $oid = spl_object_hash($coll); - if (isset($this->collectionUpdates[$oid])) { + if (isset($this->scheduledCollectionUpdates[$oid])) { return; } - $this->collectionUpdates[$oid] = $coll; + $this->scheduledCollectionUpdates[$oid] = $coll; $this->scheduleCollectionOwner($coll); } @@ -2581,12 +2572,12 @@ public function unscheduleCollectionUpdate(PersistentCollectionInterface $coll): } $oid = spl_object_hash($coll); - if (! isset($this->collectionUpdates[$oid])) { + if (! isset($this->scheduledCollectionUpdates[$oid])) { return; } $topmostOwner = $this->getOwningDocument($coll->getOwner()); - unset($this->collectionUpdates[$oid]); + unset($this->scheduledCollectionUpdates[$oid]); unset($this->hasScheduledCollections[spl_object_hash($topmostOwner)][$oid]); } @@ -2599,7 +2590,7 @@ public function unscheduleCollectionUpdate(PersistentCollectionInterface $coll): */ public function isCollectionScheduledForUpdate(PersistentCollectionInterface $coll): bool { - return isset($this->collectionUpdates[spl_object_hash($coll)]); + return isset($this->scheduledCollectionUpdates[spl_object_hash($coll)]); } /** @@ -2939,7 +2930,7 @@ public function getDocumentIdentifier(object $document) */ public function hasPendingInsertions(): bool { - return ! empty($this->documentInsertions); + return ! empty($this->scheduledDocumentInsertions); } /** @@ -3034,7 +3025,7 @@ public function propertyChanged($sender, $propertyName, $oldValue, $newValue) */ public function getScheduledDocumentInsertions(): array { - return $this->documentInsertions; + return $this->scheduledDocumentInsertions; } /** @@ -3044,7 +3035,7 @@ public function getScheduledDocumentInsertions(): array */ public function getScheduledDocumentUpserts(): array { - return $this->documentUpserts; + return $this->scheduledDocumentUpserts; } /** @@ -3054,7 +3045,7 @@ public function getScheduledDocumentUpserts(): array */ public function getScheduledDocumentUpdates(): array { - return $this->documentUpdates; + return $this->scheduledDocumentUpdates; } /** @@ -3064,7 +3055,7 @@ public function getScheduledDocumentUpdates(): array */ public function getScheduledDocumentDeletions(): array { - return $this->documentDeletions; + return $this->scheduledDocumentDeletions; } /** @@ -3076,7 +3067,7 @@ public function getScheduledDocumentDeletions(): array */ public function getScheduledCollectionDeletions(): array { - return $this->collectionDeletions; + return $this->scheduledCollectionDeletions; } /** @@ -3088,7 +3079,7 @@ public function getScheduledCollectionDeletions(): array */ public function getScheduledCollectionUpdates(): array { - return $this->collectionUpdates; + return $this->scheduledCollectionUpdates; } /** diff --git a/tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php b/tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php index ad5fe91ca8..9471db7576 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php @@ -16,10 +16,17 @@ use function array_key_exists; use function array_map; +use function count; +use function explode; use function getenv; +use function implode; use function in_array; use function iterator_to_array; +use function parse_url; use function preg_match; +use function strlen; +use function strpos; +use function substr_replace; use function version_compare; use const DOCTRINE_MONGODB_DATABASE; @@ -108,7 +115,7 @@ protected static function createMetadataDriverImpl(): MappingDriver protected static function createTestDocumentManager(): DocumentManager { $config = static::getConfiguration(); - $client = new Client(getenv('DOCTRINE_MONGODB_SERVER') ?: DOCTRINE_MONGODB_SERVER, [], ['typeMap' => ['root' => 'array', 'document' => 'array']]); + $client = new Client(self::getUri(), [], ['typeMap' => ['root' => 'array', 'document' => 'array']]); return DocumentManager::create($client, $config); } @@ -162,4 +169,43 @@ protected function requireMongoDB42(string $message): void { $this->requireVersion($this->getServerVersion(), '4.2.0', '<', $message); } + + protected static function getUri(bool $useMultipleMongoses = true): string + { + $uri = getenv('DOCTRINE_MONGODB_SERVER') ?: DOCTRINE_MONGODB_SERVER; + + return $useMultipleMongoses ? $uri : self::removeMultipleHosts($uri); + } + + /** + * Removes any hosts beyond the first in a URI. This function should only be + * used with a sharded cluster URI, but that is not enforced. + */ + protected static function removeMultipleHosts(string $uri): string + { + $parts = parse_url($uri); + + self::assertIsArray($parts); + + $hosts = explode(',', $parts['host']); + + // Nothing to do if the URI already has a single mongos host + if (count($hosts) === 1) { + return $uri; + } + + // Re-append port to last host + if (isset($parts['port'])) { + $hosts[count($hosts) - 1] .= ':' . $parts['port']; + } + + $singleHost = $hosts[0]; + $multipleHosts = implode(',', $hosts); + + $pos = strpos($uri, $multipleHosts); + + self::assertNotFalse($pos); + + return substr_replace($uri, $singleHost, $pos, strlen($multipleHosts)); + } } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkCommitConsistencyTest.php b/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkCommitConsistencyTest.php new file mode 100644 index 0000000000..ab01bbc850 --- /dev/null +++ b/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkCommitConsistencyTest.php @@ -0,0 +1,458 @@ +dm->getClient()->selectDatabase('admin')->command([ + 'configureFailPoint' => 'failCommand', + 'mode' => 'off', + ]); + + parent::tearDown(); + } + + public function testInsertErrorKeepsFailingInsertions(): void + { + $firstUser = new ForumUser(); + $firstUser->username = 'alcaeus'; + $this->uow->persist($firstUser); + + $secondUser = new ForumUser(); + $secondUser->username = 'jmikola'; + $this->uow->persist($secondUser); + + $friendUser = new FriendUser('GromNaN'); + $this->uow->persist($friendUser); + + // Add failpoint to let the first insert command fail. This affects the ForumUser documents + $this->createFailpoint('insert'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + self::assertSame( + 0, + $this->dm->getDocumentCollection(ForumUser::class)->countDocuments(), + ); + + self::assertTrue($this->uow->isScheduledForInsert($firstUser)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($firstUser)); + + self::assertTrue($this->uow->isScheduledForInsert($secondUser)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($secondUser)); + + self::assertTrue($this->uow->isScheduledForInsert($friendUser)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($friendUser)); + } + + public function testInsertErrorKeepsFailingInsertionsForDocumentClass(): void + { + // Create a unique index on the collection to let the second document fail, as using a fail point would also + // affect the first document. + $collection = $this->dm->getDocumentCollection(ForumUser::class); + $collection->createIndex(['username' => 1], ['unique' => true]); + + $firstUser = new ForumUser(); + $firstUser->username = 'alcaeus'; + $this->uow->persist($firstUser); + + $secondUser = new ForumUser(); + $secondUser->username = 'alcaeus'; + $this->uow->persist($secondUser); + + $thirdUser = new ForumUser(); + $thirdUser->username = 'jmikola'; + $this->uow->persist($thirdUser); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // One user inserted, the second insert failed, the last was skipped + self::assertSame( + 1, + $this->dm->getDocumentCollection(ForumUser::class)->countDocuments(), + ); + + // Wrong behaviour: user was saved and should no longer be scheduled for insertion + self::assertTrue($this->uow->isScheduledForInsert($firstUser)); + // Wrong behaviour: changeset should be empty + self::assertNotEquals([], $this->uow->getDocumentChangeSet($firstUser)); + + self::assertTrue($this->uow->isScheduledForInsert($secondUser)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($secondUser)); + + self::assertTrue($this->uow->isScheduledForInsert($thirdUser)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($thirdUser)); + } + + public function testInsertErrorWithEmbeddedDocumentKeepsInsertions(): void + { + // Create a unique index on the collection to let the second insert fail + $collection = $this->dm->getDocumentCollection(User::class); + $collection->createIndex(['username' => 1], ['unique' => true]); + + $firstAddress = new Address(); + $firstAddress->setCity('Olching'); + $firstUser = new User(); + $firstUser->setUsername('alcaeus'); + $firstUser->setAddress($firstAddress); + + $secondAddress = new Address(); + $secondAddress->setCity('Olching'); + $secondUser = new User(); + $secondUser->setUsername('alcaeus'); + $secondUser->setAddress($secondAddress); + + $this->uow->persist($firstUser); + $this->uow->persist($secondUser); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // First document inserted, second failed due to index error + self::assertSame(1, $collection->countDocuments()); + + // Wrong behaviour: document should no longer be scheduled and changeset should be cleared + $this->assertTrue($this->uow->isScheduledForInsert($firstUser)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($firstUser)); + + // Wrong behaviour: document should no longer be scheduled for insertion and changeset cleared + $this->assertTrue($this->uow->isScheduledForInsert($firstAddress)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($firstAddress)); + + $this->assertTrue($this->uow->isScheduledForInsert($secondUser)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($secondUser)); + $this->assertTrue($this->uow->isScheduledForInsert($secondAddress)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($secondAddress)); + } + + public function testUpsertErrorDropsFailingUpserts(): void + { + $user = new ForumUser(); + $user->id = new ObjectId(); // Specifying an identifier makes this an upsert + $user->username = 'alcaeus'; + $this->uow->persist($user); + + $this->createFailpoint('update'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // No document was inserted + self::assertSame( + 0, + $this->dm->getDocumentCollection(ForumUser::class)->countDocuments(), + ); + + self::assertTrue($this->uow->isScheduledForUpsert($user)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($user)); + } + + public function testUpdateErrorKeepsFailingUpdate(): void + { + $user = new ForumUser(); + $user->username = 'alcaeus'; + $this->uow->persist($user); + $this->uow->commit(); + + $user->username = 'jmikola'; + + // Make sure update command fails once + $this->createFailpoint('update'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // The update is kept, user data is not changed + self::assertSame( + 1, + $this->dm->getDocumentCollection(ForumUser::class)->countDocuments(['username' => 'alcaeus']), + ); + + self::assertTrue($this->uow->isScheduledForUpdate($user)); + self::assertNotEquals([], $this->uow->getDocumentChangeSet($user)); + } + + public function testUpdateErrorWithNewEmbeddedDocumentKeepsFailingChangeset(): void + { + $user = new User(); + $user->setUsername('alcaeus'); + + $this->uow->persist($user); + $this->uow->commit(); + + $address = new Address(); + $address->setCity('Olching'); + $user->setAddress($address); + + $this->createFailpoint('update'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + $this->assertTrue($this->uow->isScheduledForUpdate($user)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertTrue($this->uow->isScheduledForInsert($address)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($address)); + } + + public function testUpdateWithNewEmbeddedDocumentClearsChangesets(): void + { + $user = new User(); + $user->setUsername('alcaeus'); + + $this->uow->persist($user); + $this->uow->commit(); + + $address = new Address(); + $address->setCity('Olching'); + $user->setAddress($address); + + $this->uow->commit(); + + $this->assertFalse($this->uow->isScheduledForUpdate($user)); + $this->assertEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertFalse($this->uow->isScheduledForInsert($address)); + $this->assertEquals([], $this->uow->getDocumentChangeSet($address)); + } + + public function testUpdateErrorWithEmbeddedDocumentKeepsFailingChangeset(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $address->setCity('Munich'); + + $this->createFailpoint('update'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + $this->assertTrue($this->uow->isScheduledForUpdate($user)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertTrue($this->uow->isScheduledForUpdate($address)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($address)); + } + + public function testUpdateWithEmbeddedDocumentClearsChangesets(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $address->setCity('Munich'); + + $this->uow->commit(); + + $this->assertFalse($this->uow->isScheduledForUpdate($user)); + $this->assertEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertFalse($this->uow->isScheduledForUpdate($address)); + $this->assertEquals([], $this->uow->getDocumentChangeSet($address)); + } + + public function testUpdateErrorWithRemovedEmbeddedDocumentKeepsFailingChangeset(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $user->removeAddress(); + + $this->createFailpoint('update'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + $this->assertTrue($this->uow->isScheduledForUpdate($user)); + $this->assertNotEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertTrue($this->uow->isScheduledForDelete($address)); + + // As $address is orphaned after changeset computation, it is removed from the identity map + $this->assertFalse($this->uow->isInIdentityMap($address)); + } + + public function testUpdateWithRemovedEmbeddedDocumentClearsChangesets(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $user->removeAddress(); + + $this->uow->commit(); + + $this->assertFalse($this->uow->isScheduledForUpdate($user)); + $this->assertEquals([], $this->uow->getDocumentChangeSet($user)); + $this->assertFalse($this->uow->isScheduledForDelete($address)); + $this->assertFalse($this->uow->isInIdentityMap($address)); + } + + public function testDeleteErrorKeepsFailingDelete(): void + { + $user = new ForumUser(); + $user->username = 'alcaeus'; + $this->uow->persist($user); + $this->uow->commit(); + + $this->uow->remove($user); + + // Make sure delete command fails once + $this->createFailpoint('delete'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // The document still exists, the deletion is still scheduled + self::assertSame( + 1, + $this->dm->getDocumentCollection(ForumUser::class)->countDocuments(['username' => 'alcaeus']), + ); + + self::assertTrue($this->uow->isScheduledForDelete($user)); + } + + public function testDeleteErrorWithEmbeddedDocumentKeepsChangeset(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $this->uow->remove($user); + + // Make sure delete command fails once + $this->createFailpoint('delete'); + + try { + $this->uow->commit(); + self::fail('Expected exception when committing'); + } catch (Throwable) { + } + + // The document still exists, the deletion is still scheduled + self::assertSame( + 1, + $this->dm->getDocumentCollection(User::class)->countDocuments(['username' => 'alcaeus']), + ); + + self::assertTrue($this->uow->isScheduledForDelete($user)); + self::assertTrue($this->uow->isScheduledForDelete($address)); + } + + public function testDeleteWithEmbeddedDocumentClearsChangeset(): void + { + $address = new Address(); + $address->setCity('Olching'); + + $user = new User(); + $user->setUsername('alcaeus'); + $user->setAddress($address); + + $this->uow->persist($user); + $this->uow->commit(); + + $this->uow->remove($user); + + $this->uow->commit(); + + self::assertSame( + 0, + $this->dm->getDocumentCollection(User::class)->countDocuments(['username' => 'alcaeus']), + ); + + self::assertFalse($this->uow->isScheduledForDelete($user)); + self::assertFalse($this->uow->isScheduledForDelete($address)); + } + + /** Create a document manager with a single host to ensure failpoints target the correct server */ + protected static function createTestDocumentManager(): DocumentManager + { + $config = static::getConfiguration(); + $client = new Client(self::getUri(false), [], ['typeMap' => ['root' => 'array', 'document' => 'array']]); + + return DocumentManager::create($client, $config); + } + + private function createFailpoint(string $commandName): void + { + $this->dm->getClient()->selectDatabase('admin')->command([ + 'configureFailPoint' => 'failCommand', + 'mode' => ['times' => 1], + 'data' => [ + 'errorCode' => 192, // FailPointEnabled + 'failCommands' => [$commandName], + ], + ]); + } +}