From 2de7cada45a6a91bc2e6d2e1f3ad5102fec9f139 Mon Sep 17 00:00:00 2001
From: AdrienClairembault <aclairembault@teclib.com>
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 @@
+<?php
+
+/**
+ * -------------------------------------------------------------------------
+ * Fields plugin for GLPI
+ * -------------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of Fields.
+ *
+ * Fields is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Fields is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Fields. If not, see <http://www.gnu.org/licenses/>.
+ * -------------------------------------------------------------------------
+ * @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?= <cedric.anne@gmail.com>
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'),
+ '<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;
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);
+ }
+ );
}
}