Enhance command output; make command more generic

Author: cedric-anne

inc/fixdroppedfieldscommand.class.php inc/checkdatabasecommand.class.php

@@ -34,65 +34,72 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
-class PluginFieldsFixDroppedFieldsCommand extends AbstractCommand
+class PluginFieldsCheckDatabaseCommand extends AbstractCommand
 {
     protected function configure()
     {
-        $this->setName('plugin:fields:fixdroppedfields');
-        $this->setAliases(['fields:fixdroppedfields']);
-        $this->setDescription(
-            'Remove fields that were wrongly kept in the database following an '
-            . 'issue introduced in 1.15.0 and fixed in 1.15.3.'
+        $this->setName('plugin:fields:check_database');
+        $this->setDescription(__('Check database to detect inconsistencies.', 'fields'));
+        $this->setHelp(
+            __('This command will chec database to detect following inconsistencies:', 'fields')
+            . "\n"
+            . sprintf(
+                __('- some deleted fields may still be present in database (bug introduced in %s and fixed in version %s)', 'fields'),
+                '1.15.0',
+                '1.15.3'
+            )
         );
 
         $this->addOption(
-            "delete",
+            'fix',
             null,
             InputOption::VALUE_NONE,
-            "Use this option to actually delete data"
+            __('Use this option to actually fix database', 'fields')
         );
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
         // Read option
-        $delete = $input->getOption("delete");
+        $fix = $input->getOption('fix');
 
-        $fields = PluginFieldsMigration::fixDroppedFields(!$delete);
+        $dead_fields = PluginFieldsMigration::checkDeadFields($fix);
+        $dead_fields_count = count($dead_fields, COUNT_RECURSIVE) - count($dead_fields);
 
         // No invalid fields found
-        if (!count($fields)) {
+        if ($dead_fields_count === 0) {
             $output->writeln(
-                __("Everything is in order - no action needed.", 'fields'),
+                '<info>' . __('Everything is in order - no action needed.', 'fields') . '</info>',
             );
             return Command::SUCCESS;
         }
 
         // Indicate which fields will have been or must be deleted
-        foreach ($fields as $field) {
-            if ($delete) {
-                $info = sprintf(__("-> %s was deleted.", 'fields'), $field);
-            } else {
-                $info = sprintf(__("-> %s must be deleted.", 'fields'), $field);
-            }
+        $error = $fix
+            ? sprintf(__('Database was containing %s gone field(s).', 'fields'), $dead_fields_count)
+            : sprintf(__('Database contains %s gone field(s).', 'fields'), $dead_fields_count);
+        $output->writeln('<error>' . $error . '</error>', OutputInterface::VERBOSITY_QUIET);
 
-            $output->writeln($info);
+        foreach ($dead_fields as $table => $fields) {
+            foreach ($fields as $field) {
+                $info = $fix
+                    ? sprintf(__('-> "%s.%s" has been deleted.', 'fields'), $table, $field)
+                    : sprintf(__('-> "%s.%s" should be deleted.', 'fields'), $table, $field);
+                $output->writeln($info);
+            }
         }
 
         // Show extra info in dry-run mode
-        if (!$delete) {
-            $fields_found = sprintf(
-                __("%s field(s) need to be deleted.", 'fields'),
-                count($fields)
-            );
-            $output->writeln($fields_found);
-
+        if (!$fix) {
             // Print command to do the actual deletion
             $next_command = sprintf(
-                __("Run \"%s\" to delete the found field(s).", 'fields'),
-                "php bin/console plugin:fields:fixdroppedfields --delete"
+                __('Run "%s" to delete the found field(s).', 'fields'),
+                sprintf("php bin/console %s --fix", $this->getName())
+            );
+            $output->writeln(
+                '<comment>' . $next_command . '</comment>',
+                OutputInterface::VERBOSITY_QUIET
             );
-            $output->writeln($next_command);
         }
 
         return Command::SUCCESS;

inc/migration.class.php

@@ -88,24 +88,24 @@ public static function getSQLFields(string $field_name, string $field_type): arr
      * field from glpi_plugin_fields_fields but not from the custom container
      * table
      *
-     * This function find looks into containers tables for these fields that
-     * should have been removed and list them (dry_run = true) or delete them
-     * (dry_run = false)
+     * This function looks into containers tables for fields that
+     * should have been removed and list them.
+     * If parameter $fix is true, fields are deleted from database.
      *
-     * @param bool $dry_run
+     * @param bool $fix
      *
      * @return array
      */
-    public static function fixDroppedFields(bool $dry_run = true): array
+    public static function checkDeadFields(bool $fix): array
     {
         /** @var DBMysql $DB */
         global $DB;
 
-        // Keep track of dropped fields
-        $dropped = [];
+        $dead_fields = [];
 
         // For each existing container
-        foreach ((new PluginFieldsContainer())->find([]) as $row) {
+        $containers = (new PluginFieldsContainer())->find([]);
+        foreach ($containers as $row) {
             // Get expected fields
             $valid_fields = self::getValidFieldsForContainer($row['id']);
 
@@ -129,21 +129,25 @@ public static function fixDroppedFields(bool $dry_run = true): array
                 // Compute which fields should be removed
                 $fields_to_drop = array_diff($found_fields, $valid_fields);
 
-                // Drop fields
-                $migration = new PluginFieldsMigration(0);
-
-                foreach ($fields_to_drop as $field) {
-                    $dropped[] = "$table.$field";
-                    $migration->dropField($table, $field);
+                if (count($fields_to_drop) > 0) {
+                    $dead_fields[$table] = $fields_to_drop;
                 }
+            }
+        }
 
-                if (!$dry_run) {
-                    $migration->migrationOneTable($table);
+        if ($fix) {
+            $migration = new PluginFieldsMigration(0);
+
+            foreach ($dead_fields as $table => $fields) {
+                foreach ($fields as $field) {
+                    $migration->dropField($table, $field);
                 }
             }
+
+            $migration->executeMigration();
         }
 
-        return $dropped;
+        return $dead_fields;
     }
 
     /**
@@ -155,15 +159,11 @@ public static function fixDroppedFields(bool $dry_run = true): array
      */
     private static function getValidFieldsForContainer(int $container_id): array
     {
-        // Keep track of fields found
         $valid_fields = [];
 
         // For each defined fields in the given container
-        foreach (
-            (new PluginFieldsField())->find([
-                'plugin_fields_containers_id' => $container_id
-            ]) as $row
-        ) {
+        $fields = (new PluginFieldsField())->find(['plugin_fields_containers_id' => $container_id]);
+        foreach ($fields as $row) {
             $fields = self::getSQLFields($row['name'], $row['type']);
             array_push($valid_fields, ...array_keys($fields));
         }
@@ -199,6 +199,11 @@ private static function getCustomFieldsInContainerTable(
             'itemtype',
             'plugin_fields_containers_id',
         ];
-        return array_filter($fields, fn($f) => !in_array($f, $basic_fields));
+        return array_filter(
+            $fields,
+            function (string $field) use ($basic_fields) {
+                return !in_array($field, $basic_fields);
+            }
+        );
     }
 }