Completed
Push — master ( c08d49...226ec7 )
by Gaetano
09:37 queued 10s
created

GenerateCommand   A

Complexity

Total Complexity 42

Size/Duplication

Total Lines 309
Duplicated Lines 7.12 %

Coupling/Cohesion

Components 1
Dependencies 7

Test Coverage

Coverage 86.39%

Importance

Changes 0
Metric Value
wmc 42
lcom 1
cbo 7
dl 22
loc 309
c 0
b 0
f 0
ccs 127
cts 147
cp 0.8639
rs 9.0399

6 Methods

Rating   Name   Duplication   Size   Complexity  
A configure() 0 46 1
F execute() 0 116 17
C generateMigrationFile() 10 54 11
A getMigrationDirectory() 0 21 5
A getGeneratingExecutors() 12 12 3
A migrationContextFromParameters() 0 13 5

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like GenerateCommand often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use GenerateCommand, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
namespace Kaliop\eZMigrationBundle\Command;
4
5
use Symfony\Component\Console\Input\InputArgument;
6
use Symfony\Component\Console\Input\InputInterface;
7
use Symfony\Component\Console\Input\InputOption;
8
use Symfony\Component\Console\Output\OutputInterface;
9
use Symfony\Component\HttpFoundation\File\Exception\FileException;
10
use Symfony\Component\Yaml\Yaml;
11
use Kaliop\eZMigrationBundle\API\MigrationGeneratorInterface;
12
use Kaliop\eZMigrationBundle\API\MatcherInterface;
13
14
class GenerateCommand extends AbstractCommand
15
{
16
    const DIR_CREATE_PERMISSIONS = 0755;
17
18
    private $availableMigrationFormats = array('yml', 'php', 'sql', 'json');
19
    private $availableModes = array('create', 'update', 'delete');
20
    private $availableTypes = array('content', 'content_type', 'content_type_group', 'language', 'object_state', 'object_state_group', 'role', 'section', 'generic', 'db', 'php');
21
    private $thisBundle = 'EzMigrationBundle';
22
23
    /**
24
     * Configure the console command
25
     */
26 78
    protected function configure()
27
    {
28 78
        $this->setName('kaliop:migration:generate')
29 78
            ->setDescription('Generate a blank migration definition file.')
30 78
            ->addOption('format', null, InputOption::VALUE_REQUIRED, 'The format of migration file to generate (' . implode(', ', $this->availableMigrationFormats) . ')', 'yml')
31 78
            ->addOption('type', null, InputOption::VALUE_REQUIRED, 'The type of migration to generate (' . implode(', ', $this->availableTypes) . ')', '')
32 78
            ->addOption('mode', null, InputOption::VALUE_REQUIRED, 'The mode of the migration (' . implode(', ', $this->availableModes) . ')', 'create')
33 78
            ->addOption('match-type', null, InputOption::VALUE_REQUIRED, 'The type of identifier used to find the entity to generate the migration for', null)
34 78
            ->addOption('match-value', null, InputOption::VALUE_REQUIRED, 'The identifier value used to find the entity to generate the migration for. Can have many values separated by commas', null)
35 78
            ->addOption('match-except', null, InputOption::VALUE_NONE, 'Used to match all entities except the ones satisfying the match-value condition', null)
36 78
            ->addOption('lang', 'l', InputOption::VALUE_REQUIRED, 'The language of the migration (eng-GB, ger-DE, ...). If null, the default language of the current siteaccess is used')
37 78
            ->addOption('dbserver', null, InputOption::VALUE_REQUIRED, 'The type of the database server the sql migration is for, when type=db (mysql, postgresql, ...)', 'mysql')
38 78
            ->addOption('admin-login', 'a', InputOption::VALUE_REQUIRED, "Login of admin account used whenever elevated privileges are needed (user id 14 used by default)")
39 78
            ->addArgument('bundle', InputArgument::REQUIRED, 'The bundle to generate the migration definition file in. eg.: AcmeMigrationBundle')
40 78
            ->addArgument('name', InputArgument::OPTIONAL, 'The migration name (will be prefixed with current date)', null)
41 78
            ->setHelp(<<<EOT
42 78
The <info>kaliop:migration:generate</info> command generates a skeleton migration definition file:
43 78
44
    <info>php ezpublish/console kaliop:migration:generate bundleName</info>
45
46
You can optionally specify the file type to generate with <info>--format</info>, as well a name for the migration:
47
48
    <info>php ezpublish/console kaliop:migration:generate --format=json bundleName migrationName</info>
49
50
For SQL type migration you can optionally specify the database server type the migration is for with <info>--dbserver</info>:
51
52
    <info>php ezpublish/console kaliop:migration:generate --format=sql bundleName</info>
53
54
For role/content/content_type/language/object_state/section migrations you need to specify the entity that you want to generate the migration for:
55
56
    <info>php ezpublish/console kaliop:migration:generate --type=content --match-type=content_id --match-value=10,14 bundleName</info>
57
58
For role type migration you will receive a yaml file with the current role definition. You must define ALL the policies
59
you wish for the role. Any not defined will be removed. Example for updating an existing role:
60
61
    <info>php ezpublish/console kaliop:migration:generate --type=role --mode=update --match-type=identifier --match-value=Anonymous bundleName</info>
62
63
For freeform php migrations, you will receive a php class definition
64
65
    <info>php ezpublish/console kaliop:migration:generate --format=php bundlename classname</info>
66
67
Note that you can pass in a custom directory path instead of a bundle name, but, if you do, you will have to use the <info>--path</info>
68
option when you run the <info>migrate</info> command.
69
EOT
70
            );
71
    }
72 78
73
    /**
74
     * Run the command and display the results.
75
     *
76
     * @param InputInterface $input
77
     * @param OutputInterface $output
78
     * @return null|int null or 0 if everything went fine, or an error code
79
     * @throws \InvalidArgumentException When an unsupported file type is selected
80
     */
81
    public function execute(InputInterface $input, OutputInterface $output)
82 31
    {
83
        $bundleName = $input->getArgument('bundle');
84 31
        $name = $input->getArgument('name');
85 31
        $fileType = $input->getOption('format');
86 31
        $migrationType = $input->getOption('type');
87 31
        $matchType = $input->getOption('match-type');
88 31
        $matchValue = $input->getOption('match-value');
89 31
        $matchExcept = $input->getOption('match-except');
90 31
        $mode = $input->getOption('mode');
91 31
        $dbServer = $input->getOption('dbserver');
92 31
93 31
        if ($bundleName == $this->thisBundle) {
94
            throw new \InvalidArgumentException("It is not allowed to create migrations in bundle '$bundleName'");
95 31
        }
96
97
        // be kind to lazy users
98
        if ($migrationType == '') {
99
            if ($fileType == 'sql') {
100
                $migrationType = 'db';
101
            } elseif ($fileType == 'php') {
102
                $migrationType = 'php';
103
            } else {
104
                $migrationType = 'generic';
105 31
            }
106
        }
107
108
        if (!in_array($fileType, $this->availableMigrationFormats)) {
109
            throw new \InvalidArgumentException('Unsupported migration file format ' . $fileType);
110 31
        }
111 6
112 2
        if (!in_array($mode, $this->availableModes)) {
113 4
            throw new \InvalidArgumentException('Unsupported migration mode ' . $mode);
114 2
        }
115
116 2
        $migrationDirectory = $this->getMigrationDirectory($bundleName);
0 ignored issues
show
Bug introduced by
It seems like $bundleName defined by $input->getArgument('bundle') on line 83 can also be of type array<integer,string> or null; however, Kaliop\eZMigrationBundle...getMigrationDirectory() does only seem to accept string, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
117
118
        if (!is_dir($migrationDirectory)) {
119
            $output->writeln(sprintf(
120 31
                "Migrations directory <info>%s</info> does not exist. I will create it now....",
121
                $migrationDirectory
122
            ));
123
124 31
            if (mkdir($migrationDirectory, self::DIR_CREATE_PERMISSIONS, true)) {
125
                $output->writeln(sprintf(
126
                    "Migrations directory <info>%s</info> has been created",
127
                    $migrationDirectory
128 31
                ));
129
            } else {
130 31
                throw new FileException(sprintf(
131 1
                    "Failed to create migrations directory %s.",
132 1
                    $migrationDirectory
133 1
                ));
134
            }
135
        }
136 1
137 1
        // allow to generate migrations for many entities
138 1
        if (strpos($matchValue, ',') !== false ) {
139 1
            $matchValue = explode(',', $matchValue);
140
        }
141
142
        $parameters = array(
143
            'dbserver' => $dbServer,
144
            'matchType' => $matchType,
145
            'matchValue' => $matchValue,
146
            'matchExcept' => $matchExcept,
147
            'mode' => $mode,
148
            'lang' => $input->getOption('lang'),
149
            'adminLogin' => $input->getOption('admin-login')
150 31
        );
151 2
152
        $date = date('YmdHis');
153
154
        switch ($fileType) {
155 31
            case 'sql':
156 31
                /// @todo this logic should come from the DefinitionParser, really
157 31
                if ($name != '') {
158 31
                    $name = '_' . ltrim($name, '_');
159 31
                }
160 31
                $fileName = $date . '_' . $dbServer . $name . '.sql';
161 31
                break;
162
163
            case 'php':
164 31
                /// @todo this logic should come from the DefinitionParser, really
165
                $className = ltrim($name, '_');
166
                if ($className == '') {
167 31
                    $className = 'Migration';
168
                }
169 2
                // Make sure that php class names are unique, not only migration definition file names
170 1
                $existingMigrations = count(glob($migrationDirectory . '/*_' . $className . '*.php'));
171
                if ($existingMigrations) {
172 2
                    $className = $className . sprintf('%03d', $existingMigrations + 1);
173 2
                }
174
                $parameters = array_merge($parameters, array(
175 29
                    'class_name' => $className
176
                ));
177 2
                $fileName = $date . '_' . $className . '.php';
178 2
                break;
179 1
180
            default:
181
                if ($name == '') {
182 2
                    $name = 'placeholder';
183 2
                }
184
                $fileName = $date . '_' . $name . '.' . $fileType;
185
        }
186 2
187 2
        $path = $migrationDirectory . '/' . $fileName;
188
189 2
        $warning = $this->generateMigrationFile($path, $fileType, $migrationType, $parameters);
190 2
191
        $output->writeln(sprintf("Generated new migration file: <info>%s</info>", $path));
192
193 27
        if ($warning != '') {
194 1
            $output->writeln("<comment>$warning</comment>");
195
        }
196 27
    }
197
198
    /**
199 31
     * Generates a migration definition file.
200
     * @todo allow non-filesystem storage
201 31
     *
202
     * @param string $path filename to file to generate (full path)
203 31
     * @param string $fileType The type of migration file to generate
204
     * @param string $migrationType The type of migration to generate
205 31
     * @param array $parameters passed on to twig
206
     * @return string A warning message in case file generation was OK but there was something weird
207
     * @throws \Exception
208 31
     */
209
    protected function generateMigrationFile($path, $fileType, $migrationType, array $parameters = array())
210
    {
211
        $warning = '';
212
213
        switch ($migrationType) {
214
            case 'db':
215
            case 'generic':
216
            case 'php':
217
                // Generate migration file by template
218
                $template = $migrationType . 'Migration.' . $fileType . '.twig';
219
                $templatePath = $this->getApplication()->getKernel()->getBundle($this->thisBundle)->getPath() . '/Resources/views/MigrationTemplate/';
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Console\Application as the method getKernel() does only exist in the following sub-classes of Symfony\Component\Console\Application: Symfony\Bundle\FrameworkBundle\Console\Application. 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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
220
                if (!is_file($templatePath . $template)) {
221 31
                    throw new \Exception("The combination of migration type '$migrationType' is not supported with format '$fileType'");
222
                }
223 31
224
                $code = $this->getContainer()->get('twig')->render($this->thisBundle . ':MigrationTemplate:' . $template, $parameters);
225
                break;
226 31
227 29
            default:
228 27
                // Generate migration file by executor
229
                $executors = $this->getGeneratingExecutors();
230 6
                if (!in_array($migrationType, $executors)) {
231 6
                    throw new \Exception("It is not possible to generate a migration of type '$migrationType': executor not found or not a generator");
232 6
                }
233
                $executor = $this->getMigrationService()->getExecutor($migrationType);
234
235
                $context = $this->migrationContextFromParameters($parameters);
236 6
237 6
                $matchCondition = array($parameters['matchType'] => $parameters['matchValue']);
238
                if ($parameters['matchExcept']) {
239
                    $matchCondition = array(MatcherInterface::MATCH_NOT => $matchCondition);
240
                }
241 25
                $data = $executor->generateMigration($matchCondition, $parameters['mode'], $context);
242 25
243
                if (!is_array($data) || !count($data)) {
244
                    $warning = 'Note: the generated migration is empty';
245 25
                }
246
247 25 View Code Duplication
                switch ($fileType) {
0 ignored issues
show
Duplication introduced by
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...
248
                    case 'yml':
249 25
                        $code = Yaml::dump($data, 5);
250 25
                        break;
251 2
                    case 'json':
252
                        $code = json_encode($data, JSON_PRETTY_PRINT);
253 25
                        break;
254
                    default:
255 25
                        throw new \Exception("The combination of migration type '$migrationType' is not supported with format '$fileType'");
256
                }
257
        }
258
259
        file_put_contents($path, $code);
260 25
261 16
        return $warning;
262 16
    }
263 9
264 9
    /**
265 9
     * @param string $bundleName a bundle name or filesystem path to a directory
266
     * @return string
267
     */
268
    protected function getMigrationDirectory($bundleName)
269
    {
270
        // Allow direct usage of a directory path instead of a bundle name
271 31
        if (strpos($bundleName, '/') !== false && is_dir($bundleName)) {
272
            return rtrim($bundleName, '/');
273 31
        }
274
275
        $activeBundles = array();
276
        foreach ($this->getApplication()->getKernel()->getBundles() as $bundle) {
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Console\Application as the method getKernel() does only exist in the following sub-classes of Symfony\Component\Console\Application: Symfony\Bundle\FrameworkBundle\Console\Application. 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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
277
            $activeBundles[] = $bundle->getName();
278
        }
279
        asort($activeBundles);
280 31
        if (!in_array($bundleName, $activeBundles)) {
281
            throw new \InvalidArgumentException("Bundle '$bundleName' does not exist or it is not enabled. Try with one of:\n" . implode(', ', $activeBundles));
282
        }
283 31
284
        $bundle = $this->getApplication()->getKernel()->getBundle($bundleName);
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Console\Application as the method getKernel() does only exist in the following sub-classes of Symfony\Component\Console\Application: Symfony\Bundle\FrameworkBundle\Console\Application. 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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
285
        $migrationDirectory = $bundle->getPath() . '/' . $this->getContainer()->get('ez_migration_bundle.helper.config.resolver')->getParameter('kaliop_bundle_migration.version_directory');
286
287 31
        return $migrationDirectory;
288 31
    }
289 31
290
    /// @todo move somewhere else. Maybe to the MigrationService itself ?
291 31 View Code Duplication
    protected function getGeneratingExecutors()
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in 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...
292 31
    {
293
        $migrationService = $this->getMigrationService();
294
        $executors = $migrationService->listExecutors();
295
        foreach($executors as $key => $name) {
296 31
            $executor = $migrationService->getExecutor($name);
297 31
            if (!$executor instanceof MigrationGeneratorInterface) {
298
                unset($executors[$key]);
299 31
            }
300
        }
301
        return $executors;
302
    }
303 25
304
    /**
305 25
     * @see MigrationService::migrationContextFromParameters
306 25
     * @param array $parameters
307 25
     * @return array
308 25
     */
309 25
    protected function migrationContextFromParameters(array $parameters)
310 25
    {
311
        $context = array();
312
313 25
        if (isset($parameters['lang']) && $parameters['lang'] != '') {
314
            $context['defaultLanguageCode'] = $parameters['lang'];
315
        }
316
        if (isset($parameters['adminLogin']) && $parameters['adminLogin'] != '') {
317
            $context['adminUserLogin'] = $parameters['adminLogin'];
318
        }
319
320
        return $context;
321 25
    }
322
}
323