Merge pull request #815 from merk/command-tidy

Command tidy

CHANGELOG-3.1.md

@@ -33,3 +33,5 @@ https://github.com/FriendsOfSymfony/FOSElasticaBundle/compare/v3.0.4...v3.1.0
    that property while transforming. Combined with the above POST_TRANSFORM event
    developers can now create calculated dynamic properties on Elastica documents
    for indexing. #794
+ * Fixed a case where ProgressCommand would always ignore errors regardless of
+   --ignore-errors being passed or not.

Command/PopulateCommand.php

@@ -23,6 +23,11 @@ class PopulateCommand extends ContainerAwareCommand
      */
     private $indexManager;
 
+    /**
+     * @var ProgressClosureBuilder
+     */
+    private $progressClosureBuilder;
+
     /**
      * @var ProviderRegistry
      */
@@ -47,31 +52,39 @@ class PopulateCommand extends ContainerAwareCommand
             ->addOption('sleep', null, InputOption::VALUE_REQUIRED, 'Sleep time between persisting iterations (microseconds)', 0)
             ->addOption('batch-size', null, InputOption::VALUE_REQUIRED, 'Index packet size (overrides provider config option)')
             ->addOption('ignore-errors', null, InputOption::VALUE_NONE, 'Do not stop on errors')
+            ->addOption('no-overwrite-format', null, InputOption::VALUE_NONE, 'Prevent this command from overwriting ProgressBar\'s formats')
             ->setDescription('Populates search indexes from providers')
         ;
     }
 
     /**
-     * @see Symfony\Component\Console\Command\Command::initialize()
+     * {@inheritDoc}
      */
     protected function initialize(InputInterface $input, OutputInterface $output)
     {
         $this->indexManager = $this->getContainer()->get('fos_elastica.index_manager');
         $this->providerRegistry = $this->getContainer()->get('fos_elastica.provider_registry');
         $this->resetter = $this->getContainer()->get('fos_elastica.resetter');
+        $this->progressClosureBuilder = new ProgressClosureBuilder();
+
+        if (!$input->getOption('no-overwrite-format')) {
+            ProgressBar::setFormatDefinition('normal', " %current%/%max% [%bar%] %percent:3s%%\n%message%");
+            ProgressBar::setFormatDefinition('verbose', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%\n%message%");
+            ProgressBar::setFormatDefinition('very_verbose', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s%\n%message%");
+            ProgressBar::setFormatDefinition('debug', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s% %memory:6s%\n%message%");
+        }
     }
 
     /**
-     * @see Symfony\Component\Console\Command\Command::execute()
+     * {@inheritDoc}
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        $index         = $input->getOption('index');
-        $type          = $input->getOption('type');
-        $reset         = !$input->getOption('no-reset');
-        $options       = $input->getOptions();
-
-        $options['ignore-errors'] = $input->hasOption('ignore-errors');
+        $index = $input->getOption('index');
+        $type = $input->getOption('type');
+        $reset = !$input->getOption('no-reset');
+        $options = $input->getOptions();
+        $options['ignore-errors'] = $input->getOption('ignore-errors');
 
         if ($input->isInteractive() && $reset && $input->getOption('offset')) {
             /** @var DialogHelper $dialog */
@@ -109,73 +122,11 @@ class PopulateCommand extends ContainerAwareCommand
      */
     private function doPopulateType(ProviderInterface $provider, OutputInterface $output, $input, $type, $options)
     {
-        $loggerClosure = $this->getLoggerClosure($output, $input, $type);
+        $loggerClosure = $this->progressClosureBuilder->build($output, 'Populating', $input, $type);
 
         $provider->populate($loggerClosure, $options);
     }
 
-    /**
-     * Builds a loggerClosure to be called from inside the Provider to update the command
-     * line.
-     *
-     * @param OutputInterface $output
-     * @param string $index
-     * @param string $type
-     * @return callable
-     */
-    private function getLoggerClosure(OutputInterface $output, $index, $type)
-    {
-        if (!class_exists('Symfony\Component\Console\Helper\ProgressBar')) {
-            $lastStep = null;
-            $current = 0;
-
-            return function ($increment, $totalObjects) use ($output, $index, $type, &$lastStep, &$current) {
-                if ($current + $increment > $totalObjects) {
-                    $increment = $totalObjects - $current;
-                }
-
-                $currentTime = microtime(true);
-                $timeDifference = $currentTime - $lastStep;
-                $objectsPerSecond = $lastStep ? ($increment / $timeDifference) : $increment;
-                $lastStep = $currentTime;
-                $current += $increment;
-                $percent = 100 * $current / $totalObjects;
-
-                $output->writeln(sprintf(
-                    '<info>Populating</info> <comment>%s/%s</comment> %0.1f%% (%d/%d), %d objects/s (RAM: current=%uMo peak=%uMo)',
-                    $index,
-                    $type,
-                    $percent,
-                    $current,
-                    $totalObjects,
-                    $objectsPerSecond,
-                    round(memory_get_usage() / (1024 * 1024)),
-                    round(memory_get_peak_usage() / (1024 * 1024))
-                ));
-            };
-        }
-
-        ProgressBar::setFormatDefinition('normal', " %current%/%max% [%bar%] %percent:3s%%\n%message%");
-        ProgressBar::setFormatDefinition('verbose', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%\n%message%");
-        ProgressBar::setFormatDefinition('very_verbose', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s%\n%message%");
-        ProgressBar::setFormatDefinition('debug', " %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s% %memory:6s%\n%message%");
-        $progress = null;
-
-        return function ($increment, $totalObjects) use (&$progress, $output, $index, $type) {
-            if (null === $progress) {
-                $progress = new ProgressBar($output, $totalObjects);
-                $progress->start();
-            }
-
-            $progress->setMessage(sprintf('<info>Populating</info> <comment>%s/%s</comment>', $index, $type));
-            $progress->advance($increment);
-
-            if ($progress->getProgressPercent() >= 1.0) {
-                $progress->finish();
-            }
-        };
-    }
-
     /**
      * Recreates an index, populates its types, and refreshes the index.
      *

Command/ProgressClosureBuilder.php

@@ -0,0 +1,101 @@
+<?php
+
+/**
+ * This file is part of the FOSElasticaBundle project.
+ *
+ * (c) Tim Nagel <tim@nagel.com.au>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\ElasticaBundle\Command;
+
+use Symfony\Component\Console\Helper\ProgressBar;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class ProgressClosureBuilder
+{
+    /**
+     * Builds a loggerClosure to be called from inside the Provider to update the command
+     * line.
+     *
+     * @param OutputInterface $output
+     * @param string $action
+     * @param string $index
+     * @param string $type
+     * @return callable
+     */
+    public function build(OutputInterface $output, $action, $index, $type)
+    {
+        if (!class_exists('Symfony\Component\Console\Helper\ProgressBar') ||
+            !is_callable(array('Symfony\Component\Console\Helper\ProgressBar', 'getProgress'))) {
+            return $this->buildLegacy($output, $action, $index, $type);
+        }
+
+        $progress = null;
+
+        return function ($increment, $totalObjects, $message = null) use (&$progress, $output, $action, $index, $type) {
+            if (null === $progress) {
+                $progress = new ProgressBar($output, $totalObjects);
+                $progress->start();
+            }
+
+            if (null !== $message) {
+                $progress->clear();
+                $output->writeln(sprintf('<info>%s</info> <error>%s</error>', $action, $message));
+                $progress->display();
+            }
+
+            $progress->setMessage(sprintf('<info>%s</info> <comment>%s/%s</comment>', $action, $index, $type));
+            $progress->advance($increment);
+        };
+    }
+
+    /**
+     * Builds a legacy closure that outputs lines for each step. Used in cases
+     * where the ProgressBar component doesnt exist or does not have the correct
+     * methods to support what we need.
+     *
+     * @param OutputInterface $output
+     * @param string $action
+     * @param string $index
+     * @param string $type
+     * @return callable
+     */
+    private function buildLegacy(OutputInterface $output, $action, $index, $type)
+    {
+        $lastStep = null;
+        $current = 0;
+
+        return function ($increment, $totalObjects, $message = null) use ($output, $action, $index, $type, &$lastStep, &$current) {
+            if ($current + $increment > $totalObjects) {
+                $increment = $totalObjects - $current;
+            }
+
+            if (null !== $message) {
+                $output->writeln(sprintf('<info>%s</info> <error>%s</error>', $action, $message));
+            }
+
+            $currentTime = microtime(true);
+            $timeDifference = $currentTime - $lastStep;
+            $objectsPerSecond = $lastStep ? ($increment / $timeDifference) : $increment;
+            $lastStep = $currentTime;
+            $current += $increment;
+            $percent = 100 * $current / $totalObjects;
+
+            $output->writeln(sprintf(
+                '<info>%s</info> <comment>%s/%s</comment> %0.1f%% (%d/%d), %d objects/s (RAM: current=%uMo peak=%uMo)',
+                $action,
+                $index,
+                $type,
+                $percent,
+                $current,
+                $totalObjects,
+                $objectsPerSecond,
+                round(memory_get_usage() / (1024 * 1024)),
+                round(memory_get_peak_usage() / (1024 * 1024))
+            ));
+        };
+    }
+}

Doctrine/AbstractProvider.php

@@ -67,7 +67,7 @@ abstract class AbstractProvider extends BaseAbstractProvider
                         $this->objectPersister->insertMany($objects);
                     } catch(BulkResponseException $e) {
                         if ($loggerClosure) {
-                            $loggerClosure(sprintf('<error>%s</error>',$e->getMessage()));
+                            $loggerClosure($batchSize, $nbObjects, sprintf('<error>%s</error>', $e->getMessage()));
                         }
                     }
                 }

Provider/ProviderInterface.php

@@ -12,6 +12,11 @@ interface ProviderInterface
     /**
      * Persists all domain objects to ElasticSearch for this provider.
      *
+     * The closure can expect 2 or 3 arguments:
+     *   * The step size
+     *   * The total number of objects
+     *   * A message to output in error conditions (not normally provided)
+     *
      * @param \Closure $loggerClosure
      * @param array    $options
      * @return