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 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 |
||
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('role', 'content', 'content_type', 'content_type_group', 'object_state_group', 'section', 'generic', 'db', 'php'); |
||
21 | private $thisBundle = 'EzMigrationBundle'; |
||
22 | |||
23 | /** |
||
24 | * Configure the console command |
||
25 | */ |
||
26 | 76 | protected function configure() |
|
27 | { |
||
28 | 76 | $this->setName('kaliop:migration:generate') |
|
29 | 76 | ->setDescription('Generate a blank migration definition file.') |
|
30 | 76 | ->addOption('format', null, InputOption::VALUE_REQUIRED, 'The format of migration file to generate (' . implode(', ', $this->availableMigrationFormats) . ')', 'yml') |
|
31 | 76 | ->addOption('type', null, InputOption::VALUE_REQUIRED, 'The type of migration to generate (' . implode(', ', $this->availableTypes) . ')', '') |
|
32 | 76 | ->addOption('mode', null, InputOption::VALUE_REQUIRED, 'The mode of the migration (' . implode(', ', $this->availableModes) . ')', 'create') |
|
33 | 76 | ->addOption('match-type', null, InputOption::VALUE_REQUIRED, 'The type of identifier used to find the entity to generate the migration for', null) |
|
34 | 76 | ->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 | 76 | ->addOption('match-except', null, InputOption::VALUE_NONE, 'Used to match all entities except the ones satisfying the match-value condition', null) |
|
36 | 76 | ->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 | 76 | ->addOption('dbserver', null, InputOption::VALUE_REQUIRED, 'The type of the database server the sql migration is for, when type=db (mysql, postgresql, ...)', 'mysql') |
|
38 | 76 | ->addOption('role', null, InputOption::VALUE_REQUIRED, 'Deprecated: The role identifier (or id) that you would like to update, for type=role', null) |
|
39 | 76 | ->addOption('admin-login', 'a', InputOption::VALUE_REQUIRED, "Login of admin account used whenever elevated privileges are needed (user id 14 used by default)") |
|
40 | 76 | ->addArgument('bundle', InputArgument::REQUIRED, 'The bundle to generate the migration definition file in. eg.: AcmeMigrationBundle') |
|
41 | 76 | ->addArgument('name', InputArgument::OPTIONAL, 'The migration name (will be prefixed with current date)', null) |
|
42 | 76 | ->setHelp(<<<EOT |
|
43 | The <info>kaliop:migration:generate</info> command generates a skeleton migration definition file: |
||
44 | |||
45 | <info>php ezpublish/console kaliop:migration:generate bundleName</info> |
||
46 | |||
47 | You can optionally specify the file type to generate with <info>--format</info>, as well a name for the migration: |
||
48 | |||
49 | <info>php ezpublish/console kaliop:migration:generate --format=json bundleName migrationName</info> |
||
50 | |||
51 | For SQL type migration you can optionally specify the database server type the migration is for with <info>--dbserver</info>: |
||
52 | |||
53 | <info>php ezpublish/console kaliop:migration:generate --format=sql bundleName</info> |
||
54 | |||
55 | For role/content/content_type migrations you need to specify the entity that you want to generate the migration for: |
||
56 | |||
57 | <info>php ezpublish/console kaliop:migration:generate --type=content --match-type=content_id --match-value=10,14 bundleName</info> |
||
58 | |||
59 | For role type migration you will receive a yaml file with the current role definition. You must define ALL the policies |
||
60 | you wish for the role. Any not defined will be removed. Example for updating an existing role: |
||
61 | |||
62 | <info>php ezpublish/console kaliop:migration:generate --type=role --mode=update --match-type=identifier --match-value=Anonymous bundleName</info> |
||
63 | |||
64 | For freeform php migrations, you will receive a php class definition |
||
65 | |||
66 | <info>php ezpublish/console kaliop:migration:generate --format=php bundlename classname</info> |
||
67 | |||
68 | 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> |
||
69 | option when you run the <info>migrate</info> command. |
||
70 | EOT |
||
71 | 76 | ); |
|
72 | } |
||
73 | |||
74 | /** |
||
75 | * Run the command and display the results. |
||
76 | * |
||
77 | * @param InputInterface $input |
||
78 | * @param OutputInterface $output |
||
79 | * @return null|int null or 0 if everything went fine, or an error code |
||
80 | * @throws \InvalidArgumentException When an unsupported file type is selected |
||
81 | 31 | */ |
|
82 | public function execute(InputInterface $input, OutputInterface $output) |
||
83 | 31 | { |
|
84 | 31 | $bundleName = $input->getArgument('bundle'); |
|
85 | 31 | $name = $input->getArgument('name'); |
|
86 | 31 | $fileType = $input->getOption('format'); |
|
87 | 31 | $migrationType = $input->getOption('type'); |
|
88 | 31 | $role = $input->getOption('role'); |
|
89 | 31 | $matchType = $input->getOption('match-type'); |
|
90 | 31 | $matchValue = $input->getOption('match-value'); |
|
91 | 31 | $matchExcept = $input->getOption('match-except'); |
|
92 | 31 | $mode = $input->getOption('mode'); |
|
93 | $dbServer = $input->getOption('dbserver'); |
||
94 | 31 | ||
95 | if ($role != '') { |
||
96 | $output->writeln('<error>The "role" option is deprecated since version 3.2 and will be removed in 4.0. Use "type=role", "match-type=identifier" and "match-value" instead.</error>'); |
||
97 | $migrationType = 'role'; |
||
98 | $matchType = 'identifier'; |
||
99 | $matchValue = $role; |
||
100 | if ($mode == '') { |
||
101 | $mode = 'update'; |
||
102 | } |
||
103 | } |
||
104 | 31 | ||
105 | if ($bundleName == $this->thisBundle) { |
||
106 | throw new \InvalidArgumentException("It is not allowed to create migrations in bundle '$bundleName'"); |
||
107 | } |
||
108 | |||
109 | 31 | // be kind to lazy users |
|
110 | 6 | if ($migrationType == '') { |
|
111 | 2 | if ($fileType == 'sql') { |
|
112 | 4 | $migrationType = 'db'; |
|
113 | 2 | } elseif ($fileType == 'php') { |
|
114 | $migrationType = 'php'; |
||
115 | 2 | } else { |
|
116 | $migrationType = 'generic'; |
||
117 | } |
||
118 | } |
||
119 | 31 | ||
120 | if (!in_array($fileType, $this->availableMigrationFormats)) { |
||
121 | throw new \InvalidArgumentException('Unsupported migration file format ' . $fileType); |
||
122 | } |
||
123 | 31 | ||
124 | if (!in_array($mode, $this->availableModes)) { |
||
125 | throw new \InvalidArgumentException('Unsupported migration mode ' . $mode); |
||
126 | } |
||
127 | 31 | ||
128 | $migrationDirectory = $this->getMigrationDirectory($bundleName); |
||
|
|||
129 | 31 | ||
130 | 1 | if (!is_dir($migrationDirectory)) { |
|
131 | 1 | $output->writeln(sprintf( |
|
132 | 1 | "Migrations directory <info>%s</info> does not exist. I will create it now....", |
|
133 | $migrationDirectory |
||
134 | )); |
||
135 | 1 | ||
136 | 1 | if (mkdir($migrationDirectory, self::DIR_CREATE_PERMISSIONS, true)) { |
|
137 | 1 | $output->writeln(sprintf( |
|
138 | 1 | "Migrations directory <info>%s</info> has been created", |
|
139 | $migrationDirectory |
||
140 | )); |
||
141 | } else { |
||
142 | throw new FileException(sprintf( |
||
143 | "Failed to create migrations directory %s.", |
||
144 | $migrationDirectory |
||
145 | )); |
||
146 | } |
||
147 | } |
||
148 | |||
149 | 31 | // allow to generate migrations for many entities |
|
150 | 2 | if (strpos($matchValue, ',') !== false ) { |
|
151 | $matchValue = explode(',', $matchValue); |
||
152 | } |
||
153 | |||
154 | 31 | $parameters = array( |
|
155 | 31 | 'dbserver' => $dbServer, |
|
156 | 31 | 'matchType' => $matchType, |
|
157 | 31 | 'matchValue' => $matchValue, |
|
158 | 31 | 'matchExcept' => $matchExcept, |
|
159 | 31 | 'mode' => $mode, |
|
160 | 'lang' => $input->getOption('lang'), |
||
161 | 'adminLogin' => $input->getOption('admin-login') |
||
162 | 31 | ); |
|
163 | |||
164 | $date = date('YmdHis'); |
||
165 | 31 | ||
166 | switch ($fileType) { |
||
167 | 2 | case 'sql': |
|
168 | 1 | /// @todo this logic should come from the DefinitionParser, really |
|
169 | if ($name != '') { |
||
170 | 2 | $name = '_' . ltrim($name, '_'); |
|
171 | 2 | } |
|
172 | $fileName = $date . '_' . $dbServer . $name . '.sql'; |
||
173 | 29 | break; |
|
174 | |||
175 | 2 | case 'php': |
|
176 | 2 | /// @todo this logic should come from the DefinitionParser, really |
|
177 | 1 | $className = ltrim($name, '_'); |
|
178 | if ($className == '') { |
||
179 | $className = 'Migration'; |
||
180 | 2 | } |
|
181 | 2 | // Make sure that php class names are unique, not only migration definition file names |
|
182 | $existingMigrations = count(glob($migrationDirectory . '/*_' . $className . '*.php')); |
||
183 | if ($existingMigrations) { |
||
184 | 2 | $className = $className . sprintf('%03d', $existingMigrations + 1); |
|
185 | 2 | } |
|
186 | $parameters = array_merge($parameters, array( |
||
187 | 2 | 'class_name' => $className |
|
188 | 2 | )); |
|
189 | $fileName = $date . '_' . $className . '.php'; |
||
190 | break; |
||
191 | 27 | ||
192 | 1 | default: |
|
193 | if ($name == '') { |
||
194 | 27 | $name = 'placeholder'; |
|
195 | } |
||
196 | $fileName = $date . '_' . $name . '.' . $fileType; |
||
197 | 31 | } |
|
198 | |||
199 | 31 | $path = $migrationDirectory . '/' . $fileName; |
|
200 | |||
201 | 6 | $warning = $this->generateMigrationFile($path, $fileType, $migrationType, $parameters); |
|
202 | |||
203 | 6 | $output->writeln(sprintf("Generated new migration file: <info>%s</info>", $path)); |
|
204 | |||
205 | if ($warning != '') { |
||
206 | 6 | $output->writeln("<comment>$warning</comment>"); |
|
207 | } |
||
208 | } |
||
209 | |||
210 | /** |
||
211 | * Generates a migration definition file. |
||
212 | * @todo allow non-filesystem storage |
||
213 | * |
||
214 | * @param string $path filename to file to generate (full path) |
||
215 | * @param string $fileType The type of migration file to generate |
||
216 | * @param string $migrationType The type of migration to generate |
||
217 | * @param array $parameters passed on to twig |
||
218 | 31 | * @return string A warning message in case file generation was OK but there was something weird |
|
219 | * @throws \Exception |
||
220 | 31 | */ |
|
221 | protected function generateMigrationFile($path, $fileType, $migrationType, array $parameters = array()) |
||
222 | { |
||
223 | 31 | $warning = ''; |
|
224 | 29 | ||
225 | 27 | switch ($migrationType) { |
|
226 | case 'db': |
||
227 | 6 | case 'generic': |
|
228 | 6 | case 'php': |
|
229 | 6 | // Generate migration file by template |
|
230 | $template = $migrationType . 'Migration.' . $fileType . '.twig'; |
||
231 | $templatePath = $this->getApplication()->getKernel()->getBundle($this->thisBundle)->getPath() . '/Resources/views/MigrationTemplate/'; |
||
232 | if (!is_file($templatePath . $template)) { |
||
233 | 6 | throw new \Exception("The combination of migration type '$migrationType' is not supported with format '$fileType'"); |
|
234 | 6 | } |
|
235 | |||
236 | $code = $this->getContainer()->get('twig')->render($this->thisBundle . ':MigrationTemplate:' . $template, $parameters); |
||
237 | break; |
||
238 | 25 | ||
239 | default: |
||
240 | // Generate migration file by executor |
||
241 | $executors = $this->getGeneratingExecutors(); |
||
242 | if (!in_array($migrationType, $executors)) { |
||
243 | throw new \Exception("It is not possible to generate a migration of type '$migrationType': executor not found or not a generator"); |
||
244 | } |
||
245 | $executor = $this->getMigrationService()->getExecutor($migrationType); |
||
246 | |||
247 | $context = $this->migrationContextFromParameters($parameters); |
||
248 | |||
249 | $matchCondition = array($parameters['matchType'] => $parameters['matchValue']); |
||
250 | if ($parameters['matchExcept']) { |
||
251 | $matchCondition = array(MatcherInterface::MATCH_NOT => $matchCondition); |
||
252 | } |
||
253 | $data = $executor->generateMigration($matchCondition, $parameters['mode'], $context); |
||
254 | |||
255 | if (!is_array($data) || !count($data)) { |
||
256 | $warning = 'Note: the generated migration is empty'; |
||
257 | } |
||
258 | |||
259 | View Code Duplication | switch ($fileType) { |
|
260 | case 'yml': |
||
261 | $code = Yaml::dump($data, 5); |
||
262 | break; |
||
263 | case 'json': |
||
264 | $code = json_encode($data, JSON_PRETTY_PRINT); |
||
265 | break; |
||
266 | default: |
||
267 | throw new \Exception("The combination of migration type '$migrationType' is not supported with format '$fileType'"); |
||
268 | } |
||
269 | } |
||
270 | |||
271 | 6 | file_put_contents($path, $code); |
|
272 | |||
273 | 6 | return $warning; |
|
274 | } |
||
275 | |||
276 | /** |
||
277 | * @param string $bundleName a bundle name or filesystem path to a directory |
||
278 | * @return string |
||
279 | */ |
||
280 | 31 | protected function getMigrationDirectory($bundleName) |
|
301 | |||
302 | /// @todo move somewhere else. Maybe to the MigrationService itself ? |
||
303 | 25 | View Code Duplication | protected function getGeneratingExecutors() |
315 | |||
316 | /** |
||
317 | * @see MigrationService::migrationContextFromParameters |
||
318 | * @param array $parameters |
||
319 | * @return array |
||
320 | */ |
||
321 | protected function migrationContextFromParameters(array $parameters) |
||
334 | } |
||
335 |
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:
If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.