From 4af9f442fd940953c5211e509d3ecaca13f6c832 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Thu, 12 Mar 2015 22:17:57 +1100 Subject: [PATCH 01/13] Move common sorting code to base Transformer class --- .../AbstractElasticaToModelTransformer.php | 26 ++-------- Propel/ElasticaToModelTransformer.php | 26 ++-------- .../AbstractElasticaToModelTransformer.php | 51 +++++++++++++++++++ 3 files changed, 57 insertions(+), 46 deletions(-) create mode 100644 Transformer/AbstractElasticaToModelTransformer.php diff --git a/Doctrine/AbstractElasticaToModelTransformer.php b/Doctrine/AbstractElasticaToModelTransformer.php index a9afb22..603eafa 100755 --- a/Doctrine/AbstractElasticaToModelTransformer.php +++ b/Doctrine/AbstractElasticaToModelTransformer.php @@ -3,7 +3,7 @@ namespace FOS\ElasticaBundle\Doctrine; use FOS\ElasticaBundle\HybridResult; -use FOS\ElasticaBundle\Transformer\ElasticaToModelTransformerInterface; +use FOS\ElasticaBundle\Transformer\AbstractElasticaToModelTransformer as BaseTransformer; use FOS\ElasticaBundle\Transformer\HighlightableModelInterface; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; @@ -12,7 +12,7 @@ use Symfony\Component\PropertyAccess\PropertyAccessorInterface; * This mapper assumes an exact match between * elastica documents ids and doctrine object ids. */ -abstract class AbstractElasticaToModelTransformer implements ElasticaToModelTransformerInterface +abstract class AbstractElasticaToModelTransformer extends BaseTransformer { /** * Manager registry. @@ -38,13 +38,6 @@ abstract class AbstractElasticaToModelTransformer implements ElasticaToModelTran 'query_builder_method' => 'createQueryBuilder', ); - /** - * PropertyAccessor instance. - * - * @var PropertyAccessorInterface - */ - protected $propertyAccessor; - /** * Instantiates a new Mapper. * @@ -69,16 +62,6 @@ abstract class AbstractElasticaToModelTransformer implements ElasticaToModelTran return $this->objectClass; } - /** - * Set the PropertyAccessor. - * - * @param PropertyAccessorInterface $propertyAccessor - */ - public function setPropertyAccessor(PropertyAccessorInterface $propertyAccessor) - { - $this->propertyAccessor = $propertyAccessor; - } - /** * Transforms an array of elastica objects into an array of * model objects fetched from the doctrine repository. @@ -111,10 +94,7 @@ abstract class AbstractElasticaToModelTransformer implements ElasticaToModelTran // sort objects in the order of ids $idPos = array_flip($ids); $identifier = $this->options['identifier']; - $propertyAccessor = $this->propertyAccessor; - usort($objects, function ($a, $b) use ($idPos, $identifier, $propertyAccessor) { - return $idPos[$propertyAccessor->getValue($a, $identifier)] > $idPos[$propertyAccessor->getValue($b, $identifier)]; - }); + usort($objects, $this->getSortingClosure($idPos, $identifier)); return $objects; } diff --git a/Propel/ElasticaToModelTransformer.php b/Propel/ElasticaToModelTransformer.php index 87c4047..c80daa2 100644 --- a/Propel/ElasticaToModelTransformer.php +++ b/Propel/ElasticaToModelTransformer.php @@ -3,6 +3,7 @@ namespace FOS\ElasticaBundle\Propel; use FOS\ElasticaBundle\HybridResult; +use FOS\ElasticaBundle\Transformer\AbstractElasticaToModelTransformer; use FOS\ElasticaBundle\Transformer\ElasticaToModelTransformerInterface; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; @@ -14,7 +15,7 @@ use Symfony\Component\PropertyAccess\PropertyAccessorInterface; * * @author William Durand */ -class ElasticaToModelTransformer implements ElasticaToModelTransformerInterface +class ElasticaToModelTransformer extends AbstractElasticaToModelTransformer { /** * Propel model class to map to Elastica documents. @@ -33,13 +34,6 @@ class ElasticaToModelTransformer implements ElasticaToModelTransformerInterface 'identifier' => 'id', ); - /** - * PropertyAccessor instance. - * - * @var PropertyAccessorInterface - */ - protected $propertyAccessor; - /** * Constructor. * @@ -52,16 +46,6 @@ class ElasticaToModelTransformer implements ElasticaToModelTransformerInterface $this->options = array_merge($this->options, $options); } - /** - * Set the PropertyAccessor instance. - * - * @param PropertyAccessorInterface $propertyAccessor - */ - public function setPropertyAccessor(PropertyAccessorInterface $propertyAccessor) - { - $this->propertyAccessor = $propertyAccessor; - } - /** * Transforms an array of Elastica document into an array of Propel entities * fetched from the database. @@ -82,11 +66,7 @@ class ElasticaToModelTransformer implements ElasticaToModelTransformerInterface // Sort objects in the order of their IDs $idPos = array_flip($ids); $identifier = $this->options['identifier']; - $propertyAccessor = $this->propertyAccessor; - - $sortCallback = function ($a, $b) use ($idPos, $identifier, $propertyAccessor) { - return $idPos[$propertyAccessor->getValue($a, $identifier)] > $idPos[$propertyAccessor->getValue($b, $identifier)]; - }; + $sortCallback = $this->getSortingClosure($idPos, $identifier); if (is_object($objects)) { $objects->uasort($sortCallback); diff --git a/Transformer/AbstractElasticaToModelTransformer.php b/Transformer/AbstractElasticaToModelTransformer.php new file mode 100644 index 0000000..2b1de5c --- /dev/null +++ b/Transformer/AbstractElasticaToModelTransformer.php @@ -0,0 +1,51 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\ElasticaBundle\Transformer; + +use Symfony\Component\PropertyAccess\PropertyAccessorInterface; + +abstract class AbstractElasticaToModelTransformer implements ElasticaToModelTransformerInterface +{ + /** + * PropertyAccessor instance. + * + * @var PropertyAccessorInterface + */ + protected $propertyAccessor; + + /** + * Set the PropertyAccessor instance. + * + * @param PropertyAccessorInterface $propertyAccessor + */ + public function setPropertyAccessor(PropertyAccessorInterface $propertyAccessor) + { + $this->propertyAccessor = $propertyAccessor; + } + + /** + * Returns a sorting closure to be used with usort() to put retrieved objects + * back in the order that they were returned by ElasticSearch. + * + * @param array $idPos + * @param string $identifierPath + * @return callable + */ + protected function getSortingClosure(array $idPos, $identifierPath) + { + $propertyAccessor = $this->propertyAccessor; + + return function ($a, $b) use ($idPos, $identifierPath, $propertyAccessor) { + return $idPos[$propertyAccessor->getValue($a, $identifierPath)] > $idPos[$propertyAccessor->getValue($b, $identifierPath)]; + }; + } +} From d5a9b7b235bdbdbc757ed7c28b7608fe40f1312b Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Thu, 12 Mar 2015 22:21:53 +1100 Subject: [PATCH 02/13] Add missing method to HighlightableModelInterface. This is not a BC break - the method has always been required and lacking the method would cause a fatal error. --- Transformer/HighlightableModelInterface.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Transformer/HighlightableModelInterface.php b/Transformer/HighlightableModelInterface.php index 70b8ed3..96c6c7c 100644 --- a/Transformer/HighlightableModelInterface.php +++ b/Transformer/HighlightableModelInterface.php @@ -3,10 +3,17 @@ namespace FOS\ElasticaBundle\Transformer; /** - * Maps Elastica documents with model objects. + * Indicates that the model should have elastica highlights injected. */ interface HighlightableModelInterface { + /** + * Returns a unique identifier for the model. + * + * @return mixed + */ + public function getId(); + /** * Set ElasticSearch highlight data. * From 84e5831a813f53f236a4b3027ee49dd71bc99895 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Thu, 12 Mar 2015 23:08:32 +1100 Subject: [PATCH 03/13] Fixing scrutinizer issues --- DependencyInjection/FOSElasticaExtension.php | 3 +-- .../AbstractElasticaToModelTransformer.php | 10 ++++++---- Doctrine/AbstractProvider.php | 2 +- Doctrine/Listener.php | 8 ++++---- Index/IndexManager.php | 5 +++++ Index/MappingBuilder.php | 6 +++--- Persister/ObjectPersisterInterface.php | 18 +++++++++--------- Persister/ObjectSerializerPersister.php | 6 +++++- Propel/ElasticaToModelTransformer.php | 2 +- Serializer/Callback.php | 12 ++++-------- Tests/Doctrine/AbstractProviderTest.php | 1 - Tests/Functional/app/ORM/IndexableService.php | 4 ++-- ...lasticaToModelTransformerCollectionTest.php | 2 +- .../ElasticaToModelTransformerCollection.php | 2 +- 14 files changed, 43 insertions(+), 38 deletions(-) diff --git a/DependencyInjection/FOSElasticaExtension.php b/DependencyInjection/FOSElasticaExtension.php index b26e50c..ec45e25 100644 --- a/DependencyInjection/FOSElasticaExtension.php +++ b/DependencyInjection/FOSElasticaExtension.php @@ -502,7 +502,7 @@ class FOSElasticaExtension extends Extension break; } - if ($tagName) { + if (null !== $tagName) { foreach ($this->getDoctrineEvents($typeConfig) as $event) { $listenerDef->addTag($tagName, array('event' => $event)); } @@ -527,7 +527,6 @@ class FOSElasticaExtension extends Extension break; default: throw new InvalidArgumentException(sprintf('Cannot determine events for driver "%s"', $typeConfig['driver'])); - break; } $events = array(); diff --git a/Doctrine/AbstractElasticaToModelTransformer.php b/Doctrine/AbstractElasticaToModelTransformer.php index 603eafa..0263f42 100755 --- a/Doctrine/AbstractElasticaToModelTransformer.php +++ b/Doctrine/AbstractElasticaToModelTransformer.php @@ -2,10 +2,10 @@ namespace FOS\ElasticaBundle\Doctrine; +use Doctrine\Common\Persistence\ManagerRegistry; use FOS\ElasticaBundle\HybridResult; use FOS\ElasticaBundle\Transformer\AbstractElasticaToModelTransformer as BaseTransformer; use FOS\ElasticaBundle\Transformer\HighlightableModelInterface; -use Symfony\Component\PropertyAccess\PropertyAccessorInterface; /** * Maps Elastica documents with Doctrine objects @@ -16,6 +16,8 @@ abstract class AbstractElasticaToModelTransformer extends BaseTransformer { /** * Manager registry. + * + * @var ManagerRegistry */ protected $registry = null; @@ -41,11 +43,11 @@ abstract class AbstractElasticaToModelTransformer extends BaseTransformer /** * Instantiates a new Mapper. * - * @param object $registry + * @param ManagerRegistry $registry * @param string $objectClass * @param array $options */ - public function __construct($registry, $objectClass, array $options = array()) + public function __construct(ManagerRegistry $registry, $objectClass, array $options = array()) { $this->registry = $registry; $this->objectClass = $objectClass; @@ -118,7 +120,7 @@ abstract class AbstractElasticaToModelTransformer extends BaseTransformer } /** - * {@inheritdoc} + * {@inheritDoc} */ public function getIdentifierField() { diff --git a/Doctrine/AbstractProvider.php b/Doctrine/AbstractProvider.php index e250e56..9845edc 100644 --- a/Doctrine/AbstractProvider.php +++ b/Doctrine/AbstractProvider.php @@ -71,7 +71,7 @@ abstract class AbstractProvider extends BaseAbstractProvider $objects = $this->getSlice($queryBuilder, $batchSize, $offset, $objects); $objects = array_filter($objects, array($this, 'isObjectIndexable')); - if ($objects) { + if (!empty($objects)) { if (!$ignoreErrors) { $this->objectPersister->insertMany($objects); } else { diff --git a/Doctrine/Listener.php b/Doctrine/Listener.php index c35b947..a1d3585 100644 --- a/Doctrine/Listener.php +++ b/Doctrine/Listener.php @@ -3,8 +3,8 @@ namespace FOS\ElasticaBundle\Doctrine; use Doctrine\Common\Persistence\Event\LifecycleEventArgs; -use FOS\ElasticaBundle\Persister\ObjectPersisterInterface; use FOS\ElasticaBundle\Persister\ObjectPersister; +use FOS\ElasticaBundle\Persister\ObjectPersisterInterface; use FOS\ElasticaBundle\Provider\IndexableInterface; use Psr\Log\LoggerInterface; use Symfony\Component\PropertyAccess\PropertyAccess; @@ -19,14 +19,14 @@ class Listener /** * Object persister. * - * @var ObjectPersister + * @var ObjectPersisterInterface */ protected $objectPersister; /** * Configuration for the listener. * - * @var string + * @var array */ private $config; @@ -84,7 +84,7 @@ class Listener $this->objectPersister = $objectPersister; $this->propertyAccessor = PropertyAccess::createPropertyAccessor(); - if ($logger) { + if ($logger && $this->objectPersister instanceof ObjectPersister) { $this->objectPersister->setLogger($logger); } } diff --git a/Index/IndexManager.php b/Index/IndexManager.php index 13aecad..98ce870 100644 --- a/Index/IndexManager.php +++ b/Index/IndexManager.php @@ -6,6 +6,11 @@ use FOS\ElasticaBundle\Elastica\Index; class IndexManager { + /** + * @var Index + */ + private $defaultIndex; + /** * @var array */ diff --git a/Index/MappingBuilder.php b/Index/MappingBuilder.php index 93a3fcc..e03bf54 100644 --- a/Index/MappingBuilder.php +++ b/Index/MappingBuilder.php @@ -38,13 +38,13 @@ class MappingBuilder } $mapping = array(); - if ($typeMappings) { + if (!empty($typeMappings)) { $mapping['mappings'] = $typeMappings; } // 'warmers' => $indexConfig->getWarmers(), $settings = $indexConfig->getSettings(); - if ($settings) { + if (!empty($settings)) { $mapping['settings'] = $settings; } @@ -95,7 +95,7 @@ class MappingBuilder $mapping['_meta']['model'] = $typeConfig->getModel(); } - if (!$mapping) { + if (empty($mapping)) { // Empty mapping, we want it encoded as a {} instead of a [] $mapping = new \stdClass(); } diff --git a/Persister/ObjectPersisterInterface.php b/Persister/ObjectPersisterInterface.php index 9163320..f624971 100644 --- a/Persister/ObjectPersisterInterface.php +++ b/Persister/ObjectPersisterInterface.php @@ -10,6 +10,15 @@ namespace FOS\ElasticaBundle\Persister; */ interface ObjectPersisterInterface { + /** + * Checks if this persister can handle the given object or not. + * + * @param mixed $object + * + * @return boolean + */ + public function handlesObject($object); + /** * Insert one object into the type * The object will be transformed to an elastica document. @@ -66,13 +75,4 @@ interface ObjectPersisterInterface * @param array $identifiers array of domain model object identifiers */ public function deleteManyByIdentifiers(array $identifiers); - - /** - * If the object persister handles the given object. - * - * @param object $object - * - * @return bool - */ - public function handlesObject($object); } diff --git a/Persister/ObjectSerializerPersister.php b/Persister/ObjectSerializerPersister.php index 018f851..792aa9a 100644 --- a/Persister/ObjectSerializerPersister.php +++ b/Persister/ObjectSerializerPersister.php @@ -18,12 +18,16 @@ class ObjectSerializerPersister extends ObjectPersister protected $serializer; /** + * @param Type $type + * @param ModelToElasticaTransformerInterface $transformer * @param string $objectClass + * @param callable $serializer */ public function __construct(Type $type, ModelToElasticaTransformerInterface $transformer, $objectClass, $serializer) { parent::__construct($type, $transformer, $objectClass, array()); - $this->serializer = $serializer; + + $this->serializer = $serializer; } /** diff --git a/Propel/ElasticaToModelTransformer.php b/Propel/ElasticaToModelTransformer.php index c80daa2..d143478 100644 --- a/Propel/ElasticaToModelTransformer.php +++ b/Propel/ElasticaToModelTransformer.php @@ -85,7 +85,7 @@ class ElasticaToModelTransformer extends AbstractElasticaToModelTransformer $objects = $this->transform($elasticaObjects); $result = array(); - for ($i = 0; $i < count($elasticaObjects); $i++) { + for ($i = 0, $j = count($elasticaObjects); $i < $j; $i++) { $result[] = new HybridResult($elasticaObjects[$i], $objects[$i]); } diff --git a/Serializer/Callback.php b/Serializer/Callback.php index 38d93dc..004d133 100644 --- a/Serializer/Callback.php +++ b/Serializer/Callback.php @@ -23,10 +23,8 @@ class Callback { $this->groups = $groups; - if ($this->groups) { - if (!$this->serializer instanceof SerializerInterface) { - throw new \RuntimeException('Setting serialization groups requires using "JMS\Serializer\Serializer".'); - } + if (!empty($this->groups) && !$this->serializer instanceof SerializerInterface) { + throw new \RuntimeException('Setting serialization groups requires using "JMS\Serializer\Serializer".'); } } @@ -34,10 +32,8 @@ class Callback { $this->version = $version; - if ($this->version) { - if (!$this->serializer instanceof SerializerInterface) { - throw new \RuntimeException('Setting serialization version requires using "JMS\Serializer\Serializer".'); - } + if ($this->version && !$this->serializer instanceof SerializerInterface) { + throw new \RuntimeException('Setting serialization version requires using "JMS\Serializer\Serializer".'); } } diff --git a/Tests/Doctrine/AbstractProviderTest.php b/Tests/Doctrine/AbstractProviderTest.php index 24e3d15..7b41837 100644 --- a/Tests/Doctrine/AbstractProviderTest.php +++ b/Tests/Doctrine/AbstractProviderTest.php @@ -58,7 +58,6 @@ class AbstractProviderTest extends \PHPUnit_Framework_TestCase ->with('index', 'type', $this->anything()) ->will($this->returnValue(true)); - $providerInvocationOffset = 2; $previousSlice = array(); foreach ($objectsByIteration as $i => $objects) { diff --git a/Tests/Functional/app/ORM/IndexableService.php b/Tests/Functional/app/ORM/IndexableService.php index 018451e..8f17bd0 100644 --- a/Tests/Functional/app/ORM/IndexableService.php +++ b/Tests/Functional/app/ORM/IndexableService.php @@ -13,12 +13,12 @@ namespace FOS\ElasticaBundle\Tests\Functional\app\ORM; class IndexableService { - public function isIndexable($object) + public function isIndexable() { return true; } - public static function isntIndexable($object) + public static function isntIndexable() { return false; } diff --git a/Tests/Transformer/ElasticaToModelTransformerCollectionTest.php b/Tests/Transformer/ElasticaToModelTransformerCollectionTest.php index d83f596..56a7200 100644 --- a/Tests/Transformer/ElasticaToModelTransformerCollectionTest.php +++ b/Tests/Transformer/ElasticaToModelTransformerCollectionTest.php @@ -37,7 +37,7 @@ class ElasticaToModelTransformerCollectionTest extends \PHPUnit_Framework_TestCa $this->collection = new ElasticaToModelTransformerCollection($this->transformers = array( 'type1' => $transformer1, 'type2' => $transformer2, - ), array()); + )); } public function testGetObjectClass() diff --git a/Transformer/ElasticaToModelTransformerCollection.php b/Transformer/ElasticaToModelTransformerCollection.php index 041e361..9920f43 100644 --- a/Transformer/ElasticaToModelTransformerCollection.php +++ b/Transformer/ElasticaToModelTransformerCollection.php @@ -81,7 +81,7 @@ class ElasticaToModelTransformerCollection implements ElasticaToModelTransformer $objects = $this->transform($elasticaObjects); $result = array(); - for ($i = 0; $i < count($elasticaObjects); $i++) { + for ($i = 0, $j = count($elasticaObjects); $i < $j; $i++) { $result[] = new HybridResult($elasticaObjects[$i], $objects[$i]); } From 6a07f7b24ee2fb44d4bf069f09568f6a521920c3 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Fri, 13 Mar 2015 19:10:24 +1100 Subject: [PATCH 04/13] More QA --- Paginator/RawPaginatorAdapter.php | 2 +- Tests/Command/ResetCommandTest.php | 2 +- Tests/Doctrine/AbstractListenerTest.php | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Paginator/RawPaginatorAdapter.php b/Paginator/RawPaginatorAdapter.php index 4dfb32a..c270f00 100644 --- a/Paginator/RawPaginatorAdapter.php +++ b/Paginator/RawPaginatorAdapter.php @@ -74,7 +74,7 @@ class RawPaginatorAdapter implements PaginatorAdapterInterface ? (integer) $this->query->getParam('size') : null; - if ($size && $size < $offset + $itemCountPerPage) { + if (null !== $size && $size < $offset + $itemCountPerPage) { $itemCountPerPage = $size - $offset; } diff --git a/Tests/Command/ResetCommandTest.php b/Tests/Command/ResetCommandTest.php index eb0aaa9..d63b380 100644 --- a/Tests/Command/ResetCommandTest.php +++ b/Tests/Command/ResetCommandTest.php @@ -9,8 +9,8 @@ use Symfony\Component\DependencyInjection\Container; class ResetCommandTest extends \PHPUnit_Framework_TestCase { + private $command; private $resetter; - private $indexManager; public function setup() diff --git a/Tests/Doctrine/AbstractListenerTest.php b/Tests/Doctrine/AbstractListenerTest.php index d6e2cb7..dcaf7d6 100644 --- a/Tests/Doctrine/AbstractListenerTest.php +++ b/Tests/Doctrine/AbstractListenerTest.php @@ -272,6 +272,7 @@ namespace FOS\ElasticaBundle\Tests\Doctrine\Listener; class Entity { private $id; + public $identifier; /** * @param integer $id From bb4618c101e8cda1419414688fa3020b3dfb40c6 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Fri, 13 Mar 2015 19:34:56 +1100 Subject: [PATCH 05/13] Even more QA --- Event/IndexEvent.php | 38 +++++++++++++++++++ Event/IndexPopulateEvent.php | 20 ++-------- Event/IndexResetEvent.php | 22 ++--------- Event/TypeResetEvent.php | 18 ++------- Propel/Provider.php | 2 +- Provider/AbstractProvider.php | 2 +- Serializer/Callback.php | 4 +- Tests/Persister/ObjectPersisterTest.php | 2 +- .../ObjectSerializerPersisterTest.php | 2 +- Tests/RepositoryTest.php | 4 +- .../ModelToElasticaAutoTransformerTest.php | 2 +- ...delToElasticaIdentifierTransformerTest.php | 2 +- 12 files changed, 58 insertions(+), 60 deletions(-) create mode 100644 Event/IndexEvent.php diff --git a/Event/IndexEvent.php b/Event/IndexEvent.php new file mode 100644 index 0000000..ed71d78 --- /dev/null +++ b/Event/IndexEvent.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\ElasticaBundle\Event; + +use Symfony\Component\EventDispatcher\Event; + +class IndexEvent extends Event +{ + /** + * @var string + */ + private $index; + + /** + * @param string $index + */ + public function __construct($index) + { + $this->index = $index; + } + + /** + * @return string + */ + public function getIndex() + { + return $this->index; + } +} diff --git a/Event/IndexPopulateEvent.php b/Event/IndexPopulateEvent.php index ab61f0a..b35105a 100644 --- a/Event/IndexPopulateEvent.php +++ b/Event/IndexPopulateEvent.php @@ -11,23 +11,16 @@ namespace FOS\ElasticaBundle\Event; -use Symfony\Component\EventDispatcher\Event; - /** * Index Populate Event. * * @author Oleg Andreyev */ -class IndexPopulateEvent extends Event +class IndexPopulateEvent extends IndexEvent { const PRE_INDEX_POPULATE = 'elastica.index.index_pre_populate'; const POST_INDEX_POPULATE = 'elastica.index.index_post_populate'; - /** - * @var string - */ - private $index; - /** * @var bool */ @@ -45,19 +38,12 @@ class IndexPopulateEvent extends Event */ public function __construct($index, $reset, $options) { - $this->index = $index; + parent::__construct($index); + $this->reset = $reset; $this->options = $options; } - /** - * @return string - */ - public function getIndex() - { - return $this->index; - } - /** * @return boolean */ diff --git a/Event/IndexResetEvent.php b/Event/IndexResetEvent.php index 3fb3625..871915a 100644 --- a/Event/IndexResetEvent.php +++ b/Event/IndexResetEvent.php @@ -11,14 +11,12 @@ namespace FOS\ElasticaBundle\Event; -use Symfony\Component\EventDispatcher\Event; - /** * Index ResetEvent. * * @author Oleg Andreyev */ -class IndexResetEvent extends Event +class IndexResetEvent extends IndexEvent { const PRE_INDEX_RESET = 'elastica.index.pre_reset'; const POST_INDEX_RESET = 'elastica.index.post_reset'; @@ -28,11 +26,6 @@ class IndexResetEvent extends Event */ private $force; - /** - * @var string - */ - private $index; - /** * @var bool */ @@ -45,17 +38,10 @@ class IndexResetEvent extends Event */ public function __construct($index, $populating, $force) { - $this->force = $force; - $this->index = $index; - $this->populating = $populating; - } + parent::__construct($index); - /** - * @return string - */ - public function getIndex() - { - return $this->index; + $this->force = $force; + $this->populating = $populating; } /** diff --git a/Event/TypeResetEvent.php b/Event/TypeResetEvent.php index 0b21a06..98fa2f4 100644 --- a/Event/TypeResetEvent.php +++ b/Event/TypeResetEvent.php @@ -18,16 +18,11 @@ use Symfony\Component\EventDispatcher\Event; * * @author Oleg Andreyev */ -class TypeResetEvent extends Event +class TypeResetEvent extends IndexEvent { const PRE_TYPE_RESET = 'elastica.index.type_pre_reset'; const POST_TYPE_RESET = 'elastica.index.type_post_reset'; - /** - * @var string - */ - private $index; - /** * @var string */ @@ -39,16 +34,9 @@ class TypeResetEvent extends Event */ public function __construct($index, $type) { - $this->type = $type; - $this->index = $index; - } + parent::__construct($index); - /** - * @return string - */ - public function getIndex() - { - return $this->index; + $this->type = $type; } /** diff --git a/Propel/Provider.php b/Propel/Provider.php index f8397f9..f28faa4 100644 --- a/Propel/Provider.php +++ b/Propel/Provider.php @@ -30,7 +30,7 @@ class Provider extends AbstractProvider ->getArrayCopy(); $objects = array_filter($objects, array($this, 'isObjectIndexable')); - if ($objects) { + if (!empty($objects)) { $this->objectPersister->insertMany($objects); } diff --git a/Provider/AbstractProvider.php b/Provider/AbstractProvider.php index 05fa525..a743d17 100644 --- a/Provider/AbstractProvider.php +++ b/Provider/AbstractProvider.php @@ -25,7 +25,7 @@ abstract class AbstractProvider implements ProviderInterface protected $options; /** - * @var Indexable + * @var IndexableInterface */ private $indexable; diff --git a/Serializer/Callback.php b/Serializer/Callback.php index 004d133..61da997 100644 --- a/Serializer/Callback.php +++ b/Serializer/Callback.php @@ -8,7 +8,7 @@ use JMS\Serializer\SerializerInterface; class Callback { protected $serializer; - protected $groups; + protected $groups = array(); protected $version; public function setSerializer($serializer) @@ -41,7 +41,7 @@ class Callback { $context = $this->serializer instanceof SerializerInterface ? SerializationContext::create()->enableMaxDepthChecks() : array(); - if ($this->groups) { + if (!empty($this->groups)) { $context->setGroups($this->groups); } diff --git a/Tests/Persister/ObjectPersisterTest.php b/Tests/Persister/ObjectPersisterTest.php index c3da687..06039f0 100644 --- a/Tests/Persister/ObjectPersisterTest.php +++ b/Tests/Persister/ObjectPersisterTest.php @@ -203,7 +203,7 @@ class ObjectPersisterTest extends \PHPUnit_Framework_TestCase private function getTransformer() { $transformer = new ModelToElasticaAutoTransformer(); - $transformer->setPropertyAccessor(PropertyAccess::getPropertyAccessor()); + $transformer->setPropertyAccessor(PropertyAccess::createPropertyAccessor()); return $transformer; } diff --git a/Tests/Persister/ObjectSerializerPersisterTest.php b/Tests/Persister/ObjectSerializerPersisterTest.php index 0b348d1..0536d06 100644 --- a/Tests/Persister/ObjectSerializerPersisterTest.php +++ b/Tests/Persister/ObjectSerializerPersisterTest.php @@ -112,7 +112,7 @@ class ObjectSerializerPersisterTest extends \PHPUnit_Framework_TestCase private function getTransformer() { $transformer = new ModelToElasticaIdentifierTransformer(); - $transformer->setPropertyAccessor(PropertyAccess::getPropertyAccessor()); + $transformer->setPropertyAccessor(PropertyAccess::createPropertyAccessor()); return $transformer; } diff --git a/Tests/RepositoryTest.php b/Tests/RepositoryTest.php index 3f0509a..7702af2 100644 --- a/Tests/RepositoryTest.php +++ b/Tests/RepositoryTest.php @@ -57,10 +57,10 @@ class RepositoryTest extends \PHPUnit_Framework_TestCase /** * @param string $testQuery - * @param int $testLimit + * @param mixed $testLimit * @param string $method * - * @return \FOS\ElasticaBundle\Finder\TransformedFinder|\PHPUnit_Framework_MockObject_MockObject + * @return \FOS\ElasticaBundle\Finder\TransformedFinder */ private function getFinderMock($testQuery, $testLimit = null, $method = 'find') { diff --git a/Tests/Transformer/ModelToElasticaAutoTransformerTest.php b/Tests/Transformer/ModelToElasticaAutoTransformerTest.php index 8d07aeb..f45134e 100644 --- a/Tests/Transformer/ModelToElasticaAutoTransformerTest.php +++ b/Tests/Transformer/ModelToElasticaAutoTransformerTest.php @@ -417,7 +417,7 @@ class ModelToElasticaAutoTransformerTest extends \PHPUnit_Framework_TestCase private function getTransformer($dispatcher = null) { $transformer = new ModelToElasticaAutoTransformer(array(), $dispatcher); - $transformer->setPropertyAccessor(PropertyAccess::getPropertyAccessor()); + $transformer->setPropertyAccessor(PropertyAccess::createPropertyAccessor()); return $transformer; } diff --git a/Tests/Transformer/ModelToElasticaIdentifierTransformerTest.php b/Tests/Transformer/ModelToElasticaIdentifierTransformerTest.php index 14d678e..aa3d7b7 100644 --- a/Tests/Transformer/ModelToElasticaIdentifierTransformerTest.php +++ b/Tests/Transformer/ModelToElasticaIdentifierTransformerTest.php @@ -51,7 +51,7 @@ class ModelToElasticaIdentifierTransformerTest extends \PHPUnit_Framework_TestCa private function getTransformer() { $transformer = new ModelToElasticaIdentifierTransformer(); - $transformer->setPropertyAccessor(PropertyAccess::getPropertyAccessor()); + $transformer->setPropertyAccessor(PropertyAccess::createPropertyAccessor()); return $transformer; } From 9cf0117c71229ba62050f4de02233d540f7baad8 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 00:51:07 +1100 Subject: [PATCH 06/13] AliasProcessor --- Exception/AliasIsIndexException.php | 2 +- Index/AliasProcessor.php | 145 ++++++++++-------- Tests/Index/AliasProcessorTest.php | 223 ++++++++++++++++++++++++++++ 3 files changed, 305 insertions(+), 65 deletions(-) create mode 100644 Tests/Index/AliasProcessorTest.php diff --git a/Exception/AliasIsIndexException.php b/Exception/AliasIsIndexException.php index 1df7a62..9af6ee3 100644 --- a/Exception/AliasIsIndexException.php +++ b/Exception/AliasIsIndexException.php @@ -6,6 +6,6 @@ class AliasIsIndexException extends \Exception { public function __construct($indexName) { - parent::__construct(sprintf('Expected alias %s instead of index', $indexName)); + parent::__construct(sprintf('Expected %s to be an alias but it is an index.', $indexName)); } } diff --git a/Index/AliasProcessor.php b/Index/AliasProcessor.php index c4f1464..f4e4d00 100644 --- a/Index/AliasProcessor.php +++ b/Index/AliasProcessor.php @@ -54,37 +54,47 @@ class AliasProcessor $client = $index->getClient(); $aliasName = $indexConfig->getElasticSearchName(); - $oldIndexName = false; + $oldIndexName = null; $newIndexName = $index->getName(); try { - $aliasedIndexes = $this->getAliasedIndexes($client, $aliasName); + $oldIndexName = $this->getAliasedIndex($client, $aliasName); } catch (AliasIsIndexException $e) { if (!$force) { throw $e; } $this->deleteIndex($client, $aliasName); - $aliasedIndexes = array(); } - if (count($aliasedIndexes) > 1) { - throw new \RuntimeException( - sprintf( - 'Alias %s is used for multiple indexes: [%s]. - Make sure it\'s either not used or is assigned to one index only', - $aliasName, - implode(', ', $aliasedIndexes) - ) - ); + try { + $aliasUpdateRequest = $this->buildAliasUpdateRequest($oldIndexName, $aliasName, $newIndexName); + $client->request('_aliases', 'POST', $aliasUpdateRequest); + } catch (ExceptionInterface $e) { + $this->cleanupRenameFailure($client, $newIndexName, $e); } + // Delete the old index after the alias has been switched + if (null !== $oldIndexName) { + $this->deleteIndex($client, $oldIndexName); + } + } + + /** + * Builds an ElasticSearch request to rename or create an alias. + * + * @param string|null $aliasedIndex + * @param string $aliasName + * @param string $newIndexName + * @return array + */ + private function buildAliasUpdateRequest($aliasedIndex, $aliasName, $newIndexName) + { $aliasUpdateRequest = array('actions' => array()); - if (count($aliasedIndexes) === 1) { + if (null !== $aliasedIndex) { // if the alias is set - add an action to remove it - $oldIndexName = $aliasedIndexes[0]; $aliasUpdateRequest['actions'][] = array( - 'remove' => array('index' => $oldIndexName, 'alias' => $aliasName), + 'remove' => array('index' => $aliasedIndex, 'alias' => $aliasName), ); } @@ -93,58 +103,68 @@ class AliasProcessor 'add' => array('index' => $newIndexName, 'alias' => $aliasName), ); - try { - $client->request('_aliases', 'POST', $aliasUpdateRequest); - } catch (ExceptionInterface $renameAliasException) { - $additionalError = ''; - // if we failed to move the alias, delete the newly built index - try { - $index->delete(); - } catch (ExceptionInterface $deleteNewIndexException) { - $additionalError = sprintf( - 'Tried to delete newly built index %s, but also failed: %s', - $newIndexName, - $deleteNewIndexException->getMessage() - ); - } + return $aliasUpdateRequest; + } - throw new \RuntimeException( - sprintf( - 'Failed to updated index alias: %s. %s', - $renameAliasException->getMessage(), - $additionalError ?: sprintf('Newly built index %s was deleted', $newIndexName) - ), 0, $renameAliasException + /** + * Cleans up an index when we encounter a failure to rename the alias. + * + * @param Client $client + * @param $indexName + * @param \Exception $renameAliasException + */ + private function cleanupRenameFailure(Client $client, $indexName, \Exception $renameAliasException) + { + $additionalError = ''; + try { + $this->deleteIndex($client, $indexName); + } catch (ExceptionInterface $deleteNewIndexException) { + $additionalError = sprintf( + 'Tried to delete newly built index %s, but also failed: %s', + $indexName, + $deleteNewIndexException->getMessage() ); } - // Delete the old index after the alias has been switched - if ($oldIndexName) { - $oldIndex = new Index($client, $oldIndexName); - try { - $oldIndex->delete(); - } catch (ExceptionInterface $deleteOldIndexException) { - throw new \RuntimeException( - sprintf( - 'Failed to delete old index %s with message: %s', - $oldIndexName, - $deleteOldIndexException->getMessage() - ), 0, $deleteOldIndexException - ); - } + throw new \RuntimeException(sprintf( + 'Failed to updated index alias: %s. %s', + $renameAliasException->getMessage(), + $additionalError ?: sprintf('Newly built index %s was deleted', $indexName) + ), 0, $renameAliasException); + } + + /** + * Delete an index. + * + * @param Client $client + * @param string $indexName Index name to delete + */ + private function deleteIndex(Client $client, $indexName) + { + try { + $path = sprintf("%s", $indexName); + $client->request($path, Request::DELETE); + } catch (ExceptionInterface $deleteOldIndexException) { + throw new \RuntimeException(sprintf( + 'Failed to delete index %s with message: %s', + $indexName, + $deleteOldIndexException->getMessage() + ), 0, $deleteOldIndexException); } } /** - * Returns array of indexes which are mapped to given alias. + * Returns the name of a single index that an alias points to or throws + * an exception if there is more than one. * * @param Client $client * @param string $aliasName Alias name * - * @return array + * @return string|null * * @throws AliasIsIndexException */ - private function getAliasedIndexes(Client $client, $aliasName) + private function getAliasedIndex(Client $client, $aliasName) { $aliasesInfo = $client->request('_aliases', 'GET')->getData(); $aliasedIndexes = array(); @@ -163,18 +183,15 @@ class AliasProcessor } } - return $aliasedIndexes; - } + if (count($aliasedIndexes) > 1) { + throw new \RuntimeException(sprintf( + 'Alias %s is used for multiple indexes: [%s]. Make sure it\'s'. + 'either not used or is assigned to one index only', + $aliasName, + implode(', ', $aliasedIndexes) + )); + } - /** - * Delete an index. - * - * @param Client $client - * @param string $indexName Index name to delete - */ - private function deleteIndex(Client $client, $indexName) - { - $path = sprintf("%s", $indexName); - $client->request($path, Request::DELETE); + return array_shift($aliasedIndexes); } } diff --git a/Tests/Index/AliasProcessorTest.php b/Tests/Index/AliasProcessorTest.php new file mode 100644 index 0000000..f1592b2 --- /dev/null +++ b/Tests/Index/AliasProcessorTest.php @@ -0,0 +1,223 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\ElasticaBundle\Tests\Index; + +use Elastica\Exception\ResponseException; +use Elastica\Request; +use Elastica\Response; +use FOS\ElasticaBundle\Configuration\IndexConfig; +use FOS\ElasticaBundle\Index\AliasProcessor; + +class AliasProcessorTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var AliasProcessor + */ + private $processor; + + /** + * @dataProvider getSetRootNameData + * @param string $name + * @param array $configArray + * @param string $resultStartsWith + */ + public function testSetRootName($name, $configArray, $resultStartsWith) + { + $indexConfig = new IndexConfig($name, array(), $configArray); + $index = $this->getMockBuilder('FOS\\ElasticaBundle\\Elastica\\Index') + ->disableOriginalConstructor() + ->getMock(); + $index->expects($this->once()) + ->method('overrideName') + ->with($this->stringStartsWith($resultStartsWith)); + + $this->processor->setRootName($indexConfig, $index); + } + + public function testSwitchAliasNoAliasSet() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array())); + $client->expects($this->at(1)) + ->method('request') + ->with('_aliases', 'POST', array('actions' => array( + array('add' => array('index' => 'unique_name', 'alias' => 'name')) + ))); + + $this->processor->switchIndexAlias($indexConfig, $index, false); + } + + public function testSwitchAliasExistingAliasSet() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'old_unique_name' => array('aliases' => array('name')) + ))); + $client->expects($this->at(1)) + ->method('request') + ->with('_aliases', 'POST', array('actions' => array( + array('remove' => array('index' => 'old_unique_name', 'alias' => 'name')), + array('add' => array('index' => 'unique_name', 'alias' => 'name')) + ))); + + $this->processor->switchIndexAlias($indexConfig, $index, false); + } + + /** + * @expectedException \RuntimeException + */ + public function testSwitchAliasThrowsWhenMoreThanOneExists() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'old_unique_name' => array('aliases' => array('name')), + 'another_old_unique_name' => array('aliases' => array('name')) + ))); + + $this->processor->switchIndexAlias($indexConfig, $index, false); + } + + /** + * @expectedException \FOS\ElasticaBundle\Exception\AliasIsIndexException + */ + public function testSwitchAliasThrowsWhenAliasIsAnIndex() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'name' => array(), + ))); + + $this->processor->switchIndexAlias($indexConfig, $index, false); + } + + public function testSwitchAliasDeletesIndexCollisionIfForced() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'name' => array(), + ))); + $client->expects($this->at(1)) + ->method('request') + ->with('name', 'DELETE'); + + $this->processor->switchIndexAlias($indexConfig, $index, true); + } + + public function testSwitchAliasDeletesOldIndex() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'old_unique_name' => array('aliases' => array('name')), + ))); + $client->expects($this->at(1)) + ->method('request') + ->with('_aliases', 'POST', array('actions' => array( + array('remove' => array('index' => 'old_unique_name', 'alias' => 'name')), + array('add' => array('index' => 'unique_name', 'alias' => 'name')) + ))); + $client->expects($this->at(2)) + ->method('request') + ->with('old_unique_name', 'DELETE'); + + $this->processor->switchIndexAlias($indexConfig, $index, true); + } + + public function testSwitchAliasCleansUpOnRenameFailure() + { + $indexConfig = new IndexConfig('name', array(), array()); + list($index, $client) = $this->getMockedIndex('unique_name'); + + $client->expects($this->at(0)) + ->method('request') + ->with('_aliases', 'GET') + ->willReturn(new Response(array( + 'old_unique_name' => array('aliases' => array('name')), + ))); + $client->expects($this->at(1)) + ->method('request') + ->with('_aliases', 'POST', array('actions' => array( + array('remove' => array('index' => 'old_unique_name', 'alias' => 'name')), + array('add' => array('index' => 'unique_name', 'alias' => 'name')) + ))) + ->will($this->throwException(new ResponseException(new Request(''), new Response('')))); + $client->expects($this->at(2)) + ->method('request') + ->with('unique_name', 'DELETE'); + // Not an annotation: we do not want a RuntimeException until now. + $this->setExpectedException('RuntimeException'); + + $this->processor->switchIndexAlias($indexConfig, $index, true); + } + + public function getSetRootNameData() + { + return array( + array('name', array(), 'name_'), + array('name', array('elasticSearchName' => 'notname'), 'notname_') + ); + } + + protected function setUp() + { + $this->processor = new AliasProcessor(); + } + + private function getMockedIndex($name) + { + $index = $this->getMockBuilder('FOS\\ElasticaBundle\\Elastica\\Index') + ->disableOriginalConstructor() + ->getMock(); + + $client = $this->getMockBuilder('Elastica\\Client') + ->disableOriginalConstructor() + ->getMock(); + $index->expects($this->any()) + ->method('getClient') + ->willReturn($client); + + $index->expects($this->any()) + ->method('getName') + ->willReturn($name); + + return array($index, $client); + } +} From 72a9dfa267ff5d302a4b0b595e1a74d88021cc06 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 16:08:50 +1100 Subject: [PATCH 07/13] Indexable service improvements --- CHANGELOG-3.1.md | 1 + Index/AliasProcessor.php | 2 +- Provider/Indexable.php | 112 ++++++++++++++++++++++--------- Resources/doc/types.md | 7 +- Tests/Provider/IndexableTest.php | 7 ++ 5 files changed, 95 insertions(+), 34 deletions(-) diff --git a/CHANGELOG-3.1.md b/CHANGELOG-3.1.md index 478840e..10d7887 100644 --- a/CHANGELOG-3.1.md +++ b/CHANGELOG-3.1.md @@ -44,3 +44,4 @@ https://github.com/FriendsOfSymfony/FOSElasticaBundle/compare/v3.0.4...v3.1.0 * New events `PRE_INDEX_RESET`, `POST_INDEX_RESET`, `PRE_TYPE_RESET` and `POST_TYPE_RESET` are run before and after operations that will reset an index. #744 + * Added indexable callback support for the __invoke method of a service. #823 diff --git a/Index/AliasProcessor.php b/Index/AliasProcessor.php index f4e4d00..d8f608d 100644 --- a/Index/AliasProcessor.php +++ b/Index/AliasProcessor.php @@ -110,7 +110,7 @@ class AliasProcessor * Cleans up an index when we encounter a failure to rename the alias. * * @param Client $client - * @param $indexName + * @param string $indexName * @param \Exception $renameAliasException */ private function cleanupRenameFailure(Client $client, $indexName, \Exception $renameAliasException) diff --git a/Provider/Indexable.php b/Provider/Indexable.php index c72c9b9..6946b54 100644 --- a/Provider/Indexable.php +++ b/Provider/Indexable.php @@ -55,6 +55,7 @@ class Indexable implements IndexableInterface /** * @param array $callbacks + * @param ContainerInterface $container */ public function __construct(array $callbacks, ContainerInterface $container) { @@ -112,39 +113,48 @@ class Indexable implements IndexableInterface return $callback; } - if (is_array($callback)) { - list($class, $method) = $callback + array(null, null); - - if (is_object($class)) { - $class = get_class($class); - } - - if (strpos($class, '@') === 0) { - $service = $this->container->get(substr($class, 1)); - - return array($service, $method); - } - - if ($class && $method) { - throw new \InvalidArgumentException(sprintf('Callback for type "%s", "%s::%s()", is not callable.', $type, $class, $method)); - } + if (is_array($callback) && !is_object($callback[0])) { + return $this->processArrayToCallback($type, $callback); } - if (is_string($callback) && $expression = $this->getExpressionLanguage()) { - $callback = new Expression($callback); - - try { - $expression->compile($callback, array('object', $this->getExpressionVar($object))); - - return $callback; - } catch (SyntaxError $e) { - throw new \InvalidArgumentException(sprintf('Callback for type "%s" is an invalid expression', $type), $e->getCode(), $e); - } + if (is_string($callback)) { + return $this->buildExpressionCallback($type, $object, $callback); } throw new \InvalidArgumentException(sprintf('Callback for type "%s" is not a valid callback.', $type)); } + /** + * Processes a string expression into an Expression. + * + * @param string $type + * @param mixed $object + * @param string $callback + * + * @return Expression + */ + private function buildExpressionCallback($type, $object, $callback) + { + $expression = $this->getExpressionLanguage(); + if (!$expression) { + throw new \RuntimeException('Unable to process an expression without the ExpressionLanguage component.'); + } + + try { + $callback = new Expression($callback); + $expression->compile($callback, array( + 'object', $this->getExpressionVar($object) + )); + + return $callback; + } catch (SyntaxError $e) { + throw new \InvalidArgumentException(sprintf( + 'Callback for type "%s" is an invalid expression', + $type + ), $e->getCode(), $e); + } + } + /** * Retreives a cached callback, or creates a new callback if one is not found. * @@ -163,15 +173,13 @@ class Indexable implements IndexableInterface } /** - * @return bool|ExpressionLanguage + * Returns the ExpressionLanguage class if it is available. + * + * @return ExpressionLanguage|null */ private function getExpressionLanguage() { - if (null === $this->expressionLanguage) { - if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { - return false; - } - + if (null === $this->expressionLanguage && class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) { $this->expressionLanguage = new ExpressionLanguage(); } @@ -179,14 +187,54 @@ class Indexable implements IndexableInterface } /** + * Returns the variable name to be used to access the object when using the ExpressionLanguage + * component to parse and evaluate an expression. + * * @param mixed $object * * @return string */ private function getExpressionVar($object = null) { + if (!is_object($object)) { + return 'object'; + } + $ref = new \ReflectionClass($object); return strtolower($ref->getShortName()); } + + /** + * Processes an array into a callback. Replaces the first element with a service if + * it begins with an @. + * + * @param string $type + * @param array $callback + * @return array + */ + private function processArrayToCallback($type, array $callback) + { + list($class, $method) = $callback + array(null, '__invoke'); + + if (strpos($class, '@') === 0) { + $service = $this->container->get(substr($class, 1)); + $callback = array($service, $method); + + if (!is_callable($callback)) { + throw new \InvalidArgumentException(sprintf( + 'Method "%s" on service "%s" is not callable.', + $method, + substr($class, 1) + )); + } + + return $callback; + } + + throw new \InvalidArgumentException(sprintf( + 'Unable to parse callback array for type "%s"', + $type + )); + } } diff --git a/Resources/doc/types.md b/Resources/doc/types.md index 2d575cd..0e82c60 100644 --- a/Resources/doc/types.md +++ b/Resources/doc/types.md @@ -201,13 +201,18 @@ index enabled users. The callback option supports multiple approaches: * A method on the object itself provided as a string. `enabled` will call - `Object->enabled()` + `Object->enabled()`. Note that this does not support chaining methods with dot notation + like property paths. To achieve something similar use the ExpressionLanguage option + below. * An array of a service id and a method which will be called with the object as the first and only argument. `[ @my_custom_service, 'userIndexable' ]` will call the userIndexable method on a service defined as my_custom_service. * An array of a class and a static method to call on that class which will be called with the object as the only argument. `[ 'Acme\DemoBundle\IndexableChecker', 'isIndexable' ]` will call Acme\DemoBundle\IndexableChecker::isIndexable($object) +* A single element array with a service id can be used if the service has an __invoke + method. Such an invoke method must accept a single parameter for the object to be indexed. + `[ @my_custom_invokable_service ]` * If you have the ExpressionLanguage component installed, A valid ExpressionLanguage expression provided as a string. The object being indexed will be supplied as `object` in the expression. `object.isEnabled() or object.shouldBeIndexedAnyway()`. For more diff --git a/Tests/Provider/IndexableTest.php b/Tests/Provider/IndexableTest.php index 3080c88..f8a4564 100644 --- a/Tests/Provider/IndexableTest.php +++ b/Tests/Provider/IndexableTest.php @@ -55,6 +55,7 @@ class IndexableTest extends \PHPUnit_Framework_TestCase { return array( array('nonexistentEntityMethod'), + array(array('@indexableService', 'internalMethod')), array(array(new IndexableDecider(), 'internalMethod')), array(42), array('entity.getIsIndexable() && nonexistentEntityFunction()'), @@ -67,6 +68,7 @@ class IndexableTest extends \PHPUnit_Framework_TestCase array('isIndexable', false), array(array(new IndexableDecider(), 'isIndexable'), true), array(array('@indexableService', 'isIndexable'), true), + array(array('@indexableService'), true), array(function (Entity $entity) { return $entity->maybeIndex(); }, true), array('entity.maybeIndex()', true), array('!object.isIndexable() && entity.property == "abc"', true), @@ -111,4 +113,9 @@ class IndexableDecider protected function internalMethod() { } + + public function __invoke($object) + { + return true; + } } From d4f01e8d2e479b660ba5675c7257c78a5e9e8b89 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 18:47:24 +1100 Subject: [PATCH 08/13] Cast result from ExpressionLanguage eval to bool --- Provider/Indexable.php | 2 +- Tests/Provider/IndexableTest.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Provider/Indexable.php b/Provider/Indexable.php index 6946b54..c26da5a 100644 --- a/Provider/Indexable.php +++ b/Provider/Indexable.php @@ -82,7 +82,7 @@ class Indexable implements IndexableInterface } if ($callback instanceof Expression) { - return $this->getExpressionLanguage()->evaluate($callback, array( + return (bool) $this->getExpressionLanguage()->evaluate($callback, array( 'object' => $object, $this->getExpressionVar($object) => $object, )); diff --git a/Tests/Provider/IndexableTest.php b/Tests/Provider/IndexableTest.php index f8a4564..e122ec1 100644 --- a/Tests/Provider/IndexableTest.php +++ b/Tests/Provider/IndexableTest.php @@ -73,6 +73,8 @@ class IndexableTest extends \PHPUnit_Framework_TestCase array('entity.maybeIndex()', true), array('!object.isIndexable() && entity.property == "abc"', true), array('entity.property != "abc"', false), + array('["array", "values"]', true), + array('[]', false) ); } From 3bb2f384babf9f72461ddab6f332c7989376ad81 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 19:53:05 +1100 Subject: [PATCH 09/13] Provider refactoring --- Doctrine/AbstractProvider.php | 159 +++++++++++------------- Doctrine/MongoDB/Provider.php | 10 +- Doctrine/ORM/Provider.php | 12 +- Doctrine/ORM/SliceFetcher.php | 3 + Propel/Provider.php | 32 +++-- Provider/AbstractProvider.php | 127 +++++++++++++++++-- Tests/Doctrine/AbstractProviderTest.php | 4 +- 7 files changed, 232 insertions(+), 115 deletions(-) diff --git a/Doctrine/AbstractProvider.php b/Doctrine/AbstractProvider.php index 9845edc..f56fe52 100644 --- a/Doctrine/AbstractProvider.php +++ b/Doctrine/AbstractProvider.php @@ -7,6 +7,7 @@ use Elastica\Exception\Bulk\ResponseException as BulkResponseException; use FOS\ElasticaBundle\Persister\ObjectPersisterInterface; use FOS\ElasticaBundle\Provider\AbstractProvider as BaseAbstractProvider; use FOS\ElasticaBundle\Provider\IndexableInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; abstract class AbstractProvider extends BaseAbstractProvider { @@ -26,7 +27,7 @@ abstract class AbstractProvider extends BaseAbstractProvider * @param ObjectPersisterInterface $objectPersister * @param IndexableInterface $indexable * @param string $objectClass - * @param array $options + * @param array $baseOptions * @param ManagerRegistry $managerRegistry * @param SliceFetcherInterface $sliceFetcher */ @@ -34,71 +35,106 @@ abstract class AbstractProvider extends BaseAbstractProvider ObjectPersisterInterface $objectPersister, IndexableInterface $indexable, $objectClass, - array $options, + array $baseOptions, ManagerRegistry $managerRegistry, SliceFetcherInterface $sliceFetcher = null ) { - parent::__construct($objectPersister, $indexable, $objectClass, array_merge(array( - 'clear_object_manager' => true, - 'debug_logging' => false, - 'ignore_errors' => false, - 'query_builder_method' => 'createQueryBuilder', - ), $options)); + parent::__construct($objectPersister, $indexable, $objectClass, $baseOptions); $this->managerRegistry = $managerRegistry; $this->sliceFetcher = $sliceFetcher; } + /** + * Counts objects that would be indexed using the query builder. + * + * @param object $queryBuilder + * + * @return integer + */ + abstract protected function countObjects($queryBuilder); + + /** + * Creates the query builder, which will be used to fetch objects to index. + * + * @param string $method + * + * @return object + */ + abstract protected function createQueryBuilder($method); + + /** + * Fetches a slice of objects using the query builder. + * + * @param object $queryBuilder + * @param integer $limit + * @param integer $offset + * + * @return array + */ + abstract protected function fetchSlice($queryBuilder, $limit, $offset); + /** * {@inheritDoc} */ - public function populate(\Closure $loggerClosure = null, array $options = array()) + protected function doPopulate($options, \Closure $loggerClosure = null) { - if (!$this->options['debug_logging']) { - $logger = $this->disableLogging(); - } - - $queryBuilder = $this->createQueryBuilder(); - $nbObjects = $this->countObjects($queryBuilder); - $offset = isset($options['offset']) ? intval($options['offset']) : 0; - $sleep = isset($options['sleep']) ? intval($options['sleep']) : 0; - $batchSize = isset($options['batch-size']) ? intval($options['batch-size']) : $this->options['batch_size']; - $ignoreErrors = isset($options['ignore-errors']) ? $options['ignore-errors'] : $this->options['ignore_errors']; $manager = $this->managerRegistry->getManagerForClass($this->objectClass); - $objects = array(); - for (; $offset < $nbObjects; $offset += $batchSize) { - $objects = $this->getSlice($queryBuilder, $batchSize, $offset, $objects); - $objects = array_filter($objects, array($this, 'isObjectIndexable')); + $queryBuilder = $this->createQueryBuilder($options['query_builder_method']); + $nbObjects = $this->countObjects($queryBuilder); + $offset = $options['offset']; - if (!empty($objects)) { - if (!$ignoreErrors) { + $objects = array(); + for (; $offset < $nbObjects; $offset += $options['batch_size']) { + try { + $objects = $this->getSlice($queryBuilder, $options['batch_size'], $offset, $objects); + $objects = $this->filterObjects($options, $objects); + + if (!empty($objects)) { $this->objectPersister->insertMany($objects); - } else { - try { - $this->objectPersister->insertMany($objects); - } catch (BulkResponseException $e) { - if ($loggerClosure) { - $loggerClosure($batchSize, $nbObjects, sprintf('%s', $e->getMessage())); - } - } + } + } catch (BulkResponseException $e) { + if (!$options['ignore_errors']) { + throw $e; + } + + if (null !== $loggerClosure) { + $loggerClosure( + $options['batch_size'], + $nbObjects, + sprintf('%s', $e->getMessage()) + ); } } - if ($this->options['clear_object_manager']) { + if ($options['clear_object_manager']) { $manager->clear(); } - usleep($sleep); + usleep($options['sleep']); - if ($loggerClosure) { - $loggerClosure($batchSize, $nbObjects); + if (null !== $loggerClosure) { + $loggerClosure($options['batch_size'], $nbObjects); } } + } - if (!$this->options['debug_logging']) { - $this->enableLogging($logger); - } + /** + * {@inheritDoc} + */ + protected function configureOptions() + { + parent::configureOptions(); + + $this->resolver->setDefaults(array( + 'clear_object_manager' => true, + 'debug_logging' => false, + 'ignore_errors' => false, + 'offset' => 0, + 'query_builder_method' => 'createQueryBuilder', + 'sleep' => 0 + )); } /** @@ -131,47 +167,4 @@ abstract class AbstractProvider extends BaseAbstractProvider $identifierFieldNames ); } - - /** - * Counts objects that would be indexed using the query builder. - * - * @param object $queryBuilder - * - * @return integer - */ - abstract protected function countObjects($queryBuilder); - - /** - * Disables logging and returns the logger that was previously set. - * - * @return mixed - */ - abstract protected function disableLogging(); - - /** - * Reenables the logger with the previously returned logger from disableLogging();. - * - * @param mixed $logger - * - * @return mixed - */ - abstract protected function enableLogging($logger); - - /** - * Fetches a slice of objects using the query builder. - * - * @param object $queryBuilder - * @param integer $limit - * @param integer $offset - * - * @return array - */ - abstract protected function fetchSlice($queryBuilder, $limit, $offset); - - /** - * Creates the query builder, which will be used to fetch objects to index. - * - * @return object - */ - abstract protected function createQueryBuilder(); } diff --git a/Doctrine/MongoDB/Provider.php b/Doctrine/MongoDB/Provider.php index 6b2bd2c..e4b08c5 100644 --- a/Doctrine/MongoDB/Provider.php +++ b/Doctrine/MongoDB/Provider.php @@ -44,7 +44,7 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::countObjects() + * {@inheritDoc} */ protected function countObjects($queryBuilder) { @@ -58,7 +58,7 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::fetchSlice() + * {@inheritDoc} */ protected function fetchSlice($queryBuilder, $limit, $offset) { @@ -75,13 +75,13 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::createQueryBuilder() + * {@inheritDoc} */ - protected function createQueryBuilder() + protected function createQueryBuilder($method) { return $this->managerRegistry ->getManagerForClass($this->objectClass) ->getRepository($this->objectClass) - ->{$this->options['query_builder_method']}(); + ->{$method}(); } } diff --git a/Doctrine/ORM/Provider.php b/Doctrine/ORM/Provider.php index fc59667..303242a 100644 --- a/Doctrine/ORM/Provider.php +++ b/Doctrine/ORM/Provider.php @@ -46,7 +46,7 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::countObjects() + * {@inheritDoc} */ protected function countObjects($queryBuilder) { @@ -69,7 +69,9 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::fetchSlice() + * This method should remain in sync with SliceFetcher::fetch until it is deprecated and removed. + * + * {@inheritDoc} */ protected function fetchSlice($queryBuilder, $limit, $offset) { @@ -103,14 +105,14 @@ class Provider extends AbstractProvider } /** - * @see FOS\ElasticaBundle\Doctrine\AbstractProvider::createQueryBuilder() + * {@inheritDoc} */ - protected function createQueryBuilder() + protected function createQueryBuilder($method) { return $this->managerRegistry ->getManagerForClass($this->objectClass) ->getRepository($this->objectClass) // ORM query builders require an alias argument - ->{$this->options['query_builder_method']}(static::ENTITY_ALIAS); + ->{$method}(static::ENTITY_ALIAS); } } diff --git a/Doctrine/ORM/SliceFetcher.php b/Doctrine/ORM/SliceFetcher.php index 86ad1b4..ac6c816 100644 --- a/Doctrine/ORM/SliceFetcher.php +++ b/Doctrine/ORM/SliceFetcher.php @@ -14,6 +14,9 @@ use FOS\ElasticaBundle\Doctrine\SliceFetcherInterface; class SliceFetcher implements SliceFetcherInterface { /** + * This method should remain in sync with Provider::fetchSlice until that method is deprecated and + * removed. + * * {@inheritdoc} */ public function fetch($queryBuilder, $limit, $offset, array $previousSlice, array $identifierFieldNames) diff --git a/Propel/Provider.php b/Propel/Provider.php index f28faa4..57d7176 100644 --- a/Propel/Provider.php +++ b/Propel/Provider.php @@ -14,31 +14,43 @@ class Provider extends AbstractProvider /** * {@inheritDoc} */ - public function populate(\Closure $loggerClosure = null, array $options = array()) + public function doPopulate($options, \Closure $loggerClosure = null) { $queryClass = $this->objectClass.'Query'; $nbObjects = $queryClass::create()->count(); - $offset = isset($options['offset']) ? intval($options['offset']) : 0; - $sleep = isset($options['sleep']) ? intval($options['sleep']) : 0; - $batchSize = isset($options['batch-size']) ? intval($options['batch-size']) : $this->options['batch_size']; - for (; $offset < $nbObjects; $offset += $batchSize) { + $offset = $options['offset']; + + for (; $offset < $nbObjects; $offset += $options['batch_size']) { $objects = $queryClass::create() - ->limit($batchSize) + ->limit($options['batch_size']) ->offset($offset) ->find() ->getArrayCopy(); - - $objects = array_filter($objects, array($this, 'isObjectIndexable')); + $objects = $this->filterObjects($options, $objects); if (!empty($objects)) { $this->objectPersister->insertMany($objects); } - usleep($sleep); + usleep($options['sleep']); if ($loggerClosure) { - $loggerClosure($batchSize, $nbObjects); + $loggerClosure($options['batch_size'], $nbObjects); } } } + + /** + * {@inheritDoc} + */ + protected function disableLogging() + { + } + + /** + * {@inheritDoc} + */ + protected function enableLogging($logger) + { + } } diff --git a/Provider/AbstractProvider.php b/Provider/AbstractProvider.php index a743d17..87614cd 100644 --- a/Provider/AbstractProvider.php +++ b/Provider/AbstractProvider.php @@ -3,6 +3,7 @@ namespace FOS\ElasticaBundle\Provider; use FOS\ElasticaBundle\Persister\ObjectPersisterInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; /** * AbstractProvider. @@ -10,9 +11,9 @@ use FOS\ElasticaBundle\Persister\ObjectPersisterInterface; abstract class AbstractProvider implements ProviderInterface { /** - * @var ObjectPersisterInterface + * @var array */ - protected $objectPersister; + protected $baseOptions; /** * @var string @@ -20,9 +21,14 @@ abstract class AbstractProvider implements ProviderInterface protected $objectClass; /** - * @var array + * @var ObjectPersisterInterface */ - protected $options; + protected $objectPersister; + + /** + * @var OptionsResolver + */ + protected $resolver; /** * @var IndexableInterface @@ -35,26 +41,114 @@ abstract class AbstractProvider implements ProviderInterface * @param ObjectPersisterInterface $objectPersister * @param IndexableInterface $indexable * @param string $objectClass - * @param array $options + * @param array $baseOptions */ public function __construct( ObjectPersisterInterface $objectPersister, IndexableInterface $indexable, $objectClass, - array $options = array() + array $baseOptions = array() ) { + $this->baseOptions = $baseOptions; $this->indexable = $indexable; $this->objectClass = $objectClass; $this->objectPersister = $objectPersister; + $this->resolver = new OptionsResolver(); + $this->configureOptions(); + } - $this->options = array_merge(array( + /** + * {@inheritDoc} + */ + public function populate(\Closure $loggerClosure = null, array $options = array()) + { + $options = $this->resolveOptions($options); + + $logger = !$options['debug_logging'] ? + $this->disableLogging() : + null; + + $this->doPopulate($options, $loggerClosure); + + if (null !== $logger) { + $this->enableLogging($logger); + } + } + + /** + * Disables logging and returns the logger that was previously set. + * + * @return mixed + */ + abstract protected function disableLogging(); + + /** + * Perform actual population. + * + * @param array $options + * @param \Closure $loggerClosure + */ + abstract protected function doPopulate($options, \Closure $loggerClosure = null); + + /** + * Reenables the logger with the previously returned logger from disableLogging();. + * + * @param mixed $logger + * + * @return mixed + */ + abstract protected function enableLogging($logger); + + /** + * Configures the option resolver. + */ + protected function configureOptions() + { + $this->resolver->setDefaults(array( 'batch_size' => 100, - ), $options); + 'skip_indexable_check' => false, + )); + + $this->resolver->setRequired(array( + 'indexName', + 'typeName', + )); + } + + + /** + * Filters objects away if they are not indexable. + * + * @param array $options + * @param array $objects + * @return array + */ + protected function filterObjects(array $options, array $objects) + { + if ($options['skip_indexable_check']) { + return $objects; + } + + $index = $options['indexName']; + $type = $options['typeName']; + + $return = array(); + foreach ($objects as $object) { + if (!$this->indexable->isObjectIndexable($index, $type, $object)) { + continue; + } + + $return[] = $object; + } + + return $return; } /** * Checks if a given object should be indexed or not. * + * @deprecated To be removed in 4.0 + * * @param object $object * * @return bool @@ -62,8 +156,8 @@ abstract class AbstractProvider implements ProviderInterface protected function isObjectIndexable($object) { return $this->indexable->isObjectIndexable( - $this->options['indexName'], - $this->options['typeName'], + $this->baseOptions['indexName'], + $this->baseOptions['typeName'], $object ); } @@ -82,4 +176,17 @@ abstract class AbstractProvider implements ProviderInterface return sprintf('(RAM : current=%uMo peak=%uMo)', $memory, $memoryMax); } + + /** + * Merges the base options provided by the class with options passed to the populate + * method and runs them through the resolver. + * + * @param array $options + * + * @return array + */ + protected function resolveOptions(array $options) + { + return $this->resolver->resolve(array_merge($this->baseOptions, $options)); + } } diff --git a/Tests/Doctrine/AbstractProviderTest.php b/Tests/Doctrine/AbstractProviderTest.php index 7b41837..aa28a4c 100644 --- a/Tests/Doctrine/AbstractProviderTest.php +++ b/Tests/Doctrine/AbstractProviderTest.php @@ -252,7 +252,7 @@ class AbstractProviderTest extends \PHPUnit_Framework_TestCase $this->setExpectedException('Elastica\Exception\Bulk\ResponseException'); - $provider->populate(null, array('ignore-errors' => false)); + $provider->populate(null, array('ignore_errors' => false)); } public function testPopulateRunsIndexCallable() @@ -280,7 +280,7 @@ class AbstractProviderTest extends \PHPUnit_Framework_TestCase $this->objectPersister->expects($this->once()) ->method('insertMany') - ->with(array(1 => 2)); + ->with(array(2)); $provider->populate(); } From 9c1c771799f030c8ae15b597714d36d697056c22 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 19:54:01 +1100 Subject: [PATCH 10/13] Mark getSlice private --- Doctrine/AbstractProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doctrine/AbstractProvider.php b/Doctrine/AbstractProvider.php index f56fe52..ec198a8 100644 --- a/Doctrine/AbstractProvider.php +++ b/Doctrine/AbstractProvider.php @@ -148,7 +148,7 @@ abstract class AbstractProvider extends BaseAbstractProvider * * @return array */ - protected function getSlice($queryBuilder, $limit, $offset, $lastSlice) + private function getSlice($queryBuilder, $limit, $offset, $lastSlice) { if (!$this->sliceFetcher) { return $this->fetchSlice($queryBuilder, $limit, $offset); From ac98549eb563058a6367294d2a52d4e6edc9936a Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 19:54:23 +1100 Subject: [PATCH 11/13] Fix translation of option keys for the Provider --- Command/PopulateCommand.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Command/PopulateCommand.php b/Command/PopulateCommand.php index e3d2918..08b501a 100644 --- a/Command/PopulateCommand.php +++ b/Command/PopulateCommand.php @@ -90,8 +90,12 @@ class PopulateCommand extends ContainerAwareCommand $index = $input->getOption('index'); $type = $input->getOption('type'); $reset = !$input->getOption('no-reset'); - $options = $input->getOptions(); - $options['ignore-errors'] = $input->getOption('ignore-errors'); + $options = array( + 'batch_size' => $input->getOption('batch-size'), + 'ignore_errors' => $input->getOption('ignore-errors'), + 'offset' => $input->getOption('offset'), + 'sleep' => $input->getOption('sleep') + ); if ($input->isInteractive() && $reset && $input->getOption('offset')) { /** @var DialogHelper $dialog */ From 5181b0293379d83a02ef593da40cd77bbe73dae9 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Sat, 14 Mar 2015 22:14:24 +1100 Subject: [PATCH 12/13] Resetter tests --- Index/Resetter.php | 12 +- Tests/Index/ResetterTest.php | 371 +++++++++++++++++++---------------- 2 files changed, 206 insertions(+), 177 deletions(-) diff --git a/Index/Resetter.php b/Index/Resetter.php index e144a62..2b39157 100644 --- a/Index/Resetter.php +++ b/Index/Resetter.php @@ -86,12 +86,12 @@ class Resetter */ public function resetIndex($indexName, $populating = false, $force = false) { - $event = new IndexResetEvent($indexName, $populating, $force); - $this->dispatcher->dispatch(IndexResetEvent::PRE_INDEX_RESET, $event); - $indexConfig = $this->configManager->getIndexConfiguration($indexName); $index = $this->indexManager->getIndex($indexName); + $event = new IndexResetEvent($indexName, $populating, $force); + $this->dispatcher->dispatch(IndexResetEvent::PRE_INDEX_RESET, $event); + if ($indexConfig->isUseAlias()) { $this->aliasProcessor->setRootName($indexConfig, $index); } @@ -117,12 +117,12 @@ class Resetter */ public function resetIndexType($indexName, $typeName) { - $event = new TypeResetEvent($indexName, $typeName); - $this->dispatcher->dispatch(TypeResetEvent::PRE_TYPE_RESET, $event); - $typeConfig = $this->configManager->getTypeConfiguration($indexName, $typeName); $type = $this->indexManager->getIndex($indexName)->getType($typeName); + $event = new TypeResetEvent($indexName, $typeName); + $this->dispatcher->dispatch(TypeResetEvent::PRE_TYPE_RESET, $event); + try { $type->delete(); } catch (ResponseException $e) { diff --git a/Tests/Index/ResetterTest.php b/Tests/Index/ResetterTest.php index 35a0bd9..9b4cd05 100644 --- a/Tests/Index/ResetterTest.php +++ b/Tests/Index/ResetterTest.php @@ -5,8 +5,14 @@ namespace FOS\ElasticaBundle\Tests\Index; use Elastica\Exception\ResponseException; use Elastica\Request; use Elastica\Response; +use Elastica\Type; use Elastica\Type\Mapping; use FOS\ElasticaBundle\Configuration\IndexConfig; +use FOS\ElasticaBundle\Configuration\TypeConfig; +use FOS\ElasticaBundle\Elastica\Index; +use FOS\ElasticaBundle\Event\IndexResetEvent; +use FOS\ElasticaBundle\Event\TypeResetEvent; +use FOS\ElasticaBundle\Index\AliasProcessor; use FOS\ElasticaBundle\Index\Resetter; class ResetterTest extends \PHPUnit_Framework_TestCase @@ -16,230 +22,253 @@ class ResetterTest extends \PHPUnit_Framework_TestCase */ private $resetter; - private $configManager; - private $indexManager; private $aliasProcessor; + private $configManager; + private $dispatcher; + private $elasticaClient; + private $indexManager; private $mappingBuilder; - public function setUp() - { - $this->markTestIncomplete('To be rewritten'); - $this->configManager = $this->getMockBuilder('FOS\\ElasticaBundle\\Configuration\\ConfigManager') - ->disableOriginalConstructor() - ->getMock(); - $this->indexManager = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\IndexManager') - ->disableOriginalConstructor() - ->getMock(); - $this->aliasProcessor = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\AliasProcessor') - ->disableOriginalConstructor() - ->getMock(); - $this->mappingBuilder = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\MappingBuilder') - ->disableOriginalConstructor() - ->getMock(); - $this->resetter = $this->getMockBuilder('FOS\ElasticaBundle\Index\Resetter') - ->disableOriginalConstructor() - ->getMock(); - - $this->resetter = new Resetter($this->configManager, $this->indexManager, $this->aliasProcessor, $this->mappingBuilder, $this->resetter); - - /*$this->indexConfigsByName = array( - 'foo' => array( - 'index' => $this->getMockElasticaIndex(), - 'config' => array( - 'properties' => array( - 'a' => array( - 'dynamic_templates' => array(), - 'properties' => array(), - ), - 'b' => array('properties' => array()), - ), - ), - ), - 'bar' => array( - 'index' => $this->getMockElasticaIndex(), - 'config' => array( - 'properties' => array( - 'a' => array('properties' => array()), - 'b' => array('properties' => array()), - ), - ), - ), - 'parent' => array( - 'index' => $this->getMockElasticaIndex(), - 'config' => array( - 'properties' => array( - 'a' => array( - 'properties' => array( - 'field_2' => array() - ), - '_parent' => array( - 'type' => 'b', - 'property' => 'b', - 'identifier' => 'id' - ), - ), - 'b' => array('properties' => array()), - ), - ), - ), - );*/ - } - public function testResetAllIndexes() { + $indexName = 'index1'; + $indexConfig = new IndexConfig($indexName, array(), array()); + $this->mockIndex($indexName, $indexConfig); + $this->configManager->expects($this->once()) ->method('getIndexNames') - ->will($this->returnValue(array('index1'))); + ->will($this->returnValue(array($indexName))); - $this->configManager->expects($this->once()) - ->method('getIndexConfiguration') - ->with('index1') - ->will($this->returnValue(new IndexConfig('index1', array(), array()))); + $this->dispatcherExpects(array( + array(IndexResetEvent::PRE_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')), + array(IndexResetEvent::POST_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')) + )); - $this->indexManager->expects($this->once()) - ->method('getIndex') - ->with('index1') - ->will($this->returnValue()); + $this->elasticaClient->expects($this->exactly(2)) + ->method('request') + ->withConsecutive( + array('index1/', 'DELETE'), + array('index1/', 'PUT', array(), array()) + ); - /*$this->indexConfigsByName['foo']['index']->expects($this->once()) - ->method('create') - ->with($this->indexConfigsByName['foo']['config'], true); - - $this->indexConfigsByName['bar']['index']->expects($this->once()) - ->method('create') - ->with($this->indexConfigsByName['bar']['config'], true); - - $resetter = new Resetter($this->indexConfigsByName);*/ $this->resetter->resetAllIndexes(); } public function testResetIndex() { - $this->indexConfigsByName['foo']['index']->expects($this->once()) - ->method('create') - ->with($this->indexConfigsByName['foo']['config'], true); + $indexConfig = new IndexConfig('index1', array(), array()); + $this->mockIndex('index1', $indexConfig); - $this->indexConfigsByName['bar']['index']->expects($this->never()) - ->method('create'); + $this->dispatcherExpects(array( + array(IndexResetEvent::PRE_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')), + array(IndexResetEvent::POST_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')) + )); - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndex('foo'); + $this->elasticaClient->expects($this->exactly(2)) + ->method('request') + ->withConsecutive( + array('index1/', 'DELETE'), + array('index1/', 'PUT', array(), array()) + ); + + $this->resetter->resetIndex('index1'); + } + + public function testResetIndexWithDifferentName() + { + $indexConfig = new IndexConfig('index1', array(), array( + 'elasticSearchName' => 'notIndex1' + )); + $this->mockIndex('index1', $indexConfig); + $this->dispatcherExpects(array( + array(IndexResetEvent::PRE_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')), + array(IndexResetEvent::POST_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')) + )); + + $this->elasticaClient->expects($this->exactly(2)) + ->method('request') + ->withConsecutive( + array('index1/', 'DELETE'), + array('index1/', 'PUT', array(), array()) + ); + + $this->resetter->resetIndex('index1'); + } + + public function testResetIndexWithDifferentNameAndAlias() + { + $indexConfig = new IndexConfig('index1', array(), array( + 'elasticSearchName' => 'notIndex1', + 'useAlias' => true + )); + $index = $this->mockIndex('index1', $indexConfig); + $this->dispatcherExpects(array( + array(IndexResetEvent::PRE_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')), + array(IndexResetEvent::POST_INDEX_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\IndexResetEvent')) + )); + + $this->aliasProcessor->expects($this->once()) + ->method('switchIndexAlias') + ->with($indexConfig, $index, false); + + $this->elasticaClient->expects($this->exactly(2)) + ->method('request') + ->withConsecutive( + array('index1/', 'DELETE'), + array('index1/', 'PUT', array(), array()) + ); + + $this->resetter->resetIndex('index1'); } /** * @expectedException \InvalidArgumentException */ - public function testResetIndexShouldThrowExceptionForInvalidIndex() + public function testFailureWhenMissingIndexDoesntDispatch() { - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndex('baz'); + $this->configManager->expects($this->once()) + ->method('getIndexConfiguration') + ->with('nonExistant') + ->will($this->throwException(new \InvalidArgumentException)); + + $this->indexManager->expects($this->never()) + ->method('getIndex'); + + $this->resetter->resetIndex('nonExistant'); } - public function testResetIndexType() + public function testResetType() { - $type = $this->getMockElasticaType(); + $typeConfig = new TypeConfig('type', array(), array()); + $this->mockType('type', 'index', $typeConfig); - $this->indexConfigsByName['foo']['index']->expects($this->once()) - ->method('getType') - ->with('a') - ->will($this->returnValue($type)); + $this->dispatcherExpects(array( + array(TypeResetEvent::PRE_TYPE_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\TypeResetEvent')), + array(TypeResetEvent::POST_TYPE_RESET, $this->isInstanceOf('FOS\\ElasticaBundle\\Event\\TypeResetEvent')) + )); - $type->expects($this->once()) - ->method('delete'); + $this->elasticaClient->expects($this->exactly(2)) + ->method('request') + ->withConsecutive( + array('index/type/', 'DELETE'), + array('index/type/_mapping', 'PUT', array('type' => array()), array()) + ); - $mapping = Mapping::create($this->indexConfigsByName['foo']['config']['properties']['a']['properties']); - $mapping->setParam('dynamic_templates', $this->indexConfigsByName['foo']['config']['properties']['a']['dynamic_templates']); - $type->expects($this->once()) - ->method('setMapping') - ->with($mapping); - - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndexType('foo', 'a'); + $this->resetter->resetIndexType('index', 'type'); } /** * @expectedException \InvalidArgumentException */ - public function testResetIndexTypeShouldThrowExceptionForInvalidIndex() + public function testNonExistantResetType() { - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndexType('baz', 'a'); + $this->configManager->expects($this->once()) + ->method('getTypeConfiguration') + ->with('index', 'nonExistant') + ->will($this->throwException(new \InvalidArgumentException)); + + $this->indexManager->expects($this->never()) + ->method('getIndex'); + + $this->resetter->resetIndexType('index', 'nonExistant'); } - /** - * @expectedException \InvalidArgumentException - */ - public function testResetIndexTypeShouldThrowExceptionForInvalidType() + public function testPostPopulateWithoutAlias() { - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndexType('foo', 'c'); + $this->mockIndex('index', new IndexConfig('index', array(), array())); + + $this->indexManager->expects($this->never()) + ->method('getIndex'); + $this->aliasProcessor->expects($this->never()) + ->method('switchIndexAlias'); + + $this->resetter->postPopulate('index'); } - public function testResetIndexTypeIgnoreTypeMissingException() + public function testPostPopulate() { - $type = $this->getMockElasticaType(); + $indexConfig = new IndexConfig('index', array(), array( 'useAlias' => true)); + $index = $this->mockIndex('index', $indexConfig); - $this->indexConfigsByName['foo']['index']->expects($this->once()) - ->method('getType') - ->with('a') - ->will($this->returnValue($type)); + $this->aliasProcessor->expects($this->once()) + ->method('switchIndexAlias') + ->with($indexConfig, $index); - $type->expects($this->once()) - ->method('delete') - ->will($this->throwException(new ResponseException( - new Request(''), - new Response(array('error' => 'TypeMissingException[[de_20131022] type[bla] missing]', 'status' => 404))) - )); - - $mapping = Mapping::create($this->indexConfigsByName['foo']['config']['properties']['a']['properties']); - $mapping->setParam('dynamic_templates', $this->indexConfigsByName['foo']['config']['properties']['a']['dynamic_templates']); - $type->expects($this->once()) - ->method('setMapping') - ->with($mapping); - - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndexType('foo', 'a'); + $this->resetter->postPopulate('index'); } - public function testIndexMappingForParent() + private function dispatcherExpects(array $events) { - $type = $this->getMockElasticaType(); + $expectation = $this->dispatcher->expects($this->exactly(count($events))) + ->method('dispatch'); - $this->indexConfigsByName['parent']['index']->expects($this->once()) - ->method('getType') - ->with('a') - ->will($this->returnValue($type)); - - $type->expects($this->once()) - ->method('delete'); - - $mapping = Mapping::create($this->indexConfigsByName['parent']['config']['properties']['a']['properties']); - $mapping->setParam('_parent', array('type' => 'b')); - $type->expects($this->once()) - ->method('setMapping') - ->with($mapping); - - $resetter = new Resetter($this->indexConfigsByName); - $resetter->resetIndexType('parent', 'a'); + call_user_func_array(array($expectation, 'withConsecutive'), $events); } - /** - * @return \Elastica\Index - */ - private function getMockElasticaIndex() + private function mockIndex($indexName, IndexConfig $config, $mapping = array()) { - return $this->getMockBuilder('Elastica\Index') + $this->configManager->expects($this->atLeast(1)) + ->method('getIndexConfiguration') + ->with($indexName) + ->will($this->returnValue($config)); + $index = new Index($this->elasticaClient, $indexName); + $this->indexManager->expects($this->any()) + ->method('getIndex') + ->with($indexName) + ->willReturn($index); + $this->mappingBuilder->expects($this->any()) + ->method('buildIndexMapping') + ->with($config) + ->willReturn($mapping); + + return $index; + } + + private function mockType($typeName, $indexName, TypeConfig $config, $mapping = array()) + { + $this->configManager->expects($this->atLeast(1)) + ->method('getTypeConfiguration') + ->with($indexName, $typeName) + ->will($this->returnValue($config)); + $index = new Index($this->elasticaClient, $indexName); + $this->indexManager->expects($this->once()) + ->method('getIndex') + ->with($indexName) + ->willReturn($index); + $this->mappingBuilder->expects($this->once()) + ->method('buildTypeMapping') + ->with($config) + ->willReturn($mapping); + + return $index; + } + + protected function setUp() + { + $this->aliasProcessor = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\AliasProcessor') ->disableOriginalConstructor() ->getMock(); - } - - /** - * @return \Elastica\Type - */ - private function getMockElasticaType() - { - return $this->getMockBuilder('Elastica\Type') + $this->configManager = $this->getMockBuilder('FOS\\ElasticaBundle\\Configuration\\ConfigManager') ->disableOriginalConstructor() ->getMock(); + $this->dispatcher = $this->getMockBuilder('Symfony\\Component\\EventDispatcher\\EventDispatcherInterface') + ->getMock(); + $this->elasticaClient = $this->getMockBuilder('Elastica\\Client') + ->disableOriginalConstructor() + ->getMock(); + $this->indexManager = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\IndexManager') + ->disableOriginalConstructor() + ->getMock(); + $this->mappingBuilder = $this->getMockBuilder('FOS\\ElasticaBundle\\Index\\MappingBuilder') + ->disableOriginalConstructor() + ->getMock(); + + $this->resetter = new Resetter( + $this->configManager, + $this->indexManager, + $this->aliasProcessor, + $this->mappingBuilder, + $this->dispatcher + ); } } From 447d29ab9c37c3ce913a0692fa0cd4c271aa5cd3 Mon Sep 17 00:00:00 2001 From: Tim Nagel Date: Wed, 18 Mar 2015 09:38:47 +1100 Subject: [PATCH 13/13] Release 3.1.0 --- CHANGELOG-3.1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG-3.1.md b/CHANGELOG-3.1.md index 10d7887..aa431e3 100644 --- a/CHANGELOG-3.1.md +++ b/CHANGELOG-3.1.md @@ -9,7 +9,7 @@ https://github.com/FriendsOfSymfony/FOSElasticaBundle/commit/XXX where XXX is the commit hash. To get the diff between two versions, go to https://github.com/FriendsOfSymfony/FOSElasticaBundle/compare/v3.0.4...v3.1.0 -* 3.1.0 (Unreleased) +* 3.1.0 (2015-03-18) * BC BREAK: `Doctrine\Listener#scheduleForDeletion` access changed to private. * BC BREAK: `ObjectPersisterInterface` gains the method `handlesObject` that