From 0de48d2190b03583703c3084a1a8296e4d8cb17b Mon Sep 17 00:00:00 2001 From: nurikabe Date: Sat, 29 Mar 2014 18:36:06 -0400 Subject: [PATCH] Use identifiers for bulk delete rather than cloning objects --- Doctrine/Listener.php | 41 ++++++++++++++++++++++--- Persister/ObjectPersister.php | 10 ++++++ Persister/ObjectPersisterInterface.php | 7 +++++ Tests/Doctrine/AbstractListenerTest.php | 23 ++++++-------- 4 files changed, 63 insertions(+), 18 deletions(-) diff --git a/Doctrine/Listener.php b/Doctrine/Listener.php index 43ac744..b6217a6 100644 --- a/Doctrine/Listener.php +++ b/Doctrine/Listener.php @@ -9,7 +9,12 @@ use FOS\ElasticaBundle\Persister\ObjectPersister; use Symfony\Component\ExpressionLanguage\Expression; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; use Symfony\Component\ExpressionLanguage\SyntaxError; +use Symfony\Component\PropertyAccess\PropertyAccess; +/** + * Automatically update ElasticSearch based on changes to the Doctrine source + * data. One listener is generated for each Doctrine entity / ElasticSearch type. + */ class Listener implements EventSubscriber { /** @@ -48,10 +53,14 @@ class Listener implements EventSubscriber protected $isIndexableCallback; /** - * Objects scheduled for insertion, replacement, or removal + * Objects scheduled for insertion and replacement */ public $scheduledForInsertion = array(); public $scheduledForUpdate = array(); + + /** + * IDs of objects scheduled for removal + */ public $scheduledForDeletion = array(); /** @@ -61,6 +70,13 @@ class Listener implements EventSubscriber */ protected $expressionLanguage; + /** + * PropertyAccessor instance + * + * @var PropertyAccessorInterface + */ + protected $propertyAccessor; + /** * Constructor. * @@ -75,6 +91,8 @@ class Listener implements EventSubscriber $this->objectClass = $objectClass; $this->events = $events; $this->esIdentifierField = $esIdentifierField; + + $this->propertyAccessor = PropertyAccess::createPropertyAccessor(); } /** @@ -195,20 +213,21 @@ class Listener implements EventSubscriber $this->scheduledForUpdate[] = $entity; } else { // Delete if no longer indexable - $this->scheduledForDeletion[] = clone $entity; + $this->scheduleForDeletion($entity); } } } /** - * Delete objects preRemove instead of postRemove so that we have access to the id + * Delete objects preRemove instead of postRemove so that we have access to the id. Because this is called + * preRemove, first check that the entity is managed by Doctrine */ public function preRemove(EventArgs $eventArgs) { $entity = $eventArgs->getEntity(); if ($entity instanceof $this->objectClass) { - $this->scheduledForDeletion[] = clone $entity; + $this->scheduleForDeletion($entity); } } @@ -224,7 +243,7 @@ class Listener implements EventSubscriber $this->objectPersister->replaceMany($this->scheduledForUpdate); } if (count($this->scheduledForDeletion)) { - $this->objectPersister->deleteMany($this->scheduledForDeletion); + $this->objectPersister->deleteManyByIdentifiers($this->scheduledForDeletion); } } @@ -245,4 +264,16 @@ class Listener implements EventSubscriber { $this->persistScheduled(); } + + /** + * Record the specified identifier to delete. Do not need to entire object. + * @param mixed $object + * @return mixed + */ + protected function scheduleForDeletion($object) + { + if ($identifierValue = $this->propertyAccessor->getValue($object, $this->esIdentifierField)) { + $this->scheduledForDeletion[] = $identifierValue; + } + } } diff --git a/Persister/ObjectPersister.php b/Persister/ObjectPersister.php index 3592a78..64cf5db 100644 --- a/Persister/ObjectPersister.php +++ b/Persister/ObjectPersister.php @@ -126,6 +126,16 @@ class ObjectPersister implements ObjectPersisterInterface $this->type->deleteDocuments($documents); } + /** + * Bulk deletes records from an array of identifiers + * + * @param array $identifiers array of domain model object identifiers + */ + public function deleteManyByIdentifiers(array $identifiers) + { + $this->type->getIndex()->getClient()->deleteIds($identifiers, $this->type->getIndex(), $this->type); + } + /** * Transforms an object to an elastica document * diff --git a/Persister/ObjectPersisterInterface.php b/Persister/ObjectPersisterInterface.php index a25aafc..2b4c8ee 100644 --- a/Persister/ObjectPersisterInterface.php +++ b/Persister/ObjectPersisterInterface.php @@ -61,4 +61,11 @@ interface ObjectPersisterInterface * @param array $objects array of domain model objects */ function deleteMany(array $objects); + + /** + * Bulk deletes records from an array of identifiers + * + * @param array $identifiers array of domain model object identifiers + */ + public function deleteManyByIdentifiers(array $identifiers); } diff --git a/Tests/Doctrine/AbstractListenerTest.php b/Tests/Doctrine/AbstractListenerTest.php index 8e49f24..ee657f1 100644 --- a/Tests/Doctrine/AbstractListenerTest.php +++ b/Tests/Doctrine/AbstractListenerTest.php @@ -100,13 +100,13 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $listener->postUpdate($eventArgs); $this->assertEmpty($listener->scheduledForUpdate); - $this->assertEquals($entity, current($listener->scheduledForDeletion)); + $this->assertEquals($entity->getId(), current($listener->scheduledForDeletion)); $persister->expects($this->never()) ->method('replaceOne'); $persister->expects($this->once()) - ->method('deleteMany') - ->with(array($entity)); + ->method('deleteManyByIdentifiers') + ->with(array($entity->getId())); $listener->postFlush($eventArgs); } @@ -133,13 +133,11 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $listener = $this->createListener($persister, get_class($entity), array()); $listener->preRemove($eventArgs); - $scheduledClone = current($listener->scheduledForDeletion); - $this->assertEquals($entity, $scheduledClone); - $this->assertNotSame($entity, $scheduledClone); + $this->assertEquals($entity->getId(), current($listener->scheduledForDeletion)); $persister->expects($this->once()) - ->method('deleteMany') - ->with(array($entity)); + ->method('deleteManyByIdentifiers') + ->with(array($entity->getId())); $listener->postFlush($eventArgs); } @@ -151,6 +149,7 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $persister = $this->getMockPersister(); $entity = new Listener\Entity(1); + $entity->identifier = 'foo'; $eventArgs = $this->createLifecycleEventArgs($entity, $objectManager); $objectManager->expects($this->any()) @@ -166,13 +165,11 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $listener = $this->createListener($persister, get_class($entity), array(), 'identifier'); $listener->preRemove($eventArgs); - $scheduledClone = current($listener->scheduledForDeletion); - $this->assertEquals($entity, $scheduledClone); - $this->assertNotSame($entity, $scheduledClone); + $this->assertEquals($entity->identifier, current($listener->scheduledForDeletion)); $persister->expects($this->once()) - ->method('deleteMany') - ->with(array($entity)); + ->method('deleteManyByIdentifiers') + ->with(array($entity->identifier)); $listener->postFlush($eventArgs); }