From 65be0a415ea8b716582e4aa435c11528ae74b8ea Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Wed, 25 Jan 2012 19:51:10 +0000 Subject: [PATCH] Fixing issue with removing mapped Doctrine entities/documents --- DependencyInjection/FOQElasticaExtension.php | 25 ++++++--- Doctrine/AbstractListener.php | 26 +++++++-- Doctrine/MongoDB/Listener.php | 11 +++- Doctrine/ORM/Listener.php | 12 ++++- Persister/ObjectPersister.php | 22 ++++++-- Persister/ObjectPersisterInterface.php | 9 ++++ Resources/config/mongodb.xml | 1 + Resources/config/orm.xml | 1 + Tests/Doctrine/MongoDB/ListenerTest.php | 57 +++++++++++++++++--- Tests/Doctrine/ORM/ListenerTest.php | 57 +++++++++++++++++--- 10 files changed, 191 insertions(+), 30 deletions(-) diff --git a/DependencyInjection/FOQElasticaExtension.php b/DependencyInjection/FOQElasticaExtension.php index 7b149a4..9df6634 100644 --- a/DependencyInjection/FOQElasticaExtension.php +++ b/DependencyInjection/FOQElasticaExtension.php @@ -316,13 +316,8 @@ class FOQElasticaExtension extends Extension $listenerDef = new DefinitionDecorator($abstractListenerId); $listenerDef->replaceArgument(0, new Reference($objectPersisterId)); $listenerDef->replaceArgument(1, $typeConfig['model']); - $events = array(); - $doctrineEvents = array('insert' => 'postPersist', 'update' => 'postUpdate', 'delete' => 'postRemove'); - foreach ($doctrineEvents as $event => $doctrineEvent) { - if (isset($typeConfig['listener'][$event]) && $typeConfig['listener'][$event]) { - $events[] = $doctrineEvent; - } - } + $listenerDef->replaceArgument(3, $typeConfig['identifier']); + $events = $this->getDoctrineEvents($typeConfig); $listenerDef->replaceArgument(2, $events); switch ($typeConfig['driver']) { case 'orm': $listenerDef->addTag('doctrine.event_subscriber'); break; @@ -333,6 +328,22 @@ class FOQElasticaExtension extends Extension return $listenerId; } + private function getDoctrineEvents(array $typeConfig) + { + $events = array(); + $eventMapping = array( + 'insert' => array('postPersist'), + 'update' => array('postUpdate'), + 'delete' => array('postRemove', 'preRemove') + ); + foreach ($eventMapping as $event => $doctrineEvents) { + if (isset($typeConfig['listener'][$event]) && $typeConfig['listener'][$event]) { + $events = array_merge($events, $doctrineEvents); + } + } + return $events; + } + protected function loadTypeFinder(array $typeConfig, ContainerBuilder $container, $elasticaToModelId, $typeDef, $indexName, $typeName) { if (isset($typeConfig['finder']['service'])) { diff --git a/Doctrine/AbstractListener.php b/Doctrine/AbstractListener.php index 343f9c2..a16bc73 100644 --- a/Doctrine/AbstractListener.php +++ b/Doctrine/AbstractListener.php @@ -28,14 +28,19 @@ abstract class AbstractListener */ protected $events; + protected $identifier; + protected $scheduledForRemoval; + /** * Constructor **/ - public function __construct(ObjectPersisterInterface $objectPersister, $objectClass, array $events) + public function __construct(ObjectPersisterInterface $objectPersister, $objectClass, array $events, $identifier = 'id') { - $this->objectPersister = $objectPersister; - $this->objectClass = $objectClass; - $this->events = $events; + $this->objectPersister = $objectPersister; + $this->objectClass = $objectClass; + $this->events = $events; + $this->identifier = $identifier; + $this->scheduledForRemoval = new \SplObjectStorage(); } /** @@ -46,4 +51,17 @@ abstract class AbstractListener return $this->events; } + protected function scheduleForRemoval($object) + { + $getIdentifierMethod = 'get' . ucfirst($this->identifier); + $this->scheduledForRemoval[$object] = $object->$getIdentifierMethod(); + } + + protected function removeIfScheduled($object) + { + if (isset($this->scheduledForRemoval[$object])) { + $this->objectPersister->deleteById($this->scheduledForRemoval[$object]); + } + } + } diff --git a/Doctrine/MongoDB/Listener.php b/Doctrine/MongoDB/Listener.php index 89c7502..352ff36 100644 --- a/Doctrine/MongoDB/Listener.php +++ b/Doctrine/MongoDB/Listener.php @@ -26,12 +26,21 @@ class Listener extends AbstractListener implements EventSubscriber } } + public function preRemove(LifecycleEventArgs $eventArgs) + { + $document = $eventArgs->getDocument(); + + if ($document instanceof $this->objectClass) { + $this->scheduleForRemoval($document); + } + } + public function postRemove(LifecycleEventArgs $eventArgs) { $document = $eventArgs->getDocument(); if ($document instanceof $this->objectClass) { - $this->objectPersister->deleteOne($document); + $this->removeIfScheduled($document); } } } diff --git a/Doctrine/ORM/Listener.php b/Doctrine/ORM/Listener.php index 7e5bea8..4a1f30d 100644 --- a/Doctrine/ORM/Listener.php +++ b/Doctrine/ORM/Listener.php @@ -26,12 +26,22 @@ class Listener extends AbstractListener implements EventSubscriber } } + public function preRemove(LifecycleEventArgs $eventArgs) + { + $entity = $eventArgs->getEntity(); + + if ($entity instanceof $this->objectClass) { + $this->scheduleForRemoval($entity); + } + } + public function postRemove(LifecycleEventArgs $eventArgs) { $entity = $eventArgs->getEntity(); if ($entity instanceof $this->objectClass) { - $this->objectPersister->deleteOne($entity); + $this->removeIfScheduled($entity); } } + } diff --git a/Persister/ObjectPersister.php b/Persister/ObjectPersister.php index 87653f4..03a2f1e 100644 --- a/Persister/ObjectPersister.php +++ b/Persister/ObjectPersister.php @@ -61,12 +61,28 @@ class ObjectPersister implements ObjectPersisterInterface * @param object $object * @return null **/ - public function deleteOne($object) + public function deleteOne($object, $id = null) { - $document = $this->transformToElasticaDocument($object); - $this->type->deleteById($document->getId()); + if (!$id) { + $document = $this->transformToElasticaDocument($object); + $id = $document->getId(); + } + $this->type->deleteById($id); } + /** + * Deletes one object in the type by id + * + * @param mixed $id + * + * @return null + **/ + public function deleteById($id) + { + $this->type->deleteById($id); + } + + /** * Inserts an array of objects in the type * diff --git a/Persister/ObjectPersisterInterface.php b/Persister/ObjectPersisterInterface.php index 3cf12d2..962a88c 100644 --- a/Persister/ObjectPersisterInterface.php +++ b/Persister/ObjectPersisterInterface.php @@ -32,6 +32,15 @@ interface ObjectPersisterInterface **/ function deleteOne($object); + /** + * Deletes one object in the type by id + * + * @param mixed $id + * + * @return null + **/ + function deleteById($id); + /** * Inserts an array of objects in the type * diff --git a/Resources/config/mongodb.xml b/Resources/config/mongodb.xml index 1fc8fe3..af24531 100644 --- a/Resources/config/mongodb.xml +++ b/Resources/config/mongodb.xml @@ -18,6 +18,7 @@ + diff --git a/Resources/config/orm.xml b/Resources/config/orm.xml index 86869de..71fa2aa 100644 --- a/Resources/config/orm.xml +++ b/Resources/config/orm.xml @@ -18,6 +18,7 @@ + diff --git a/Tests/Doctrine/MongoDB/ListenerTest.php b/Tests/Doctrine/MongoDB/ListenerTest.php index eff42d7..bb12d20 100644 --- a/Tests/Doctrine/MongoDB/ListenerTest.php +++ b/Tests/Doctrine/MongoDB/ListenerTest.php @@ -4,7 +4,20 @@ namespace FOQ\ElasticaBundle\Tests\Doctrine\MongoDB; use FOQ\ElasticaBundle\Doctrine\MongoDB\Listener; -class Document{} +class Document +{ + + public function getId() + { + return ListenerTest::DOCUMENT_ID; + } + + public function getIdentifier() + { + return ListenerTest::DOCUMENT_IDENTIFIER; + } + +} /** * @author Richard Miller @@ -12,6 +25,9 @@ class Document{} class ListenerTest extends \PHPUnit_Framework_TestCase { + const DOCUMENT_ID = 78; + const DOCUMENT_IDENTIFIER = 826; + public function testObjectInsertedOnPersist() { $persisterMock = $this->getMockBuilder('FOQ\ElasticaBundle\Persister\ObjectPersisterInterface') @@ -33,7 +49,7 @@ class ListenerTest extends \PHPUnit_Framework_TestCase ->method('insertOne') ->with($this->equalTo($document)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); $listener->postPersist($eventArgsMock); } @@ -58,7 +74,7 @@ class ListenerTest extends \PHPUnit_Framework_TestCase ->method('replaceOne') ->with($this->equalTo($document)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); $listener->postUpdate($eventArgsMock); } @@ -75,15 +91,42 @@ class ListenerTest extends \PHPUnit_Framework_TestCase $objectName = 'FOQ\ElasticaBundle\Tests\Doctrine\MongoDB\Document'; $document = new Document(); - $eventArgsMock->expects($this->once()) + $eventArgsMock->expects($this->any()) ->method('getDocument') ->will($this->returnValue($document)); $persisterMock->expects($this->once()) - ->method('deleteOne') - ->with($this->equalTo($document)); + ->method('deleteById') + ->with($this->equalTo(self::DOCUMENT_ID)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); + $listener->preRemove($eventArgsMock); + $listener->postRemove($eventArgsMock); + } + + public function testObjectWithNonStandardIdentifierDeletedOnRemove() + { + $persisterMock = $this->getMockBuilder('FOQ\ElasticaBundle\Persister\ObjectPersisterInterface') + ->disableOriginalConstructor() + ->getMock(); + + $eventArgsMock = $this->getMockBuilder('Doctrine\ODM\MongoDB\Event\LifecycleEventArgs') + ->disableOriginalConstructor() + ->getMock(); + + $objectName = 'FOQ\ElasticaBundle\Tests\Doctrine\MongoDB\Document'; + $document = new Document(); + + $eventArgsMock->expects($this->any()) + ->method('getDocument') + ->will($this->returnValue($document)); + + $persisterMock->expects($this->once()) + ->method('deleteById') + ->with($this->equalTo(self::DOCUMENT_IDENTIFIER)); + + $listener = new Listener($persisterMock, $objectName, array(), 'identifier'); + $listener->preRemove($eventArgsMock); $listener->postRemove($eventArgsMock); } diff --git a/Tests/Doctrine/ORM/ListenerTest.php b/Tests/Doctrine/ORM/ListenerTest.php index ac733f7..9bf376b 100644 --- a/Tests/Doctrine/ORM/ListenerTest.php +++ b/Tests/Doctrine/ORM/ListenerTest.php @@ -4,7 +4,20 @@ namespace FOQ\ElasticaBundle\Tests\Doctrine\ORM; use FOQ\ElasticaBundle\Doctrine\ORM\Listener; -class Entity{} +class Entity +{ + + public function getId() + { + return ListenerTest::ENTITY_ID; + } + + public function getIdentifier() + { + return ListenerTest::ENTITY_IDENTIFIER; + } + +} /** * @author Richard Miller @@ -12,6 +25,9 @@ class Entity{} class ListenerTest extends \PHPUnit_Framework_TestCase { + const ENTITY_ID = 21; + const ENTITY_IDENTIFIER = 912; + public function testObjectInsertedOnPersist() { $persisterMock = $this->getMockBuilder('FOQ\ElasticaBundle\Persister\ObjectPersisterInterface') @@ -33,7 +49,7 @@ class ListenerTest extends \PHPUnit_Framework_TestCase ->method('insertOne') ->with($this->equalTo($entity)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); $listener->postPersist($eventArgsMock); } @@ -58,7 +74,7 @@ class ListenerTest extends \PHPUnit_Framework_TestCase ->method('replaceOne') ->with($this->equalTo($entity)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); $listener->postUpdate($eventArgsMock); } @@ -75,15 +91,42 @@ class ListenerTest extends \PHPUnit_Framework_TestCase $objectName = 'FOQ\ElasticaBundle\Tests\Doctrine\ORM\Entity'; $entity = new Entity; - $eventArgsMock->expects($this->once()) + $eventArgsMock->expects($this->any()) ->method('getEntity') ->will($this->returnValue($entity)); $persisterMock->expects($this->once()) - ->method('deleteOne') - ->with($this->equalTo($entity)); + ->method('deleteById') + ->with($this->equalTo(self::ENTITY_ID)); - $listener = new Listener($persisterMock, $objectName, array(), null); + $listener = new Listener($persisterMock, $objectName, array()); + $listener->preRemove($eventArgsMock); + $listener->postRemove($eventArgsMock); + } + + public function testObjectWithNonStandardIdentifierDeletedOnRemove() + { + $persisterMock = $this->getMockBuilder('FOQ\ElasticaBundle\Persister\ObjectPersisterInterface') + ->disableOriginalConstructor() + ->getMock(); + + $eventArgsMock = $this->getMockBuilder('Doctrine\ORM\Event\LifecycleEventArgs') + ->disableOriginalConstructor() + ->getMock(); + + $objectName = 'FOQ\ElasticaBundle\Tests\Doctrine\ORM\Entity'; + $entity = new Entity; + + $eventArgsMock->expects($this->any()) + ->method('getEntity') + ->will($this->returnValue($entity)); + + $persisterMock->expects($this->once()) + ->method('deleteById') + ->with($this->equalTo(self::ENTITY_IDENTIFIER)); + + $listener = new Listener($persisterMock, $objectName, array(), 'identifier'); + $listener->preRemove($eventArgsMock); $listener->postRemove($eventArgsMock); }