From 720917f609381921db52622a9796b2c13a84647a Mon Sep 17 00:00:00 2001 From: nurikabe Date: Mon, 24 Mar 2014 11:16:31 -0400 Subject: [PATCH 1/3] Clone entities on delete to preserve ids --- Doctrine/Listener.php | 7 +++++-- Tests/Doctrine/AbstractListenerTest.php | 8 ++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Doctrine/Listener.php b/Doctrine/Listener.php index c254513..43ac744 100644 --- a/Doctrine/Listener.php +++ b/Doctrine/Listener.php @@ -195,17 +195,20 @@ class Listener implements EventSubscriber $this->scheduledForUpdate[] = $entity; } else { // Delete if no longer indexable - $this->scheduledForDeletion[] = $entity; + $this->scheduledForDeletion[] = clone $entity; } } } + /** + * Delete objects preRemove instead of postRemove so that we have access to the id + */ public function preRemove(EventArgs $eventArgs) { $entity = $eventArgs->getEntity(); if ($entity instanceof $this->objectClass) { - $this->scheduledForDeletion[] = $entity; + $this->scheduledForDeletion[] = clone $entity; } } diff --git a/Tests/Doctrine/AbstractListenerTest.php b/Tests/Doctrine/AbstractListenerTest.php index a9eff66..8e49f24 100644 --- a/Tests/Doctrine/AbstractListenerTest.php +++ b/Tests/Doctrine/AbstractListenerTest.php @@ -133,7 +133,9 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $listener = $this->createListener($persister, get_class($entity), array()); $listener->preRemove($eventArgs); - $this->assertEquals($entity, current($listener->scheduledForDeletion)); + $scheduledClone = current($listener->scheduledForDeletion); + $this->assertEquals($entity, $scheduledClone); + $this->assertNotSame($entity, $scheduledClone); $persister->expects($this->once()) ->method('deleteMany') @@ -164,7 +166,9 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase $listener = $this->createListener($persister, get_class($entity), array(), 'identifier'); $listener->preRemove($eventArgs); - $this->assertEquals($entity, current($listener->scheduledForDeletion)); + $scheduledClone = current($listener->scheduledForDeletion); + $this->assertEquals($entity, $scheduledClone); + $this->assertNotSame($entity, $scheduledClone); $persister->expects($this->once()) ->method('deleteMany') From 361d80a720e27ef63bc5bd6a188435a7a6b6102c Mon Sep 17 00:00:00 2001 From: nurikabe Date: Mon, 24 Mar 2014 11:18:55 -0400 Subject: [PATCH 2/3] Prevent division by zero error --- Doctrine/AbstractProvider.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Doctrine/AbstractProvider.php b/Doctrine/AbstractProvider.php index 9d1575c..3fafe50 100644 --- a/Doctrine/AbstractProvider.php +++ b/Doctrine/AbstractProvider.php @@ -70,7 +70,8 @@ abstract class AbstractProvider extends BaseAbstractProvider $stepNbObjects = count($objects); $stepCount = $stepNbObjects + $offset; $percentComplete = 100 * $stepCount / $nbObjects; - $objectsPerSecond = $stepNbObjects / (microtime(true) - $stepStartTime); + $timeDifference = microtime(true) - $stepStartTime; + $objectsPerSecond = $timeDifference ? ($stepNbObjects / $timeDifference) : $stepNbObjects; $loggerClosure(sprintf('%0.1f%% (%d/%d), %d objects/s %s', $percentComplete, $stepCount, $nbObjects, $objectsPerSecond, $this->getMemoryUsage())); } } From 0de48d2190b03583703c3084a1a8296e4d8cb17b Mon Sep 17 00:00:00 2001 From: nurikabe Date: Sat, 29 Mar 2014 18:36:06 -0400 Subject: [PATCH 3/3] 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); }