MigrateCommand::execute() - Code Metrics - Inspection of "Better error messages from migrate and generate co..." - kaliop-uk/ezmigrationbundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 956196...175692 )

by Gaetano

created 2016-08-05 17:10 UTC

MigrateCommand::execute() D

↳ Parent: MigrateCommand

Complexity

Conditions	17
Paths	175

Size

Total Lines	96
Code Lines	60

Duplication

Lines	14
Ratio	14.58 %

Importance

Changes	3
Bugs	1	Features	1

Metric	Value
c	3
b	1
f	1
dl	14
loc	96
rs	4.5364
cc	17
eloc	60
nc	175
nop	2

How to fix Long Method Complexity

<?php

namespace Kaliop\eZMigrationBundle\Command;

use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Input\InputOption;
use Kaliop\eZMigrationBundle\API\Value\MigrationDefinition;
use Kaliop\eZMigrationBundle\API\Value\Migration;
use eZ\Publish\API\Repository\Exceptions\ContentTypeFieldDefinitionValidationException;

/**
 * Command to execute the available migration definitions.
 */
class MigrateCommand extends AbstractCommand
{
    /**
     * Set up the command.
     *
     * Define the name, options and help text.
     */
    protected function configure()
    {
        parent::configure();

        $this
            ->setName('kaliop:migration:migrate')
            ->setAliases(array('kaliop:migration:update'))
            ->setDescription('Execute available migration definitions.')
            ->addOption(
                'path',
                null,
                InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY,
                "The directory or file to load the migration definitions from"
            )
            ->addOption('ignore-failures', null, InputOption::VALUE_NONE, "Keep executing migration even if one fails")
            ->addOption('clear-cache', null, InputOption::VALUE_NONE, "Clear the cache after the command finishes")
            ->addOption('no-interaction', 'n', InputOption::VALUE_NONE, "Do not ask any interactive question")
            ->setHelp(<<<EOT
The <info>kaliop:migration:migrate</info> command loads and executes migrations:

    <info>./ezpublish/console kaliop:migration:migrate</info>

You can optionally specify the path to migration definitions with <info>--path</info>:

    <info>./ezpublish/console kaliop:migrations:migrate --path=/path/to/bundle/version_directory --path=/path/to/bundle/version_directory/single_migration_file</info>
EOT
            );
    }

    /**
     * Execute the command.
     *
     * @param InputInterface $input
     * @param OutputInterface $output
     * @return null|int null or 0 if everything went fine, or an error code
     *
     * @todo Add functionality to work with specified version files not just directories.
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $migrationsService = $this->getMigrationService();

        $migrationDefinitions = $migrationsService->getMigrationsDefinitions($input->getOption('path')) ;
        $migrations = $migrationsService->getMigrations();

        if (!count($migrationDefinitions)) {
            $output->writeln('<info>No migrations found</info>');
            return;
        }

        // filter away all migrations except 'to do' ones
        $toExecute = array();
        foreach($migrationDefinitions as $name => $migrationDefinition) {
            if (!isset($migrations[$name]) || (($migration = $migrations[$name]) && $migration->status == Migration::STATUS_TODO)) {
                $toExecute[$name] = $migrationsService->parseMigrationDefinition($migrationDefinition);
            }
        }
        // just in case...
        ksort($toExecute);

        if (!count($toExecute)) {
            $output->writeln('<info>No migrations to execute</info>');
            return;
        }

        $output->writeln("\n <info>==</info> Migrations to be executed\n");

        $data = array();
        $i = 1;
        foreach($toExecute as $name => $migrationDefinition) {
            $notes = '';
            if ($migrationDefinition->status != MigrationDefinition::STATUS_PARSED) {
                $notes = '<error>' . $migrationDefinition->parsingError . '</error>';
            }
            $data[] = array(
                $i++,
                $name,
                $notes
            );
        }

        $table = $this->getHelperSet()->get('table');
        $table
            ->setHeaders(array('#', 'Migration', 'Notes'))
            ->setRows($data);
        $table->render($output);

        $output->writeln('');
        // ask user for confirmation to make changes
        if ($input->isInteractive() && !$input->getOption('no-interaction')) {

            $dialog = $this->getHelperSet()->get('dialog');
            if (!$dialog->askConfirmation(
                $output,
                '<question>Careful, the database will be modified. Do you want to continue Y/N ?</question>',
                false
            )
            ) {
                $output->writeln('<error>Migration execution cancelled!</error>');
                return 0;
            }
        } else {
            $output->writeln("=============================================\n");
        }

        foreach($toExecute as $name => $migrationDefinition) {

            // let's skip migrations that we know are invalid - user was warned and he decide to proceed anyway
            if ($migrationDefinition->status == MigrationDefinition::STATUS_INVALID) {
                $output->writeln("<comment>Skipping $name</comment>\n");
                continue;
            }

            $output->writeln("<info>Processing $name</info>");

            try {
                $migrationsService->executeMigration($migrationDefinition);
            } catch(\Exception $e) {
                if ($input->getOption('ignore-failures')) {
                    $output->writeln("\n<error>Migration failed! Reason: ".$this->getFullExceptionMessage($e)."</error>\n");
                    continue;
                }
                $output->writeln("\n<error>Migration aborted! Reason: ".$this->getFullExceptionMessage($e)."</error>");
                return 1;
            }

            $output->writeln('');
        }

        if ($input->getOption('clear-cache')) {
            $command = $this->getApplication()->find('cache:clear');
            $inputArray = new ArrayInput(array('command' => 'cache:clear'));
            $command->run($inputArray, $output);
        }
    }

    /**
     * Turns eZPublish cryptic exceptions into something more palatable for random devs
     * @todo should this be moved to a lower layer ?
     *
     * @param \Exception $e
     * @return string
     */
    protected function getFullExceptionMessage(\Exception $e)
    {
        $message = $e->getMessage();
        if (is_a($e, '\eZ\Publish\API\Repository\Exceptions\ContentTypeFieldDefinitionValidationException') ||
            is_a($e, '\eZ\Publish\API\Repository\Exceptions\ContentTypeFieldValidationException') ||
            is_a($e, '\eZ\Publish\API\Repository\Exceptions\LimitationValidationException')
        ) {
            if (is_a($e, '\eZ\Publish\API\Repository\Exceptions\LimitationValidationException')) {
                $errorsArray = array($e->getValidationErrors());
            } else {
                $errorsArray = $e->getFieldErrors();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            }

            foreach ($errorsArray as $errors) {
                foreach ($errors as $error) {
                    /// @todo find out what is the proper eZ way of getting a translated message for these errors
                    $translatableMessage = $error->getTranslatableMessage();
                    if (is_a($e, 'eZ\Publish\API\Repository\Values\Translation\Plural')) {
                        $msgText = $translatableMessage->plural;
                    } else {
                        $msgText = $translatableMessage->message;
                    }

                    $message .= "\n" . $msgText . " - " . var_export($translatableMessage->values, true);
                }
            }
        }
        return $message;
    }
}


1		<?php
2
3		namespace Kaliop\eZMigrationBundle\Command;
4
5		use Symfony\Component\Console\Input\ArrayInput;
6		use Symfony\Component\Console\Input\InputInterface;
7		use Symfony\Component\Console\Output\OutputInterface;
8		use Symfony\Component\Console\Input\InputOption;
9		use Kaliop\eZMigrationBundle\API\Value\MigrationDefinition;
10		use Kaliop\eZMigrationBundle\API\Value\Migration;
11		use eZ\Publish\API\Repository\Exceptions\ContentTypeFieldDefinitionValidationException;
12
13		/**
14		* Command to execute the available migration definitions.
15		*/
16		class MigrateCommand extends AbstractCommand
17		{
18		/**
19		* Set up the command.
20		*
21		* Define the name, options and help text.
22		*/
23		protected function configure()
24		{
25		parent::configure();
26
27		$this
28		->setName('kaliop:migration:migrate')
29		->setAliases(array('kaliop:migration:update'))
30		->setDescription('Execute available migration definitions.')
31		->addOption(
32		'path',
33		null,
34		InputOption::VALUE_OPTIONAL \| InputOption::VALUE_IS_ARRAY,
35		"The directory or file to load the migration definitions from"
36		)
37		->addOption('ignore-failures', null, InputOption::VALUE_NONE, "Keep executing migration even if one fails")
38		->addOption('clear-cache', null, InputOption::VALUE_NONE, "Clear the cache after the command finishes")
39		->addOption('no-interaction', 'n', InputOption::VALUE_NONE, "Do not ask any interactive question")
40		->setHelp(<<<EOT
41		The <info>kaliop:migration:migrate</info> command loads and executes migrations:
42
43		<info>./ezpublish/console kaliop:migration:migrate</info>
44
45		You can optionally specify the path to migration definitions with <info>--path</info>:
46
47		<info>./ezpublish/console kaliop:migrations:migrate --path=/path/to/bundle/version_directory --path=/path/to/bundle/version_directory/single_migration_file</info>
48		EOT
49		);
50		}
51
52		/**
53		* Execute the command.
54		*
55		* @param InputInterface $input
56		* @param OutputInterface $output
57		* @return null\|int null or 0 if everything went fine, or an error code
58		*
59		* @todo Add functionality to work with specified version files not just directories.
60		*/
61		protected function execute(InputInterface $input, OutputInterface $output)
62		{
63		$migrationsService = $this->getMigrationService();
64
65		$migrationDefinitions = $migrationsService->getMigrationsDefinitions($input->getOption('path')) ;
66		$migrations = $migrationsService->getMigrations();
67
68		if (!count($migrationDefinitions)) {
69		$output->writeln('<info>No migrations found</info>');
70		return;
71		}
72
73		// filter away all migrations except 'to do' ones
74		$toExecute = array();
75		foreach($migrationDefinitions as $name => $migrationDefinition) {
76		if (!isset($migrations[$name]) \|\| (($migration = $migrations[$name]) && $migration->status == Migration::STATUS_TODO)) {
77		$toExecute[$name] = $migrationsService->parseMigrationDefinition($migrationDefinition);
78		}
79		}
80		// just in case...
81		ksort($toExecute);
82
83		if (!count($toExecute)) {
84		$output->writeln('<info>No migrations to execute</info>');
85		return;
86		}
87
88		$output->writeln("\n <info>==</info> Migrations to be executed\n");
89
90		$data = array();
91		$i = 1;
92		foreach($toExecute as $name => $migrationDefinition) {
93		$notes = '';
94		if ($migrationDefinition->status != MigrationDefinition::STATUS_PARSED) {
95		$notes = '<error>' . $migrationDefinition->parsingError . '</error>';
96		}
97		$data[] = array(
98		$i++,
99		$name,
100		$notes
101		);
102		}
103
104		$table = $this->getHelperSet()->get('table');
105		$table
106		->setHeaders(array('#', 'Migration', 'Notes'))
107		->setRows($data);
108		$table->render($output);
109
110		$output->writeln('');
111		// ask user for confirmation to make changes
112	View Code Duplication	if ($input->isInteractive() && !$input->getOption('no-interaction')) {
		0 ignored issues – show Duplication introduced 2016-08-05 09:22 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
113		$dialog = $this->getHelperSet()->get('dialog');
114		if (!$dialog->askConfirmation(
115		$output,
116		'<question>Careful, the database will be modified. Do you want to continue Y/N ?</question>',
117		false
118		)
119		) {
120		$output->writeln('<error>Migration execution cancelled!</error>');
121		return 0;
122		}
123		} else {
124		$output->writeln("=============================================\n");
125		}
126
127		foreach($toExecute as $name => $migrationDefinition) {
128
129		// let's skip migrations that we know are invalid - user was warned and he decide to proceed anyway
130		if ($migrationDefinition->status == MigrationDefinition::STATUS_INVALID) {
131		$output->writeln("<comment>Skipping $name</comment>\n");
132		continue;
133		}
134
135		$output->writeln("<info>Processing $name</info>");
136
137		try {
138		$migrationsService->executeMigration($migrationDefinition);
139		} catch(\Exception $e) {
140		if ($input->getOption('ignore-failures')) {
141		$output->writeln("\n<error>Migration failed! Reason: ".$this->getFullExceptionMessage($e)."</error>\n");
142		continue;
143		}
144		$output->writeln("\n<error>Migration aborted! Reason: ".$this->getFullExceptionMessage($e)."</error>");
145		return 1;
146		}
147
148		$output->writeln('');
149		}
150
151		if ($input->getOption('clear-cache')) {
152		$command = $this->getApplication()->find('cache:clear');
153		$inputArray = new ArrayInput(array('command' => 'cache:clear'));
154		$command->run($inputArray, $output);
155		}
156		}
157
158		/**
159		* Turns eZPublish cryptic exceptions into something more palatable for random devs
160		* @todo should this be moved to a lower layer ?
161		*
162		* @param \Exception $e
163		* @return string
164		*/
165		protected function getFullExceptionMessage(\Exception $e)
166		{
167		$message = $e->getMessage();
168		if (is_a($e, '\eZ\Publish\API\Repository\Exceptions\ContentTypeFieldDefinitionValidationException') \|\|
169		is_a($e, '\eZ\Publish\API\Repository\Exceptions\ContentTypeFieldValidationException') \|\|
170		is_a($e, '\eZ\Publish\API\Repository\Exceptions\LimitationValidationException')
171		) {
172		if (is_a($e, '\eZ\Publish\API\Repository\Exceptions\LimitationValidationException')) {
173		$errorsArray = array($e->getValidationErrors());
174		} else {
175		$errorsArray = $e->getFieldErrors();
		0 ignored issues – show Bug introduced 2016-08-05 17:20 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Exception` as the method `getFieldErrors()` does only exist in the following sub-classes of `Exception`: `eZ\Publish\API\Repositor...ieldValidationException`, `eZ\Publish\API\Repositor...tionValidationException`, `eZ\Publish\Core\Base\Exc...ieldValidationException`, `eZ\Publish\Core\Base\Exc...tionValidationException`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
176		}
177
178		foreach ($errorsArray as $errors) {
179		foreach ($errors as $error) {
180		/// @todo find out what is the proper eZ way of getting a translated message for these errors
181		$translatableMessage = $error->getTranslatableMessage();
182		if (is_a($e, 'eZ\Publish\API\Repository\Values\Translation\Plural')) {
183		$msgText = $translatableMessage->plural;
184		} else {
185		$msgText = $translatableMessage->message;
186		}
187
188		$message .= "\n" . $msgText . " - " . var_export($translatableMessage->values, true);
189		}
190		}
191		}
192		return $message;
193		}
194		}
195

kaliop-uk / ezmigrationbundle

Push — master ( 956196...175692 )

MigrateCommand::execute() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Duplication Side-by-Side

Filter issues like