From 81bf6b631751542f70963fe25a12450d693f8996 Mon Sep 17 00:00:00 2001 From: Julien Durand <300495670+jdurand-teclib@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:12:22 +0100 Subject: [PATCH 1/2] fix(47354): unclear warning messages + false positive warnings + issue with several complementary informatinons targetting the same field on different objetcts --- CHANGELOG.md | 4 + inc/commoninjectionlib.class.php | 73 +++++-- inc/contractinjection.class.php | 9 +- inc/engine.class.php | 9 +- inc/info.class.php | 31 ++- inc/infocominjection.class.php | 9 +- inc/model.class.php | 7 +- inc/networkportinjection.class.php | 46 ++-- inc/softwarelicenseinjection.class.php | 9 +- inc/softwareversioninjection.class.php | 9 +- tests/unit/AdditionalInformationTest.php | 134 ++++++++++++ tests/unit/LinkedDeviceNameNotMappedTest.php | 119 +++++++++++ tests/unit/NetworkPortInjectionTest.php | 212 +++++++++++++++++++ 13 files changed, 599 insertions(+), 72 deletions(-) create mode 100644 tests/unit/AdditionalInformationTest.php create mode 100644 tests/unit/LinkedDeviceNameNotMappedTest.php create mode 100644 tests/unit/NetworkPortInjectionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ea0b461..c6a94f82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fix the uploaded file name being lost after the client injection upload step +- Fix additional information sharing a field name across itemtypes (e.g. two device designations) overwriting each other, and show the itemtype next to each field +- Stop flagging a network port injection as partial when no connection to a network equipment port is requested +- Made several generic warnings more explicit. They explain why the mapping failed. +- Stop requesting a network port connection when its connection columns are empty ## [2.15.12] - 2026-10-05 diff --git a/inc/commoninjectionlib.class.php b/inc/commoninjectionlib.class.php index 2154699a..4bf8d2e4 100644 --- a/inc/commoninjectionlib.class.php +++ b/inc/commoninjectionlib.class.php @@ -98,6 +98,14 @@ class PluginDatainjectionCommonInjectionLib public const ERROR_IMPORT_REFUSED = 39; //Dictionnary explicitly refuse import + public const ERROR_NAME_NOT_MAPPED = 40; //Dropdown/device not created: its name field has no value + + public const WARNING_NETPORT_NOT_FOUND = 41; //Network port not connected: no matching network equipment port + + public const WARNING_NETPORT_AMBIGUOUS = 42; //Network port not connected: several network equipment ports match + + public const WARNING_NETPORT_ALREADY_CONNECTED = 43; //Network port not connected: the network equipment port is used + //Empty values public const EMPTY_VALUE = ''; @@ -1705,15 +1713,13 @@ public function processAddOrUpdate() $values = $this->getValuesForItemtype($this->primary_type); $newID = $this->effectiveAddOrUpdate($this->injectionClass, $item, $values, $add); - if (!$newID) { - $this->addCheckWarning(self::WARNING, $item::class); - } else { + if ($newID) { //Store id of the injected item $this->setValueForItemtype($this->primary_type, 'id', $newID); //If type needs it : process more data after type import - if ($this->processAfterInsertOrUpdate($this->injectionClass, $add) === false) { - $this->addCheckWarning(self::WARNING, $item::class); + if (($code = $this->processAfterInsertOrUpdate($this->injectionClass, $add)) !== null) { + $this->addCheckWarning($code, $item::class); } //$this->results['status'] = self::SUCCESS; @@ -1753,10 +1759,8 @@ public function processAddOrUpdate() $values = $this->getValuesForItemtype($itemtype); if ($this->lastCheckBeforeProcess($injectionClass)) { $tmpID = $this->effectiveAddOrUpdate($injectionClass, $item, $values, $add); - if (!$tmpID) { - $this->addCheckWarning(self::WARNING, $itemtype); - } elseif ($this->processAfterInsertOrUpdate($injectionClass, $add) === false) { - $this->addCheckWarning(self::WARNING, $itemtype); + if ($tmpID && ($code = $this->processAfterInsertOrUpdate($injectionClass, $add)) !== null) { + $this->addCheckWarning($code, $itemtype); } } } @@ -1897,6 +1901,22 @@ private function effectiveAddOrUpdate($injectionClass, $item, $values, $add = tr $newID = $injectionClass->customimport($toinject, $add, $this->rights); } elseif ($item instanceof CommonDropdown && $add && !($item instanceof SoftwareLicense)) { $newID = $item->import($toinject); + + // import() silently rejects items without a name (designation for devices): tell the user which field to map + $name_field = $item instanceof CommonDevice ? 'designation' : 'name'; + if ($newID <= 0 && empty($toinject[$name_field]) && empty($toinject['completename'])) { + $option = self::findSearchOption($options, $name_field); + $this->addCheckWarning( + self::ERROR_NAME_NOT_MAPPED, + sprintf( + __('%1$s - %2$s'), + $item::getTypeName(1), + $option['name'] ?? $name_field, + ), + ); + $this->setValueForItemtype($item::class, 'id', 0); + return 0; // reason already logged: no generic warning + } } elseif ($add) { if ($newID = $item->add($toinject)) { self::logAddOrUpdate($item, $add); @@ -1914,6 +1934,10 @@ private function effectiveAddOrUpdate($injectionClass, $item, $values, $add = tr } } + if (!$newID) { + $this->addCheckWarning(self::WARNING, $item::class); + } + $this->setValueForItemtype($item::class, 'id', $newID); return $newID; } @@ -2317,6 +2341,22 @@ public static function getLogLabel($type) $message = __('Import not allowed', 'datainjection'); break; + case self::ERROR_NAME_NOT_MAPPED: + $message = __('Item not created: a mapping is needed on the following field', 'datainjection'); + break; + + case self::WARNING_NETPORT_NOT_FOUND: + $message = __('Network port not connected: no matching network equipment port found', 'datainjection'); + break; + + case self::WARNING_NETPORT_AMBIGUOUS: + $message = __('Network port not connected: several network equipment ports match', 'datainjection'); + break; + + case self::WARNING_NETPORT_ALREADY_CONNECTED: + $message = __('Network port not connected: the network equipment port is already connected to another port', 'datainjection'); + break; + case self::FAILED: $message = __('Import failed', 'datainjection'); break; @@ -2512,17 +2552,22 @@ public static function addTemplateSearchOptions($injectionClass, &$tab) * @param PluginDatainjectionInjectionInterface $injectionClass the injection class to use * @param $add true if an item is created, false if it's an update * - * @return bool false if the injection class rejected a post-processing step + * @return int|null log code of the rejected post-processing step, null if none **/ - private function processAfterInsertOrUpdate($injectionClass, $add = true) + private function processAfterInsertOrUpdate($injectionClass, $add = true): ?int { //If itemtype implements special process after type injection if (method_exists($injectionClass, 'processAfterInsertOrUpdate')) { - //Invoke it - return $injectionClass->processAfterInsertOrUpdate($this->values, $add, $this->rights) !== false; + //Invoke it: false is a generic failure, an int is the log code explaining it + $result = $injectionClass->processAfterInsertOrUpdate($this->values, $add, $this->rights); + if (is_int($result)) { + return $result; + } + + return $result === false ? self::WARNING : null; } - return true; + return null; } } diff --git a/inc/contractinjection.class.php b/inc/contractinjection.class.php index 047f7329..d85b9df9 100644 --- a/inc/contractinjection.class.php +++ b/inc/contractinjection.class.php @@ -125,13 +125,12 @@ public function addOrUpdateObject($values = [], $options = []) /** - * @param array $info array - * @param array $option array - **/ - public function showAdditionalInformation($info = [], $option = []) + * @param array $option array + **/ + public function showAdditionalInformation(PluginDatainjectionInfo $info, $option = []) { - $name = "info[" . $option['linkfield'] . "]"; + $name = $info->getInputName(); switch ($option['displaytype']) { case 'alert': diff --git a/inc/engine.class.php b/inc/engine.class.php index 810b93f1..af0d475d 100644 --- a/inc/engine.class.php +++ b/inc/engine.class.php @@ -222,12 +222,9 @@ public function addAdditionalInformations() $additional_infos = []; foreach ($this->model->getInfos() as $info) { - if ( - isset($this->infos[$info->getValue()]) - && PluginDatainjectionInfo::keepInfo($info, $this->infos[$info->getValue()]) - ) { - $additional_infos[$info->getInfosType()][$info->getValue()] - = $this->infos[$info->getValue()]; + $value = $info->getPostedValue($this->infos); + if ($value !== null && PluginDatainjectionInfo::keepInfo($info, $value)) { + $additional_infos[$info->getInfosType()][$info->getValue()] = $value; } } diff --git a/inc/info.class.php b/inc/info.class.php index 82758ef6..899eb3dc 100644 --- a/inc/info.class.php +++ b/inc/info.class.php @@ -72,6 +72,26 @@ public function getInfosType() return $this->fields["itemtype"]; } + /** + * Name of the form input: keyed by itemtype too, as several itemtypes may share a field name + */ + public function getInputName(): string + { + return 'info[' . $this->getInfosType() . '][' . $this->getValue() . ']'; + } + + /** + * Value filled by the user for this info, as posted by the form built with getInputName() + * + * @param array $values infos posted by the user + * + * @return mixed|null + */ + public function getPostedValue(array $values) + { + return $values[$this->getInfosType()][$this->getValue()] ?? null; + } + /** * @param PluginDatainjectionModel $model PluginDatainjectionModel object * @param boolean $canedit (false by default) @@ -309,7 +329,7 @@ public static function displayAdditionalInformation(PluginDatainjectionInfo $inf $info->fields['value'], ); if ($option) { - echo "" . $option['name'] . ""; + echo "" . htmlescape(sprintf(__('%1$s - %2$s'), $info->fields['itemtype']::getTypeName(1), $option['name'])) . ""; self::showAdditionalInformation($info, $option, $injectionClass, $values); echo ""; } @@ -330,13 +350,8 @@ public static function showAdditionalInformation( $values = [] ) { - $name = "info[" . $option['linkfield'] . "]"; - - if (isset($_SESSION['datainjection']['infos'][$option['linkfield']])) { - $value = $_SESSION['datainjection']['infos'][$option['linkfield']]; - } else { - $value = ''; - } + $name = $info->getInputName(); + $value = $info->getPostedValue($_SESSION['datainjection']['infos'] ?? []) ?? ''; switch ($option['displaytype']) { case 'text': diff --git a/inc/infocominjection.class.php b/inc/infocominjection.class.php index 243f1a67..f6348ede 100644 --- a/inc/infocominjection.class.php +++ b/inc/infocominjection.class.php @@ -125,13 +125,12 @@ public function getOptions($primary_type = '') /** - * @param array $info array - * @param array $option array - **/ - public function showAdditionalInformation($info = [], $option = []) + * @param array $option array + **/ + public function showAdditionalInformation(PluginDatainjectionInfo $info, $option = []) { - $name = "info[" . $option['linkfield'] . "]"; + $name = $info->getInputName(); switch ($option['displaytype']) { case 'sink_type': diff --git a/inc/model.class.php b/inc/model.class.php index 6a72d63a..2a42e2dc 100644 --- a/inc/model.class.php +++ b/inc/model.class.php @@ -1214,11 +1214,12 @@ public function checkMandatoryFields($fields) $item = new $itemtype(); $option = $item->getSearchOptionByField('field', $info->getValue()); $tocheck = (!isset($option['datatype']) || ($option['datatype'] != 'bool')); + $value = $info->getPostedValue($fields); if ( - !isset($fields[$info->getValue()]) + $value === null //Check if no value defined only when it's not a yes/no - || ($tocheck && !$fields[$info->getValue()]) - || ($fields[$info->getValue()] == 'NULL') + || ($tocheck && !$value) + || ($value == 'NULL') ) { $check = false; break; diff --git a/inc/networkportinjection.class.php b/inc/networkportinjection.class.php index a9611b0c..3c61225c 100644 --- a/inc/networkportinjection.class.php +++ b/inc/networkportinjection.class.php @@ -301,22 +301,20 @@ public function lastCheck($values = []) * @param array $values * @param boolean $add (true by default) * @param array|null $rights array + * @return bool|int|null true or null if ok, a PluginDatainjectionCommonInjectionLib log code if the connection failed **/ public function processAfterInsertOrUpdate($values, $add = true, $rights = []) { /** @var DBmysql $DB */ global $DB; - //Should the port be connected to another one ? - $use_name = (isset($values['NetworkPort']["netname"]) - || !empty($values['NetworkPort']["netname"])); - $use_logical_number = (isset($values['NetworkPort']["netport"]) - || !empty($values['NetworkPort']["netport"])); - $use_mac = (isset($values['NetworkPort']["netmac"]) - || !empty($values['NetworkPort']["netmac"])); + //Should the port be connected to another one ? (an empty mapped cell is not a request) + $use_name = (string) ($values['NetworkPort']["netname"] ?? '') !== ''; + $use_logical_number = (string) ($values['NetworkPort']["netport"] ?? '') !== ''; + $use_mac = (string) ($values['NetworkPort']["netmac"] ?? '') !== ''; if (!$use_name && !$use_logical_number && !$use_mac) { - return false; + return true; // nothing to connect is not a failure } // Find port in database @@ -351,21 +349,27 @@ public function processAfterInsertOrUpdate($values, $add = true, $rights = []) 'WHERE' => $criteria, ]); - //if at least one parameter is given - if (count($result) === 1) { - //Get data for this port - $netport = $result->current(); - $netport_netport = new NetworkPort_NetworkPort(); - //If this port already connected to another one ? - if (!$netport_netport->getOppositeContact((int) $netport['id'])) { - //No, add a new port to port connection - $tmp['networkports_id_1'] = $values['NetworkPort']['id']; - $tmp['networkports_id_2'] = $netport['id']; - $netport_netport->add($tmp); - } + if (count($result) === 0) { + return PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_NOT_FOUND; + } + + if (count($result) > 1) { + return PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_AMBIGUOUS; + } + + $netport = $result->current(); + $netport_netport = new NetworkPort_NetworkPort(); + $opposite = $netport_netport->getOppositeContact((int) $netport['id']); + if (!$opposite) { + //Not connected yet: add a new port to port connection + $tmp['networkports_id_1'] = $values['NetworkPort']['id']; + $tmp['networkports_id_2'] = $netport['id']; + $netport_netport->add($tmp); + } elseif ((int) $opposite !== (int) $values['NetworkPort']['id']) { + // Connected to another port; already connected to this one (re-import) is fine + return PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_ALREADY_CONNECTED; } - //TODO add injection warning if no port found or more than one return null; } } diff --git a/inc/softwarelicenseinjection.class.php b/inc/softwarelicenseinjection.class.php index ae1791e0..4b7f1794 100644 --- a/inc/softwarelicenseinjection.class.php +++ b/inc/softwarelicenseinjection.class.php @@ -100,13 +100,12 @@ public function getOptions($primary_type = '') /** - * @param array $info array - * @param array $option array - **/ - public function showAdditionalInformation($info = [], $option = []) + * @param array $option array + **/ + public function showAdditionalInformation(PluginDatainjectionInfo $info, $option = []) { - $name = "info[" . $option['linkfield'] . "]"; + $name = $info->getInputName(); switch ($option['displaytype']) { case 'computer': diff --git a/inc/softwareversioninjection.class.php b/inc/softwareversioninjection.class.php index 202d2029..5f4bba65 100644 --- a/inc/softwareversioninjection.class.php +++ b/inc/softwareversioninjection.class.php @@ -96,13 +96,12 @@ public function getOptions($primary_type = '') /** - * @param array $info array - * @param array $option array - **/ - public function showAdditionalInformation($info = [], $option = []) + * @param array $option array + **/ + public function showAdditionalInformation(PluginDatainjectionInfo $info, $option = []) { - $name = "info[" . $option['linkfield'] . "]"; + $name = $info->getInputName(); switch ($option['displaytype']) { case 'computer': Computer::dropdown( diff --git a/tests/unit/AdditionalInformationTest.php b/tests/unit/AdditionalInformationTest.php new file mode 100644 index 00000000..f366d82f --- /dev/null +++ b/tests/unit/AdditionalInformationTest.php @@ -0,0 +1,134 @@ +. + * ------------------------------------------------------------------------- + * @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 Computer; +use DeviceMemory; +use DeviceProcessor; +use Glpi\Tests\DbTestCase; +use Item_DeviceMemory; +use Item_DeviceProcessor; +use PluginDatainjectionCommonInjectionLib; +use PluginDatainjectionEngine; +use PluginDatainjectionInfo; +use PluginDatainjectionMapping; +use PluginDatainjectionModel; +use Session; + +final class AdditionalInformationTest extends DbTestCase +{ + /** + * Model on Computer, CSV maps only the name; both device designations come from additional information + */ + private function createModel(): PluginDatainjectionModel + { + $model = $this->createItem(PluginDatainjectionModel::class, [ + 'name' => 'Test_model_infos_' . random_int(1, PHP_INT_MAX), + 'itemtype' => 'Computer', + 'behavior_add' => 1, + 'behavior_update' => 1, + 'users_id' => Session::getLoginUserID(), + ]); + $this->createItem(PluginDatainjectionMapping::class, [ + 'models_id' => $model->getID(), + 'itemtype' => 'Computer', + 'rank' => 0, + 'name' => 'Name', + 'value' => 'name', + 'is_mandatory' => 1, + ]); + foreach (['DeviceMemory', 'DeviceProcessor'] as $itemtype) { + $this->createItem(PluginDatainjectionInfo::class, [ + 'models_id' => $model->getID(), + 'itemtype' => $itemtype, + 'value' => 'designation', + 'is_mandatory' => 1, + ]); + } + + return $model; + } + + public function testSameFieldOnSeveralItemtypesKeepsEachValue(): void + { + $this->login(); + + $entities_id = $this->getTestRootEntity(true); + $computer = $this->createItem(Computer::class, [ + 'name' => 'Test_Computer_infos_' . random_int(1, PHP_INT_MAX), + 'entities_id' => $entities_id, + ]); + $model = $this->createModel(); + $suffix = random_int(1, PHP_INT_MAX); + $infos = [ + 'DeviceMemory' => ['designation' => 'Test_RAM_' . $suffix], + 'DeviceProcessor' => ['designation' => 'Test_CPU_' . $suffix], + ]; + + self::assertTrue($model->checkMandatoryFields($infos)); + self::assertFalse($model->checkMandatoryFields(['DeviceMemory' => $infos['DeviceMemory']])); + + $engine = new PluginDatainjectionEngine($model, $infos, $entities_id); + $results = $engine->injectLine([$computer->fields['name']], 2); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + + $memory = new DeviceMemory(); + self::assertTrue($memory->getFromDBByCrit(['designation' => 'Test_RAM_' . $suffix])); + self::assertSame(1, countElementsInTable(Item_DeviceMemory::getTable(), [ + 'devicememories_id' => $memory->getID(), + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + ])); + + $processor = new DeviceProcessor(); + self::assertTrue($processor->getFromDBByCrit(['designation' => 'Test_CPU_' . $suffix])); + self::assertSame(1, countElementsInTable(Item_DeviceProcessor::getTable(), [ + 'deviceprocessors_id' => $processor->getID(), + 'itemtype' => 'Computer', + 'items_id' => $computer->getID(), + ])); + } + + public function testFormInputIsNamedAfterItemtypeAndField(): void + { + $this->login(); + + $info = new PluginDatainjectionInfo(); + $info->fields = ['itemtype' => 'DeviceProcessor', 'value' => 'designation', 'is_mandatory' => 0]; + + ob_start(); + PluginDatainjectionInfo::displayAdditionalInformation($info); + $html = ob_get_clean(); + + self::assertStringContainsString("name='info[DeviceProcessor][designation]'", $html); + self::assertStringContainsString(DeviceProcessor::getTypeName(1) . ' - ', $html); + } +} diff --git a/tests/unit/LinkedDeviceNameNotMappedTest.php b/tests/unit/LinkedDeviceNameNotMappedTest.php new file mode 100644 index 00000000..4a970676 --- /dev/null +++ b/tests/unit/LinkedDeviceNameNotMappedTest.php @@ -0,0 +1,119 @@ +. + * ------------------------------------------------------------------------- + * @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 Computer; +use DeviceProcessor; +use Glpi\Tests\DbTestCase; +use Item_DeviceProcessor; +use PluginDatainjectionCommonInjectionLib; +use PluginDatainjectionComputerInjection; +use PluginDatainjectionModel; +use PluginDatainjectionSession; +use Session; + +final class LinkedDeviceNameNotMappedTest extends DbTestCase +{ + private function injectComputerWithProcessor(array $processor): array + { + $computer_name = 'Test_Computer_cpu_' . random_int(1, PHP_INT_MAX); + $entities_id = $this->getTestRootEntity(true); + $this->createItem(Computer::class, ['name' => $computer_name, 'entities_id' => $entities_id]); + + $lib = new PluginDatainjectionCommonInjectionLib( + new PluginDatainjectionComputerInjection(), + [ + 'Computer' => ['name' => $computer_name], + 'DeviceProcessor' => $processor, + ], + [ + 'rights' => ['can_add' => true, 'can_update' => true], + 'mandatory_fields' => ['Computer' => ['name' => true]], + 'entities_id' => $entities_id, + ], + ); + $lib->processAddOrUpdate(); + + return $lib->getInjectionResults(); + } + + public function testProcessorWithoutDesignationReportsTheFieldToMap(): void + { + $this->login(); + + $before = countElementsInTable(DeviceProcessor::getTable()); + $results = $this->injectComputerWithProcessor(['frequency_default' => '3900']); + + self::assertSame(PluginDatainjectionCommonInjectionLib::WARNING, $results['status']); + self::assertContains( + [PluginDatainjectionCommonInjectionLib::ERROR_NAME_NOT_MAPPED, DeviceProcessor::getTypeName(1) . ' - ' . __('Name')], + $results[PluginDatainjectionCommonInjectionLib::ACTION_CHECK], + ); + self::assertNotContains( + [PluginDatainjectionCommonInjectionLib::WARNING, DeviceProcessor::class], + $results[PluginDatainjectionCommonInjectionLib::ACTION_CHECK], + ); + self::assertSame($before, countElementsInTable(DeviceProcessor::getTable())); + + // Same message as displayed in the log popup and the PDF export + $model = $this->createItem(PluginDatainjectionModel::class, [ + 'name' => 'Test_model_' . random_int(1, PHP_INT_MAX), + 'itemtype' => 'Computer', + 'behavior_add' => 1, + 'users_id' => Session::getLoginUserID(), + ]); + PluginDatainjectionSession::setParam('results', json_encode([$results + ['line' => 2]])); + $logs = PluginDatainjectionModel::prepareLogResults($model->getID()); + self::assertStringContainsString( + 'Item not created: a mapping is needed on the following field (' . DeviceProcessor::getTypeName(1) . ' - ' . __('Name') . ')', + $logs[PluginDatainjectionCommonInjectionLib::FAILED][0]['check_message'], + ); + } + + public function testProcessorWithDesignationIsLinkedWithItsDefaultFrequency(): void + { + $this->login(); + + $designation = 'Test_CPU_' . random_int(1, PHP_INT_MAX); + $results = $this->injectComputerWithProcessor(['designation' => $designation, 'frequency_default' => '3900']); + + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + + $processor = new DeviceProcessor(); + self::assertTrue($processor->getFromDBByCrit(['designation' => $designation])); + self::assertSame(1, countElementsInTable(Item_DeviceProcessor::getTable(), [ + 'deviceprocessors_id' => $processor->getID(), + 'itemtype' => 'Computer', + 'items_id' => $results['Computer'], + 'frequency' => 3900, + ])); + } +} diff --git a/tests/unit/NetworkPortInjectionTest.php b/tests/unit/NetworkPortInjectionTest.php new file mode 100644 index 00000000..02b8a212 --- /dev/null +++ b/tests/unit/NetworkPortInjectionTest.php @@ -0,0 +1,212 @@ +. + * ------------------------------------------------------------------------- + * @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 Computer; +use Glpi\Tests\DbTestCase; +use NetworkEquipment; +use NetworkPort; +use NetworkPort_NetworkPort; +use PHPUnit\Framework\Attributes\DataProvider; +use PluginDatainjectionCommonInjectionLib; +use PluginDatainjectionComputerInjection; + +final class NetworkPortInjectionTest extends DbTestCase +{ + private function injectComputerWithPort(array $port, ?string $computer_name = null): array + { + $entities_id = $this->getTestRootEntity(true); + if ($computer_name === null || countElementsInTable(Computer::getTable(), ['name' => $computer_name]) === 0) { + $computer_name ??= 'Test_Computer_port_' . random_int(1, PHP_INT_MAX); + $this->createItem(Computer::class, ['name' => $computer_name, 'entities_id' => $entities_id]); + } + + $lib = new PluginDatainjectionCommonInjectionLib( + new PluginDatainjectionComputerInjection(), + [ + 'Computer' => ['name' => $computer_name], + 'NetworkPort' => $port, + ], + [ + 'rights' => ['can_add' => true, 'can_update' => true], + 'mandatory_fields' => ['Computer' => ['name' => true]], + 'entities_id' => $entities_id, + 'checks' => ['port_unicity' => PluginDatainjectionCommonInjectionLib::UNICITY_NETPORT_MACADDRESS], + ], + ); + $lib->processAddOrUpdate(); + + return $lib->getInjectionResults(); + } + + public static function noConnectionProvider(): iterable + { + yield 'connection fields not mapped' => [[]]; + yield 'connection fields mapped empty' => [['netname' => '', 'netport' => '', 'netmac' => '']]; + } + + #[DataProvider('noConnectionProvider')] + public function testPortWithoutConnectionToSwitchIsNotAWarning(array $connection): void + { + $this->login(); + + $results = $this->injectComputerWithPort(['mac' => 'AA:11:BB:22:CC:33'] + $connection); + + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + self::assertSame(1, countElementsInTable(NetworkPort::getTable(), [ + 'itemtype' => 'Computer', + 'items_id' => $results['Computer'], + 'mac' => 'aa:11:bb:22:cc:33', + ])); + } + + public static function switchCriteriaProvider(): iterable + { + yield 'switch name and port number' => [fn(string $switch, string $mac) => ['netname' => $switch, 'netport' => '5']]; + yield 'switch port mac' => [fn(string $switch, string $mac) => ['netmac' => $mac]]; + } + + #[DataProvider('switchCriteriaProvider')] + public function testPortIsConnectedToTheRequestedSwitchPort(callable $criteria): void + { + $this->login(); + + $suffix = random_int(1, PHP_INT_MAX); + $switch_name = 'Test_Switch_' . $suffix; + $switch_mac = sprintf('02:00:%02x:%02x:%02x:%02x', ...array_values(unpack('C4', pack('N', $suffix)))); + $switch = $this->createItem(NetworkEquipment::class, [ + 'name' => $switch_name, + 'entities_id' => $this->getTestRootEntity(true), + ]); + $switch_port = $this->createItem(NetworkPort::class, [ + 'itemtype' => NetworkEquipment::class, + 'items_id' => $switch->getID(), + 'entities_id' => $this->getTestRootEntity(true), + 'logical_number' => 5, + 'mac' => $switch_mac, + 'instantiation_type' => 'NetworkPortEthernet', + ]); + + $results = $this->injectComputerWithPort(['mac' => '02:aa:bb:cc:dd:ee'] + $criteria($switch_name, $switch_mac)); + + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status']); + $computer_port = new NetworkPort(); + self::assertTrue($computer_port->getFromDBByCrit(['itemtype' => 'Computer', 'items_id' => $results['Computer']])); + self::assertSame( + $switch_port->getID(), + (new NetworkPort_NetworkPort())->getOppositeContact($computer_port->getID()), + ); + } + + private function assertPortNotConnected(array $results, int $code): void + { + self::assertSame(PluginDatainjectionCommonInjectionLib::WARNING, $results['status']); + self::assertContains([$code, NetworkPort::class], $results[PluginDatainjectionCommonInjectionLib::ACTION_CHECK]); + + // The port itself is still created, only the connection is missing + $computer_port = new NetworkPort(); + self::assertTrue($computer_port->getFromDBByCrit(['itemtype' => 'Computer', 'items_id' => $results['Computer']])); + self::assertFalse((new NetworkPort_NetworkPort())->getOppositeContact($computer_port->getID())); + } + + private function createSwitchPort(string $switch_name, int $logical_number): NetworkPort + { + $switch = new NetworkEquipment(); + if (!$switch->getFromDBByCrit(['name' => $switch_name])) { + $switch = $this->createItem(NetworkEquipment::class, [ + 'name' => $switch_name, + 'entities_id' => $this->getTestRootEntity(true), + ]); + } + + return $this->createItem(NetworkPort::class, [ + 'itemtype' => NetworkEquipment::class, + 'items_id' => $switch->getID(), + 'entities_id' => $this->getTestRootEntity(true), + 'logical_number' => $logical_number, + 'instantiation_type' => 'NetworkPortEthernet', + ]); + } + + public function testPortIsReportedWhenTheRequestedSwitchPortIsNotFoundOrAmbiguous(): void + { + $this->login(); + + // Not found: no switch with this name + $results = $this->injectComputerWithPort(['mac' => '02:aa:bb:cc:dd:01', 'netname' => 'Unknown_Switch_' . random_int(1, PHP_INT_MAX)]); + $this->assertPortNotConnected($results, PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_NOT_FOUND); + self::assertStringContainsString( + 'no matching network equipment port found', + PluginDatainjectionCommonInjectionLib::getLogLabel(PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_NOT_FOUND), + ); + + // Ambiguous: the switch has two ports and only its name is given + $switch_name = 'Test_Switch_' . random_int(1, PHP_INT_MAX); + $this->createSwitchPort($switch_name, 1); + $this->createSwitchPort($switch_name, 2); + + $results = $this->injectComputerWithPort(['mac' => '02:aa:bb:cc:dd:02', 'netname' => $switch_name]); + $this->assertPortNotConnected($results, PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_AMBIGUOUS); + } + + public function testPortIsReportedWhenTheSwitchPortIsAlreadyConnectedElsewhere(): void + { + $this->login(); + + $switch_name = 'Test_Switch_' . random_int(1, PHP_INT_MAX); + $this->createSwitchPort($switch_name, 5); + $connection = ['netname' => $switch_name, 'netport' => '5']; + + $first = $this->injectComputerWithPort(['mac' => '02:aa:bb:cc:dd:03'] + $connection); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $first['status']); + + $results = $this->injectComputerWithPort(['mac' => '02:aa:bb:cc:dd:04'] + $connection); + $this->assertPortNotConnected($results, PluginDatainjectionCommonInjectionLib::WARNING_NETPORT_ALREADY_CONNECTED); + } + + public function testReimportingAnAlreadyConnectedPortIsNotAWarning(): void + { + $this->login(); + + $switch_name = 'Test_Switch_' . random_int(1, PHP_INT_MAX); + $switch_port = $this->createSwitchPort($switch_name, 5); + $computer_name = 'Test_Computer_port_' . random_int(1, PHP_INT_MAX); + $port = ['mac' => '02:aa:bb:cc:dd:05', 'netname' => $switch_name, 'netport' => '5']; + + foreach ([1, 2] as $run) { + $results = $this->injectComputerWithPort($port, $computer_name); + self::assertSame(PluginDatainjectionCommonInjectionLib::SUCCESS, $results['status'], 'run ' . $run); + } + + self::assertSame(1, countElementsInTable(NetworkPort::getTable(), ['itemtype' => 'Computer', 'items_id' => $results['Computer']])); + self::assertSame(1, countElementsInTable(NetworkPort_NetworkPort::getTable(), ['networkports_id_2' => $switch_port->getID()])); + } +} From c2a2a9e88a96fc0781345d9498098d27f601cf2d Mon Sep 17 00:00:00 2001 From: Julien Durand <300495670+jdurand-teclib@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:35:51 +0100 Subject: [PATCH 2/2] fix(47354): Misleading error message --- inc/commoninjectionlib.class.php | 6 +++--- tests/unit/LinkedDeviceNameNotMappedTest.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/inc/commoninjectionlib.class.php b/inc/commoninjectionlib.class.php index 4bf8d2e4..6cb7fa6f 100644 --- a/inc/commoninjectionlib.class.php +++ b/inc/commoninjectionlib.class.php @@ -98,7 +98,7 @@ class PluginDatainjectionCommonInjectionLib public const ERROR_IMPORT_REFUSED = 39; //Dictionnary explicitly refuse import - public const ERROR_NAME_NOT_MAPPED = 40; //Dropdown/device not created: its name field has no value + public const ERROR_NAME_NOT_MAPPED = 40; //Dropdown/device not created: its name field is not mapped or empty in the row public const WARNING_NETPORT_NOT_FOUND = 41; //Network port not connected: no matching network equipment port @@ -1902,7 +1902,7 @@ private function effectiveAddOrUpdate($injectionClass, $item, $values, $add = tr } elseif ($item instanceof CommonDropdown && $add && !($item instanceof SoftwareLicense)) { $newID = $item->import($toinject); - // import() silently rejects items without a name (designation for devices): tell the user which field to map + // import() silently rejects items without a name (designation for devices): tell the user which field to map or fill $name_field = $item instanceof CommonDevice ? 'designation' : 'name'; if ($newID <= 0 && empty($toinject[$name_field]) && empty($toinject['completename'])) { $option = self::findSearchOption($options, $name_field); @@ -2342,7 +2342,7 @@ public static function getLogLabel($type) break; case self::ERROR_NAME_NOT_MAPPED: - $message = __('Item not created: a mapping is needed on the following field', 'datainjection'); + $message = __('Item not created: the following field is not mapped or is empty', 'datainjection'); break; case self::WARNING_NETPORT_NOT_FOUND: diff --git a/tests/unit/LinkedDeviceNameNotMappedTest.php b/tests/unit/LinkedDeviceNameNotMappedTest.php index 4a970676..8b16e9c3 100644 --- a/tests/unit/LinkedDeviceNameNotMappedTest.php +++ b/tests/unit/LinkedDeviceNameNotMappedTest.php @@ -93,7 +93,7 @@ public function testProcessorWithoutDesignationReportsTheFieldToMap(): void PluginDatainjectionSession::setParam('results', json_encode([$results + ['line' => 2]])); $logs = PluginDatainjectionModel::prepareLogResults($model->getID()); self::assertStringContainsString( - 'Item not created: a mapping is needed on the following field (' . DeviceProcessor::getTypeName(1) . ' - ' . __('Name') . ')', + 'Item not created: the following field is not mapped or is empty (' . DeviceProcessor::getTypeName(1) . ' - ' . __('Name') . ')', $logs[PluginDatainjectionCommonInjectionLib::FAILED][0]['check_message'], ); }