From 52fca750b00320968a957a86e2b22a22fd117446 Mon Sep 17 00:00:00 2001 From: Timmy38 Date: Wed, 22 Jul 2026 17:37:19 +0200 Subject: [PATCH 1/4] =?UTF-8?q?N=C2=B09541=20itop-virtualization-mgmt=20ca?= =?UTF-8?q?nnot=20currently=20be=20uninstalled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- datamodels/2.x/installation.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/datamodels/2.x/installation.xml b/datamodels/2.x/installation.xml index 2b5185b35..25dab5ab9 100755 --- a/datamodels/2.x/installation.xml +++ b/datamodels/2.x/installation.xml @@ -74,6 +74,7 @@ itop-virtualization-mgmt true + no From 837b433622c4049cfdf43494872df5fed11afdbb Mon Sep 17 00:00:00 2001 From: Molkobain Date: Wed, 22 Jul 2026 22:37:34 +0200 Subject: [PATCH 2/4] =?UTF-8?q?N=C2=B09831=20-=20Fix=20DataCleanupService?= =?UTF-8?q?=20so=20it=20handles=20external=20keys=20the=20same=20way=20as?= =?UTF-8?q?=20\Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan:?= =?UTF-8?q?:DeletionPlanForReferencingClasses=20and=20\DBObject::MakeDelet?= =?UTF-8?q?ionPlan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Check if ext. key is nullable first - Then check if its a DEL_MANUAL option --- .../src/Service/DataCleanupService.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Service/DataCleanupService.php b/datamodels/2.x/combodo-data-feature-removal/src/Service/DataCleanupService.php index e2d3956b7..180ef7af2 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Service/DataCleanupService.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Service/DataCleanupService.php @@ -134,10 +134,6 @@ class DataCleanupService /** @var DBObject $oDependentObj */ while ($oDependentObj = $oSet->Fetch()) { $iDeletePropagationOption = $oExtKeyAttDef->GetDeletionPropagationOption(); - if ($iDeletePropagationOption == DEL_MANUAL) { - $this->oObjectService->SetIssue(get_class($oDependentObj)); - continue; - } if ($oExtKeyAttDef->IsNullAllowed()) { // Optional external key, list to reset @@ -152,6 +148,12 @@ class DataCleanupService return false; } } else { + // Mandatory external key + if ($iDeletePropagationOption == DEL_MANUAL) { + // Cannot be deleted automatically, must be handled manually + $this->oObjectService->SetIssue(get_class($oDependentObj)); + continue; + } // Propagate deletion only if not visited if ($this->IsVisited($oDependentObj)) { continue; From c68887c2bc204dcda42ae90e6ad63b1a78bc5bfc Mon Sep 17 00:00:00 2001 From: Molkobain Date: Wed, 22 Jul 2026 22:42:27 +0200 Subject: [PATCH 3/4] =?UTF-8?q?N=C2=B09831=20-=20Fix=20ext.=20management?= =?UTF-8?q?=20deletion=20plan=20analysis=20by=20avoiding=20overwriting=20e?= =?UTF-8?q?ntity=20for=20a=20previously=20processed=20class?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Service/StaticDeletionPlan.php | 8 ++- .../DataCleanupServiceTest.php | 52 ++++++++++++++++ .../StaticDeletionPlanTest.php | 61 ++++++++++++++++++- .../data_cleanup_delta.xml | 60 ++++++++++++++++++ 4 files changed, 177 insertions(+), 4 deletions(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php b/datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php index 82cf65157..6c3b790c4 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php @@ -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); } diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/DataCleanupServiceTest.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/DataCleanupServiceTest.php index cbf3b54f2..9df032c77 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/DataCleanupServiceTest.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/DataCleanupServiceTest.php @@ -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(<<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(<<GetCleanupSummary($aClasses); + + $aExpected = [ + ['DFRManualNullable', 1, 0, 0 ], + ['DFRToRemoveLeaf', 0, 1, 0 ], + ]; + $this->AssertSummaryEquals($aExpected, $aRes); + } + private function AssertSummaryEquals(array $expected, $actual, $sMessage = '') { $aExpected = []; diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php index bd0e00a47..ca292f3ca 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php @@ -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(<<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(<<GetCleanupSummary($aClasses); + + $aExpected = [ + 'DFRToRemoveLeaf' => ['iDeleteCount' => 1], + 'DFRManual' => ['iDeleteCount' => 1, 'iIssueCount' => 1], + ]; + $this->AssertSummaryEquals($aExpected, $aRes); + } + public function testCircularRefsShouldNotRunInfinitely() { $this->GivenDFRObjectsInDB(<< cmdbAbstractObject + + + bizmodel,searchable + false + dfrmanualnullable + + + + + + + + + + name + + true + + + all + + + extkey_id + + + true + DFRToRemove + DEL_MANUAL + all + + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + cmdbAbstractObject + bizmodel,searchable @@ -718,6 +770,14 @@ + + + + + + + + From 76a410e00de423643f60392cbe0979f6e605c58b Mon Sep 17 00:00:00 2001 From: Molkobain Date: Thu, 23 Jul 2026 10:44:54 +0200 Subject: [PATCH 4/4] =?UTF-8?q?Revert=20"N=C2=B09541=20itop-virtualization?= =?UTF-8?q?-mgmt=20cannot=20currently=20be=20uninstalled"=20now=20that=20N?= =?UTF-8?q?=C2=B09831=20has=20been=20fixed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 52fca750b00320968a957a86e2b22a22fd117446. --- datamodels/2.x/installation.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/datamodels/2.x/installation.xml b/datamodels/2.x/installation.xml index 25dab5ab9..2b5185b35 100755 --- a/datamodels/2.x/installation.xml +++ b/datamodels/2.x/installation.xml @@ -74,7 +74,6 @@ itop-virtualization-mgmt true - no