Merge pull request #515 from nurikabe/master

Clone entities on delete to preserve ids

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()));
             }
         }

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,17 +213,21 @@ class Listener implements EventSubscriber
                 $this->scheduledForUpdate[] = $entity;
             } else {
                 // Delete if no longer indexable
-                $this->scheduledForDeletion[] = $entity;
+                $this->scheduleForDeletion($entity);
             }
         }
     }
 
+    /**
+     * 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[] = $entity;
+            $this->scheduleForDeletion($entity);
         }
     }
 
@@ -221,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);
         }
     }
 
@@ -242,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;
+        }
+    }
 }

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
      *

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);
 }

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,11 +133,11 @@ abstract class ListenerTest extends \PHPUnit_Framework_TestCase
         $listener = $this->createListener($persister, get_class($entity), array());
         $listener->preRemove($eventArgs);
 
-        $this->assertEquals($entity, current($listener->scheduledForDeletion));
+        $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);
     }
@@ -149,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())
@@ -164,11 +165,11 @@ 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));
+        $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);
     }