From 86cdaa7c3cf48bdaa123ae2e3a01bfdfa6185e11 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 10 Apr 2012 17:23:09 -0400 Subject: [PATCH] [Listener] Support additional indexable callback types Support service/method tuples as indexable callbacks. Closures are also supported, although they cannot be injected by the service container. The indexable callback is now injected via a setter and validated when set (instead of during event processing). --- DependencyInjection/Configuration.php | 4 +- DependencyInjection/FOQElasticaExtension.php | 4 +- Doctrine/AbstractListener.php | 105 ++++++++++++++++-- Doctrine/MongoDB/Listener.php | 32 +----- Doctrine/ORM/Listener.php | 32 +----- Tests/Doctrine/AbstractListenerTest.php | 108 ++++++++++++++++++- 6 files changed, 215 insertions(+), 70 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 6417f59..a06f209 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -91,7 +91,6 @@ class Configuration ->thenInvalid('The driver %s is not supported. Please choose one of '.json_encode($this->supportedDrivers)) ->end() ->end() - ->scalarNode('is_indexable_callback')->defaultNull()->end() ->scalarNode('identifier')->defaultValue('id')->end() ->arrayNode('provider') ->children() @@ -107,6 +106,7 @@ class Configuration ->scalarNode('update')->defaultTrue()->end() ->scalarNode('delete')->defaultTrue()->end() ->scalarNode('service')->end() + ->scalarNode('is_indexable_callback')->defaultNull()->end() ->end() ->end() ->arrayNode('finder') @@ -167,7 +167,6 @@ class Configuration ->thenInvalid('The driver %s is not supported. Please choose one of '.json_encode($this->supportedDrivers)) ->end() ->end() - ->scalarNode('is_indexable_callback')->defaultNull()->end() ->scalarNode('model')->end() ->scalarNode('repository')->end() ->scalarNode('identifier')->defaultValue('id')->end() @@ -185,6 +184,7 @@ class Configuration ->scalarNode('update')->defaultTrue()->end() ->scalarNode('delete')->defaultTrue()->end() ->scalarNode('service')->end() + ->scalarNode('is_indexable_callback')->defaultNull()->end() ->end() ->end() ->arrayNode('finder') diff --git a/DependencyInjection/FOQElasticaExtension.php b/DependencyInjection/FOQElasticaExtension.php index 81f409c..413344e 100644 --- a/DependencyInjection/FOQElasticaExtension.php +++ b/DependencyInjection/FOQElasticaExtension.php @@ -305,12 +305,14 @@ class FOQElasticaExtension extends Extension $listenerDef->replaceArgument(0, new Reference($objectPersisterId)); $listenerDef->replaceArgument(1, $typeConfig['model']); $listenerDef->replaceArgument(3, $typeConfig['identifier']); - $listenerDef->replaceArgument(4, $typeConfig['is_indexable_callback']); $listenerDef->replaceArgument(2, $this->getDoctrineEvents($typeConfig)); switch ($typeConfig['driver']) { case 'orm': $listenerDef->addTag('doctrine.event_subscriber'); break; case 'mongodb': $listenerDef->addTag('doctrine.odm.mongodb.event_subscriber'); break; } + if (isset($typeConfig['listener']['is_indexable_callback'])) { + $listenerDef->addMethodCall('setIsIndexableCallback', array($typeConfig['listener']['is_indexable_callback'])); + } $container->setDefinition($listenerId, $listenerDef); return $listenerId; diff --git a/Doctrine/AbstractListener.php b/Doctrine/AbstractListener.php index e8a75b5..796cf19 100644 --- a/Doctrine/AbstractListener.php +++ b/Doctrine/AbstractListener.php @@ -2,10 +2,11 @@ namespace FOQ\ElasticaBundle\Doctrine; +use Doctrine\Common\EventSubscriber; +use Doctrine\Common\Persistence\ObjectManager; use FOQ\ElasticaBundle\Persister\ObjectPersisterInterface; -use Symfony\Component\HttpKernel\Log\LoggerInterface; -abstract class AbstractListener +abstract class AbstractListener implements EventSubscriber { /** * Object persister @@ -28,39 +29,123 @@ abstract class AbstractListener */ protected $events; + /** + * Name of domain model field used as the ES identifier + * + * @var string + */ protected $esIdentifierField; - protected $scheduledForRemoval; + /** + * Callback for determining if an object should be indexed + * + * @var mixed + */ protected $isIndexableCallback; /** - * Constructor - **/ - public function __construct(ObjectPersisterInterface $objectPersister, $objectClass, array $events, $esIdentifierField = 'id', $isIndexableCallback = null) + * Objects scheduled for removal + * + * @var array + */ + private $scheduledForRemoval = array(); + + /** + * Constructor. + * + * @param ObjectPersisterInterface $objectPersister + * @param string $objectClass + * @param array $events + * @param string $esIdentifierField + */ + public function __construct(ObjectPersisterInterface $objectPersister, $objectClass, array $events, $esIdentifierField = 'id') { $this->objectPersister = $objectPersister; $this->objectClass = $objectClass; $this->events = $events; $this->esIdentifierField = $esIdentifierField; - $this->scheduledForRemoval = array(); - $this->isIndexableCallback = $isIndexableCallback; } /** - * @return array + * @see Doctrine\Common\EventSubscriber::getSubscribedEvents() */ public function getSubscribedEvents() { return $this->events; } - protected function scheduleForRemoval($object, $objectManager) + /** + * Set the callback for determining object index eligibility. + * + * If callback is a string, it must be public method on the object class + * that expects no arguments and returns a boolean. Otherwise, the callback + * should expect the object for consideration as its only argument and + * return a boolean. + * + * @param callback $callback + * @throw RuntimeException if the callback is not callable + */ + public function setIsIndexableCallback($callback) + { + if (is_string($callback)) { + if (!is_callable(array($this->objectClass, $callback))) { + throw new \RuntimeException(sprintf('Indexable callback %s::%s() is not callable.', $this->objectClass, $callback)); + } + } elseif (!is_callable($callback)) { + if (is_array($callback)) { + list($class, $method) = $callback + array(null, null); + if (is_object($class)) { + $class = get_class($class); + } + if ($class && $method) { + throw new \RuntimeException(sprintf('Indexable callback %s::%s() is not callable.', $class, $method)); + } + } + throw new \RuntimeException('Indexable callback is not callable.'); + } + + $this->isIndexableCallback = $callback; + } + + /** + * Return whether the object is indexable with respect to the callback. + * + * @param object $object + * @return boolean + */ + protected function isObjectIndexable($object) + { + if (!$this->isIndexableCallback) { + return true; + } + + return is_string($this->isIndexableCallback) + ? call_user_func(array($object, $this->isIndexableCallback)) + : call_user_func($this->isIndexableCallback, $object); + } + + /** + * Schedules the object for removal. + * + * This is usually called during the pre-remove event. + * + * @param object $object + * @param ObjectManager $objectManager + */ + protected function scheduleForRemoval($object, ObjectManager $objectManager) { $metadata = $objectManager->getClassMetadata($this->objectClass); $esId = $metadata->getFieldValue($object, $this->esIdentifierField); $this->scheduledForRemoval[spl_object_hash($object)] = $esId; } + /** + * Removes the object if it was scheduled for removal. + * + * This is usually called during the post-remove event. + * + * @param object $object + */ protected function removeIfScheduled($object) { $objectHash = spl_object_hash($object); diff --git a/Doctrine/MongoDB/Listener.php b/Doctrine/MongoDB/Listener.php index d83bafc..1bddf2f 100644 --- a/Doctrine/MongoDB/Listener.php +++ b/Doctrine/MongoDB/Listener.php @@ -2,29 +2,17 @@ namespace FOQ\ElasticaBundle\Doctrine\MongoDB; -use FOQ\ElasticaBundle\Doctrine\AbstractListener; use Doctrine\ODM\MongoDB\Event\LifecycleEventArgs; -use Doctrine\Common\EventSubscriber; +use FOQ\ElasticaBundle\Doctrine\AbstractListener; -class Listener extends AbstractListener implements EventSubscriber +class Listener extends AbstractListener { public function postPersist(LifecycleEventArgs $eventArgs) { $document = $eventArgs->getDocument(); - if ($document instanceof $this->objectClass) { - if ($this->isIndexableCallback && !is_callable(array($document, $this->isIndexableCallback))) { - if (method_exists($document, $this->isIndexableCallback)) { - $exception = sprintf('The specified check method %s::%s is out of scope.', $this->objectClass, $this->isIndexableCallback); - } else { - $exception = sprintf('The specified check method %s::%s does not exist', $this->objectClass, $this->isIndexableCallback); - } - throw new \RuntimeException($exception); - } - - if (($this->isIndexableCallback && call_user_func(array($document, $this->isIndexableCallback))) || !$this->isIndexableCallback) { - $this->objectPersister->insertOne($document); - } + if ($document instanceof $this->objectClass && $this->isObjectIndexable($document)) { + $this->objectPersister->insertOne($document); } } @@ -33,17 +21,7 @@ class Listener extends AbstractListener implements EventSubscriber $document = $eventArgs->getDocument(); if ($document instanceof $this->objectClass) { - - if ($this->isIndexableCallback && !is_callable(array($document, $this->isIndexableCallback))) { - if (method_exists($document, $this->isIndexableCallback)) { - $exception = sprintf('The specified check method %s::%s is out of scope.', $this->objectClass, $this->isIndexableCallback); - } else { - $exception = sprintf('The specified check method %s::%s does not exist', $this->objectClass, $this->isIndexableCallback); - } - throw new \RuntimeException($exception); - } - - if (($this->isIndexableCallback && call_user_func(array($document, $this->isIndexableCallback))) || !$this->isIndexableCallback) { + if ($this->isObjectIndexable($document)) { $this->objectPersister->replaceOne($document); } else { $this->scheduleForRemoval($document, $eventArgs->getDocumentManager()); diff --git a/Doctrine/ORM/Listener.php b/Doctrine/ORM/Listener.php index 3eb153b..c335669 100644 --- a/Doctrine/ORM/Listener.php +++ b/Doctrine/ORM/Listener.php @@ -2,29 +2,17 @@ namespace FOQ\ElasticaBundle\Doctrine\ORM; -use FOQ\ElasticaBundle\Doctrine\AbstractListener; use Doctrine\ORM\Event\LifecycleEventArgs; -use Doctrine\Common\EventSubscriber; +use FOQ\ElasticaBundle\Doctrine\AbstractListener; -class Listener extends AbstractListener implements EventSubscriber +class Listener extends AbstractListener { public function postPersist(LifecycleEventArgs $eventArgs) { $entity = $eventArgs->getEntity(); - if ($entity instanceof $this->objectClass) { - if ($this->isIndexableCallback && !is_callable(array($entity, $this->isIndexableCallback))) { - if (method_exists($entity, $this->isIndexableCallback)) { - $exception = sprintf('The specified check method %s::%s is out of scope.', $this->objectClass, $this->isIndexableCallback); - } else { - $exception = sprintf('The specified check method %s::%s does not exist', $this->objectClass, $this->isIndexableCallback); - } - throw new \RuntimeException($exception); - } - - if (($this->isIndexableCallback && call_user_func(array($entity, $this->isIndexableCallback))) || !$this->isIndexableCallback) { - $this->objectPersister->insertOne($entity); - } + if ($entity instanceof $this->objectClass && $this->isObjectIndexable($entity)) { + $this->objectPersister->insertOne($entity); } } @@ -33,17 +21,7 @@ class Listener extends AbstractListener implements EventSubscriber $entity = $eventArgs->getEntity(); if ($entity instanceof $this->objectClass) { - - if ($this->isIndexableCallback && !is_callable(array($entity, $this->isIndexableCallback))) { - if (method_exists($entity, $this->isIndexableCallback)) { - $exception = sprintf('The specified check method %s::%s is out of scope.', $this->objectClass, $this->isIndexableCallback); - } else { - $exception = sprintf('The specified check method %s::%s does not exist', $this->objectClass, $this->isIndexableCallback); - } - throw new \RuntimeException($exception); - } - - if (($this->isIndexableCallback && call_user_func(array($entity, $this->isIndexableCallback))) || !$this->isIndexableCallback) { + if ($this->isObjectIndexable($entity)) { $this->objectPersister->replaceOne($entity); } else { $this->scheduleForRemoval($entity, $eventArgs->getEntityManager()); diff --git a/Tests/Doctrine/AbstractListenerTest.php b/Tests/Doctrine/AbstractListenerTest.php index d8d9fab..16845e9 100644 --- a/Tests/Doctrine/AbstractListenerTest.php +++ b/Tests/Doctrine/AbstractListenerTest.php @@ -22,6 +22,24 @@ abstract class AbstractListenerTest extends \PHPUnit_Framework_TestCase $listener->postPersist($eventArgs); } + /** + * @dataProvider provideIsIndexableCallbacks + */ + public function testNonIndexableObjectNotInsertedOnPersist($isIndexableCallback) + { + $persister = $this->getMockPersister(); + + $entity = new Listener\Entity(1, false); + $eventArgs = $this->createLifecycleEventArgs($entity, $this->getMockObjectManager()); + + $persister->expects($this->never()) + ->method('insertOne'); + + $listener = $this->createListener($persister, get_class($entity), array()); + $listener->setIsIndexableCallback($isIndexableCallback); + $listener->postPersist($eventArgs); + } + public function testObjectReplacedOnUpdate() { $persister = $this->getMockPersister(); @@ -33,17 +51,54 @@ abstract class AbstractListenerTest extends \PHPUnit_Framework_TestCase ->method('replaceOne') ->with($entity); + $persister->expects($this->never()) + ->method('deleteById'); + $listener = $this->createListener($persister, get_class($entity), array()); $listener->postUpdate($eventArgs); } + /** + * @dataProvider provideIsIndexableCallbacks + */ + public function testNonIndexableObjectRemovedOnUpdate($isIndexableCallback) + { + $classMetadata = $this->getMockClassMetadata(); + $objectManager = $this->getMockObjectManager(); + $persister = $this->getMockPersister(); + + $entity = new Listener\Entity(1, false); + $eventArgs = $this->createLifecycleEventArgs($entity, $objectManager); + + $objectManager->expects($this->any()) + ->method('getClassMetadata') + ->with(get_class($entity)) + ->will($this->returnValue($classMetadata)); + + $classMetadata->expects($this->any()) + ->method('getFieldValue') + ->with($entity, 'id') + ->will($this->returnValue($entity->getId())); + + $persister->expects($this->never()) + ->method('replaceOne'); + + $persister->expects($this->once()) + ->method('deleteById') + ->with($entity->getId()); + + $listener = $this->createListener($persister, get_class($entity), array()); + $listener->setIsIndexableCallback($isIndexableCallback); + $listener->postUpdate($eventArgs); + } + public function testObjectDeletedOnRemove() { $classMetadata = $this->getMockClassMetadata(); $objectManager = $this->getMockObjectManager(); $persister = $this->getMockPersister(); - $entity = new Listener\Entity(78); + $entity = new Listener\Entity(1); $eventArgs = $this->createLifecycleEventArgs($entity, $objectManager); $objectManager->expects($this->any()) @@ -71,7 +126,7 @@ abstract class AbstractListenerTest extends \PHPUnit_Framework_TestCase $objectManager = $this->getMockObjectManager(); $persister = $this->getMockPersister(); - $entity = new Listener\Entity(826); + $entity = new Listener\Entity(1); $eventArgs = $this->createLifecycleEventArgs($entity, $objectManager); $objectManager->expects($this->any()) @@ -93,6 +148,34 @@ abstract class AbstractListenerTest extends \PHPUnit_Framework_TestCase $listener->postRemove($eventArgs); } + /** + * @dataProvider provideInvalidIsIndexableCallbacks + * @expectedException RuntimeException + */ + public function testInvalidIsIndexableCallbacks($isIndexableCallback) + { + $listener = $this->createListener($this->getMockPersister(), 'FOQ\ElasticaBundle\Tests\Doctrine\Listener\Entity', array()); + $listener->setIsIndexableCallback($isIndexableCallback); + } + + public function provideInvalidIsIndexableCallbacks() + { + return array( + array('nonexistentEntityMethod'), + array(array(new Listener\IndexableDecider(), 'internalMethod')), + array(42), + ); + } + + public function provideIsIndexableCallbacks() + { + return array( + array('getIsIndexable'), + array(array(new Listener\IndexableDecider(), 'isIndexable')), + array(function(Listener\Entity $entity) { return $entity->getIsIndexable(); }), + ); + } + abstract protected function getLifecycleEventArgsClass(); abstract protected function getListenerClass(); @@ -140,14 +223,33 @@ namespace FOQ\ElasticaBundle\Tests\Doctrine\Listener; class Entity { private $id; + private $isIndexable; - public function __construct($id) + public function __construct($id, $isIndexable = true) { $this->id = $id; + $this->isIndexable = $isIndexable; } public function getId() { return $this->id; } + + public function getIsIndexable() + { + return $this->isIndexable; + } +} + +class IndexableDecider +{ + public function isIndexable(Entity $entity) + { + return $entity->getIsIndexable(); + } + + protected function internalMethod() + { + } }