From 0b53539e0119b7a2f4fd9ff97ac979fd4306c57e Mon Sep 17 00:00:00 2001 From: Matthieu de Canteloube Date: Fri, 16 Oct 2015 12:10:25 +0200 Subject: [PATCH 1/5] Fix return values of clear and reindex commands --- Command/ClearCommand.php | 12 +++++++++++- Command/ReindexCommand.php | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Command/ClearCommand.php b/Command/ClearCommand.php index 56c369f6..7d579896 100644 --- a/Command/ClearCommand.php +++ b/Command/ClearCommand.php @@ -58,7 +58,17 @@ protected function execute(InputInterface $input, OutputInterface $output) $nIndexed += $this->clear($className); } - return $nIndexed; + switch ($nIndexed) { + case 0: + $output->writeln('No entity cleared'); + break; + case 1: + $output->writeln('1 entity cleared'); + break; + default: + $output->writeln(sprintf('%s entities cleared', $nIndexed)); + break; + } } public function clear($className) diff --git a/Command/ReindexCommand.php b/Command/ReindexCommand.php index b6de7407..05fa86ee 100644 --- a/Command/ReindexCommand.php +++ b/Command/ReindexCommand.php @@ -73,7 +73,17 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->getContainer()->get('algolia.indexer')->waitForAlgoliaTasks(); } - return $nIndexed; + switch ($nIndexed) { + case 0: + $output->writeln('No entity indexed'); + break; + case 1: + $output->writeln('1 entity indexed'); + break; + default: + $output->writeln(sprintf('%s entities indexed', $nIndexed)); + break; + } } public function reIndex($className, $batchSize = 1000, $safe = true) From 2b95c1b6eacb7ee1ae8e4cd033c0a6eba6ab561e Mon Sep 17 00:00:00 2001 From: Matthieu de Canteloube Date: Fri, 16 Oct 2015 18:52:55 +0200 Subject: [PATCH 2/5] Fix memory crash when reindexing many entities --- Command/ReindexCommand.php | 3 ++- Indexer/ManualIndexer.php | 22 +++++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/Command/ReindexCommand.php b/Command/ReindexCommand.php index 05fa86ee..79ec6311 100644 --- a/Command/ReindexCommand.php +++ b/Command/ReindexCommand.php @@ -92,7 +92,8 @@ public function reIndex($className, $batchSize = 1000, $safe = true) return $reIndexer->reIndex($className, [ 'batchSize' => $batchSize, - 'safe' => $safe + 'safe' => $safe, + 'clearEntityManager' => true, ]); } } diff --git a/Indexer/ManualIndexer.php b/Indexer/ManualIndexer.php index c9bcdf40..c4367f15 100644 --- a/Indexer/ManualIndexer.php +++ b/Indexer/ManualIndexer.php @@ -72,7 +72,7 @@ private function batchArray(array $entities, $batchSize, $callback) return count($entities); } - private function batchQuery($entityName, $query, $batchSize, $callback) + private function batchQuery($entityName, $query, $batchSize, $callback, $clearEntityManager = false) { if (!$query) { $query = $this->entityManager->createQueryBuilder()->select('e')->from($entityName, 'e')->getQuery(); @@ -98,6 +98,10 @@ private function batchQuery($entityName, $query, $batchSize, $callback) $nEntities += count($batch); $callback($batch); } + + if ($clearEntityManager) { + $this->entityManager->clear(); + } } return $nEntities; @@ -122,7 +126,8 @@ public function index($entities, array $options = array()) $defaults = [ 'batchSize' => 1000, 'query' => null, - 'indexName' => null // default is to let the engine guess + 'indexName' => null, // default is to let the engine guess + 'clearEntityManager' => false, ]; $options = array_merge($defaults, $options); @@ -134,7 +139,7 @@ public function index($entities, array $options = array()) } elseif (is_string($entities)) { return $this->batchQuery($entities, $options['query'], $options['batchSize'], function ($batch) use ($options) { $this->doIndex($batch, $options['indexName']); - }); + }, $options['clearEntityManager']); } elseif (is_object($entities)) { $this->doIndex([$entities], $options['indexName']); @@ -161,7 +166,8 @@ public function unIndex($entities, array $options = array()) { $defaults = [ 'batchSize' => 1000, - 'query' => null + 'query' => null, + 'clearEntityManager' => false, ]; $options = array_merge($defaults, $options); @@ -173,7 +179,7 @@ public function unIndex($entities, array $options = array()) } elseif (is_string($entities)) { return $this->batchQuery($entities, $options['query'], $options['batchSize'], function ($batch) { $this->doUnIndex($batch); - }); + }, $options['clearEntityManager']); } elseif (is_object($entities)) { $this->doUnIndex([$entities]); @@ -212,7 +218,8 @@ public function reIndex($entityName, array $options = array()) $defaults = [ 'safe' => true, 'batchSize' => 1000, - 'query' => null + 'query' => null, + 'clearEntityManager' => false, ]; $options = array_merge($defaults, $options); @@ -242,7 +249,8 @@ public function reIndex($entityName, array $options = array()) $nProcessed = $this->index($entityName, [ 'batchSize' => $options['batchSize'], 'query' => $options['query'], - 'indexName' => $indexTo + 'indexName' => $indexTo, + 'clearEntityManager' => $options['clearEntityManager'], ]); if ($options['safe']) { From 9c25661d32e91f63f9664ac862ee5ae7571f9973 Mon Sep 17 00:00:00 2001 From: Maxime Locqueville Date: Thu, 3 Dec 2015 11:45:20 +0100 Subject: [PATCH 3/5] Enhance error reporting when trying to index a relation that is not configured to be indexed. Fixes #33 --- Indexer/Indexer.php | 12 ++++++++++-- Indexer/ManualIndexer.php | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Indexer/Indexer.php b/Indexer/Indexer.php index bdaa010b..a0ce2c03 100644 --- a/Indexer/Indexer.php +++ b/Indexer/Indexer.php @@ -304,7 +304,11 @@ private function extractPropertyValue($entity, $field, $depth) if (count($value) > 0) { - $this->discoverEntity(reset($value), $this->em); + if (! $this->discoverEntity(reset($value), $this->em)) { + throw new NotAnAlgoliaEntity( + 'Tried to index `'.$field.'` relation which is a `'.get_class(reset($value)).'` instance, which is not recognized as an entity to index.' + ); + } } $value = array_map(function ($val) use ($depth) { @@ -319,7 +323,11 @@ private function extractPropertyValue($entity, $field, $depth) return null; } - $this->discoverEntity($value, $this->em); + if (! $this->discoverEntity($value, $this->em)) { + throw new NotAnAlgoliaEntity( + 'Tried to index `'.$field.'` relation which is a `'.get_class($value).'` instance, which is not recognized as an entity to index.' + ); + } $value = $this->getFieldsForAlgolia($value, null, $depth + 1); } diff --git a/Indexer/ManualIndexer.php b/Indexer/ManualIndexer.php index c4367f15..3d642c92 100644 --- a/Indexer/ManualIndexer.php +++ b/Indexer/ManualIndexer.php @@ -191,7 +191,11 @@ public function clear($entityName) { $className = $this->entityManager->getRepository($entityName)->getClassName(); - $this->indexer->discoverEntity($className, $this->entityManager); + if (!$this->indexer->discoverEntity($className, $this->entityManager)) { + throw new NotAnAlgoliaEntity( + 'Tried to index entity of class `'.get_class($className).'`, which is not recognized as an entity to index.' + ); + } $targetIndexName = $this->indexer->getAlgoliaIndexName($className); @@ -226,7 +230,11 @@ public function reIndex($entityName, array $options = array()) $className = $this->entityManager->getRepository($entityName)->getClassName(); - $this->indexer->discoverEntity($className, $this->entityManager); + if (!$this->indexer->discoverEntity($className, $this->entityManager)) { + throw new NotAnAlgoliaEntity( + 'Tried to index entity of class `'.get_class($className).'`, which is not recognized as an entity to index.' + ); + } $targetIndexName = $this->indexer->getAlgoliaIndexName($className); From 642cab5b96ad41d967a24a044406dafad19b20bd Mon Sep 17 00:00:00 2001 From: Maxime Locqueville Date: Mon, 7 Dec 2015 12:11:13 +0100 Subject: [PATCH 4/5] Fix potential issue with proxies --- Indexer/Indexer.php | 10 ++++++++-- Mapping/Loader/AnnotationLoader.php | 8 ++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Indexer/Indexer.php b/Indexer/Indexer.php index a0ce2c03..7d1f97b1 100644 --- a/Indexer/Indexer.php +++ b/Indexer/Indexer.php @@ -113,12 +113,17 @@ public function getMetaDataLoader() return new AnnotationLoader(); } + private function removeProxy($class) + { + /* Avoid proxy class form symfony */ + return str_replace("Proxies\\__CG__\\", "", $class); + } + private function get_class($entity) { $class = get_class($entity); - /* Avoid proxy class form symfony */ - $class = str_replace("Proxies\\__CG__\\", "", $class); + $class = $this->removeProxy($class); return $class; } @@ -140,6 +145,7 @@ public function discoverEntity($entity_or_class, EntityManager $em) $class = $this->get_class($entity); } else { $class = $em->getRepository($entity_or_class)->getClassName(); + $class = $this->removeProxy($class); $reflClass = new \ReflectionClass($class); if ($reflClass->isAbstract()) { diff --git a/Mapping/Loader/AnnotationLoader.php b/Mapping/Loader/AnnotationLoader.php index 03ceee54..58f2f0c4 100644 --- a/Mapping/Loader/AnnotationLoader.php +++ b/Mapping/Loader/AnnotationLoader.php @@ -40,6 +40,12 @@ protected function getAnnotationReader() return self::$annotationReader; } + private function removeProxy($class) + { + /* Avoid proxy class form symfony */ + return str_replace("Proxies\\__CG__\\", "", $class); + } + /** * @return Description */ @@ -47,6 +53,8 @@ public function getMetaData($entity, EntityManager $em) { $class = get_class($entity); + $class = $this->removeProxy($class); + $description = new Description($class); $refl = new \ReflectionClass($entity); From b32767ac472f318ab1f7600065c2876bfd285ebc Mon Sep 17 00:00:00 2001 From: Maxime Locqueville Date: Mon, 7 Dec 2015 16:31:44 +0100 Subject: [PATCH 5/5] Fix proxies + getter issues. Fixes #33 --- Indexer/Indexer.php | 14 ++++---------- Indexer/ManualIndexer.php | 2 +- Mapping/Loader/AnnotationLoader.php | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Indexer/Indexer.php b/Indexer/Indexer.php index 7d1f97b1..742e1981 100644 --- a/Indexer/Indexer.php +++ b/Indexer/Indexer.php @@ -4,6 +4,7 @@ use Doctrine\Common\Persistence\Proxy; use Doctrine\ORM\EntityManager; +use Symfony\Component\PropertyAccess\PropertyAccess; use Algolia\AlgoliaSearchBundle\Exception\UnknownEntity; use Algolia\AlgoliaSearchBundle\Exception\NoPrimaryKey; @@ -151,7 +152,7 @@ public function discoverEntity($entity_or_class, EntityManager $em) if ($reflClass->isAbstract()) { return false; } - + $entity = $reflClass->newInstanceWithoutConstructor(); } @@ -287,17 +288,10 @@ function isEntity(EntityManager $em, $class) return ! $em->getMetadataFactory()->isTransient($class); } - /** - * OOP? Encapsulation? No thanks! :) - * http://php.net/manual/en/closure.bind.php - */ private function extractPropertyValue($entity, $field, $depth) { - $privateGetter = \Closure::bind(function ($field) { - return $this->$field; - }, $entity, $entity); - - $value = $privateGetter($field); + $accessor = PropertyAccess::createPropertyAccessor(); + $value = $accessor->getValue($entity, $field); if ($value instanceof \Doctrine\Common\Collections\Collection) { diff --git a/Indexer/ManualIndexer.php b/Indexer/ManualIndexer.php index 3d642c92..de97e4be 100644 --- a/Indexer/ManualIndexer.php +++ b/Indexer/ManualIndexer.php @@ -232,7 +232,7 @@ public function reIndex($entityName, array $options = array()) if (!$this->indexer->discoverEntity($className, $this->entityManager)) { throw new NotAnAlgoliaEntity( - 'Tried to index entity of class `'.get_class($className).'`, which is not recognized as an entity to index.' + 'Tried to index entity of class `'.$className.'`, which is not recognized as an entity to index.' ); } diff --git a/Mapping/Loader/AnnotationLoader.php b/Mapping/Loader/AnnotationLoader.php index 58f2f0c4..c69f9031 100644 --- a/Mapping/Loader/AnnotationLoader.php +++ b/Mapping/Loader/AnnotationLoader.php @@ -57,7 +57,7 @@ public function getMetaData($entity, EntityManager $em) $description = new Description($class); - $refl = new \ReflectionClass($entity); + $refl = new \ReflectionClass($class); $reader = $this->getAnnotationReader();