From ed27498a22b879b624c04cc164bb40841bcc8f6a Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 26 Oct 2026 17:30:16 +0100 Subject: [PATCH 1/3] Feat - Inject Generic Asset --- CHANGELOG.md | 1 + inc/commoninjectionlib.class.php | 46 +++- inc/info.class.php | 2 +- inc/injectiontype.class.php | 8 + phpstan.neon | 1 + psalm.xml | 1 + rector.php | 1 + src/AssetInjection.php | 209 +++++++++++++++++++ tests/unit/AssetInjectionTest.php | 335 ++++++++++++++++++++++++++++++ 9 files changed, 595 insertions(+), 9 deletions(-) create mode 100644 src/AssetInjection.php create mode 100644 tests/unit/AssetInjectionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 46dc0a19..7d4d61d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Add Transifex translations link to README +- Inject generic assets, including their type, model, comment and custom fields ## [2.16.0] - 2026-10-05 diff --git a/inc/commoninjectionlib.class.php b/inc/commoninjectionlib.class.php index 97d6ceb9..198b286c 100644 --- a/inc/commoninjectionlib.class.php +++ b/inc/commoninjectionlib.class.php @@ -32,6 +32,8 @@ use Glpi\DBAL\QuerySubQuery; use Glpi\Exception\Http\HttpException; use Glpi\Features\AssignableItem; +use Glpi\Asset\Asset; +use GlpiPlugin\Datainjection\AssetInjection; use function Safe\preg_match; use function Safe\preg_replace; @@ -316,7 +318,10 @@ public static function isFieldADropdown($field_type) public static function getItemtypeInstanceByInjection($injectionClassName) { - if (!is_a($injectionClassName, PluginDatainjectionInjectionInterface::class, true)) { + if ( + !is_a($injectionClassName, PluginDatainjectionInjectionInterface::class, true) + || is_a($injectionClassName, AssetInjection::class, true) + ) { throw new HttpException(500, 'Class ' . $injectionClassName . ' is not a valid class'); } @@ -338,7 +343,8 @@ public static function getItemtypeInstanceByInjection($injectionClassName) */ public static function getItemtypeByInjection($injectionClassName) { - if (!is_a($injectionClassName, CommonDBTM::class, true)) { + // A generic asset injection class needs its itemtype: it cannot be instanciated from its name + if (!is_a($injectionClassName, CommonDBTM::class, true) || is_a($injectionClassName, AssetInjection::class, true)) { throw new HttpException(500, 'Class ' . $injectionClassName . ' is not a valid class'); } @@ -355,6 +361,10 @@ public static function getItemtypeByInjection($injectionClassName) */ public static function getItemtypeByInjectionClass($injectionClass) { + // All generic assets share the same table + if ($injectionClass instanceof AssetInjection) { + return $injectionClass->itemtype; + } return Toolbox::ucfirst(getItemTypeForTable($injectionClass->getTable())); } @@ -369,6 +379,9 @@ public static function getItemtypeByInjectionClass($injectionClass) */ public static function getInjectionClassInstance($itemtype) { + if (is_a($itemtype, Asset::class, true)) { + return AssetInjection::forItemtype($itemtype); + } if (!isPluginItemType($itemtype)) { $itemtype = (new ReflectionClass($itemtype))->getShortName(); // get shortname of class when full namespaced class given (from GLPI) @@ -527,8 +540,12 @@ private function getFloatFormat() private function getItemInstance() { - $classname = $this->injectionClass::class; - return self::getItemtypeInstanceByInjection($classname); + $itemtype = $this->primary_type; + if (!is_a($itemtype, CommonDBTM::class, true)) { + throw new HttpException(500, 'Class ' . $itemtype . ' is not a valid class'); + } + + return new $itemtype(); } @@ -626,7 +643,8 @@ private function getFieldValue( case 'dropdown': case 'relation': - $tmptype = getItemTypeForTable($searchOption['table']); + // Generic assets types and models, or custom dropdowns, share their table with other itemtypes + $tmptype = $searchOption['itemtype'] ?? getItemTypeForTable($searchOption['table']); if (!is_a($tmptype, CommonDBTM::class, true)) { return; } @@ -900,6 +918,7 @@ private function findSingle($item, $searchOption, $entity, $value) } $where[$searchOption['field']] = $value; + $where += $item::getSystemSQLCriteria(); $result = $DB->request([ 'SELECT' => ['id'], @@ -1010,7 +1029,7 @@ private function setValueForItemtype($itemtype, $field, $value, $fromdb = false) 'is_active', ]; if (empty($value) && $value !== 0 && $value !== '0') { - if (isForeignKeyField($field) || (str_contains($field, 'is_')) || (method_exists($injectionClass, 'isNullable') && !$injectionClass->isNullable($field))) { + if (isForeignKeyField($field) || (str_starts_with($field, 'is_')) || (method_exists($injectionClass, 'isNullable') && !$injectionClass->isNullable($field))) { //If the field concernes groupds, unseting it instead of setting it to 0 in order to avoid associating the item to a non existing group (id 0) $group_fields = ['groups_id_tech', 'groups_id', 'groups_id_normal']; if (in_array($field, $group_fields) && Toolbox::hasTrait($itemtype, AssignableItem::class)) { @@ -1053,7 +1072,7 @@ private function getTemplateIDByName($itemtype, $name) 'WHERE' => [ 'is_template' => 1, 'template_name' => $name, - ], + ] + $itemtype::getSystemSQLCriteria(), ]); if (count($result) > 0) { @@ -2128,6 +2147,11 @@ private function dataAlreadyInDB($injectionClass, $itemtype) 'FROM' => 'glpi_useremails', 'WHERE' => ['email' => $this->getValueByItemtypeAndName($itemtype, $field)], ]); + } elseif ( + $injectionClass instanceof AssetInjection + && ($criterion = $injectionClass->getCustomFieldCriterion($field, $this->getValueByItemtypeAndName($itemtype, $field))) instanceof QueryExpression + ) { + $where[] = $criterion; } else { $where[$field] = $this->getValueByItemtypeAndName($itemtype, $field); } @@ -2165,6 +2189,9 @@ private function dataAlreadyInDB($injectionClass, $itemtype) } } + // Restrict to the items of the itemtype when its table is shared (generic assets) + $where += $item::getSystemSQLCriteria(); + $result = $DB->request([ 'FROM' => $injectionClass->getTable(), 'WHERE' => $where, @@ -2447,8 +2474,11 @@ public static function addToSearchOptions( ); $main_itemtype = self::getItemtypeByInjectionClass($injectionClass); + $main_table = getTableForItemType($main_itemtype); foreach ($type_searchOptions as &$value) { - $value_itemtype = getItemTypeForTable($value['table']); + $value_itemtype = $value['table'] === $main_table + ? $main_itemtype + : $value['itemtype'] ?? getItemTypeForTable($value['table']); $value_itemtype_name_singular = $value_itemtype::getTypeName(1); $value_itemtype_name_plural = $value_itemtype::getTypeName(Session::getPluralNumber()); if ( diff --git a/inc/info.class.php b/inc/info.class.php index 4527d57e..f44e8d62 100644 --- a/inc/info.class.php +++ b/inc/info.class.php @@ -359,7 +359,7 @@ public static function showAdditionalInformation( } Dropdown::show( - getItemTypeForTable($option['table']), + $option['itemtype'] ?? getItemTypeForTable($option['table']), ['name' => $name, 'value' => $value, ], diff --git a/inc/injectiontype.class.php b/inc/injectiontype.class.php index 8da2105e..3b7b1095 100644 --- a/inc/injectiontype.class.php +++ b/inc/injectiontype.class.php @@ -28,6 +28,7 @@ * ------------------------------------------------------------------------- */ use Glpi\Exception\Http\HttpException; +use Glpi\Asset\AssetDefinitionManager; use function Safe\json_decode; use function Safe\json_encode; @@ -85,6 +86,13 @@ public static function getItemtypes($only_primary = false) } } + foreach (AssetDefinitionManager::getInstance()->getDefinitions(true) as $definition) { + $itemtype = $definition->getAssetClassName(); + if ($itemtype::canCreate()) { + $values[$itemtype] = $definition->getTranslatedName(); + } + } + asort($values); return $values; } diff --git a/phpstan.neon b/phpstan.neon index db69ea19..275425e8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -9,6 +9,7 @@ parameters: - ajax - front - inc + - src - hook.php - setup.php scanDirectories: diff --git a/psalm.xml b/psalm.xml index a482e8d3..7164584e 100644 --- a/psalm.xml +++ b/psalm.xml @@ -7,6 +7,7 @@ + diff --git a/rector.php b/rector.php index efe32495..d081ce84 100644 --- a/rector.php +++ b/rector.php @@ -49,6 +49,7 @@ __DIR__ . '/ajax', __DIR__ . '/front', __DIR__ . '/inc', + __DIR__ . '/src', __DIR__ . '/tests', ]); diff --git a/src/AssetInjection.php b/src/AssetInjection.php new file mode 100644 index 00000000..87e19bb4 --- /dev/null +++ b/src/AssetInjection.php @@ -0,0 +1,209 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2007-2023 by DataInjection plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/datainjection + * ------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Datainjection; + +use CommonDBTM; +use Glpi\Asset\Asset; +use Glpi\Asset\CustomFieldDefinition; +use Glpi\DBAL\QueryExpression; +use Glpi\DBAL\QueryFunction; +use Glpi\DBAL\QueryIdentifier; +use PluginDatainjectionCommonInjectionLib; +use PluginDatainjectionInjectionInterface; +use Search; + +/** + * Injection of the generic assets (GLPI custom assets). + * + * Their classes are final and share the same table: the injected itemtype is given + * to the instance instead of being the parent class. + */ +final class AssetInjection extends CommonDBTM implements PluginDatainjectionInjectionInterface +{ + /** @var class-string */ + public string $itemtype; + + /** + * @param class-string $itemtype + */ + public static function forItemtype(string $itemtype): self + { + $injection = new self(); + $injection->itemtype = $itemtype; + + return $injection; + } + + public static function getTable($classname = null) + { + return Asset::getTable(); + } + + public function isPrimaryType() + { + return true; + } + + public function connectedTo() + { + return []; + } + + public function isNullable($field) + { + return true; // By default, all fields can be null + } + + /** + * @see plugins/datainjection/inc/PluginDatainjectionInjectionInterface::getOptions() + **/ + public function getOptions($primary_type = '') + { + $tab = Search::getOptions($this->itemtype); + + //Specific to groups (simple group, not technical) + $tab[71]['linkfield'] = 'groups_id_normal'; + + //Remove some options because some fields cannot be imported + $blacklist = PluginDatainjectionCommonInjectionLib::getBlacklistedOptions($this->itemtype); + $notimportable = [ + 91, 92, 93, 101, 102, 103, 104, 105, 106, 107, 998, 999, // location fields + 250, // asset definition, set by the asset itself + ]; + + $options['ignore_fields'] = array_merge($blacklist, $notimportable); + $options['displaytype'] = [ + 'user' => [24, 70], + 'multiline_text' => [16], + ]; + + $tab = PluginDatainjectionCommonInjectionLib::addToSearchOptions($tab, $options, $this); + + // Capacities add the fields of linked items (operating system, volumes...), which are injected with their own type + $tab = array_filter( + $tab, + fn(array $option) => $this->isField($option['linkfield']) + || in_array($option['linkfield'], ['groups_id', 'groups_id_normal', 'templates_id'], true), + ); + + // Added after the generic filter, which keeps only the core options (id < 1000) + foreach ($this->itemtype::getDefinition()->getCustomFieldDefinitions() as $custom_field) { + $option = $this->getCustomFieldOption($custom_field); + if ($option !== null) { + $tab[$option['id']] = $option; + } + } + + return $tab; + } + + /** + * The core search option of a custom field cannot be injected as is: its linkfield is the + * JSON column shared by all custom fields. Values are injected as "custom_", + * which the asset stores into this column. + * + * @return array|null null if the field cannot be injected + */ + private function getCustomFieldOption(CustomFieldDefinition $custom_field): ?array + { + $core_option = $custom_field->getFieldType()->getSearchOption(); + $types = [ + // datatype => [displaytype, checktype] + 'string' => ['text', 'text'], + 'text' => ['multiline_text', 'text'], + 'number' => ['decimal', 'float'], + 'bool' => ['bool', 'bool'], + 'date' => ['text', 'date'], + 'datetime' => ['text', 'datetime'], + 'itemlink' => ['dropdown', 'text'], + ]; + if ( + $core_option === null + || !isset($types[$core_option['datatype']]) + // Values of a multiple dropdown cannot be told apart in a single CSV cell + || ($custom_field->fields['field_options']['multiple'] ?? false) + || $custom_field->getFieldType()->getOptionValues()['hidden'] + ) { + return null; + } + + $linkfield = 'custom_' . $custom_field->fields['system_name']; + [$displaytype, $checktype] = $types[$core_option['datatype']]; + + return [ + 'id' => $core_option['id'], + 'name' => $custom_field->computeFriendlyName(), + 'table' => $core_option['table'], + // A dropdown is searched by its name, other values are stored as is + 'field' => $displaytype === 'dropdown' ? $core_option['field'] : $linkfield, + 'linkfield' => $linkfield, + // Needed for dropdowns: custom dropdowns share a table, which cannot give back their itemtype + 'itemtype' => $core_option['itemtype'] ?? $this->itemtype, + 'datatype' => $core_option['datatype'], + 'displaytype' => $displaytype, + 'checktype' => $checktype, + 'injectable' => PluginDatainjectionCommonInjectionLib::FIELD_INJECTABLE, + ]; + } + + /** + * Criterion to find an asset by a custom field, whose value is stored in a JSON column + * + * @return QueryExpression|null null if the field is not a custom field + */ + public function getCustomFieldCriterion(string $field, mixed $value): ?QueryExpression + { + global $DB; + + foreach ($this->itemtype::getDefinition()->getCustomFieldDefinitions() as $custom_field) { + if ('custom_' . $custom_field->fields['system_name'] === $field) { + return new QueryExpression( + QueryFunction::jsonUnquote(QueryFunction::jsonExtract([ + new QueryIdentifier('custom_fields'), + new QueryExpression($DB::quoteValue('$."' . $custom_field->getID() . '"')), + ])) . ' = ' . $DB::quoteValue((string) $value), + ); + } + } + + return null; + } + + /** + * @see plugins/datainjection/inc/PluginDatainjectionInjectionInterface::addOrUpdateObject() + **/ + public function addOrUpdateObject($values = [], $options = []) + { + $lib = new PluginDatainjectionCommonInjectionLib($this, $values, $options); + $lib->processAddOrUpdate(); + return $lib->getInjectionResults(); + } +} diff --git a/tests/unit/AssetInjectionTest.php b/tests/unit/AssetInjectionTest.php new file mode 100644 index 00000000..ee48d9a0 --- /dev/null +++ b/tests/unit/AssetInjectionTest.php @@ -0,0 +1,335 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2007-2023 by DataInjection plugin team. + * @license GPLv2 https://www.gnu.org/licenses/gpl-2.0.html + * @link https://github.com/pluginsGLPI/datainjection + * ------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Datainjection\Tests\Unit; + +use Glpi\Asset\Asset; +use Glpi\Asset\AssetDefinition; +use Glpi\Asset\Capacity; +use Glpi\Asset\Capacity\HasAntivirusCapacity; +use Glpi\Asset\Capacity\HasInfocomCapacity; +use Glpi\Asset\Capacity\HasOperatingSystemCapacity; +use Glpi\Asset\Capacity\HasSoftwaresCapacity; +use Glpi\Asset\Capacity\HasVolumesCapacity; +use Glpi\Asset\CustomFieldDefinition; +use Glpi\Asset\CustomFieldType\BooleanType; +use Glpi\Asset\CustomFieldType\DateTimeType; +use Glpi\Asset\CustomFieldType\DateType; +use Glpi\Asset\CustomFieldType\DropdownType; +use Glpi\Asset\CustomFieldType\NumberType; +use Glpi\Asset\CustomFieldType\StringType; +use Glpi\Asset\CustomFieldType\TextType; +use Glpi\Exception\Http\HttpException; +use Glpi\Tests\DbTestCase; +use GlpiPlugin\Datainjection\AssetInjection; +use Group; +use Infocom; +use Item_OperatingSystem; +use Location; +use Manufacturer; +use OperatingSystem; +use PluginDatainjectionCommonInjectionLib; +use PluginDatainjectionInjectionType; + +final class AssetInjectionTest extends DbTestCase +{ + public function testAssetDefinitionIsInjectable(): void + { + $definition = $this->initAssetDefinition(); + $this->login(); // load the rights of the new definition + + $itemtype = $definition->getAssetClassName(); + self::assertSame($definition->getTranslatedName(), PluginDatainjectionInjectionType::getItemtypes(true)[$itemtype] ?? null); + + $injection = PluginDatainjectionCommonInjectionLib::getInjectionClassInstance($itemtype); + self::assertInstanceOf(AssetInjection::class, $injection); + self::assertSame($itemtype, PluginDatainjectionCommonInjectionLib::getItemtypeByInjectionClass($injection)); + } + + public function testAssetDefinitionWithoutRightIsNotInjectable(): void + { + $definition = $this->initAssetDefinition(profiles: []); + $this->login(); + + self::assertArrayNotHasKey($definition->getAssetClassName(), PluginDatainjectionInjectionType::getItemtypes(true)); + } + + public function testOptions(): void + { + $definition = $this->initAssetDefinition(capacities: [ + new Capacity(name: HasOperatingSystemCapacity::class), + new Capacity(name: HasVolumesCapacity::class), + new Capacity(name: HasSoftwaresCapacity::class), + new Capacity(name: HasAntivirusCapacity::class), + ]); + $this->createCustomField($definition, 'reference', StringType::class); + $this->createCustomField($definition, 'tags', DropdownType::class, Location::class, ['multiple' => true]); + $this->createCustomField($definition, 'secret', StringType::class, field_options: ['hidden' => [-1]]); + $this->login(); + + $options = $this->getInjection($definition)->getOptions(); + $by_linkfield = array_column($options, null, 'linkfield'); + + foreach (['name', 'serial', 'otherserial', 'locations_id', 'manufacturers_id', 'states_id', 'custom_reference'] as $linkfield) { + self::assertArrayHasKey($linkfield, $by_linkfield); + } + + self::assertSame('dropdown', $by_linkfield['assets_assettypes_id']['displaytype']); + self::assertSame('dropdown', $by_linkfield['assets_assetmodels_id']['displaytype']); + self::assertSame('multiline_text', $by_linkfield['comment']['displaytype']); + self::assertSame('user', $by_linkfield['users_id']['displaytype']); + self::assertArrayHasKey('groups_id_normal', $by_linkfield); + self::assertArrayHasKey('groups_id', $by_linkfield); + + // Each option is stored in the asset itself, linked items have their own injection type + foreach (array_keys($by_linkfield) as $linkfield) { + self::assertTrue( + $this->getInjection($definition)->isField($linkfield) + || in_array($linkfield, ['groups_id', 'groups_id_normal', 'templates_id', 'custom_reference'], true), + "{$linkfield} is not stored in the asset", + ); + } + + self::assertArrayNotHasKey('assets_assetdefinitions_id', $by_linkfield); + self::assertArrayNotHasKey('custom_fields', $by_linkfield); + self::assertArrayNotHasKey('custom_tags', $by_linkfield); + self::assertArrayNotHasKey('custom_secret', $by_linkfield); + } + + public function testInjectAsset(): void + { + $definition = $this->initAssetDefinition(); + $dropdown_definition = $this->initDropdownDefinition(); + foreach ( + [ + 'reference' => StringType::class, + 'notes' => TextType::class, + 'quantity' => NumberType::class, + 'delivery' => DateType::class, + 'audited' => DateTimeType::class, + 'is_spare' => BooleanType::class, + ] as $system_name => $type + ) { + $this->createCustomField($definition, $system_name, $type); + } + + $this->createCustomField($definition, 'room', DropdownType::class, Location::class); + $this->createCustomField($definition, 'color', DropdownType::class, $dropdown_definition->getDropdownClassName()); + $group = $this->createItem(Group::class, ['name' => 'Users group', 'entities_id' => 0, 'is_recursive' => 1]); + $tech_group = $this->createItem(Group::class, ['name' => 'Tech group', 'entities_id' => 0, 'is_recursive' => 1]); + $this->login(); + + $results = $this->inject($definition, [ + 'name' => 'Injected asset', + 'serial' => 'SN-001', + 'comment' => "first line\nsecond line", + 'assets_assettypes_id' => 'Server', + 'assets_assetmodels_id' => 'X42', + 'manufacturers_id' => 'ACME', + 'groups_id_normal' => $group->fields['name'], + 'groups_id' => $tech_group->fields['name'], + 'custom_reference' => 'REF-42', + 'custom_notes' => 'Some notes', + 'custom_quantity' => '7', + 'custom_delivery' => '2026-10-23', + 'custom_audited' => '2026-10-23 10:30:00', + 'custom_is_spare' => '1', + 'custom_room' => 'Room 101', + 'custom_color' => 'Blue', + ]); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_ADD, $results['type']); + + $asset = $this->getItem($definition, $results); + self::assertSame('Injected asset', $asset->fields['name']); + self::assertSame('SN-001', $asset->fields['serial']); + self::assertSame("first line\nsecond line", $asset->fields['comment']); + self::assertSame($definition->getID(), $asset->fields['assets_assetdefinitions_id']); + + // Missing dropdowns are created for the injected asset definition + $type = $definition->getAssetTypeClassName(); + self::assertSame('Server', $type::getById($asset->fields['assets_assettypes_id'])->fields['name']); + $model = $definition->getAssetModelClassName(); + self::assertSame('X42', $model::getById($asset->fields['assets_assetmodels_id'])->fields['name']); + self::assertSame('ACME', Manufacturer::getById($asset->fields['manufacturers_id'])->fields['name']); + self::assertSame([$group->getID()], $asset->fields['groups_id']); + self::assertSame([$tech_group->getID()], $asset->fields['groups_id_tech']); + + self::assertSame('REF-42', $asset->fields['custom_reference']); + self::assertSame('Some notes', $asset->fields['custom_notes']); + self::assertEquals(7, $asset->fields['custom_quantity']); + self::assertSame('2026-10-23', $asset->fields['custom_delivery']); + self::assertSame('2026-10-23 10:30:00', $asset->fields['custom_audited']); + self::assertTrue((bool) $asset->fields['custom_is_spare']); + self::assertSame('Room 101', Location::getById($asset->fields['custom_room'])->fields['name']); + $color = $dropdown_definition->getDropdownClassName(); + self::assertSame('Blue', $color::getById($asset->fields['custom_color'])->fields['name']); + } + + public function testUpdateOnlyMatchesAssetsOfTheSameDefinition(): void + { + $definition = $this->initAssetDefinition(); + $other_definition = $this->initAssetDefinition(); + $this->login(); + + $other = $this->getItem($other_definition, $this->inject($other_definition, ['name' => 'Shared name', 'serial' => 'OTHER'])); + + $added = $this->inject($definition, ['name' => 'Shared name', 'serial' => 'FIRST']); + self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_ADD, $added['type']); + + $updated = $this->inject($definition, ['name' => 'Shared name', 'serial' => 'SECOND']); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $updated['status']); + self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_UPDATE, $updated['type']); + self::assertSame($this->getItem($definition, $added)->getID(), $this->getItem($definition, $updated)->getID()); + self::assertSame('SECOND', $this->getItem($definition, $updated)->fields['serial']); + + self::assertTrue($other->getFromDB($other->getID())); + self::assertSame('OTHER', $other->fields['serial']); + } + + public function testUpdateMatchesACustomField(): void + { + $definition = $this->initAssetDefinition(); + $this->createCustomField($definition, 'reference', StringType::class); + $this->login(); + + $mandatory = ['custom_reference' => true]; + $added = $this->inject($definition, ['custom_reference' => 'REF-1', 'name' => 'Before'], mandatory_fields: $mandatory); + $this->inject($definition, ['custom_reference' => 'REF-2', 'name' => 'Other'], mandatory_fields: $mandatory); + + $updated = $this->inject($definition, ['custom_reference' => 'REF-1', 'name' => 'After'], mandatory_fields: $mandatory); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $updated['status']); + self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_UPDATE, $updated['type']); + self::assertSame($this->getItem($definition, $added)->getID(), $this->getItem($definition, $updated)->getID()); + self::assertSame('After', $this->getItem($definition, $updated)->fields['name']); + } + + public function testEmptyCustomFieldIsNotSetToZero(): void + { + $definition = $this->initAssetDefinition(); + // Contains "is_", like the boolean fields which are set to 0 when empty + $this->createCustomField($definition, 'basis_note', StringType::class); + $this->login(); + + $results = $this->inject($definition, ['name' => 'Empty custom field', 'custom_basis_note' => '']); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + self::assertSame('', $this->getItem($definition, $results)->fields['custom_basis_note']); + } + + public function testInjectionClassCannotBeResolvedFromItsName(): void + { + $this->expectException(HttpException::class); + PluginDatainjectionCommonInjectionLib::getItemtypeByInjection(AssetInjection::class); + } + + public function testInjectLinkedItems(): void + { + $definition = $this->initAssetDefinition(capacities: [ + new Capacity(name: HasOperatingSystemCapacity::class), + new Capacity(name: HasInfocomCapacity::class), + ]); + $this->login(); + + $results = $this->inject($definition, ['name' => 'Asset with linked items'], [ + Item_OperatingSystem::class => ['operatingsystems_id' => 'Debian'], + Infocom::class => ['order_number' => 'PO-42'], + ]); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + + $asset = $this->getItem($definition, $results); + $criteria = ['itemtype' => $asset::class, 'items_id' => $asset->getID()]; + + $item_os = new Item_OperatingSystem(); + self::assertTrue($item_os->getFromDBByCrit($criteria)); + self::assertSame('Debian', OperatingSystem::getById($item_os->fields['operatingsystems_id'])->fields['name']); + + $infocom = new Infocom(); + self::assertTrue($infocom->getFromDBByCrit($criteria)); + self::assertSame('PO-42', $infocom->fields['order_number']); + } + + private function createCustomField( + AssetDefinition $definition, + string $system_name, + string $type, + ?string $itemtype = null, + array $field_options = [], + ): void { + $this->createItem(CustomFieldDefinition::class, [ + 'assets_assetdefinitions_id' => $definition->getID(), + 'system_name' => $system_name, + 'label' => ucfirst($system_name), + 'type' => $type, + 'itemtype' => $itemtype, + 'field_options' => $field_options, + ], ['itemtype', 'field_options']); + } + + private function getInjection(AssetDefinition $definition): AssetInjection + { + $injection = PluginDatainjectionCommonInjectionLib::getInjectionClassInstance($definition->getAssetClassName()); + self::assertInstanceOf(AssetInjection::class, $injection); + + return $injection; + } + + private function inject( + AssetDefinition $definition, + array $values, + array $linked_values = [], + array $mandatory_fields = ['name' => true], + ): array { + $itemtype = $definition->getAssetClassName(); + + return $this->getInjection($definition)->addOrUpdateObject( + // Added by the engine + [$itemtype => $values + ['entities_id' => $this->getTestRootEntity(true)]] + $linked_values, + [ + 'rights' => [ + 'can_add' => true, + 'can_update' => true, + 'add_dropdown' => true, + 'overwrite_notempty_fields' => true, + ], + 'mandatory_fields' => [$itemtype => $mandatory_fields], + 'entities_id' => $this->getTestRootEntity(true), + ], + ); + } + + private function getItem(AssetDefinition $definition, array $results): Asset + { + $itemtype = $definition->getAssetClassName(); + $asset = new $itemtype(); + self::assertTrue($asset->getFromDB((int) $results[$itemtype])); + + return $asset; + } +} From 720da90f7e98e7d23a380f50aa0e3abbb05e8d1c Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Tue, 27 Oct 2026 16:41:33 +0100 Subject: [PATCH 2/3] Review --- src/AssetInjection.php | 13 ++++++- tests/unit/AssetInjectionTest.php | 63 ++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/AssetInjection.php b/src/AssetInjection.php index 87e19bb4..658b0fc8 100644 --- a/src/AssetInjection.php +++ b/src/AssetInjection.php @@ -36,10 +36,13 @@ use Glpi\DBAL\QueryExpression; use Glpi\DBAL\QueryFunction; use Glpi\DBAL\QueryIdentifier; +use InvalidArgumentException; use PluginDatainjectionCommonInjectionLib; use PluginDatainjectionInjectionInterface; use Search; +use function Safe\json_encode; + /** * Injection of the generic assets (GLPI custom assets). * @@ -185,11 +188,19 @@ public function getCustomFieldCriterion(string $field, mixed $value): ?QueryExpr foreach ($this->itemtype::getDefinition()->getCustomFieldDefinitions() as $custom_field) { if ('custom_' . $custom_field->fields['system_name'] === $field) { + // Stored the same way as the asset does (e.g. "1" becomes true for a yes/no field) + try { + $value = $custom_field->getFieldType()->formatValueForDB($value); + } catch (InvalidArgumentException) { + // Not a valid value for this field: it matches no asset + } + + // Numbers and booleans are stored as JSON values, e.g. 7.0 or true return new QueryExpression( QueryFunction::jsonUnquote(QueryFunction::jsonExtract([ new QueryIdentifier('custom_fields'), new QueryExpression($DB::quoteValue('$."' . $custom_field->getID() . '"')), - ])) . ' = ' . $DB::quoteValue((string) $value), + ])) . ' = ' . $DB::quoteValue(is_string($value) ? $value : json_encode($value)), ); } } diff --git a/tests/unit/AssetInjectionTest.php b/tests/unit/AssetInjectionTest.php index ee48d9a0..879e5fc5 100644 --- a/tests/unit/AssetInjectionTest.php +++ b/tests/unit/AssetInjectionTest.php @@ -55,6 +55,7 @@ use Location; use Manufacturer; use OperatingSystem; +use PHPUnit\Framework\Attributes\DataProvider; use PluginDatainjectionCommonInjectionLib; use PluginDatainjectionInjectionType; @@ -214,17 +215,67 @@ public function testUpdateOnlyMatchesAssetsOfTheSameDefinition(): void self::assertSame('OTHER', $other->fields['serial']); } - public function testUpdateMatchesACustomField(): void + public function testDropdownsOfAnotherDefinitionAreNotReused(): void { + $definition_a = $this->initAssetDefinition(); + $definition_b = $this->initAssetDefinition(); + $colors_a = $this->initDropdownDefinition()->getDropdownClassName(); + $colors_b = $this->initDropdownDefinition()->getDropdownClassName(); + $this->createCustomField($definition_a, 'color', DropdownType::class, $colors_a); + $this->createCustomField($definition_b, 'color', DropdownType::class, $colors_b); + + // Definition A already has the same names + $type_a = $this->createItem($definition_a->getAssetTypeClassName(), ['name' => 'Server']); + $model_a = $this->createItem($definition_a->getAssetModelClassName(), ['name' => 'X42']); + $color_a = $this->createItem($colors_a, ['name' => 'Blue']); + $this->login(); + + $results = $this->inject($definition_b, [ + 'name' => 'Asset of B', + 'assets_assettypes_id' => 'Server', + 'assets_assetmodels_id' => 'X42', + 'custom_color' => 'Blue', + ]); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + + $asset = $this->getItem($definition_b, $results); + self::assertNotEquals($type_a->getID(), $asset->fields['assets_assettypes_id']); + self::assertNotEquals($model_a->getID(), $asset->fields['assets_assetmodels_id']); + self::assertNotEquals($color_a->getID(), $asset->fields['custom_color']); + + // The items are created for definition B + $type_b = $definition_b->getAssetTypeClassName(); + self::assertSame('Server', $type_b::getById($asset->fields['assets_assettypes_id'])->fields['name']); + $model_b = $definition_b->getAssetModelClassName(); + self::assertSame('X42', $model_b::getById($asset->fields['assets_assetmodels_id'])->fields['name']); + self::assertSame('Blue', $colors_b::getById($asset->fields['custom_color'])->fields['name']); + } + + public static function customFieldMatchProvider(): iterable + { + yield 'string' => [StringType::class, [], 'Réf/1', 'Réf/1', 'Réf/2']; + yield 'integer written differently' => [NumberType::class, [], '7', '07', '8']; + yield 'decimal written differently' => [NumberType::class, ['step' => 0.01], '7', '7.00', '7.5']; + yield 'yes/no stored as a boolean' => [BooleanType::class, [], '1', '1', '0']; + } + + #[DataProvider('customFieldMatchProvider')] + public function testUpdateMatchesACustomField( + string $type, + array $field_options, + string $stored_value, + string $matching_value, + string $other_value, + ): void { $definition = $this->initAssetDefinition(); - $this->createCustomField($definition, 'reference', StringType::class); + $this->createCustomField($definition, 'key', $type, field_options: $field_options); $this->login(); - $mandatory = ['custom_reference' => true]; - $added = $this->inject($definition, ['custom_reference' => 'REF-1', 'name' => 'Before'], mandatory_fields: $mandatory); - $this->inject($definition, ['custom_reference' => 'REF-2', 'name' => 'Other'], mandatory_fields: $mandatory); + $mandatory = ['custom_key' => true]; + $added = $this->inject($definition, ['custom_key' => $stored_value, 'name' => 'Before'], mandatory_fields: $mandatory); + $this->inject($definition, ['custom_key' => $other_value, 'name' => 'Other'], mandatory_fields: $mandatory); - $updated = $this->inject($definition, ['custom_reference' => 'REF-1', 'name' => 'After'], mandatory_fields: $mandatory); + $updated = $this->inject($definition, ['custom_key' => $matching_value, 'name' => 'After'], mandatory_fields: $mandatory); self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $updated['status']); self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_UPDATE, $updated['type']); self::assertSame($this->getItem($definition, $added)->getID(), $this->getItem($definition, $updated)->getID()); From 8ff83e76b9f3ecd788cf63124e956d9665fc852e Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Thu, 29 Oct 2026 15:22:50 +0100 Subject: [PATCH 3/3] Fix the ignored non-overwrite option for custom fields --- inc/commoninjectionlib.class.php | 4 ++++ src/AssetInjection.php | 22 ++++++++++++++++++++++ tests/unit/AssetInjectionTest.php | 25 ++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/inc/commoninjectionlib.class.php b/inc/commoninjectionlib.class.php index 198b286c..b7f1a56a 100644 --- a/inc/commoninjectionlib.class.php +++ b/inc/commoninjectionlib.class.php @@ -2199,6 +2199,10 @@ private function dataAlreadyInDB($injectionClass, $itemtype) if (count($result) > 0) { $db_fields = $result->current(); + if ($injectionClass instanceof AssetInjection) { + $db_fields += $injectionClass->getCustomFieldValues($db_fields); + } + foreach ($db_fields as $key => $value) { $this->setValueForItemtype($itemtype, $key, $value, true); } diff --git a/src/AssetInjection.php b/src/AssetInjection.php index 658b0fc8..31a94df0 100644 --- a/src/AssetInjection.php +++ b/src/AssetInjection.php @@ -41,6 +41,7 @@ use PluginDatainjectionInjectionInterface; use Search; +use function Safe\json_decode; use function Safe\json_encode; /** @@ -208,6 +209,27 @@ public function getCustomFieldCriterion(string $field, mixed $value): ?QueryExpr return null; } + /** + * Stored values of the custom fields of an asset, by injected field ("custom_"), + * so that they are handled like the other values of the asset (e.g. not overwritten) + * + * @param array $db_fields asset row + */ + public function getCustomFieldValues(array $db_fields): array + { + $stored = json_decode($db_fields['custom_fields'] ?? '[]', true); + $values = []; + foreach ($this->itemtype::getDefinition()->getCustomFieldDefinitions() as $custom_field) { + $value = $stored[$custom_field->getID()] ?? null; + if ($value !== null && $this->getCustomFieldOption($custom_field) !== null) { + // As the other DB values, e.g. "1" for true, to be checked like the CSV values + $values['custom_' . $custom_field->fields['system_name']] = is_bool($value) ? (string) (int) $value : (string) $value; + } + } + + return $values; + } + /** * @see plugins/datainjection/inc/PluginDatainjectionInjectionInterface::addOrUpdateObject() **/ diff --git a/tests/unit/AssetInjectionTest.php b/tests/unit/AssetInjectionTest.php index 879e5fc5..255413b4 100644 --- a/tests/unit/AssetInjectionTest.php +++ b/tests/unit/AssetInjectionTest.php @@ -294,6 +294,28 @@ public function testEmptyCustomFieldIsNotSetToZero(): void self::assertSame('', $this->getItem($definition, $results)->fields['custom_basis_note']); } + public function testNotEmptyCustomFieldIsNotOverwritten(): void + { + $definition = $this->initAssetDefinition(); + $this->createCustomField($definition, 'reference', StringType::class); + $this->createCustomField($definition, 'notes', StringType::class); + $this->login(); + + $added = $this->inject($definition, ['name' => 'Asset', 'custom_reference' => 'REF-1']); + + $updated = $this->inject( + $definition, + ['name' => 'Asset', 'custom_reference' => 'REF-2', 'custom_notes' => 'Some notes'], + overwrite: false, + ); + self::assertSame(PluginDatainjectionCommonInjectionLib::IMPORT_UPDATE, $updated['type']); + + $asset = $this->getItem($definition, $added); + self::assertSame('REF-1', $asset->fields['custom_reference']); + // An empty field is still filled + self::assertSame('Some notes', $asset->fields['custom_notes']); + } + public function testInjectionClassCannotBeResolvedFromItsName(): void { $this->expectException(HttpException::class); @@ -356,6 +378,7 @@ private function inject( array $values, array $linked_values = [], array $mandatory_fields = ['name' => true], + bool $overwrite = true, ): array { $itemtype = $definition->getAssetClassName(); @@ -367,7 +390,7 @@ private function inject( 'can_add' => true, 'can_update' => true, 'add_dropdown' => true, - 'overwrite_notempty_fields' => true, + 'overwrite_notempty_fields' => $overwrite, ], 'mandatory_fields' => [$itemtype => $mandatory_fields], 'entities_id' => $this->getTestRootEntity(true),