Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore document proxy state to uninitialized on load exception #2521

Merged
merged 1 commit into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use ProxyManager\Factory\LazyLoadingGhostFactory;
use ProxyManager\Proxy\GhostObjectInterface;
use ReflectionProperty;
use Throwable;

use function array_filter;
use function count;
Expand Down Expand Up @@ -108,7 +109,15 @@ private function createInitializer(
$initializer = null;
$identifier = $metadata->getIdentifierValue($ghostObject);

if (! $documentPersister->load(['_id' => $identifier], $ghostObject)) {
try {
$document = $documentPersister->load(['_id' => $identifier], $ghostObject);
} catch (Throwable $exception) {
$initializer = $originalInitializer;

throw $exception;
}

if (! $document) {
$initializer = $originalInitializer;

if (! $this->lifecycleEventManager->documentNotFound($ghostObject, $identifier)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
<?php

declare(strict_types=1);

namespace Doctrine\ODM\MongoDB\Tests\Proxy\Factory;

use Closure;
use Doctrine\ODM\MongoDB\DocumentManager;
use Doctrine\ODM\MongoDB\Event\DocumentNotFoundEventArgs;
use Doctrine\ODM\MongoDB\Events;
use Doctrine\ODM\MongoDB\LockException;
use Doctrine\ODM\MongoDB\Tests\BaseTestCase;
use Documents\Cart;
use MongoDB\Client;
use MongoDB\Collection;
use MongoDB\Database;
use PHPUnit\Framework\MockObject\MockObject;
use ProxyManager\Proxy\GhostObjectInterface;

class StaticProxyFactoryTest extends BaseTestCase
{
/** @var Client|MockObject */
private Client $client;

public function setUp(): void
{
parent::setUp();

$this->dm = $this->createMockedDocumentManager();
}

public function testProxyInitializeWithException(): void
{
$collection = $this->createMock(Collection::class);
$database = $this->createMock(Database::class);

$this->client->expects($this->once())
->method('selectDatabase')
->willReturn($database);

$database->expects($this->once())
->method('selectCollection')
->willReturn($collection);

$collection->expects($this->once())
->method('findOne')
->willThrowException(LockException::lockFailed(null));
Copy link
Member

@malarzm malarzm Apr 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn so here goes nothing: how about instead of mocking 3 levels deep we mock DocumentPersister, have it thrown an exception on load? This would require manipulating UnitOfWork::$documentPersisters with reflection though, but the test should be easier to maintain with real database connection. For your consideration :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I also wanted to mock DocumentPersister::load, but DocumentPersister is a final class. So my best shot was to mock Collection::findOne

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed then!


$uow = $this->dm->getUnitOfWork();

$proxy = $this->dm->getReference(Cart::class, '123');
self::assertInstanceOf(GhostObjectInterface::class, $proxy);

$closure = static function (DocumentNotFoundEventArgs $eventArgs) {
self::fail('DocumentNotFoundListener should not be called');
};
$this->dm->getEventManager()->addEventListener(Events::documentNotFound, new DocumentNotFoundListener($closure));

try {
$proxy->initializeProxy();
self::fail('An exception should have been thrown');
} catch (LockException $exception) {
self::assertInstanceOf(LockException::class, $exception);
}

$uow->computeChangeSets();

self::assertFalse($proxy->isProxyInitialized(), 'Proxy should not be initialized');
}

public function tearDown(): void
{
// db connection is mocked, nothing to clean up
}

private function createMockedDocumentManager(): DocumentManager
{
$config = $this->getConfiguration();

$this->client = $this->createMock(Client::class);

return DocumentManager::create($this->client, $config);
}
}

class DocumentNotFoundListener
{
private Closure $closure;

public function __construct(Closure $closure)
{
$this->closure = $closure;
}

public function documentNotFound(DocumentNotFoundEventArgs $eventArgs): void
{
$closure = $this->closure;
$closure($eventArgs);
}
}