From 46003140af3bf9896735ee0a8c8b5e1a9de792f0 Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Wed, 5 Apr 2023 07:22:30 -0300 Subject: [PATCH] Improve exception message at `ModelManager::batchDelete()` (#813) --- src/Model/ModelManager.php | 29 +++++- tests/Model/ModelManagerTest.php | 146 +++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 3 deletions(-) diff --git a/src/Model/ModelManager.php b/src/Model/ModelManager.php index a423107c..6f9b3a89 100755 --- a/src/Model/ModelManager.php +++ b/src/Model/ModelManager.php @@ -39,6 +39,8 @@ final class ModelManager implements ModelManagerInterface, ProxyResolverInterfac { public const ID_SEPARATOR = '-'; + private const BATCH_SIZE = 20; + public function __construct( private ManagerRegistry $registry, private PropertyAccessorInterface $propertyAccessor @@ -219,7 +221,7 @@ public function addIdentifiersToQuery(string $class, BaseProxyQueryInterface $qu $queryBuilder->field('_id')->in($idx); } - public function batchDelete(string $class, BaseProxyQueryInterface $query): void + public function batchDelete(string $class, BaseProxyQueryInterface $query, int $batchSize = self::BATCH_SIZE): void { if (!$query instanceof ProxyQueryInterface) { throw new \TypeError(sprintf('The query MUST implement %s.', ProxyQueryInterface::class)); @@ -234,13 +236,15 @@ public function batchDelete(string $class, BaseProxyQueryInterface $query): void \assert($iterator instanceof Iterator); $i = 0; + $confirmedDeletionsCount = 0; try { foreach ($iterator as $object) { $documentManager->remove($object); - if (0 === (++$i % 20)) { + if (0 === (++$i % $batchSize)) { $documentManager->flush(); + $confirmedDeletionsCount = $i; $documentManager->clear(); } } @@ -248,8 +252,27 @@ public function batchDelete(string $class, BaseProxyQueryInterface $query): void $documentManager->flush(); $documentManager->clear(); } catch (Exception|MongoDBException $exception) { + $id = null; + + if (isset($object)) { + $id = $this->getNormalizedIdentifier($object); + } + + if (null === $id) { + throw new ModelManagerException( + sprintf('Failed to perform batch deletion for "%s" objects', $class), + $exception->getCode(), + $exception + ); + } + + $msg = 'Failed to delete object "%s" (id: %s) while performing batch deletion'; + if ($i > $batchSize) { + $msg .= sprintf(' (%u objects were successfully deleted before this error)', $confirmedDeletionsCount); + } + throw new ModelManagerException( - sprintf('Failed to delete object: %s', $class), + sprintf($msg, $class, $id), $exception->getCode(), $exception ); diff --git a/tests/Model/ModelManagerTest.php b/tests/Model/ModelManagerTest.php index 5d3d39f9..f833bb12 100644 --- a/tests/Model/ModelManagerTest.php +++ b/tests/Model/ModelManagerTest.php @@ -13,16 +13,26 @@ namespace Sonata\DoctrineMongoDBAdminBundle\Tests\Model; +use Doctrine\Common\EventManager; +use Doctrine\ODM\MongoDB\Configuration; use Doctrine\ODM\MongoDB\DocumentManager; +use Doctrine\ODM\MongoDB\Hydrator\HydratorFactory; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\Query\Builder; +use Doctrine\ODM\MongoDB\Query\Query; use Doctrine\ODM\MongoDB\Repository\DocumentRepository; +use Doctrine\ODM\MongoDB\UnitOfWork; +use MongoDB\Collection; +use MongoDB\Driver\Exception\RuntimeException; use PHPUnit\Framework\MockObject\Stub; +use PHPUnit\Framework\MockObject\Stub\Exception as ExceptionStub; use PHPUnit\Framework\TestCase; +use Sonata\AdminBundle\Exception\ModelManagerException; use Sonata\DoctrineMongoDBAdminBundle\Datagrid\ProxyQuery; use Sonata\DoctrineMongoDBAdminBundle\Model\ModelManager; use Sonata\DoctrineMongoDBAdminBundle\Tests\ClassMetadataAnnotationTrait; use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\DocumentWithReferences; +use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\EmbeddedDocument; use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\SimpleDocumentWithPrivateSetter; use Sonata\DoctrineMongoDBAdminBundle\Tests\Fixtures\Document\TestDocument; use Symfony\Bridge\Doctrine\ManagerRegistry; @@ -199,6 +209,142 @@ public function supportsQueryDataProvider(): iterable yield [false, new \stdClass()]; } + /** + * @return iterable>> + * + * @phpstan-return iterable, 2: array}> + */ + public function failingBatchDeleteProvider(): iterable + { + yield [ + '#^Failed to delete object "Sonata\\\DoctrineMongoDBAdminBundle\\\Tests\\\Fixtures\\\Document\\\DocumentWithReferences"' + .' \(id: [a-z0-9]{32}\) while performing batch deletion \(20 objects were successfully deleted before this error\)$#', + array_fill(0, 21, new DocumentWithReferences('test', new EmbeddedDocument())), + [null, static::throwException(new RuntimeException())], + ]; + + yield [ + '#^Failed to delete object "Sonata\\\DoctrineMongoDBAdminBundle\\\Tests\\\Fixtures\\\Document\\\DocumentWithReferences"' + .' \(id: [a-z0-9]{32}\) while performing batch deletion$#', + [new DocumentWithReferences('test', new EmbeddedDocument()), new DocumentWithReferences('test', new EmbeddedDocument())], + [static::throwException(new RuntimeException())], + ]; + + yield [ + '#^Failed to perform batch deletion for "Sonata\\\DoctrineMongoDBAdminBundle\\\Tests\\\Fixtures\\\Document\\\DocumentWithReferences"' + .' objects$#', + [], + [static::throwException(new RuntimeException())], + ]; + } + + /** + * @param array $result + * @param array $onConsecutiveFlush + * + * @dataProvider failingBatchDeleteProvider + */ + public function testFailingBatchDelete(string $expectedExceptionMessage, array $result, array $onConsecutiveFlush): void + { + $batchSize = 20; + + $classMetadata = $this->createMock(ClassMetadata::class); + $classMetadata->expects([] === $result ? static::never() : static::atLeastOnce()) + ->method('newInstance') + ->willReturn(new DocumentWithReferences('test', new EmbeddedDocument())); + $classMetadata->name = DocumentWithReferences::class; + + $dm = $this->createMock(DocumentManager::class); + $dm + ->expects([] === $result ? static::never() : static::atLeastOnce()) + ->method('contains') + ->willReturnCallback(static fn (object $document): bool => $document instanceof DocumentWithReferences); + + $collection = $this->createMock(Collection::class); + $collection + ->expects(static::atLeastOnce()) + ->method('find') + ->willReturn((static function () use ($result): \Traversable { + foreach ($result as $document) { + yield [ + '_id' => $document->id, + ]; + } + })()); + + $queryBuilder = $this->createMock(Builder::class); + $queryBuilder + ->expects(static::atLeastOnce()) + ->method('getQuery') + ->willReturn(new Query( + $dm, + $classMetadata, + $collection, + [ + 'type' => Query::TYPE_FIND, + 'query' => ['$id' => '00000000000011190000000000000000'], + ] + )); + + $documentRepository = $this->createMock(DocumentRepository::class); + $documentRepository + ->expects(static::atLeastOnce()) + ->method('createQueryBuilder') + ->willReturn($queryBuilder); + + $dm + ->expects(static::atLeastOnce()) + ->method('getRepository') + ->with(DocumentWithReferences::class) + ->willReturn($documentRepository); + $dm->expects([] === $result ? static::never() : static::atLeastOnce()) + ->method('getClassMetadata') + ->with(DocumentWithReferences::class) + ->willReturn($classMetadata); + + $registry = $this->createMock(ManagerRegistry::class); + $registry->expects(static::atLeastOnce()) + ->method('getManagerForClass') + ->with(DocumentWithReferences::class) + ->willReturn($dm); + + $dm + ->expects(static::exactly(\count($result))) + ->method('remove'); + $dm + ->expects(static::exactly([] === $result ? 1 : (int) ceil(\count($result) / $batchSize))) + ->method('flush') + ->will(static::onConsecutiveCalls( + ...$onConsecutiveFlush + )); + + $eventManager = new EventManager(); + $hydratorFactory = new HydratorFactory( + $dm, + $eventManager, + sys_get_temp_dir(), + 'Sonata\DoctrineMongoDBAdminBundle\Tests\Hydrator', + Configuration::AUTOGENERATE_FILE_NOT_EXISTS + ); + $uow = new UnitOfWork($dm, $eventManager, $hydratorFactory); + /** @psalm-suppress InternalMethod */ + $hydratorFactory->setUnitOfWork($uow); + + $dm + ->expects(static::atLeastOnce()) + ->method('getUnitOfWork') + ->willReturn($uow); + + $modelManager = new ModelManager($registry, $this->propertyAccessor); + + $proxyQuery = $modelManager->createQuery(DocumentWithReferences::class); + + $this->expectException(ModelManagerException::class); + $this->expectExceptionMessageMatches($expectedExceptionMessage); + + $modelManager->batchDelete(DocumentWithReferences::class, $proxyQuery, $batchSize); + } + /** * @phpstan-template T of object * @phpstan-param class-string $class