Skip to content

Commit 1bee128

Browse files
cedric-annecedric-anne
committed
Throw exception when no item is found by getItemByTypeName() 
1 parent 236d6b8 commit 1bee128

File tree

4 files changed

+56
-43
lines changed

4 files changed

+56
-43
lines changed

‎phpunit/functional/DbUtilsTest.php

+16-12
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,10 @@ private function runGetAncestorsOf($cache = false, $hit = false)
860860
//test with new sub entity
861861
//Cache tests:
862862
//Cache is updated on entity creation; so even if we do not expect $hit; we got it.
863-
$new_id = getItemByTypeName('Entity', 'Sub child entity', true);
864-
if (!$new_id) {
865-
$entity = new \Entity();
863+
$entity = new \Entity();
864+
if ($entity->getFromDBByCrit(['name' => 'Sub child entity'])) {
865+
$new_id = $entity->getID();
866+
} else {
866867
$new_id = $entity->add([
867868
'name' => 'Sub child entity',
868869
'entities_id' => $ent1
@@ -884,9 +885,10 @@ private function runGetAncestorsOf($cache = false, $hit = false)
884885
}
885886

886887
//test with another new sub entity
887-
$new_id2 = getItemByTypeName('Entity', 'Sub child entity 2', true);
888-
if (!$new_id2) {
889-
$entity = new \Entity();
888+
$entity = new \Entity();
889+
if ($entity->getFromDBByCrit(['name' => 'Sub child entity 2'])) {
890+
$new_id2 = $entity->getID();
891+
} else {
890892
$new_id2 = $entity->add([
891893
'name' => 'Sub child entity 2',
892894
'entities_id' => $ent2
@@ -1030,9 +1032,10 @@ private function runGetSonsOf($cache = false, $hit = false)
10301032
//test with new sub entity
10311033
//Cache tests:
10321034
//Cache is updated on entity creation; so even if we do not expect $hit; we got it.
1033-
$new_id = getItemByTypeName('Entity', 'Sub child entity', true);
1034-
if (!$new_id) {
1035-
$entity = new \Entity();
1035+
$entity = new \Entity();
1036+
if ($entity->getFromDBByCrit(['name' => 'Sub child entity'])) {
1037+
$new_id = $entity->getID();
1038+
} else {
10361039
$new_id = (int)$entity->add([
10371040
'name' => 'Sub child entity',
10381041
'entities_id' => $ent1
@@ -1053,9 +1056,10 @@ private function runGetSonsOf($cache = false, $hit = false)
10531056
}
10541057

10551058
//test with another new sub entity
1056-
$new_id2 = getItemByTypeName('Entity', 'Sub child entity 2', true);
1057-
if (!$new_id2) {
1058-
$entity = new \Entity();
1059+
$entity = new \Entity();
1060+
if ($entity->getFromDBByCrit(['name' => 'Sub child entity 2'])) {
1061+
$new_id2 = $entity->getID();
1062+
} else {
10591063
$new_id2 = (int)$entity->add([
10601064
'name' => 'Sub child entity 2',
10611065
'entities_id' => $ent1

‎phpunit/functional/Glpi/Inventory/Assets/NetworkPortTest.php

+8-11
Original file line numberDiff line numberDiff line change
@@ -332,17 +332,14 @@ public function testPrepare($xml, $ports, $connections, $vlans, $aggregates)
332332
$data = $converter->convert($xml);
333333
$json = json_decode($data);
334334

335-
$neteq = getItemByTypeName('NetworkEquipment', 'My network equipment');
336-
if ($neteq === false) {
337-
$neteq = new \NetworkEquipment();
338-
$this->assertGreaterThan(
339-
0,
340-
$neteq->add([
341-
'name' => 'My network equipment',
342-
'entities_id' => 0
343-
])
344-
);
345-
}
335+
$neteq = new \NetworkEquipment();
336+
$this->assertGreaterThan(
337+
0,
338+
$neteq->add([
339+
'name' => 'My network equipment',
340+
'entities_id' => 0
341+
])
342+
);
346343

347344
$asset = new \Glpi\Inventory\Asset\NetworkPort($neteq, $json->content->network_ports);
348345
$asset->setExtraData((array)$json->content);

‎src/Glpi/Dashboard/Dashboard.php

+9
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,15 @@ public function load(bool $force = false)
101101
}
102102

103103

104+
public function getID()
105+
{
106+
// Force usage of the `id` field
107+
if (isset($this->fields['id'])) {
108+
return (int)$this->fields['id'];
109+
}
110+
return -1;
111+
}
112+
104113
public function getFromDB($ID)
105114
{
106115
/** @var \DBmysql $DB */

‎tests/src/autoload/functions.php

+23-20
Original file line numberDiff line numberDiff line change
@@ -739,23 +739,26 @@ function loadDataset()
739739
} else if ($k == 'items_id' && isset($input['itemtype']) && isset($ids[$input['itemtype']][$v]) && !is_numeric($v)) {
740740
$input[$k] = $ids[$input['itemtype']][$v];
741741
} else if ($foreigntype && $foreigntype != 'UNKNOWN' && !is_numeric($v)) {
742-
// not found in ids array, then must get it from DB
743-
if ($obj = getItemByTypeName($foreigntype, $v)) {
744-
$input[$k] = $obj->getID();
745-
}
742+
// not found in ids array, then must get it from DB
743+
$foreign_id = getItemByTypeName($foreigntype, $v, true);
744+
$input[$k] = $foreign_id;
745+
746+
$ids[$foreigntype][$v] = $foreign_id; // cache ID
746747
}
747748
}
748749

749-
if (isset($input['name']) && $item = getItemByTypeName($type, $input['name'])) {
750-
$input['id'] = $ids[$type][$input['name']] = $item->getField('id');
751-
$item->update($input);
750+
$item = getItemForItemtype($type);
751+
$name_field = $item::getNameField();
752+
753+
if (isset($input[$name_field]) && $item->getFromDBByCrit([$name_field => $input[$name_field]])) {
754+
// Update existing item
755+
$item->update([$item::getIndexName() => $item->getID()] + $input);
752756
} else {
753757
// Not found, create it
754-
$item = getItemForItemtype($type);
755-
$id = $item->add($input);
756-
if (isset($input['name'])) {
757-
$ids[$type][$input['name']] = $id;
758-
}
758+
$item->add($input);
759+
}
760+
if (isset($input[$name_field])) {
761+
$ids[$type][$input[$name_field]] = $item->getID(); // cache ID
759762
}
760763
}
761764
}
@@ -774,17 +777,17 @@ function loadDataset()
774777
/**
775778
* Test helper, search an item from its type and name
776779
*
777-
* @param string $type
778-
* @param string $name
779-
* @param boolean $onlyid
780-
* @return CommonDBTM|false the item, or its id
780+
* @param string $type
781+
* @param string $name
782+
* @param bool $onlyid
783+
* @return CommonDBTM|int the item, or its id
781784
*/
782-
function getItemByTypeName($type, $name, $onlyid = false)
785+
function getItemByTypeName(string $type, string $name, bool $onlyid = false): CommonDBTM|int
783786
{
784787
$item = getItemForItemtype($type);
785788
$nameField = $type::getNameField();
786-
if ($item->getFromDBByCrit([$nameField => $name])) {
787-
return ($onlyid ? $item->getField('id') : $item);
789+
if (!$item->getFromDBByCrit([$nameField => $name])) {
790+
throw new \RuntimeException(sprintf('Unable to load the `%s` item with the name `%s`.', $type, $name));
788791
}
789-
return false;
792+
return ($onlyid ? $item->getID() : $item);
790793
}

0 commit comments

Comments
 (0)