From 995933ded2cd2d91d791bf4dfcd56acb3c7801ea Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Tue, 30 Jun 2026 17:04:20 +0200 Subject: [PATCH 01/10] fix(migration): table rename exceeds table name length limit --- inc/container.class.php | 75 ++++++++++++++++++++++----- inc/toolbox.class.php | 13 ++++- tests/Units/FieldContainerTest.php | 83 ++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 tests/Units/FieldContainerTest.php diff --git a/inc/container.class.php b/inc/container.class.php index 6f38c4dd..b1df0878 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -255,6 +255,17 @@ public static function installBaseData(Migration $migration, $version) } } + $container_obj = new self(); + foreach ($DB->request(['FROM' => $table]) as $container) { + $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + if (empty($container['itemtypes']) || self::checkContainerName($container)) { + continue; + } + + $container = $container_obj->setContainerName($container); + $container_obj->update($container); + } + return true; } @@ -703,19 +714,8 @@ public function prepareInputForAdd($input) } $input = PluginFieldsToolbox::prepareLabel($input); - - //reject adding when container name is too long for mysql table name - foreach ($input['itemtypes'] as $itemtype) { - $tmp = getTableForItemType(self::getClassname($itemtype, $input['name'])); - if (strlen($tmp) > 64) { - Session::AddMessageAfterRedirect( - __('Container name is too long for database (digits in name are replaced by characters, try to remove them)', 'fields'), - false, - ERROR, - ); - - return false; - } + if ($input === false) { + return false; } //check for already existing container with same name @@ -2416,4 +2416,53 @@ public function getCloneRelations(): array PluginFieldsProfile::class, ]; } + + public static function checkContainerName(array $container): bool + { + if (empty($container['name'])) { + return false; + } + + if (empty($container['itemtypes'])) { + return true; + } + + // Names made entirely of digits are invalid (generated prior to Fields 1.9.2). + if (preg_match('/^\d+$/', (string) $container['name'])) { + return false; + } + + if (!is_array($container['itemtypes'])) { + $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + } + + foreach ($container['itemtypes'] as $itemtype) { + if (strlen(getTableForItemType(self::getClassname($itemtype, $container['name']))) > 64) { + return false; + } + } + + return true; + } + + public function setContainerName(array $container): array + { + $base = empty($container['label']) ? $container['name'] : $container['label']; + $new_name = (new PluginFieldsToolbox())->getSystemNameFromLabel((string) $base); + + if (!is_array($container['itemtypes'])) { + $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + } + + foreach ($container['itemtypes'] as $itemtype) { + while ($new_name !== '' && strlen(getTableForItemType(self::getClassname($itemtype, $new_name))) > 64) { + $new_name = substr($new_name, 0, -1); + } + } + + $container['name'] = $new_name; + $container['label'] = $new_name; + + return $container; + } } diff --git a/inc/toolbox.class.php b/inc/toolbox.class.php index f1922a6c..fc506f60 100644 --- a/inc/toolbox.class.php +++ b/inc/toolbox.class.php @@ -396,13 +396,22 @@ public static function sanitizeLabel(string $label): string * Centralises the prepare-label pattern used in prepareInputForAdd/Update. * * @param array $input Input array with at least a 'label' key. - * @return array Updated input with sanitized 'label' and derived 'name'. + * @return array|bool Updated input with sanitized 'label' and derived 'name', or false on error. */ - public static function prepareLabel(array $input): array + public static function prepareLabel(array $input): array|bool { $input['label'] = self::sanitizeLabel((string) ($input['label'] ?? '')); $input['name'] = (new self())->getSystemNameFromLabel($input['label']); + if (!PluginFieldsContainer::checkContainerName($input)) { + Session::addMessageAfterRedirect( + __('Container name is too long for database', 'fields'), + false, + ERROR, + ); + return false; + } + return $input; } } diff --git a/tests/Units/FieldContainerTest.php b/tests/Units/FieldContainerTest.php new file mode 100644 index 00000000..341aed3c --- /dev/null +++ b/tests/Units/FieldContainerTest.php @@ -0,0 +1,83 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2013-2023 by Fields plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/fields + * ------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Field\Tests\Units; + +use Computer; +use Glpi\Tests\DbTestCase; +use GlpiPlugin\Field\Tests\FieldTestTrait; +use PHPUnit\Framework\Attributes\DataProvider; +use PluginFieldsContainer; + +require_once __DIR__ . '/../FieldTestCase.php'; + +final class FieldContainerTest extends DbTestCase +{ + use FieldTestTrait; + + public function tearDown(): void + { + $this->tearDownFieldTest(); + parent::tearDown(); + } + + public static function provideCheckContainerName(): iterable + { + yield 'empty name' => [ + false, + '', + ]; + yield 'wrong numeric name' => [ + false, + '11753069', + ]; + yield 'valid name' => [ + true, + 'testCheckContainerName', + ]; + yield 'valid numeric name' => [ + false, + 'd7', + ]; + } + + #[DataProvider('provideCheckContainerName')] + public function testCheckContainerName(bool $expected, string $label): void + { + $container = new PluginFieldsContainer(); + $input = [ + 'label' => $label, + 'itemtypes' => [Computer::class], + 'type' => 'tab', + ]; + $result = $container->add($input); + $this->assertSame($expected, $result); + } +} From 122b25cbfd7c9977aabd17bb402ff607695add2a Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Wed, 1 Jul 2026 10:12:34 +0200 Subject: [PATCH 02/10] add test --- tests/Units/ContainerTest.php | 55 ++++++++++++++++++++ tests/Units/FieldContainerTest.php | 83 ------------------------------ 2 files changed, 55 insertions(+), 83 deletions(-) delete mode 100644 tests/Units/FieldContainerTest.php diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index a7fa7098..18b636f8 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -106,4 +106,59 @@ public function testAddWithValidItemtypesSucceeds(): void $this->assertGreaterThan(0, $container->getID()); } + + /** + * Provides invalid container names for testing. + */ + public static function provideInvalidContainerName(): iterable + { + yield 'wrong numeric name' => ['11753069']; + yield 'long name' => [str_repeat('a', 256)]; + } + + /** + * Tests that adding a container with an invalid name is rejected. + */ + #[DataProvider('provideInvalidContainerName')] + public function testInvalidCheckContainerName(string $label): void + { + $container = new PluginFieldsContainer(); + $input = [ + 'label' => $label, + 'itemtypes' => [Computer::class], + 'type' => 'tab', + 'entities_id' => 0, + ]; + $result = $container->add($input); + $this->assertFalse($result); + } + + /** + * Provides valid container names for testing. + */ + public static function provideSuccessContainerName(): iterable + { + yield 'empty name' => ['']; + yield 'valid name' => ['Valid Container Name']; + yield 'another valid name' => ['111']; + yield 'numeric and letter name' => ['d7']; + } + + /** + * Tests that adding a container with a valid name succeeds. + */ + #[DataProvider('provideSuccessContainerName')] + public function testSuccessCheckContainerName(string $label): void + { + $container = $this->createFieldContainer([ + 'label' => $label . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + + $this->assertGreaterThan(0, $container->getID()); + } } diff --git a/tests/Units/FieldContainerTest.php b/tests/Units/FieldContainerTest.php deleted file mode 100644 index 341aed3c..00000000 --- a/tests/Units/FieldContainerTest.php +++ /dev/null @@ -1,83 +0,0 @@ -. - * ------------------------------------------------------------------------- - * @copyright Copyright (C) 2013-2023 by Fields plugin team. - * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html - * @link https://github.com/pluginsGLPI/fields - * ------------------------------------------------------------------------- - */ - -namespace GlpiPlugin\Field\Tests\Units; - -use Computer; -use Glpi\Tests\DbTestCase; -use GlpiPlugin\Field\Tests\FieldTestTrait; -use PHPUnit\Framework\Attributes\DataProvider; -use PluginFieldsContainer; - -require_once __DIR__ . '/../FieldTestCase.php'; - -final class FieldContainerTest extends DbTestCase -{ - use FieldTestTrait; - - public function tearDown(): void - { - $this->tearDownFieldTest(); - parent::tearDown(); - } - - public static function provideCheckContainerName(): iterable - { - yield 'empty name' => [ - false, - '', - ]; - yield 'wrong numeric name' => [ - false, - '11753069', - ]; - yield 'valid name' => [ - true, - 'testCheckContainerName', - ]; - yield 'valid numeric name' => [ - false, - 'd7', - ]; - } - - #[DataProvider('provideCheckContainerName')] - public function testCheckContainerName(bool $expected, string $label): void - { - $container = new PluginFieldsContainer(); - $input = [ - 'label' => $label, - 'itemtypes' => [Computer::class], - 'type' => 'tab', - ]; - $result = $container->add($input); - $this->assertSame($expected, $result); - } -} From 33ff918ffc96a510b817355d79eef8864bafee7d Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Wed, 1 Jul 2026 10:24:39 +0200 Subject: [PATCH 03/10] generate template --- inc/container.class.php | 1 + 1 file changed, 1 insertion(+) diff --git a/inc/container.class.php b/inc/container.class.php index b1df0878..22090c2c 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -257,6 +257,7 @@ public static function installBaseData(Migration $migration, $version) $container_obj = new self(); foreach ($DB->request(['FROM' => $table]) as $container) { + self::generateTemplate($container); $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); if (empty($container['itemtypes']) || self::checkContainerName($container)) { continue; From f2b29354d76a4ba79d0f1b301df5963654133bca Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Wed, 1 Jul 2026 11:07:01 +0200 Subject: [PATCH 04/10] fix --- inc/container.class.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/inc/container.class.php b/inc/container.class.php index 22090c2c..d89c96b0 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -258,8 +258,8 @@ public static function installBaseData(Migration $migration, $version) $container_obj = new self(); foreach ($DB->request(['FROM' => $table]) as $container) { self::generateTemplate($container); - $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); - if (empty($container['itemtypes']) || self::checkContainerName($container)) { + $itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + if (empty($itemtypes) || self::checkContainerName($container)) { continue; } @@ -2433,11 +2433,12 @@ public static function checkContainerName(array $container): bool return false; } - if (!is_array($container['itemtypes'])) { - $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + $itemtypes = $container['itemtypes']; + if (is_string($itemtypes)) { + $itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($itemtypes); } - foreach ($container['itemtypes'] as $itemtype) { + foreach ($itemtypes as $itemtype) { if (strlen(getTableForItemType(self::getClassname($itemtype, $container['name']))) > 64) { return false; } @@ -2450,12 +2451,12 @@ public function setContainerName(array $container): array { $base = empty($container['label']) ? $container['name'] : $container['label']; $new_name = (new PluginFieldsToolbox())->getSystemNameFromLabel((string) $base); - - if (!is_array($container['itemtypes'])) { - $container['itemtypes'] = PluginFieldsToolbox::decodeJSONItemtypes($container['itemtypes']); + $itemtypes = $container['itemtypes']; + if (is_string($itemtypes)) { + $itemtypes = PluginFieldsToolbox::decodeJSONItemtypes($itemtypes); } - foreach ($container['itemtypes'] as $itemtype) { + foreach ($itemtypes as $itemtype) { while ($new_name !== '' && strlen(getTableForItemType(self::getClassname($itemtype, $new_name))) > 64) { $new_name = substr($new_name, 0, -1); } From e73e65505ee1327c83a40c185c7f787fcf040509 Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Wed, 1 Jul 2026 15:24:21 +0200 Subject: [PATCH 05/10] improve test --- inc/container.class.php | 8 ++++++-- tests/Units/ContainerTest.php | 26 +++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/inc/container.class.php b/inc/container.class.php index d89c96b0..93e99721 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -660,6 +660,10 @@ public function defineTabs($options = []) public function prepareInputForUpdate($input) { + if (!isset($input['itemtypes'])) { + $input['itemtypes'] = $this->fields['itemtypes']; + } + return PluginFieldsToolbox::prepareLabel($input); } @@ -2420,11 +2424,11 @@ public function getCloneRelations(): array public static function checkContainerName(array $container): bool { - if (empty($container['name'])) { + if (!isset($container['name']) || empty($container['name'])) { return false; } - if (empty($container['itemtypes'])) { + if (!isset($container['itemtypes']) || empty($container['itemtypes'])) { return true; } diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index 18b636f8..b6ba50fe 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -112,7 +112,7 @@ public function testAddWithValidItemtypesSucceeds(): void */ public static function provideInvalidContainerName(): iterable { - yield 'wrong numeric name' => ['11753069']; + yield 'wrong numeric name' => ['7777777777']; yield 'long name' => [str_repeat('a', 256)]; } @@ -131,6 +131,17 @@ public function testInvalidCheckContainerName(string $label): void ]; $result = $container->add($input); $this->assertFalse($result); + + $container = $this->createFieldContainer([ + 'label' => 'container ' . $this->getUniqueString(), + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $result = $container->update(['id' => $container->getID(), 'label' => $label]); + $this->assertFalse($result); } /** @@ -160,5 +171,18 @@ public function testSuccessCheckContainerName(string $label): void ]); $this->assertGreaterThan(0, $container->getID()); + $container = $this->createFieldContainer([ + 'label' => 'testSuccessCheckContainerName', + 'type' => 'tab', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $update = $container->update([ + 'id' => $container->getID(), + 'label' => $label . $this->getUniqueString(), + ]); + $this->assertTrue($update); } } From 74d7be5b688176d90dd5d7f8a70e41296c136341 Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Wed, 1 Jul 2026 15:29:35 +0200 Subject: [PATCH 06/10] Update CHANGELOG.md --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f81f8269..93df674f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [unreleased] +## [Unreleased] +### Fixed + +- Fixed table renaming during migration when tables with numeric names exceeded the maximum table name length. ## [1.24.2] - 2026-06-30 From 45d3e4e8dc84c717da9b6da7f824e235ab007a5b Mon Sep 17 00:00:00 2001 From: Stanislas Date: Fri, 3 Jul 2026 10:32:20 +0200 Subject: [PATCH 07/10] Update inc/container.class.php --- inc/container.class.php | 1 - 1 file changed, 1 deletion(-) diff --git a/inc/container.class.php b/inc/container.class.php index 93e99721..05d6fe60 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -2467,7 +2467,6 @@ public function setContainerName(array $container): array } $container['name'] = $new_name; - $container['label'] = $new_name; return $container; } From e924581a3f0c268d216bcdfca153f2d2fc1d696b Mon Sep 17 00:00:00 2001 From: Stanislas Date: Fri, 3 Jul 2026 10:32:26 +0200 Subject: [PATCH 08/10] Update inc/container.class.php --- inc/container.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/container.class.php b/inc/container.class.php index 05d6fe60..b1ec4303 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -264,7 +264,7 @@ public static function installBaseData(Migration $migration, $version) } $container = $container_obj->setContainerName($container); - $container_obj->update($container); + return $container_obj->update($container); } return true; From 5d58c7e99b26694c3412985f24386078a82edd6e Mon Sep 17 00:00:00 2001 From: Stanislas Date: Fri, 3 Jul 2026 10:32:32 +0200 Subject: [PATCH 09/10] Update inc/toolbox.class.php --- inc/toolbox.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/toolbox.class.php b/inc/toolbox.class.php index fc506f60..d5edfa4e 100644 --- a/inc/toolbox.class.php +++ b/inc/toolbox.class.php @@ -405,7 +405,7 @@ public static function prepareLabel(array $input): array|bool if (!PluginFieldsContainer::checkContainerName($input)) { Session::addMessageAfterRedirect( - __('Container name is too long for database', 'fields'), + __('Container name is invalid or too long for database', 'fields'), false, ERROR, ); From 375f4f2a1f0485ddf88d1947d968cfe4a529a114 Mon Sep 17 00:00:00 2001 From: Stanislas Date: Fri, 3 Jul 2026 10:39:28 +0200 Subject: [PATCH 10/10] Update tests/Units/ContainerTest.php --- tests/Units/ContainerTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index b6ba50fe..610fceab 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -184,5 +184,11 @@ public function testSuccessCheckContainerName(string $label): void 'label' => $label . $this->getUniqueString(), ]); $this->assertTrue($update); + // remove the container because tearsdown is only triggered when another test is run, + // and this test is the last one to be run + $delete = $container->delete([ + 'id' => $container->getID(), + ], true); + $this->assertTrue($delete); } }