From 2de7cada45a6a91bc2e6d2e1f3ad5102fec9f139 Mon Sep 17 00:00:00 2001 From: AdrienClairembault Date: Fri, 3 Jun 2022 14:01:52 +0200 Subject: [PATCH 1/2] Command: fix dropped fields --- inc/fixdroppedfieldscommand.class.php | 100 +++++++++++++++++++++ inc/migration.class.php | 120 ++++++++++++++++++++++++++ 2 files changed, 220 insertions(+) create mode 100644 inc/fixdroppedfieldscommand.class.php diff --git a/inc/fixdroppedfieldscommand.class.php b/inc/fixdroppedfieldscommand.class.php new file mode 100644 index 00000000..85512979 --- /dev/null +++ b/inc/fixdroppedfieldscommand.class.php @@ -0,0 +1,100 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2013-2022 by Fields plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/fields + * ------------------------------------------------------------------------- + */ + +use Glpi\Console\AbstractCommand; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class PluginFieldsFixDroppedFieldsCommand 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->addOption( + "delete", + null, + InputOption::VALUE_NONE, + "Use this option to actually delete data" + ); + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + // Read option + $delete = $input->getOption("delete"); + + $fields = PluginFieldsMigration::fixDroppedFields(!$delete); + + // No invalid fields found + if (!count($fields)) { + $output->writeln( + __("Everything is in order - no action needed.", 'fields'), + ); + 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); + } + + $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); + + // 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" + ); + $output->writeln($next_command); + } + + return Command::SUCCESS; + } +} diff --git a/inc/migration.class.php b/inc/migration.class.php index bcf3c33d..9799452a 100644 --- a/inc/migration.class.php +++ b/inc/migration.class.php @@ -81,4 +81,124 @@ public static function getSQLFields(string $field_name, string $field_type): arr return $fields; } + + /** + * An issue affected field removal in 1.15.0, 1.15.1 and 1.15.2. + * Using these versions, removing a field from a container would drop the + * 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) + * + * @param bool $dry_run + * + * @return array + */ + public static function fixDroppedFields(bool $dry_run = true): array + { + /** @var DBMysql $DB */ + global $DB; + + // Keep track of dropped fields + $dropped = []; + + // For each existing container + foreach ((new PluginFieldsContainer())->find([]) as $row) { + // Get expected fields + $valid_fields = self::getValidFieldsForContainer($row['id']); + + // Read itemtypes and container name + $itemtypes = importArrayFromDB($row['itemtypes']); + $name = $row['name']; + + // One table to handle per itemtype + foreach ($itemtypes as $itemtype) { + // Build table name + $table = getTableForItemType("PluginFields{$itemtype}{$name}"); + + if (!$DB->tableExists($table)) { + // Missing table; skip (abnormal) + continue; + } + + // Get the actual fields defined in the container table + $found_fields = self::getCustomFieldsInContainerTable($table); + + // 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 (!$dry_run) { + $migration->migrationOneTable($table); + } + } + } + + return $dropped; + } + + /** + * Get all fields defined for a container in glpi_plugin_fields_fields + * + * @param int $container_id Id of the container + * + * @return 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 = self::getSQLFields($row['name'], $row['type']); + array_push($valid_fields, ...array_keys($fields)); + } + + return $valid_fields; + } + + /** + * Get custom fields in a given container table + * This means all fields found in the table expect those defined in + * $basic_fields + * + * @param string $table + * + * @return array + */ + private static function getCustomFieldsInContainerTable( + string $table + ): array { + /** @var DBMysql $DB */ + global $DB; + + // Read table fields + $fields = $DB->listFields($table); + + // Reduce to fields name only + $fields = array_column($fields, "Field"); + + // Remove basic fields + $basic_fields = [ + 'id', + 'items_id', + 'itemtype', + 'plugin_fields_containers_id', + ]; + return array_filter($fields, fn($f) => !in_array($f, $basic_fields)); + } } From 30361c9b825d38a096598045874e9bff8405a7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Fri, 10 Jun 2022 12:37:50 +0200 Subject: [PATCH 2/2] Enhance command output; make command more generic --- ...ass.php => checkdatabasecommand.class.php} | 65 ++++++++++--------- inc/migration.class.php | 53 ++++++++------- 2 files changed, 65 insertions(+), 53 deletions(-) rename inc/{fixdroppedfieldscommand.class.php => checkdatabasecommand.class.php} (52%) diff --git a/inc/fixdroppedfieldscommand.class.php b/inc/checkdatabasecommand.class.php similarity index 52% rename from inc/fixdroppedfieldscommand.class.php rename to inc/checkdatabasecommand.class.php index 85512979..955b4594 100644 --- a/inc/fixdroppedfieldscommand.class.php +++ b/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'), + '' . __('Everything is in order - no action needed.', 'fields') . '', ); 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 . '', 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( + '' . $next_command . '', + OutputInterface::VERBOSITY_QUIET ); - $output->writeln($next_command); } return Command::SUCCESS; diff --git a/inc/migration.class.php b/inc/migration.class.php index 9799452a..b66a4c04 100644 --- a/inc/migration.class.php +++ b/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); + } + ); } }