composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-08-13 17:28:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

N°9831 - Fix ext. management deletion plan analysis by avoiding overwriting entity for a previously processed class

Browse Source
This commit is contained in:
Molkobain
2026-07-22 22:42:27 +02:00
parent 837b433622
commit c68887c2bc
4 changed files with 177 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php
View File

@@ -59,9 +59,11 @@ class StaticDeletionPlan
{
foreach ($aClasses as $sClass) {
$oDeletionPlanItem = $this->GetInitialClassDeletionPlan($sClass);
$oDeletionPlanEntity = new DeletionPlanEntity();
$oDeletionPlanEntity->oDelete->Merge($oDeletionPlanItem);
$this->aDeletionPlan[$sClass] = $oDeletionPlanEntity;
// N°9831 Do not overwrite existing entity as it may already exist for this class if a previously processed class references it (issues/updates already accumulated must be kept)
if (false === array_key_exists($sClass, $this->aDeletionPlan)) {
$this->aDeletionPlan[$sClass] = new DeletionPlanEntity();
}
$this->aDeletionPlan[$sClass]->oDelete->Merge($oDeletionPlanItem);
$this->DeletionPlanForReferencingClasses($sClass);
}

52
tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/DataCleanupServiceTest.php
View File

@@ -163,6 +163,58 @@ class DataCleanupServiceTest extends \AbstractCleanup
$this->AssertSummaryEquals($aExpected, $aRes);
}
/**
* N°9831
*
* A nullable external key configured as DEL_MANUAL must NOT be treated as an issue: it is simply
* reset (set to null), exactly like core DBObject::MakeDeletionPlan does. DEL_MANUAL only blocks
* for mandatory keys. This mirrors the real "Hypervisor.farm_id -> Farm" case (nullable + DEL_MANUAL).
*
* Regression guard for the ordering of the IsNullAllowed()/DEL_MANUAL checks in RecursiveDeletion:
* reverting that change would make ExecuteCleanup throw here instead of resetting the object.
*/
public function testExecuteCleanup_NullableManualShouldResetAndNotThrow()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRManualNullable_1
EOF);
$aClasses = [ 'DFRToRemoveLeaf' ];
$oService = new DataCleanupService();
// Must NOT throw: the nullable external key is reset, not reported as an issue
$aRes = $oService->ExecuteCleanup($aClasses);
$aExpected = [
['DFRManualNullable', 1, 0 ],
['DFRToRemoveLeaf', 0, 1 ],
];
$this->AssertSummaryEquals($aExpected, $aRes);
}
/**
* N°9831
*
* Summary counterpart of the test above: a nullable DEL_MANUAL external key is reported as an
* update (reset), with an issue count of 0. Reverting the RecursiveDeletion change would report
* it as an issue instead.
*/
public function testGetCleanupSummary_NullableManualShouldBeUpdateNotIssue()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRManualNullable_1
EOF);
$aClasses = [ 'DFRToRemoveLeaf' ];
$oService = new DataCleanupService();
$aRes = $oService->GetCleanupSummary($aClasses);
$aExpected = [
['DFRManualNullable', 1, 0, 0 ],
['DFRToRemoveLeaf', 0, 1, 0 ],
];
$this->AssertSummaryEquals($aExpected, $aRes);
}
private function AssertSummaryEquals(array $expected, $actual, $sMessage = '')
{
$aExpected = [];

61
tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php
View File

@@ -9,7 +9,6 @@ namespace Combodo\iTop\Test\UnitTest\Module\DataFeatureRemoval;
use Combodo\iTop\DataFeatureRemoval\Entity\DataCleanupSummaryEntity;
use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan;
use MetaModel;
require_once __DIR__."/AbstractCleanup.php";
class StaticDeletionPlanTest extends \AbstractCleanup
@@ -150,6 +149,66 @@ EOF);
$this->AssertSummaryEquals($aExpected, $aRes);
}
/**
* N°9831
*
* A DEL_MANUAL issue must still be reported when the referencing class (DFRManual) is itself part
* of the classes to remove AND is processed AFTER the class it points to (DFRToRemoveLeaf).
*
* This mirrors the real "VirtualMachine -> VirtualHost" case: the setup audit sorts the removed
* classes alphabetically and drops abstract classes, so a referencing class such as VirtualMachine
* ends up processed after its concrete targets (Farm/Hypervisor/Cloud). Here DFRToRemove is abstract
* and DFRToRemoveLeaf is its concrete child, while DFRManual points to DFRToRemove with a mandatory
* DEL_MANUAL external key.
*
* Regression: the plan entity accumulating the issue used to be overwritten when the referencing
* class was processed as a top-level class, silently dropping the issue (no blocking message on the
* summary page, then a crash at execution time).
*/
public function testGetCleanupSummary_IssueKeptWhenReferencingClassListedAfterTarget(): void
{
$this->GivenDFRObjectsInDB(<<<EOF
create DFRToRemoveLeaf (name = DFRToRemoveLeaf_1)
create DFRManual (name = DFRManual_1, extkey_id = DFRToRemoveLeaf_1)
EOF);
// Target listed BEFORE the referencing class, as produced by the alphabetically sorted audit
$aClasses = ['DFRToRemoveLeaf', 'DFRManual'];
$oService = new StaticDeletionPlan();
$aRes = $oService->GetCleanupSummary($aClasses);
$aExpected = [
'DFRToRemoveLeaf' => ['iDeleteCount' => 1],
'DFRManual' => ['iDeleteCount' => 1, 'iIssueCount' => 1],
];
$this->AssertSummaryEquals($aExpected, $aRes);
}
/**
* N°9831
*
* Control case: the same issue must also be reported when the referencing class is listed BEFORE
* its target (this order already worked, e.g. "Enclosure -> Rack"). Together with the test above,
* this locks in that issue detection is independent of the processing order.
*/
public function testGetCleanupSummary_IssueKeptWhenReferencingClassListedBeforeTarget(): void
{
$this->GivenDFRObjectsInDB(<<<EOF
create DFRToRemoveLeaf (name = DFRToRemoveLeaf_1)
create DFRManual (name = DFRManual_1, extkey_id = DFRToRemoveLeaf_1)
EOF);
$aClasses = ['DFRManual', 'DFRToRemoveLeaf'];
$oService = new StaticDeletionPlan();
$aRes = $oService->GetCleanupSummary($aClasses);
$aExpected = [
'DFRToRemoveLeaf' => ['iDeleteCount' => 1],
'DFRManual' => ['iDeleteCount' => 1, 'iIssueCount' => 1],
];
$this->AssertSummaryEquals($aExpected, $aRes);
}
public function testCircularRefsShouldNotRunInfinitely()
{
$this->GivenDFRObjectsInDB(<<<EOF

60
tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml
View File

@@ -319,6 +319,58 @@
</presentation>
<parent>cmdbAbstractObject</parent>
</class>
<class id="DFRManualNullable" _created_in="itop-structure" _delta="define">
<properties>
<category>bizmodel,searchable</category>
<abstract>false</abstract>
<db_table>dfrmanualnullable</db_table>
<naming>
<attributes/>
</naming>
<reconciliation>
<attributes/>
</reconciliation>
</properties>
<fields>
<field id="name" xsi:type="AttributeString">
<sql>name</sql>
<default_value/>
<is_null_allowed>true</is_null_allowed>
<validation_pattern/>
<dependencies/>
<tracking_level>all</tracking_level>
</field>
<field id="extkey_id" xsi:type="AttributeExternalKey">
<sql>extkey_id</sql>
<filter/>
<dependencies/>
<is_null_allowed>true</is_null_allowed>
<target_class>DFRToRemove</target_class>
<on_target_delete>DEL_MANUAL</on_target_delete>
<tracking_level>all</tracking_level>
</field>
</fields>
<methods/>
<presentation>
<list>
<items/>
</list>
<search>
<items/>
</search>
<details>
<items>
<item id="name">
<rank>10</rank>
</item>
<item id="extkey_id">
<rank>20</rank>
</item>
</items>
</details>
</presentation>
<parent>cmdbAbstractObject</parent>
</class>
<class id="DFRLeafNotToRemove" _created_in="itop-structure" _delta="define">
<properties>
<category>bizmodel,searchable</category>
@@ -718,6 +770,14 @@
<entry id="Class:DFRManual/Attribute:name+" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManual/Attribute:extkey_id" _delta="define"><![CDATA[Dfrtoremove id]]></entry>
<entry id="Class:DFRManual/Attribute:extkey_id+" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManualNullable/Name" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManualNullable/ComplementaryName" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManualNullable" _delta="define"><![CDATA[DFRManualNullable]]></entry>
<entry id="Class:DFRManualNullable+" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManualNullable/Attribute:name" _delta="define"><![CDATA[Name]]></entry>
<entry id="Class:DFRManualNullable/Attribute:name+" _delta="define"><![CDATA[]]></entry>
<entry id="Class:DFRManualNullable/Attribute:extkey_id" _delta="define"><![CDATA[Dfrtoremove id]]></entry>
<entry id="Class:DFRManualNullable/Attribute:extkey_id+" _delta="define"><![CDATA[]]></entry>
</entries>
</dictionary>
</dictionaries>