Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Glpi/Inventory/Asset/Software.php
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,9 @@ private function populateSoftware()
'name' => new QueryParam(),
'manufacturers_id' => new QueryParam(),
],
// Prefer an active software over a trashed duplicate, deterministically (lowest id wins).
'ORDER' => ['is_deleted ASC', 'id ASC'],
'LIMIT' => 1,
];

$it = new DBmysqlIterator(null);
Expand Down Expand Up @@ -664,7 +667,7 @@ private function populateSoftware()
);
$results = $stmt->get_result();

while ($row = $results->fetch_object()) {
if ($row = $results->fetch_object()) {
$this->softwares[$key] = $row->id;
}
}
Expand Down
102 changes: 102 additions & 0 deletions tests/functional/Glpi/Inventory/Assets/SoftwareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,108 @@ public function testDuplicatedSoft()
$this->doInventory($xml_source, true);
}

public function testDuplicatedSoftPrefersActiveOverDeleted()
{
$this->login();

$soft = new \Software();
$version = new SoftwareVersion();

// First inventory: let GLPI create the software (and its manufacturer)
// exactly as production does, so it is guaranteed to be matched again by
// the inventory lookup later. This is the legitimate "active" entry and,
// being created first, it gets the lower id.
$xml_first = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>
<REQUEST>
<CONTENT>
<SOFTWARES>
<NAME>Duplicate Test Software</NAME>
<PUBLISHER>Duplicate Test Corp</PUBLISHER>
<VERSION>1.0.0</VERSION>
</SOFTWARES>
<HARDWARE>
<NAME>pc-dup-active</NAME>
</HARDWARE>
<BIOS>
<SSN>dupactive01</SSN>
</BIOS>
<VERSIONCLIENT>FusionInventory-Agent_v2.3.19</VERSIONCLIENT>
</CONTENT>
<DEVICEID>test-pc-dup-active</DEVICEID>
<QUERY>INVENTORY</QUERY>
</REQUEST>";

$this->doInventory($xml_first, true);

$softs = $soft->find(['name' => 'Duplicate Test Software']);
$this->assertCount(1, $softs, 'the first inventory must create exactly one software');
$active = array_pop($softs);
$active_softwares_id = (int) $active['id'];

// Duplicate created SECOND (higher id) with the very same identifying
// fields, then trashed (soft-deleted, not purged). Copying the fields
// from the inventory-created row guarantees it is a true competing match
// for the inventory lookup.
$deleted_softwares_id = $soft->add([
'name' => $active['name'],
'manufacturers_id' => $active['manufacturers_id'],
'entities_id' => $active['entities_id'],
'is_recursive' => $active['is_recursive'],
]);
$this->assertGreaterThan($active_softwares_id, $deleted_softwares_id);
$this->assertTrue((bool) $soft->delete(['id' => $deleted_softwares_id]));
$this->assertTrue($soft->getFromDB($deleted_softwares_id));
$this->assertEquals(1, $soft->fields['is_deleted']);

// Second inventory brings a NEW version. It must attach to the active
// software, not to the trashed duplicate.
$xml_second = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>
<REQUEST>
<CONTENT>
<SOFTWARES>
<NAME>Duplicate Test Software</NAME>
<PUBLISHER>Duplicate Test Corp</PUBLISHER>
<VERSION>2.0.0</VERSION>
</SOFTWARES>
<HARDWARE>
<NAME>pc-dup-active</NAME>
</HARDWARE>
<BIOS>
<SSN>dupactive01</SSN>
</BIOS>
<VERSIONCLIENT>FusionInventory-Agent_v2.3.19</VERSIONCLIENT>
</CONTENT>
<DEVICEID>test-pc-dup-active</DEVICEID>
<QUERY>INVENTORY</QUERY>
</REQUEST>";

$this->doInventory($xml_second, true);

$on_active = $version->find([
'name' => '2.0.0',
'softwares_id' => $active_softwares_id,
]);
$this->assertCount(
1,
$on_active,
'the new version/installation must attach to the active software, not the deleted duplicate'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertions only look at glpi_softwareversions; the message claims "version/installation" but no Item_SoftwareVersion row is checked. Adding one assertion on the installation would cover the part of the bug that actually affects the asset sheet.

$installs = (new Item_SoftwareVersion())->find(['softwareversions_id' => (int) current($on_active)['id']]);
$this->assertCount(1, $installs);

);

// The installation row (Item_SoftwareVersion) is what drives the asset
// sheet, so verify it too points at the active software's version, not
// only that the version record exists.
$installs = (new \Item_SoftwareVersion())->find([
'softwareversions_id' => (int) current($on_active)['id'],
]);
$this->assertCount(1, $installs, 'the installation must be linked to the active software version');

$on_deleted = $version->find([
'name' => '2.0.0',
'softwares_id' => $deleted_softwares_id,
]);
$this->assertCount(0, $on_deleted, 'the trashed duplicate must not receive the new version/installation');
}

public function testSameSoft()
{
global $DB;
Expand Down
Loading