From 41d4f04375ce49af7445d49f1228448c3ff3dd7b Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 08:33:40 +0200 Subject: [PATCH 01/15] =?UTF-8?q?N=C2=B09639=20-=20Uninstallation=20Analys?= =?UTF-8?q?is=20better=20count=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 4 +- .../DataFeatureRemovalController.php | 2 + .../src/Service/StaticDeletionPlan.php | 163 ++++++++++++++++++ .../vendor/composer/autoload_classmap.php | 1 + .../vendor/composer/autoload_static.php | 1 + .../unattended-install/xml_setup/upgrade.xml | 2 +- setup/wizard.php | 2 - setup/wizardcontroller.class.inc.php | 1 - setup/wizardsteps/WizardStep.php | 4 - .../AbstractCleanup.php | 56 ++++++ .../DataCleanupServiceTest.php | 37 +--- .../StaticDeletionPlanTest.php | 102 +++++++++++ .../data_cleanup_delta.xml | 96 ++++++++--- .../simulate-audit-from-setup.php | 2 +- 14 files changed, 405 insertions(+), 68 deletions(-) create mode 100644 datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php create mode 100644 tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php create mode 100644 tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 8b17a931db..f5698f647c 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -916,7 +916,9 @@ public static function QueryToCol($sSql, $col) $aColumn = []; $aData = self::QueryToArray($sSql); foreach ($aData as $aRow) { - @$aColumn[] = $aRow[$col]; + if ($aRow[$col] !== null) { + $aColumn[] = $aRow[$col]; + } } return $aColumn; } diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php index 1bbe4f0191..299c47cfb3 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php @@ -18,6 +18,7 @@ use Combodo\iTop\DataFeatureRemoval\Helper\DataFeatureRemovalLog; use Combodo\iTop\DataFeatureRemoval\Service\DataCleanupService; use Combodo\iTop\DataFeatureRemoval\Service\DataFeatureRemoverExtensionService; +use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; use ContextTag; @@ -168,6 +169,7 @@ public function OperationAnalysisResult(): void $oSetupAudit = new SetupAudit($sSourceEnv); $aGetRemovedClasses = array_keys($oSetupAudit->RunDataAudit()); DataFeatureRemovalLog::Debug(__METHOD__, null, ['aGetRemovedClasses' => $aGetRemovedClasses]); + $aDeletionPlan = (new StaticDeletionPlan())->GetStaticDeletionPlan($aGetRemovedClasses); $aParams['aClasses'] = $aGetRemovedClasses; 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 new file mode 100644 index 0000000000..28047d8723 --- /dev/null +++ b/datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php @@ -0,0 +1,163 @@ + [ + * 'delete' => [ids], + * 'delete_sql' => string, + * 'update_extkey_nullable' => [ids], + * 'update_extkey_nullable_sql' => [sSQL], + * 'update_hierarchical' => [ids], + * 'update_hierarchical_sql' => [sSQL], + * 'issue' => [id], + * ]]; + * + * @throws \CoreException + */ + public function GetStaticDeletionPlan(array $aClasses): array + { + foreach ($aClasses as $sClass) { + [$sDeleteSQL, $aIds] = $this->GetInitialClassDeletionPlan($sClass); + $this->aDeletionPlan[$sClass] = [ + 'delete' => $aIds, + 'delete_sql' => $sDeleteSQL, + ]; + + $this->DeletionPlanForReferencingClasses($sClass); + } + + return $this->aDeletionPlan; + } + + private function DeletionPlanForReferencingClasses(string $sClass): void + { + $sIdsToRemove = implode(', ', $this->aDeletionPlan[$sClass]['delete']); + $aReferencingMe = MetaModel::EnumReferencingClasses($sClass); + foreach ($aReferencingMe as $sRemoteClass => $aExtKeys) { + $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); + /** @var \AttributeExternalKey $oExtKeyAttDef */ + foreach ($aExtKeys as $sExtKeyAttCode => $oExtKeyAttDef) { + // skip if this external key is behind an external field + if (!$oExtKeyAttDef->IsExternalKey(EXTKEY_ABSOLUTE)) { + continue; + } + + if ($oExtKeyAttDef->IsNullAllowed()) { + // update + [$sUpdateSQL, $aIds] = $this->UpdateExtKeyNullable($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable_sql'][$sExtKeyAttCode] = $sUpdateSQL; + $this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable'] ?? [], $aIds)); + } else { + // delete + $aRemoteIdsToRemove = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + + $iDeletePropagationOption = $oExtKeyAttDef->GetDeletionPropagationOption(); + if ($iDeletePropagationOption == DEL_MANUAL) { + // Issue, do not recurse + if (count($aRemoteIdsToRemove) > 0) { + $this->aDeletionPlan[$sRemoteClass]['issue'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['issue'] ?? [], $aRemoteIdsToRemove)); + } + continue; + } + + if (($iDeletePropagationOption == DEL_MOVEUP) && ($oExtKeyAttDef->IsHierarchicalKey())) { + // update hierarchical keys due to row cleanup in the same table + $sIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]['delete']); + [$sUpdateSQL, $aIds] = $this->UpdateHierarchicalExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $this->aDeletionPlan[$sRemoteClass]['update_hierarchical_sql'][$sExtKeyAttCode] = $sUpdateSQL; + $this->aDeletionPlan[$sRemoteClass]['update_hierarchical'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['update_hierarchical'] ?? [], $aIds)); + // do not recurse + continue; + } + + // Delete entries in Remote Class + $this->aDeletionPlan[$sRemoteClass]['delete'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['delete'] ?? [], $aRemoteIdsToRemove)); + $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); + $this->aDeletionPlan[$sRemoteClass]['delete_sql'] = "DELETE FROM $sRemoteTable WHERE id IN ($sRemoteIdsToDelete)"; + + $this->DeletionPlanForReferencingClasses($sRemoteClass); + } + } + } + } + + /** + * @param string $sRemoteTable + * @param string $sExtKeyAttCode + * @param string $sIdsToRemoveInTargetClass + * + * @return array + */ + public function UpdateExtKeyNullable(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array + { + $aIds = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemoveInTargetClass); + + $sUpdateSQL = << $baseDir . '/src/Service/DataFeatureRemoverExtensionService.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\ObjectService' => $baseDir . '/src/Service/ObjectService.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\ObjectServiceSummary' => $baseDir . '/src/Service/ObjectServiceSummary.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Service\\StaticDeletionPlan' => $baseDir . '/src/Service/StaticDeletionPlan.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\iObjectService' => $baseDir . '/src/Service/iObjectService.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', ); diff --git a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php index 94e5a3715f..06de56135f 100644 --- a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php +++ b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php @@ -31,6 +31,7 @@ class ComposerStaticInit4f96a7199e2c0d90e547333758b26464 'Combodo\\iTop\\DataFeatureRemoval\\Service\\DataFeatureRemoverExtensionService' => __DIR__ . '/../..' . '/src/Service/DataFeatureRemoverExtensionService.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\ObjectService' => __DIR__ . '/../..' . '/src/Service/ObjectService.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\ObjectServiceSummary' => __DIR__ . '/../..' . '/src/Service/ObjectServiceSummary.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Service\\StaticDeletionPlan' => __DIR__ . '/../..' . '/src/Service/StaticDeletionPlan.php', 'Combodo\\iTop\\DataFeatureRemoval\\Service\\iObjectService' => __DIR__ . '/../..' . '/src/Service/iObjectService.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', ); diff --git a/setup/unattended-install/xml_setup/upgrade.xml b/setup/unattended-install/xml_setup/upgrade.xml index 0272bd2551..bef34398af 100644 --- a/setup/unattended-install/xml_setup/upgrade.xml +++ b/setup/unattended-install/xml_setup/upgrade.xml @@ -41,5 +41,5 @@ - off + on diff --git a/setup/wizard.php b/setup/wizard.php index b1f6e4c0dd..d1c7040ce7 100644 --- a/setup/wizard.php +++ b/setup/wizard.php @@ -74,7 +74,6 @@ function json_decode($json, $assoc = null) // The configuration file already exists if (!is_writable($sConfigFile)) { SetupUtils::ExitReadOnlyMode(false); // Reset readonly mode in case of problem - SetupUtils::EraseSetupToken(); $sRelativePath = utils::GetConfigFilePathRelative(ITOP_DEFAULT_ENV); $oP = new SetupPage('Installation Cannot Continue'); $oP->add("

Fatal error

\n"); @@ -87,7 +86,6 @@ function json_decode($json, $assoc = null) $oP->p($sButtonsHtml); $oP->output(); - // Prevent token creation exit; } else { chmod($sConfigFile, 0440); diff --git a/setup/wizardcontroller.class.inc.php b/setup/wizardcontroller.class.inc.php index 3015b35a69..76ba0bd587 100644 --- a/setup/wizardcontroller.class.inc.php +++ b/setup/wizardcontroller.class.inc.php @@ -196,7 +196,6 @@ protected function DisplayStep(WizardStep $oStep): void SetupLog::Info("=== Setup screen: ".$oStep->GetTitle().' ('.get_class($oStep).')'); $oPage = new SetupPage($oStep->GetTitle()); $oPage->LinkScriptFromAppRoot('setup/setup.js'); - $oStep->PreFormDisplay($oPage); $oPage->add('
'); $oPage->add('
'); diff --git a/setup/wizardsteps/WizardStep.php b/setup/wizardsteps/WizardStep.php index cc9f2190d8..a953c52756 100644 --- a/setup/wizardsteps/WizardStep.php +++ b/setup/wizardsteps/WizardStep.php @@ -76,10 +76,6 @@ public function PostFormDisplay(SetupPage $oPage) { } - public function PreFormDisplay(SetupPage $oPage) - { - } - protected function CheckDependencies() { if (is_null($this->bDependencyCheck)) { diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php new file mode 100644 index 0000000000..61891dc5d9 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php @@ -0,0 +1,56 @@ +aIdByClass = []; + $aTree = explode("\n", $sTree); + foreach ($aTree as $sLine) { + if (trim($sLine) === '') { + continue; + } + $this->GivenDFRTreeLineInDB($sLine); + } + } + + protected function GivenDFRTreeLineInDB(string $sLine) + { + [$sLeft, $sRight] = explode('<-', $sLine); + $sLeft = trim($sLeft); + + $iLeftId = $this->aIdByObjectName[$sLeft] ?? 0; + if ($iLeftId === 0) { + [$sChildClass] = explode('_', $sLeft, 2); + $iLeftId = $this->GivenObjectInDB($sChildClass, ['name' => $sLeft]); + $this->aIdByClass[$sChildClass][] = $iLeftId; + $this->aIdByObjectName[$sLeft] = $iLeftId; + } + + $sRight = trim($sRight); + if (preg_match("/(?(?[^_]+)_\d+)(\s+\((?\w+)\))?/", $sRight, $aMatches) !== false) { + } + + [$sChildClass] = explode('_', $sRight, 2); + + $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sRight, 'extkey_id' => $iLeftId]); + $this->aIdByClass[$sChildClass][] = $iRightId; + $this->aIdByObjectName[$sRight] = $iRightId; + } +} 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 6acb5f026c..0d3d04062c 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 @@ -34,15 +34,10 @@ * @see DataCleanupSummaryEntity * @see ItopDataTestCase */ -class DataCleanupServiceTest extends ItopCustomDatamodelTestCase +class DataCleanupServiceTest extends \AbstractCleanup { private ExecutionLimits&MockObject $oExecutionLimits; - public function GetDatamodelDeltaAbsPath(): string - { - return __DIR__.'/data_cleanup_delta.xml'; - } - //--- GetCleanupSummary tests --- /** @@ -261,36 +256,6 @@ public function testExecuteCleanup_StopInProcessKeepDatabaseOk(int $iExecutionCo $this->AssertSummaryEquals($aExpected, $aRes); } - private function GivenDFRTreeInDB(string $sTree) - { - $aTree = explode("\n", $sTree); - foreach ($aTree as $sLine) { - if (trim($sLine) === "") { - continue; - } - $this->GivenDFRTreeLineInDB($sLine); - } - } - - private array $aIdByObjectName = []; - private function GivenDFRTreeLineInDB(string $sLine) - { - [$sLeft, $sRight] = explode('<-', $sLine); - $sLeft = trim($sLeft); - - $iLeftId = $this->aIdByObjectName[$sLeft] ?? 0; - if ($iLeftId === 0) { - [$sChildClass, ] = explode('_', $sLeft, 2); - $iLeftId = $this->GivenObjectInDB($sChildClass, ['name' => $sLeft]); - $this->aIdByObjectName[$sLeft] = $iLeftId; - } - - $sRight = trim($sRight); - [$sChildClass, ] = explode('_', $sRight, 2); - $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sRight, 'extkey_id' => $iLeftId]); - $this->aIdByObjectName[$sRight] = $iRightId; - } - private function GivenExecutionLimits(int $iStopAfterCallNumberReached): void { $matcher = $this->any(); 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 new file mode 100644 index 0000000000..5afed8bcbc --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php @@ -0,0 +1,102 @@ +GivenDFRTreeInDB(<<GetInitialClassDeletionPlan('DFRToRemoveLeaf'); + self::assertCount(2, $aRes[1]); + self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $aRes[1]); + $sTable = MetaModel::DBGetTable('DFRToRemoveLeaf'); + $sExpectedSQL = "DELETE FROM $sTable"; + self::assertEquals($sExpectedSQL, $aRes[0]); + } + + public function testUpdateExtKeyNullable() + { + $this->GivenDFRTreeInDB(<<UpdateExtKeyNullable( + $sRemoteTable, + 'extkey_id', + implode(',', $this->aIdByClass['DFRToRemoveLeaf']) + ); + $sUpdateSQL = $aRes[0]; + $aIds = $aRes[1]; + + // THEN + $sExpectedSQLEnd = " IN (".implode(',', $this->aIdByClass['DFRToRemoveLeaf']).")"; + self::assertStringEndsWith($sExpectedSQLEnd, $sUpdateSQL); + + self::assertCount(3, $aIds); + $sIdsToRemoveInTargetClass = implode(',', $this->aIdByClass['DFRToRemoveLeaf']); + $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteTable, 'extkey_id', $sIdsToRemoveInTargetClass); + + self::assertEquals($aExpectedIds, $aIds); + + // var_export($aRes); + // var_export($this->aIdByClass); + } + + /** + * Tests that GetCleanupSummary returns an empty array when passed null as input. + */ + public function testGetCleanupSummaryReturnsEmptyArrayWhenNull(): void + { + $oService = new StaticDeletionPlan(); + $aResult = $oService->GetStaticDeletionPlan([]); + + $this->assertIsArray($aResult, 'Expected result to be an array when input is null.'); + $this->assertEmpty($aResult, 'Expected result to be empty array when input is null.'); + } + + public function testExecuteCleanup_DeleteOneObjPerClass() + { + $this->GivenDFRTreeInDB(<<GetStaticDeletionPlan($aClasses); + + var_export($aRes); + + var_export($this->aIdByClass); + } +} diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml index a338e1fbc5..9a551fd43b 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml @@ -71,6 +71,15 @@ DEL_AUTO all + + extkey2_id + + + true + DFRToRemove + DEL_AUTO + all + @@ -145,11 +154,11 @@ cmdbAbstractObject - + bizmodel,searchable false - dfrtoremoveleaf + dfrremovedcollateralcascade @@ -158,14 +167,23 @@ - - desc + + name true all + + extkey_id + + + false + DFRRemovedCollateral + DEL_AUTO + all + @@ -180,19 +198,19 @@ 10 - + 20 - DFRToRemove + cmdbAbstractObject - + bizmodel,searchable false - dfrremovedcollateralcascade + dfrmanual @@ -214,8 +232,8 @@ false - DFRRemovedCollateral - DEL_AUTO + DFRToRemove + DEL_MANUAL all @@ -240,11 +258,11 @@ cmdbAbstractObject - + bizmodel,searchable false - dfrmanual + dfrleafnottoremove @@ -253,21 +271,55 @@ - - name + + info true all - - extkey_id - + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + DFRToRemove + + + + bizmodel,searchable + false + dfrtoremoveleaf + + + + + + + + + + desc + + true + - false - DFRToRemove - DEL_MANUAL all @@ -284,13 +336,13 @@ 10 - + 20 - cmdbAbstractObject + DFRToRemove diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php index e48d3f2d1e..9ff9c1bc0d 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php @@ -59,7 +59,7 @@ function GetLastestInstallFile(): ?string } } -$sRemovedExtensions = utils::ReadParam('removed_modules', '', false, 'raw'); +$sRemovedExtensions = utils::ReadParam('removed_modules', 'itop-container-mgmt', false, 'raw'); $aRemovedExtensionsAndModules = []; if (mb_strlen($sRemovedExtensions) > 0) { $aRemovedExtensionsAndModules = explode(',', $sRemovedExtensions); From fc9df64eeaa3d7079f8a9df6f258c31576b2737f Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 08:34:55 +0200 Subject: [PATCH 02/15] =?UTF-8?q?N=C2=B09639=20-=20Fix=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/unattended-install/xml_setup/upgrade.xml | 2 +- .../combodo-data-feature-removal/StaticDeletionPlanTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/setup/unattended-install/xml_setup/upgrade.xml b/setup/unattended-install/xml_setup/upgrade.xml index bef34398af..0272bd2551 100644 --- a/setup/unattended-install/xml_setup/upgrade.xml +++ b/setup/unattended-install/xml_setup/upgrade.xml @@ -41,5 +41,5 @@ - on + off 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 5afed8bcbc..406d855a8b 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 @@ -7,8 +7,6 @@ * @license http://opensource.org/licenses/AGPL-3.0 */ -use CMDBSource; -use Combodo\iTop\DataFeatureRemoval\Service\DataCleanupService; use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan; use MetaModel; @@ -98,5 +96,7 @@ public function testExecuteCleanup_DeleteOneObjPerClass() var_export($aRes); var_export($this->aIdByClass); + + self::assertTrue(true); } } From bcb42fdc8145180052bdc93c002b8dd2ea4a9fad Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 11:17:51 +0200 Subject: [PATCH 03/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20deletion=20p?= =?UTF-8?q?lan=20handling=20with=20new=20entity=20classes=20for=20improved?= =?UTF-8?q?=20structure=20and=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../DataFeatureRemovalController.php | 2 +- .../src/Entity/DeletionPlanEntity.php | 27 +++++ .../src/Entity/DeletionPlanItem.php | 35 ++++++ .../src/Service/StaticDeletionPlan.php | 104 +++++++++++------- .../vendor/composer/autoload_classmap.php | 2 + .../vendor/composer/autoload_static.php | 2 + .../unattended-install/xml_setup/upgrade.xml | 2 +- .../AbstractCleanup.php | 14 +-- .../StaticDeletionPlanTest.php | 90 ++++++++++++--- 9 files changed, 219 insertions(+), 59 deletions(-) create mode 100644 datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php create mode 100644 datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php index 299c47cfb3..28065f2cdd 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php @@ -274,7 +274,7 @@ private function GetExecutionSummaryTable(): array private function GetDeletionPlanSummaryTable(array $aRemovedClasses): array { $sName = 'DeletionPlanSummary'; - $oDataCleanupService = new DataCleanupService(); + $oDataCleanupService = new StaticDeletionPlan(); $aDeletionPlanSummaryEntities = $oDataCleanupService->GetCleanupSummary($aRemovedClasses); $aColumns = ['Class', 'Delete Count' , 'Update Count', 'Issue Count']; $aRows = []; diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php new file mode 100644 index 0000000000..debc726abf --- /dev/null +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php @@ -0,0 +1,27 @@ +oDelete = $oDelete ?? new DeletionPlanItem(); + $this->oUpdate = $oUpdate ?? new DeletionPlanItem(); + $this->oIssue = $oIssue ?? new DeletionPlanItem(); + } +} diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php new file mode 100644 index 0000000000..81de7a261b --- /dev/null +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php @@ -0,0 +1,35 @@ +aQueries = $aQueries; + $this->aIds = $aIds; + } + + public function Merge(DeletionPlanItem $oItem): void + { + $this->aQueries = array_merge($this->aQueries, $oItem->aQueries); + $this->aIds = array_unique(array_merge($this->aIds, $oItem->aIds)); + } + + public function Count(): int + { + return count($this->aIds); + } +} 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 28047d8723..e35ee088e5 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 @@ -8,35 +8,59 @@ namespace Combodo\iTop\DataFeatureRemoval\Service; use CMDBSource; +use Combodo\iTop\DataFeatureRemoval\Entity\DataCleanupSummaryEntity; +use Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanEntity; +use Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem; use MetaModel; class StaticDeletionPlan { + /** @var array */ private array $aDeletionPlan = []; + /** + * Get a summary of the deletion plan computed for the classes. + * The result is used for display + * + * @param array|null $aClasses + * + * @return array<\Combodo\iTop\DataFeatureRemoval\Entity\DataCleanupSummaryEntity> + * @throws \CoreException + * @throws \CoreUnexpectedValue + * @throws \MySQLException + * @throws \Combodo\iTop\DataFeatureRemoval\Helper\DataFeatureRemovalException + */ + public function GetCleanupSummary(?array $aClasses): array + { + $aSummary = []; + $aDeletionPlan = $this->GetStaticDeletionPlan($aClasses ?? []); + + foreach ($aDeletionPlan as $sClass => $oDeletionPlanEntity) { + $oDataCleanupSummary = new DataCleanupSummaryEntity($sClass); + $oDataCleanupSummary->iUpdateCount = $oDeletionPlanEntity->oUpdate->Count(); + $oDataCleanupSummary->iDeleteCount = $oDeletionPlanEntity->oDelete->Count(); + $oDataCleanupSummary->iIssueCount = $oDeletionPlanEntity->oIssue->Count(); + + $aSummary[$sClass] = $oDataCleanupSummary; + } + + return $aSummary; + } + /** * @param array $aClasses Classes to clean entirely * - * @return array ['class' => [ - * 'delete' => [ids], - * 'delete_sql' => string, - * 'update_extkey_nullable' => [ids], - * 'update_extkey_nullable_sql' => [sSQL], - * 'update_hierarchical' => [ids], - * 'update_hierarchical_sql' => [sSQL], - * 'issue' => [id], - * ]]; + * @return array ['class' => DeletionPlanEntity]; * * @throws \CoreException */ public function GetStaticDeletionPlan(array $aClasses): array { foreach ($aClasses as $sClass) { - [$sDeleteSQL, $aIds] = $this->GetInitialClassDeletionPlan($sClass); - $this->aDeletionPlan[$sClass] = [ - 'delete' => $aIds, - 'delete_sql' => $sDeleteSQL, - ]; + $oDeletionPlanItem = $this->GetInitialClassDeletionPlan($sClass); + $oDeletionPlanEntity = new DeletionPlanEntity(); + $oDeletionPlanEntity->oDelete->Merge($oDeletionPlanItem); + $this->aDeletionPlan[$sClass] = $oDeletionPlanEntity; $this->DeletionPlanForReferencingClasses($sClass); } @@ -46,10 +70,14 @@ public function GetStaticDeletionPlan(array $aClasses): array private function DeletionPlanForReferencingClasses(string $sClass): void { - $sIdsToRemove = implode(', ', $this->aDeletionPlan[$sClass]['delete']); + $sIdsToRemove = implode(', ', $this->aDeletionPlan[$sClass]->oDelete->aIds); $aReferencingMe = MetaModel::EnumReferencingClasses($sClass); foreach ($aReferencingMe as $sRemoteClass => $aExtKeys) { $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); + if (!isset($this->aDeletionPlan[$sRemoteClass])) { + $this->aDeletionPlan[$sRemoteClass] = new DeletionPlanEntity(); + } + $oDeletionPlanEntity = $this->aDeletionPlan[$sRemoteClass]; /** @var \AttributeExternalKey $oExtKeyAttDef */ foreach ($aExtKeys as $sExtKeyAttCode => $oExtKeyAttDef) { // skip if this external key is behind an external field @@ -59,9 +87,8 @@ private function DeletionPlanForReferencingClasses(string $sClass): void if ($oExtKeyAttDef->IsNullAllowed()) { // update - [$sUpdateSQL, $aIds] = $this->UpdateExtKeyNullable($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); - $this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable_sql'][$sExtKeyAttCode] = $sUpdateSQL; - $this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['update_extkey_nullable'] ?? [], $aIds)); + $oUpdateItem = $this->UpdateExtKeyNullable($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $oDeletionPlanEntity->oUpdate->Merge($oUpdateItem); } else { // delete $aRemoteIdsToRemove = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); @@ -69,28 +96,28 @@ private function DeletionPlanForReferencingClasses(string $sClass): void $iDeletePropagationOption = $oExtKeyAttDef->GetDeletionPropagationOption(); if ($iDeletePropagationOption == DEL_MANUAL) { // Issue, do not recurse - if (count($aRemoteIdsToRemove) > 0) { - $this->aDeletionPlan[$sRemoteClass]['issue'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['issue'] ?? [], $aRemoteIdsToRemove)); - } + $oDeletionPlanItem = new DeletionPlanItem(aIds: $aRemoteIdsToRemove); + $oDeletionPlanEntity->oIssue->Merge($oDeletionPlanItem); continue; } if (($iDeletePropagationOption == DEL_MOVEUP) && ($oExtKeyAttDef->IsHierarchicalKey())) { // update hierarchical keys due to row cleanup in the same table - $sIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]['delete']); - [$sUpdateSQL, $aIds] = $this->UpdateHierarchicalExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); - $this->aDeletionPlan[$sRemoteClass]['update_hierarchical_sql'][$sExtKeyAttCode] = $sUpdateSQL; - $this->aDeletionPlan[$sRemoteClass]['update_hierarchical'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['update_hierarchical'] ?? [], $aIds)); + $sIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]->oDelete->aIds); + $oUpdateItem = $this->UpdateHierarchicalExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $oDeletionPlanEntity->oUpdate->Merge($oUpdateItem); // do not recurse continue; } // Delete entries in Remote Class - $this->aDeletionPlan[$sRemoteClass]['delete'] = array_unique(array_merge($this->aDeletionPlan[$sRemoteClass]['delete'] ?? [], $aRemoteIdsToRemove)); - $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); - $this->aDeletionPlan[$sRemoteClass]['delete_sql'] = "DELETE FROM $sRemoteTable WHERE id IN ($sRemoteIdsToDelete)"; + if (count($aRemoteIdsToRemove) !== 0) { + $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); + $sSQL = "DELETE FROM $sRemoteTable WHERE id IN ($sRemoteIdsToDelete)"; + $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem([$sSQL], $aRemoteIdsToRemove)); - $this->DeletionPlanForReferencingClasses($sRemoteClass); + $this->DeletionPlanForReferencingClasses($sRemoteClass); + } } } } @@ -101,9 +128,9 @@ private function DeletionPlanForReferencingClasses(string $sClass): void * @param string $sExtKeyAttCode * @param string $sIdsToRemoveInTargetClass * - * @return array + * @return \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem */ - public function UpdateExtKeyNullable(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array + public function UpdateExtKeyNullable(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { $aIds = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemoveInTargetClass); @@ -113,10 +140,10 @@ public function UpdateExtKeyNullable(string $sRemoteTable, string $sExtKeyAttCod WHERE updated.$sExtKeyAttCode IN ($sIdsToRemoveInTargetClass) SQL; - return [$sUpdateSQL, $aIds]; + return new DeletionPlanItem([$sExtKeyAttCode => $sUpdateSQL], $aIds); } - public function UpdateHierarchicalExtKey(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array + public function UpdateHierarchicalExtKey(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { $sUpdateSQL = << $sUpdateSQL], $aIds); } public function GetRemoteIdsForExtKey(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array { + if (\utils::IsNullOrEmptyString($sIdsToRemoveInTargetClass)) { + return []; + } $sSQL = "SELECT id FROM $sRemoteTable WHERE $sExtKeyAttCode IN ($sIdsToRemoveInTargetClass)"; return CMDBSource::QueryToCol($sSQL, 'id'); @@ -146,18 +176,18 @@ public function GetRemoteIdsForExtKey(string $sRemoteTable, string $sExtKeyAttCo /** * @param string $sClass * - * @return array + * @return \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem * @throws \CoreException * @throws \MySQLException */ - public function GetInitialClassDeletionPlan(string $sClass): array + public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem { $sTable = MetaModel::DBGetTable($sClass); $sSQL = "SELECT id FROM $sTable"; $aIds = CMDBSource::QueryToCol($sSQL, 'id'); $sDeleteSQL = "DELETE FROM $sTable"; - return [$sDeleteSQL, $aIds]; + return new DeletionPlanItem([$sDeleteSQL], $aIds); } } diff --git a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_classmap.php b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_classmap.php index 6527138d59..b8d4886620 100644 --- a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_classmap.php +++ b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_classmap.php @@ -8,6 +8,8 @@ return array( 'Combodo\\iTop\\DataFeatureRemoval\\Controller\\DataFeatureRemovalController' => $baseDir . '/src/Controller/DataFeatureRemovalController.php', 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DataCleanupSummaryEntity' => $baseDir . '/src/Entity/DataCleanupSummaryEntity.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DeletionPlanEntity' => $baseDir . '/src/Entity/DeletionPlanEntity.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DeletionPlanItem' => $baseDir . '/src/Entity/DeletionPlanItem.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalConfig' => $baseDir . '/src/Helper/DataFeatureRemovalConfig.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalException' => $baseDir . '/src/Helper/DataFeatureRemovalException.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalHelper' => $baseDir . '/src/Helper/DataFeatureRemovalHelper.php', diff --git a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php index 06de56135f..a86266cd44 100644 --- a/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php +++ b/datamodels/2.x/combodo-data-feature-removal/vendor/composer/autoload_static.php @@ -23,6 +23,8 @@ class ComposerStaticInit4f96a7199e2c0d90e547333758b26464 public static $classMap = array ( 'Combodo\\iTop\\DataFeatureRemoval\\Controller\\DataFeatureRemovalController' => __DIR__ . '/../..' . '/src/Controller/DataFeatureRemovalController.php', 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DataCleanupSummaryEntity' => __DIR__ . '/../..' . '/src/Entity/DataCleanupSummaryEntity.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DeletionPlanEntity' => __DIR__ . '/../..' . '/src/Entity/DeletionPlanEntity.php', + 'Combodo\\iTop\\DataFeatureRemoval\\Entity\\DeletionPlanItem' => __DIR__ . '/../..' . '/src/Entity/DeletionPlanItem.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalConfig' => __DIR__ . '/../..' . '/src/Helper/DataFeatureRemovalConfig.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalException' => __DIR__ . '/../..' . '/src/Helper/DataFeatureRemovalException.php', 'Combodo\\iTop\\DataFeatureRemoval\\Helper\\DataFeatureRemovalHelper' => __DIR__ . '/../..' . '/src/Helper/DataFeatureRemovalHelper.php', diff --git a/setup/unattended-install/xml_setup/upgrade.xml b/setup/unattended-install/xml_setup/upgrade.xml index 0272bd2551..bef34398af 100644 --- a/setup/unattended-install/xml_setup/upgrade.xml +++ b/setup/unattended-install/xml_setup/upgrade.xml @@ -41,5 +41,5 @@ - off + on diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php index 61891dc5d9..cc292574b3 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php @@ -6,7 +6,6 @@ */ use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; -use Combodo\iTop\Test\UnitTest\ItopDataTestCase; class AbstractCleanup extends ItopCustomDatamodelTestCase { @@ -45,12 +44,13 @@ protected function GivenDFRTreeLineInDB(string $sLine) $sRight = trim($sRight); if (preg_match("/(?(?[^_]+)_\d+)(\s+\((?\w+)\))?/", $sRight, $aMatches) !== false) { - } - - [$sChildClass] = explode('_', $sRight, 2); + $sName = $aMatches['name']; + $sChildClass = $aMatches['class']; + $sExtKey = $aMatches['extkey'] ?? 'extkey_id'; - $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sRight, 'extkey_id' => $iLeftId]); - $this->aIdByClass[$sChildClass][] = $iRightId; - $this->aIdByObjectName[$sRight] = $iRightId; + $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sName, $sExtKey => $iLeftId]); + $this->aIdByClass[$sChildClass][] = $iRightId; + $this->aIdByObjectName[$sRight] = $iRightId; + } } } 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 406d855a8b..5f88460d1f 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 @@ -7,6 +7,8 @@ * @license http://opensource.org/licenses/AGPL-3.0 */ +use Combodo\iTop\DataFeatureRemoval\Helper\DataFeatureRemovalException; +use Combodo\iTop\DataFeatureRemoval\Service\DataCleanupService; use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan; use MetaModel; @@ -26,12 +28,12 @@ public function testGetInitialClassDeletionPlan() EOF); $oService = new StaticDeletionPlan(); - $aRes = $oService->GetInitialClassDeletionPlan('DFRToRemoveLeaf'); - self::assertCount(2, $aRes[1]); - self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $aRes[1]); + $oDeletionPlanItem = $oService->GetInitialClassDeletionPlan('DFRToRemoveLeaf'); + self::assertEquals(2, $oDeletionPlanItem->Count()); + self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $oDeletionPlanItem->aIds); $sTable = MetaModel::DBGetTable('DFRToRemoveLeaf'); $sExpectedSQL = "DELETE FROM $sTable"; - self::assertEquals($sExpectedSQL, $aRes[0]); + self::assertEquals($sExpectedSQL, $oDeletionPlanItem->aQueries[0]); } public function testUpdateExtKeyNullable() @@ -46,23 +48,22 @@ public function testUpdateExtKeyNullable() // WHEN $oService = new StaticDeletionPlan(); $sRemoteTable = MetaModel::DBGetTable('DFRToUpdate'); - $aRes = $oService->UpdateExtKeyNullable( + $oDeletionPlanItem = $oService->UpdateExtKeyNullable( $sRemoteTable, 'extkey_id', implode(',', $this->aIdByClass['DFRToRemoveLeaf']) ); - $sUpdateSQL = $aRes[0]; - $aIds = $aRes[1]; + $sUpdateSQL = $oDeletionPlanItem->aQueries['extkey_id']; // THEN $sExpectedSQLEnd = " IN (".implode(',', $this->aIdByClass['DFRToRemoveLeaf']).")"; self::assertStringEndsWith($sExpectedSQLEnd, $sUpdateSQL); - self::assertCount(3, $aIds); + self::assertEquals(3, $oDeletionPlanItem->Count()); $sIdsToRemoveInTargetClass = implode(',', $this->aIdByClass['DFRToRemoveLeaf']); $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteTable, 'extkey_id', $sIdsToRemoveInTargetClass); - self::assertEquals($aExpectedIds, $aIds); + self::assertEquals($aExpectedIds, $oDeletionPlanItem->aIds); // var_export($aRes); // var_export($this->aIdByClass); @@ -80,7 +81,7 @@ public function testGetCleanupSummaryReturnsEmptyArrayWhenNull(): void $this->assertEmpty($aResult, 'Expected result to be empty array when input is null.'); } - public function testExecuteCleanup_DeleteOneObjPerClass() + public function testGetStaticDeletionPlan_DeleteObjRecursively() { $this->GivenDFRTreeInDB(<<GetStaticDeletionPlan($aClasses); - var_export($aRes); + self::assertArrayHasKey('DFRRemovedCollateralCascade', $aRes); - var_export($this->aIdByClass); + // echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + // echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); + } + + public function testGetStaticDeletionPlan_IssuesArePresent() + { + $this->GivenDFRTreeInDB(<<expectException(DataFeatureRemovalException::class); + // $this->expectExceptionMessage('Deletion Plan cannot be executed due to issues'); + $oService = new StaticDeletionPlan(); + $aRes = $oService->GetStaticDeletionPlan($aClasses); + + self::assertEquals(1, $aRes['DFRManual']->oIssue->Count()); + self::assertEquals($this->aIdByClass['DFRManual'], $aRes['DFRManual']->oIssue->aIds); + + // echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + // echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); + + } + + public function testGetStaticDeletionPlan_UpdateMultipleExtKeys() + { + $this->GivenDFRTreeInDB(<<GetStaticDeletionPlan($aClasses); + + self::assertArrayHasKey('DFRToUpdate', $aRes); + + echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); } + + public function testGetCleanupSummary() + { + $this->GivenDFRTreeInDB(<<GetCleanupSummary($aClasses); + + echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); + + self::assertEquals(1, $aRes['DFRManual']->iIssueCount); + + } + } From 63879a63cc9dd530df0d954ec26afe494bfc235d Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 13:30:50 +0200 Subject: [PATCH 04/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20deletion=20p?= =?UTF-8?q?lan=20methods=20to=20use=20class=20names=20instead=20of=20table?= =?UTF-8?q?=20names=20and=20add=20TotalCount=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Entity/DeletionPlanEntity.php | 5 ++ .../src/Service/StaticDeletionPlan.php | 81 ++++++++++++++----- .../StaticDeletionPlanTest.php | 6 +- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php index debc726abf..e7dacd986a 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php @@ -24,4 +24,9 @@ public function __construct() $this->oUpdate = $oUpdate ?? new DeletionPlanItem(); $this->oIssue = $oIssue ?? new DeletionPlanItem(); } + + public function TotalCount(): int + { + return $this->oDelete->Count() + $this->oUpdate->Count() + $this->oIssue->Count(); + } } 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 e35ee088e5..df178b9b98 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 @@ -36,6 +36,9 @@ public function GetCleanupSummary(?array $aClasses): array $aDeletionPlan = $this->GetStaticDeletionPlan($aClasses ?? []); foreach ($aDeletionPlan as $sClass => $oDeletionPlanEntity) { + if ($oDeletionPlanEntity->TotalCount() === 0) { + continue; + } $oDataCleanupSummary = new DataCleanupSummaryEntity($sClass); $oDataCleanupSummary->iUpdateCount = $oDeletionPlanEntity->oUpdate->Count(); $oDataCleanupSummary->iDeleteCount = $oDeletionPlanEntity->oDelete->Count(); @@ -73,7 +76,6 @@ private function DeletionPlanForReferencingClasses(string $sClass): void $sIdsToRemove = implode(', ', $this->aDeletionPlan[$sClass]->oDelete->aIds); $aReferencingMe = MetaModel::EnumReferencingClasses($sClass); foreach ($aReferencingMe as $sRemoteClass => $aExtKeys) { - $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); if (!isset($this->aDeletionPlan[$sRemoteClass])) { $this->aDeletionPlan[$sRemoteClass] = new DeletionPlanEntity(); } @@ -87,11 +89,11 @@ private function DeletionPlanForReferencingClasses(string $sClass): void if ($oExtKeyAttDef->IsNullAllowed()) { // update - $oUpdateItem = $this->UpdateExtKeyNullable($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $oUpdateItem = $this->UpdateExtKeyNullable($sRemoteClass, $sExtKeyAttCode, $sIdsToRemove); $oDeletionPlanEntity->oUpdate->Merge($oUpdateItem); } else { // delete - $aRemoteIdsToRemove = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $aRemoteIdsToRemove = $this->GetRemoteIdsForExtKey($sRemoteClass, $sExtKeyAttCode, $sIdsToRemove); $iDeletePropagationOption = $oExtKeyAttDef->GetDeletionPropagationOption(); if ($iDeletePropagationOption == DEL_MANUAL) { @@ -104,7 +106,7 @@ private function DeletionPlanForReferencingClasses(string $sClass): void if (($iDeletePropagationOption == DEL_MOVEUP) && ($oExtKeyAttDef->IsHierarchicalKey())) { // update hierarchical keys due to row cleanup in the same table $sIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]->oDelete->aIds); - $oUpdateItem = $this->UpdateHierarchicalExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemove); + $oUpdateItem = $this->UpdateHierarchicalExtKey($sRemoteClass, $sExtKeyAttCode, $sIdsToRemove); $oDeletionPlanEntity->oUpdate->Merge($oUpdateItem); // do not recurse continue; @@ -112,7 +114,9 @@ private function DeletionPlanForReferencingClasses(string $sClass): void // Delete entries in Remote Class if (count($aRemoteIdsToRemove) !== 0) { + // TODO see ObjectService::Delete() !!!!!! $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); + $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); $sSQL = "DELETE FROM $sRemoteTable WHERE id IN ($sRemoteIdsToDelete)"; $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem([$sSQL], $aRemoteIdsToRemove)); @@ -124,38 +128,50 @@ private function DeletionPlanForReferencingClasses(string $sClass): void } /** - * @param string $sRemoteTable + * @param string $sRemoteClass * @param string $sExtKeyAttCode * @param string $sIdsToRemoveInTargetClass * * @return \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem + * @throws \CoreException */ - public function UpdateExtKeyNullable(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem + public function UpdateExtKeyNullable(string $sRemoteClass, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { - $aIds = $this->GetRemoteIdsForExtKey($sRemoteTable, $sExtKeyAttCode, $sIdsToRemoveInTargetClass); + $aIds = $this->GetRemoteIdsForExtKey($sRemoteClass, $sExtKeyAttCode, $sIdsToRemoveInTargetClass); + [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); $sUpdateSQL = << $sUpdateSQL], $aIds); } - public function UpdateHierarchicalExtKey(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem + /** + * @param string $sRemoteClass + * @param string $sExtKeyAttCode + * @param string $sIdsToRemoveInTargetClass + * + * @return \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem + * @throws \CoreException + * @throws \MySQLException + */ + public function UpdateHierarchicalExtKey(string $sRemoteClass, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { + [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); $sUpdateSQL = << $sUpdateSQL], $aIds); } - public function GetRemoteIdsForExtKey(string $sRemoteTable, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array + /** + * @param string $sRemoteClass + * @param string $sExtKeyAttCode + * @param string $sIdsToRemoveInTargetClass + * + * @return array + * @throws \CoreException + * @throws \MySQLException + */ + public function GetRemoteIdsForExtKey(string $sRemoteClass, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): array { if (\utils::IsNullOrEmptyString($sIdsToRemoveInTargetClass)) { return []; } - $sSQL = "SELECT id FROM $sRemoteTable WHERE $sExtKeyAttCode IN ($sIdsToRemoveInTargetClass)"; + [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); + $sSQL = "SELECT id FROM $sDBTable WHERE $sDBField IN ($sIdsToRemoveInTargetClass)"; return CMDBSource::QueryToCol($sSQL, 'id'); } @@ -190,4 +216,23 @@ public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem return new DeletionPlanItem([$sDeleteSQL], $aIds); } + /** + * Get database table for an attcode + * + * @param string $sRemoteClass + * @param string $sExtKeyAttCode + * + * @return array + * @throws \CoreException + * @throws \Exception + */ + public function GetDBInfoForAttcode(string $sRemoteClass, string $sExtKeyAttCode): array + { + $sRealClass = MetaModel::GetAttributeOrigin($sRemoteClass, $sExtKeyAttCode); + $sRealTable = MetaModel::DBGetTable($sRealClass); + $oAttDef = MetaModel::GetAttributeDef($sRealClass, $sExtKeyAttCode); + $sSQLAttCode = array_keys($oAttDef->GetSQLColumns())[0]; + return [$sRealTable, $sSQLAttCode]; + } + } 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 5f88460d1f..32060477c3 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 @@ -47,9 +47,9 @@ public function testUpdateExtKeyNullable() // WHEN $oService = new StaticDeletionPlan(); - $sRemoteTable = MetaModel::DBGetTable('DFRToUpdate'); + $sRemoteClass = 'DFRToUpdate'; $oDeletionPlanItem = $oService->UpdateExtKeyNullable( - $sRemoteTable, + $sRemoteClass, 'extkey_id', implode(',', $this->aIdByClass['DFRToRemoveLeaf']) ); @@ -61,7 +61,7 @@ public function testUpdateExtKeyNullable() self::assertEquals(3, $oDeletionPlanItem->Count()); $sIdsToRemoveInTargetClass = implode(',', $this->aIdByClass['DFRToRemoveLeaf']); - $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteTable, 'extkey_id', $sIdsToRemoveInTargetClass); + $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteClass, 'extkey_id', $sIdsToRemoveInTargetClass); self::assertEquals($aExpectedIds, $oDeletionPlanItem->aIds); From 4d2da15c96e02ee7d09834fa03b1907b654a77bd Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 13:44:02 +0200 Subject: [PATCH 05/15] =?UTF-8?q?N=C2=B09639=20-=20Add=20messages=20to=20t?= =?UTF-8?q?ests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../StaticDeletionPlanTest.php | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) 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 32060477c3..9e4e04158f 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 @@ -29,11 +29,11 @@ public function testGetInitialClassDeletionPlan() $oService = new StaticDeletionPlan(); $oDeletionPlanItem = $oService->GetInitialClassDeletionPlan('DFRToRemoveLeaf'); - self::assertEquals(2, $oDeletionPlanItem->Count()); - self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $oDeletionPlanItem->aIds); + self::assertEquals(2, $oDeletionPlanItem->Count(), 'All entries of root table should be removed'); + self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $oDeletionPlanItem->aIds, 'All the Ids found in root table should correspond to the one created'); $sTable = MetaModel::DBGetTable('DFRToRemoveLeaf'); $sExpectedSQL = "DELETE FROM $sTable"; - self::assertEquals($sExpectedSQL, $oDeletionPlanItem->aQueries[0]); + self::assertEquals($sExpectedSQL, $oDeletionPlanItem->aQueries[0], 'Removing elements in root class should suppress all entries'); } public function testUpdateExtKeyNullable() @@ -57,16 +57,13 @@ public function testUpdateExtKeyNullable() // THEN $sExpectedSQLEnd = " IN (".implode(',', $this->aIdByClass['DFRToRemoveLeaf']).")"; - self::assertStringEndsWith($sExpectedSQLEnd, $sUpdateSQL); + self::assertStringEndsWith($sExpectedSQLEnd, $sUpdateSQL, 'The query should be filtered with the ids of the root class'); - self::assertEquals(3, $oDeletionPlanItem->Count()); + self::assertEquals(3, $oDeletionPlanItem->Count(), 'All entries of root table should be removed'); $sIdsToRemoveInTargetClass = implode(',', $this->aIdByClass['DFRToRemoveLeaf']); $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteClass, 'extkey_id', $sIdsToRemoveInTargetClass); - self::assertEquals($aExpectedIds, $oDeletionPlanItem->aIds); - - // var_export($aRes); - // var_export($this->aIdByClass); + self::assertEquals($aExpectedIds, $oDeletionPlanItem->aIds, 'All entries pointing on root class should be removed'); } /** @@ -94,7 +91,7 @@ public function testGetStaticDeletionPlan_DeleteObjRecursively() $oService = new StaticDeletionPlan(); $aRes = $oService->GetStaticDeletionPlan($aClasses); - self::assertArrayHasKey('DFRRemovedCollateralCascade', $aRes); + self::assertArrayHasKey('DFRRemovedCollateralCascade', $aRes, 'The cleanup should descend to the cascaded classes'); // echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; // echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); @@ -111,17 +108,11 @@ public function testGetStaticDeletionPlan_IssuesArePresent() EOF); $aClasses = [ 'DFRToRemoveLeaf' ]; - // $this->expectException(DataFeatureRemovalException::class); - // $this->expectExceptionMessage('Deletion Plan cannot be executed due to issues'); $oService = new StaticDeletionPlan(); $aRes = $oService->GetStaticDeletionPlan($aClasses); - self::assertEquals(1, $aRes['DFRManual']->oIssue->Count()); - self::assertEquals($this->aIdByClass['DFRManual'], $aRes['DFRManual']->oIssue->aIds); - - // echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; - // echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); - + self::assertEquals(1, $aRes['DFRManual']->oIssue->Count(), 'Issue should be found because of DEL_MANUAL deletion policy'); + self::assertEquals($this->aIdByClass['DFRManual'], $aRes['DFRManual']->oIssue->aIds, 'Issue should be correspond to the entries created'); } public function testGetStaticDeletionPlan_UpdateMultipleExtKeys() @@ -136,10 +127,8 @@ public function testGetStaticDeletionPlan_UpdateMultipleExtKeys() $oService = new StaticDeletionPlan(); $aRes = $oService->GetStaticDeletionPlan($aClasses); - self::assertArrayHasKey('DFRToUpdate', $aRes); - - echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; - echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); + self::assertArrayHasKey('DFRToUpdate', $aRes, 'Class to update should be targeted'); + self::assertEquals(2, $aRes['DFRToUpdate']->oUpdate->Count(), 'Update should be counted only for removed pointed classes'); } public function testGetCleanupSummary() @@ -156,11 +145,7 @@ public function testGetCleanupSummary() $oService = new StaticDeletionPlan(); $aRes = $oService->GetCleanupSummary($aClasses); - echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; - echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT); - - self::assertEquals(1, $aRes['DFRManual']->iIssueCount); - + self::assertEquals(1, $aRes['DFRManual']->iIssueCount, 'Issue should have been detected during cleanup count'); } } From dc3f6a9f7272ac338b4d6a40e01eb876c1d5000b Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Jun 2026 17:20:17 +0200 Subject: [PATCH 06/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20data=20featu?= =?UTF-8?q?re=20removal=20logic=20and=20update=20database=20queries=20for?= =?UTF-8?q?=20consistency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../DataFeatureRemovalController.php | 1 + .../src/Service/StaticDeletionPlan.php | 31 +++++++++---------- .../unattended-install/xml_setup/upgrade.xml | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php index 28065f2cdd..248b7bc1d6 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php @@ -224,6 +224,7 @@ private function Compile(array $aAddedExtensions, array $aRemovedExtensions, boo $bIsDirEmpty = count(scandir($sBuildDir)) === 2; if ($bIsDirEmpty || $bForceCompilation) { + Session::Unset('bForceCompilation'); DataFeatureRemovalLog::Debug( __METHOD__, null, 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 df178b9b98..2283e0e465 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 @@ -26,9 +26,6 @@ class StaticDeletionPlan * * @return array<\Combodo\iTop\DataFeatureRemoval\Entity\DataCleanupSummaryEntity> * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \MySQLException - * @throws \Combodo\iTop\DataFeatureRemoval\Helper\DataFeatureRemovalException */ public function GetCleanupSummary(?array $aClasses): array { @@ -160,21 +157,21 @@ public function UpdateExtKeyNullable(string $sRemoteClass, string $sExtKeyAttCod */ public function UpdateHierarchicalExtKey(string $sRemoteClass, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { - [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); + [$sDBTable, $sDBField, $sDBKey] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); $sUpdateSQL = << $sUpdateSQL], $aIds); } @@ -193,10 +190,10 @@ public function GetRemoteIdsForExtKey(string $sRemoteClass, string $sExtKeyAttCo if (\utils::IsNullOrEmptyString($sIdsToRemoveInTargetClass)) { return []; } - [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); - $sSQL = "SELECT id FROM $sDBTable WHERE $sDBField IN ($sIdsToRemoveInTargetClass)"; + [$sDBTable, $sDBField, $sDBKey] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); + $sSQL = "SELECT $sDBKey FROM $sDBTable WHERE $sDBField IN ($sIdsToRemoveInTargetClass)"; - return CMDBSource::QueryToCol($sSQL, 'id'); + return CMDBSource::QueryToCol($sSQL, $sDBKey); } /** @@ -209,8 +206,9 @@ public function GetRemoteIdsForExtKey(string $sRemoteClass, string $sExtKeyAttCo public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem { $sTable = MetaModel::DBGetTable($sClass); - $sSQL = "SELECT id FROM $sTable"; - $aIds = CMDBSource::QueryToCol($sSQL, 'id'); + $sDBKey = MetaModel::DBGetKey($sClass); + $sSQL = "SELECT $sDBKey FROM $sTable"; + $aIds = CMDBSource::QueryToCol($sSQL, $sDBKey); $sDeleteSQL = "DELETE FROM $sTable"; return new DeletionPlanItem([$sDeleteSQL], $aIds); @@ -232,7 +230,8 @@ public function GetDBInfoForAttcode(string $sRemoteClass, string $sExtKeyAttCode $sRealTable = MetaModel::DBGetTable($sRealClass); $oAttDef = MetaModel::GetAttributeDef($sRealClass, $sExtKeyAttCode); $sSQLAttCode = array_keys($oAttDef->GetSQLColumns())[0]; - return [$sRealTable, $sSQLAttCode]; + $sDBKey = MetaModel::DBGetKey($sRemoteClass); + return [$sRealTable, $sSQLAttCode, $sDBKey]; } } diff --git a/setup/unattended-install/xml_setup/upgrade.xml b/setup/unattended-install/xml_setup/upgrade.xml index bef34398af..0272bd2551 100644 --- a/setup/unattended-install/xml_setup/upgrade.xml +++ b/setup/unattended-install/xml_setup/upgrade.xml @@ -41,5 +41,5 @@ - on + off From 51d5692f817bc1090ecefedd6c5cf9b8b29fe434 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 26 Jun 2026 10:39:54 +0200 Subject: [PATCH 07/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20SQL=20querie?= =?UTF-8?q?s=20to=20use=20backticks=20for=20table=20and=20field=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Service/StaticDeletionPlan.php | 48 ++++++++++--------- .../src/Controller/AjaxController.php | 3 +- .../src/Controller/UpdateController.php | 2 - setup/wizardsteps/WizStepWelcome.php | 12 ++--- .../StaticDeletionPlanTest.php | 4 +- 5 files changed, 33 insertions(+), 36 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 2283e0e465..9fd3bc92dd 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 @@ -114,7 +114,8 @@ private function DeletionPlanForReferencingClasses(string $sClass): void // TODO see ObjectService::Delete() !!!!!! $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); - $sSQL = "DELETE FROM $sRemoteTable WHERE id IN ($sRemoteIdsToDelete)"; + $sDBKey = MetaModel::DBGetKey($sRemoteClass); + $sSQL = "DELETE FROM `$sRemoteTable` WHERE `$sDBKey` IN ($sRemoteIdsToDelete)"; $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem([$sSQL], $aRemoteIdsToRemove)); $this->DeletionPlanForReferencingClasses($sRemoteClass); @@ -138,9 +139,9 @@ public function UpdateExtKeyNullable(string $sRemoteClass, string $sExtKeyAttCod [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); $sUpdateSQL = << $sUpdateSQL], $aIds); @@ -159,17 +160,17 @@ public function UpdateHierarchicalExtKey(string $sRemoteClass, string $sExtKeyAt { [$sDBTable, $sDBField, $sDBKey] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); $sUpdateSQL = <<GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); - $sSQL = "SELECT $sDBKey FROM $sDBTable WHERE $sDBField IN ($sIdsToRemoveInTargetClass)"; + $sSQL = "SELECT `$sDBKey` FROM `$sDBTable` WHERE `$sDBField` IN ($sIdsToRemoveInTargetClass)"; return CMDBSource::QueryToCol($sSQL, $sDBKey); } @@ -207,9 +208,9 @@ public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem { $sTable = MetaModel::DBGetTable($sClass); $sDBKey = MetaModel::DBGetKey($sClass); - $sSQL = "SELECT $sDBKey FROM $sTable"; + $sSQL = "SELECT `$sDBKey` FROM `$sTable`"; $aIds = CMDBSource::QueryToCol($sSQL, $sDBKey); - $sDeleteSQL = "DELETE FROM $sTable"; + $sDeleteSQL = "DELETE FROM `$sTable`"; return new DeletionPlanItem([$sDeleteSQL], $aIds); } @@ -217,21 +218,22 @@ public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem /** * Get database table for an attcode * - * @param string $sRemoteClass + * @param string $sClass * @param string $sExtKeyAttCode * * @return array * @throws \CoreException * @throws \Exception */ - public function GetDBInfoForAttcode(string $sRemoteClass, string $sExtKeyAttCode): array + public function GetDBInfoForAttcode(string $sClass, string $sExtKeyAttCode): array { - $sRealClass = MetaModel::GetAttributeOrigin($sRemoteClass, $sExtKeyAttCode); - $sRealTable = MetaModel::DBGetTable($sRealClass); - $oAttDef = MetaModel::GetAttributeDef($sRealClass, $sExtKeyAttCode); - $sSQLAttCode = array_keys($oAttDef->GetSQLColumns())[0]; - $sDBKey = MetaModel::DBGetKey($sRemoteClass); - return [$sRealTable, $sSQLAttCode, $sDBKey]; + $sOriginClass = MetaModel::GetAttributeOrigin($sClass, $sExtKeyAttCode); + $sDBTable = MetaModel::DBGetTable($sOriginClass); + $oAttDef = MetaModel::GetAttributeDef($sOriginClass, $sExtKeyAttCode); + // External key is on a single DB column + $sDBField = array_keys($oAttDef->GetSQLColumns())[0]; + $sDBKey = MetaModel::DBGetKey($sClass); + return [$sDBTable, $sDBField, $sDBKey]; } } diff --git a/datamodels/2.x/itop-core-update/src/Controller/AjaxController.php b/datamodels/2.x/itop-core-update/src/Controller/AjaxController.php index 7a100ed28a..8788f6e4d1 100644 --- a/datamodels/2.x/itop-core-update/src/Controller/AjaxController.php +++ b/datamodels/2.x/itop-core-update/src/Controller/AjaxController.php @@ -242,8 +242,7 @@ public function OperationLaunchSetup() throw new SecurityException('Access forbidden'); } - $sConfigFile = APPCONF.'production/config-itop.php'; - @chmod($sConfigFile, 0770); // Allow overwriting the file + SetupUtils::CreateSetupToken(); header('Location: ../setup/'); } diff --git a/datamodels/2.x/itop-core-update/src/Controller/UpdateController.php b/datamodels/2.x/itop-core-update/src/Controller/UpdateController.php index 259dbf2625..ca85ca22b8 100644 --- a/datamodels/2.x/itop-core-update/src/Controller/UpdateController.php +++ b/datamodels/2.x/itop-core-update/src/Controller/UpdateController.php @@ -166,8 +166,6 @@ public function OperationUpdateCoreFiles() public function OperationRunSetup() { SetupUtils::CheckSetupToken(true); - $sConfigFile = APPCONF.'production/'.ITOP_CONFIG_FILE; - @chmod($sConfigFile, 0770); $sRedirectURL = utils::GetAbsoluteUrlAppRoot().'setup/index.php'; header("Location: $sRedirectURL"); } diff --git a/setup/wizardsteps/WizStepWelcome.php b/setup/wizardsteps/WizStepWelcome.php index c9b878eb80..31ad75f12f 100644 --- a/setup/wizardsteps/WizStepWelcome.php +++ b/setup/wizardsteps/WizStepWelcome.php @@ -124,18 +124,18 @@ public function PostFormDisplay(SetupPage $oPage) if (file_exists($sBuildConfigFile)) { $oPage->add( << + - - - - -
HTML ); + $oPage->add_ready_script( + <<'); +JS + ); } } } 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 9e4e04158f..2b7433633b 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 @@ -7,8 +7,6 @@ * @license http://opensource.org/licenses/AGPL-3.0 */ -use Combodo\iTop\DataFeatureRemoval\Helper\DataFeatureRemovalException; -use Combodo\iTop\DataFeatureRemoval\Service\DataCleanupService; use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan; use MetaModel; @@ -32,7 +30,7 @@ public function testGetInitialClassDeletionPlan() self::assertEquals(2, $oDeletionPlanItem->Count(), 'All entries of root table should be removed'); self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $oDeletionPlanItem->aIds, 'All the Ids found in root table should correspond to the one created'); $sTable = MetaModel::DBGetTable('DFRToRemoveLeaf'); - $sExpectedSQL = "DELETE FROM $sTable"; + $sExpectedSQL = "DELETE FROM `$sTable`"; self::assertEquals($sExpectedSQL, $oDeletionPlanItem->aQueries[0], 'Removing elements in root class should suppress all entries'); } From 87dcbf7b53403e9283b065b5f456e431d345fb6a Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 26 Jun 2026 12:05:12 +0200 Subject: [PATCH 08/15] =?UTF-8?q?N=C2=B09639=20-=20Enhance=20deletion=20lo?= =?UTF-8?q?gic=20and=20add=20circular=20reference=20handling=20in=20cleanu?= =?UTF-8?q?p=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Service/StaticDeletionPlan.php | 2 + .../AbstractCleanup.php | 14 ++++- .../StaticDeletionPlanTest.php | 13 ++++ .../data_cleanup_delta.xml | 61 +++++++++++++++++++ 4 files changed, 87 insertions(+), 3 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 9fd3bc92dd..6099ba454d 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 @@ -210,6 +210,8 @@ public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem $sDBKey = MetaModel::DBGetKey($sClass); $sSQL = "SELECT `$sDBKey` FROM `$sTable`"; $aIds = CMDBSource::QueryToCol($sSQL, $sDBKey); + + // TODO see ObjectService::Delete() !!!!!! $sDeleteSQL = "DELETE FROM `$sTable`"; return new DeletionPlanItem([$sDeleteSQL], $aIds); diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php index cc292574b3..e3b8881347 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php @@ -48,9 +48,17 @@ protected function GivenDFRTreeLineInDB(string $sLine) $sChildClass = $aMatches['class']; $sExtKey = $aMatches['extkey'] ?? 'extkey_id'; - $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sName, $sExtKey => $iLeftId]); - $this->aIdByClass[$sChildClass][] = $iRightId; - $this->aIdByObjectName[$sRight] = $iRightId; + $iRightId = $this->aIdByObjectName[$sName] ?? 0; + if ($iRightId === 0) { + $iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sName, $sExtKey => $iLeftId]); + $this->aIdByClass[$sChildClass][] = $iRightId; + $this->aIdByObjectName[$sName] = $iRightId; + } else { + // Update object + $oObj = MetaModel::GetObject($sChildClass, $iRightId); + $oObj->Set($sExtKey, $iLeftId); + $oObj->DBUpdate(); + } } } } 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 2b7433633b..edbc5f4a79 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 @@ -146,4 +146,17 @@ public function testGetCleanupSummary() self::assertEquals(1, $aRes['DFRManual']->iIssueCount, 'Issue should have been detected during cleanup count'); } + public function testCircularRefsShouldNotRunInfinitely() + { + $this->GivenDFRTreeInDB(<<GetStaticDeletionPlan($aClasses); + echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + } } diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml index 9a551fd43b..b200c20907 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml @@ -132,6 +132,67 @@ DEL_AUTO all + + circular_id + + + true + DFRCircularRefs + DEL_AUTO + all + + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + cmdbAbstractObject + + + + bizmodel,searchable + false + dfrcircularrefs + + + + + + + + + + name + + true + + + all + + + extkey_id + + + false + DFRRemovedCollateral + DEL_AUTO + all + From e5e91e8135372c7b9986faf5e560866791809439 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 26 Jun 2026 13:40:00 +0200 Subject: [PATCH 09/15] =?UTF-8?q?N=C2=B09639=20-=20Add=20filtering=20metho?= =?UTF-8?q?ds=20to=20DeletionPlan=20classes=20for=20improved=20data=20clea?= =?UTF-8?q?nup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Entity/DeletionPlanEntity.php | 5 +++++ .../src/Entity/DeletionPlanItem.php | 5 +++++ .../src/Service/StaticDeletionPlan.php | 1 + .../combodo-data-feature-removal/StaticDeletionPlanTest.php | 4 ++++ 4 files changed, 15 insertions(+) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php index e7dacd986a..bd001e0400 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php @@ -29,4 +29,9 @@ public function TotalCount(): int { return $this->oDelete->Count() + $this->oUpdate->Count() + $this->oIssue->Count(); } + + public function FilterUpdatesByDeletes() + { + $this->oUpdate->FilterBy($this->oDelete); + } } diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php index 81de7a261b..a2f58b4106 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php @@ -32,4 +32,9 @@ public function Count(): int { return count($this->aIds); } + + public function FilterBy(DeletionPlanItem $oItem): void + { + $this->aIds = array_diff($this->aIds, $oItem->aIds); + } } 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 6099ba454d..290f429f8d 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 @@ -36,6 +36,7 @@ public function GetCleanupSummary(?array $aClasses): array if ($oDeletionPlanEntity->TotalCount() === 0) { continue; } + $oDeletionPlanEntity->FilterUpdatesByDeletes(); $oDataCleanupSummary = new DataCleanupSummaryEntity($sClass); $oDataCleanupSummary->iUpdateCount = $oDeletionPlanEntity->oUpdate->Count(); $oDataCleanupSummary->iDeleteCount = $oDeletionPlanEntity->oDelete->Count(); 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 edbc5f4a79..9d01892d46 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 @@ -156,7 +156,11 @@ public function testCircularRefsShouldNotRunInfinitely() $aClasses = [ 'DFRToRemoveLeaf' ]; $oService = new StaticDeletionPlan(); + $aRes = $oService->GetCleanupSummary($aClasses); + echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + $aRes = $oService->GetStaticDeletionPlan($aClasses); echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + } } From 357ae223fdbfe97669400ad385550549e6a79fd1 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 26 Jun 2026 13:50:09 +0200 Subject: [PATCH 10/15] =?UTF-8?q?N=C2=B09639=20-=20Add=20filtering=20metho?= =?UTF-8?q?ds=20to=20DeletionPlan=20classes=20for=20improved=20data=20clea?= =?UTF-8?q?nup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php | 1 + 1 file changed, 1 insertion(+) 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 9d01892d46..bd17cae0a0 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 @@ -162,5 +162,6 @@ public function testCircularRefsShouldNotRunInfinitely() $aRes = $oService->GetStaticDeletionPlan($aClasses); echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + self::assertTrue(true); } } From b73960a5714a005f45725648a80365c94e6bae70 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 29 Jun 2026 11:46:08 +0200 Subject: [PATCH 11/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20DeletionPlan?= =?UTF-8?q?Item=20to=20remove=20unused=20queries=20and=20simplify=20constr?= =?UTF-8?q?uctor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Entity/DeletionPlanItem.php | 6 +- .../src/Service/StaticDeletionPlan.php | 30 +- setup/runtimeenv.class.inc.php | 2 +- .../AbstractCleanup.php | 63 +++++ .../StaticDeletionPlanTest.php | 104 +++++-- .../data_cleanup_delta.xml | 265 ++++++++++++++++++ 6 files changed, 413 insertions(+), 57 deletions(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php index a2f58b4106..1f3d64d835 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php @@ -9,22 +9,18 @@ class DeletionPlanItem { - public array $aQueries = []; public array $aIds = []; /** - * @param array $aQueries * @param array $aIds */ - public function __construct(array $aQueries = [], array $aIds = []) + public function __construct(array $aIds = []) { - $this->aQueries = $aQueries; $this->aIds = $aIds; } public function Merge(DeletionPlanItem $oItem): void { - $this->aQueries = array_merge($this->aQueries, $oItem->aQueries); $this->aIds = array_unique(array_merge($this->aIds, $oItem->aIds)); } 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 290f429f8d..4a8e43e8f6 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 @@ -112,13 +112,7 @@ private function DeletionPlanForReferencingClasses(string $sClass): void // Delete entries in Remote Class if (count($aRemoteIdsToRemove) !== 0) { - // TODO see ObjectService::Delete() !!!!!! - $sRemoteIdsToDelete = implode(',', $aRemoteIdsToRemove); - $sRemoteTable = MetaModel::DBGetTable($sRemoteClass); - $sDBKey = MetaModel::DBGetKey($sRemoteClass); - $sSQL = "DELETE FROM `$sRemoteTable` WHERE `$sDBKey` IN ($sRemoteIdsToDelete)"; - $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem([$sSQL], $aRemoteIdsToRemove)); - + $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem($aRemoteIdsToRemove)); $this->DeletionPlanForReferencingClasses($sRemoteClass); } } @@ -138,14 +132,7 @@ public function UpdateExtKeyNullable(string $sRemoteClass, string $sExtKeyAttCod { $aIds = $this->GetRemoteIdsForExtKey($sRemoteClass, $sExtKeyAttCode, $sIdsToRemoveInTargetClass); - [$sDBTable, $sDBField] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); - $sUpdateSQL = << $sUpdateSQL], $aIds); + return new DeletionPlanItem($aIds); } /** @@ -160,12 +147,6 @@ public function UpdateExtKeyNullable(string $sRemoteClass, string $sExtKeyAttCod public function UpdateHierarchicalExtKey(string $sRemoteClass, string $sExtKeyAttCode, string $sIdsToRemoveInTargetClass): DeletionPlanItem { [$sDBTable, $sDBField, $sDBKey] = $this->GetDBInfoForAttcode($sRemoteClass, $sExtKeyAttCode); - $sUpdateSQL = << $sUpdateSQL], $aIds); + return new DeletionPlanItem($aIds); } /** @@ -212,10 +193,7 @@ public function GetInitialClassDeletionPlan(string $sClass): DeletionPlanItem $sSQL = "SELECT `$sDBKey` FROM `$sTable`"; $aIds = CMDBSource::QueryToCol($sSQL, $sDBKey); - // TODO see ObjectService::Delete() !!!!!! - $sDeleteSQL = "DELETE FROM `$sTable`"; - - return new DeletionPlanItem([$sDeleteSQL], $aIds); + return new DeletionPlanItem($aIds); } /** diff --git a/setup/runtimeenv.class.inc.php b/setup/runtimeenv.class.inc.php index 758cc92a91..b7cd585739 100644 --- a/setup/runtimeenv.class.inc.php +++ b/setup/runtimeenv.class.inc.php @@ -1344,7 +1344,7 @@ public function DataToCleanupAudit() $iCount = $oSetupAudit->GetDataToCleanupCount(); if ($iCount > 0) { - throw new Exception("$iCount elements require data adjustments or cleanup in the backoffice", DataAuditSequencer::DATA_AUDIT_FAILED); + throw new Exception("Elements require data adjustments or cleanup in the backoffice", DataAuditSequencer::DATA_AUDIT_FAILED); } } diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php index e3b8881347..aabf98152c 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/AbstractCleanup.php @@ -61,4 +61,67 @@ protected function GivenDFRTreeLineInDB(string $sLine) } } } + + /** + * format of object: + * [create|update] CLASS (name = NAME, ...) + * + * @param string $sObjects + * + * @return void + */ + protected function GivenDFRObjectsInDB(string $sObjects) + { + $this->aIdByClass = []; + $sObjects = explode("\n", $sObjects); + foreach ($sObjects as $sLine) { + $sLine = trim($sLine); + if ($sLine === '') { + continue; + } + $this->GivenDFRObjectLineInDB($sLine); + } + } + + protected function GivenDFRObjectLineInDB(string $sLine) + { + if (preg_match("/(?\w+)\s+(?\w+)\s*\((?[^)]+)\)/", $sLine, $aMatches) !== false) { + $sVerb = $aMatches['verb']; + $sClass = $aMatches['class']; + $sAttList = $aMatches['att_list']; + + $aAttSet = explode(',', $sAttList); + $aAttributes = []; + foreach ($aAttSet as $sAtt) { + [$sAttName, $sAttValue] = explode('=', $sAtt, 2); + $sAttName = trim($sAttName); + $sAttValue = trim($sAttValue); + $aAttributes[$sAttName] = $sAttValue; + } + + $sName = $aAttributes['name'] ?? ''; + // Transform names in external keys into ids + foreach ($aAttributes as $sAttName => $sAttValue) { + if ($sAttName !== 'name') { + $aAttributes[$sAttName] = $this->aIdByObjectName[$sAttValue] ?? $sAttValue; + } + } + + switch ($sVerb) { + case 'create': + $sId = $this->GivenObjectInDB($sClass, $aAttributes); + $this->aIdByClass[$sClass][] = $sId; + $this->aIdByObjectName[$sName] = $sId; + break; + case 'update': + $sId = $this->aIdByObjectName[$sName]; + $oObj = MetaModel::GetObject($sClass, $sId); + foreach ($aAttributes as $sAttName => $sAttValue) { + $oObj->Set($sAttName, $sAttValue); + } + $oObj->DBUpdate(); + } + } + } + } 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 bd17cae0a0..643974c95f 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 @@ -7,6 +7,7 @@ * @license http://opensource.org/licenses/AGPL-3.0 */ +use Combodo\iTop\DataFeatureRemoval\Entity\DataCleanupSummaryEntity; use Combodo\iTop\DataFeatureRemoval\Service\StaticDeletionPlan; use MetaModel; @@ -29,9 +30,6 @@ public function testGetInitialClassDeletionPlan() $oDeletionPlanItem = $oService->GetInitialClassDeletionPlan('DFRToRemoveLeaf'); self::assertEquals(2, $oDeletionPlanItem->Count(), 'All entries of root table should be removed'); self::assertEquals($this->aIdByClass['DFRToRemoveLeaf'], $oDeletionPlanItem->aIds, 'All the Ids found in root table should correspond to the one created'); - $sTable = MetaModel::DBGetTable('DFRToRemoveLeaf'); - $sExpectedSQL = "DELETE FROM `$sTable`"; - self::assertEquals($sExpectedSQL, $oDeletionPlanItem->aQueries[0], 'Removing elements in root class should suppress all entries'); } public function testUpdateExtKeyNullable() @@ -51,12 +49,8 @@ public function testUpdateExtKeyNullable() 'extkey_id', implode(',', $this->aIdByClass['DFRToRemoveLeaf']) ); - $sUpdateSQL = $oDeletionPlanItem->aQueries['extkey_id']; // THEN - $sExpectedSQLEnd = " IN (".implode(',', $this->aIdByClass['DFRToRemoveLeaf']).")"; - self::assertStringEndsWith($sExpectedSQLEnd, $sUpdateSQL, 'The query should be filtered with the ids of the root class'); - self::assertEquals(3, $oDeletionPlanItem->Count(), 'All entries of root table should be removed'); $sIdsToRemoveInTargetClass = implode(',', $this->aIdByClass['DFRToRemoveLeaf']); $aExpectedIds = $oService->GetRemoteIdsForExtKey($sRemoteClass, 'extkey_id', $sIdsToRemoveInTargetClass); @@ -129,39 +123,99 @@ public function testGetStaticDeletionPlan_UpdateMultipleExtKeys() self::assertEquals(2, $aRes['DFRToUpdate']->oUpdate->Count(), 'Update should be counted only for removed pointed classes'); } - public function testGetCleanupSummary() + public function testGetCleanupSummaryWithIssues() { - $this->GivenDFRTreeInDB(<<GivenDFRObjectsInDB(<<GetCleanupSummary($aClasses); - self::assertEquals(1, $aRes['DFRManual']->iIssueCount, 'Issue should have been detected during cleanup count'); + $aExpected = [ + 'DFRToRemoveLeaf' => ['iDeleteCount' => 1], + 'DFRRemovedCollateral' => ['iDeleteCount' => 1], + 'DFRRemovedCollateralCascade' => ['iDeleteCount' => 2], + 'DFRToUpdate' => ['iUpdateCount' => 1], + 'DFRManual' => ['iIssueCount' => 1], + ]; + + $this->AssertSummaryEquals($aExpected, $aRes); } public function testCircularRefsShouldNotRunInfinitely() { - $this->GivenDFRTreeInDB(<<GivenDFRObjectsInDB(<<GetCleanupSummary($aClasses); - echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; - $aRes = $oService->GetStaticDeletionPlan($aClasses); - echo json_encode($aRes, JSON_PRETTY_PRINT)."\n"; + $aExpected = [ + 'DFRToRemoveLeaf' => ['iDeleteCount' => 1], + 'DFRRemovedCollateral' => ['iDeleteCount' => 1], + 'DFRCircularRefs' => ['iDeleteCount' => 1], + ]; - self::assertTrue(true); + $this->AssertSummaryEquals($aExpected, $aRes); + } + + public function testMultipleExtKeys() + { + $this->GivenDFRObjectsInDB(<<GetCleanupSummary($aClasses); + + $aExpected = [ + 'DFRC1' => ['iDeleteCount' => 5], + 'DFRC2' => ['iDeleteCount' => 1], + 'DFRC3' => ['iDeleteCount' => 3], + 'DFRC4' => ['iDeleteCount' => 1], + ]; + + $this->AssertSummaryEquals($aExpected, $aRes); + + } + + private function AssertSummaryEquals(array $aExpected, array $aActual, string $sMessage = '') + { + foreach ($aExpected as $sClass => $aExpectedCounts) { + $oExpectedCleanupSummaryEntity = new DataCleanupSummaryEntity($sClass); + foreach ($aExpectedCounts as $sCount => $iExpectedValue) { + $oExpectedCleanupSummaryEntity->$sCount = $iExpectedValue; + } + + $this->assertEquals($oExpectedCleanupSummaryEntity, $aActual[$sClass], $sMessage); + } } } diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml index b200c20907..f69aa1f314 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/data_cleanup_delta.xml @@ -405,6 +405,271 @@ DFRToRemove + + + + bizmodel,searchable + true + dfrc0 + + + + + + + + + + name + + true + + + all + + + + + + + + + + +
+ + + 10 + + +
+ + cmdbAbstractObject + + + + bizmodel,searchable + false + dfrc1 + + + + + + + + + + name + + true + + + all + + + + + + + + + + +
+ + + 10 + + +
+ + DFRC0 + + + + bizmodel,searchable + false + dfrc2 + + + + + + + + + + name + + true + + + all + + + extkey1_id + + + false + DFRC0 + DEL_AUTO + all + + + extkey2_id + + + false + DFRC0 + DEL_AUTO + all + + + extkey3_id + + + false + DFRC3 + DEL_AUTO + all + + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + cmdbAbstractObject + + + + bizmodel,searchable + false + dfrc3 + + + + + + + + + + name + + true + + + all + + + extkey1_id + + + false + DFRC0 + DEL_AUTO + all + + + extkey2_id + + + true + DFRC4 + DEL_AUTO + all + + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + cmdbAbstractObject + + + + bizmodel,searchable + false + dfrc4 + + + + + + + + + + name + + true + + + all + + + extkey1_id + + + false + DFRC2 + DEL_AUTO + all + + + + + + + + + + +
+ + + 10 + + + 20 + + +
+ + cmdbAbstractObject + + From a1dcd0300c5cc982d5edebd5a6fd5ce95003643e Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Tue, 30 Jun 2026 09:13:03 +0200 Subject: [PATCH 12/15] =?UTF-8?q?N=C2=B09639=20-=20Refactor=20variable=20n?= =?UTF-8?q?ame=20for=20clarity=20in=20hierarchical=20key=20update=20proces?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Service/StaticDeletionPlan.php | 6 ++---- 1 file changed, 2 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 4a8e43e8f6..3356b77856 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 @@ -103,11 +103,9 @@ private function DeletionPlanForReferencingClasses(string $sClass): void if (($iDeletePropagationOption == DEL_MOVEUP) && ($oExtKeyAttDef->IsHierarchicalKey())) { // update hierarchical keys due to row cleanup in the same table - $sIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]->oDelete->aIds); - $oUpdateItem = $this->UpdateHierarchicalExtKey($sRemoteClass, $sExtKeyAttCode, $sIdsToRemove); + $sTargetIdsToRemove = implode(',', $this->aDeletionPlan[$sRemoteClass]->oDelete->aIds); + $oUpdateItem = $this->UpdateHierarchicalExtKey($sRemoteClass, $sExtKeyAttCode, $sTargetIdsToRemove); $oDeletionPlanEntity->oUpdate->Merge($oUpdateItem); - // do not recurse - continue; } // Delete entries in Remote Class From fe32ebcbcaa500e2661abac68871add4e9b12c08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Espi=C3=A9?= Date: Tue, 30 Jun 2026 09:14:54 +0200 Subject: [PATCH 13/15] Update tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- .../combodo-data-feature-removal/simulate-audit-from-setup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php index 9ff9c1bc0d..e48d3f2d1e 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/simulate-audit-from-setup.php @@ -59,7 +59,7 @@ function GetLastestInstallFile(): ?string } } -$sRemovedExtensions = utils::ReadParam('removed_modules', 'itop-container-mgmt', false, 'raw'); +$sRemovedExtensions = utils::ReadParam('removed_modules', '', false, 'raw'); $aRemovedExtensionsAndModules = []; if (mb_strlen($sRemovedExtensions) > 0) { $aRemovedExtensionsAndModules = explode(',', $sRemovedExtensions); From 3da34dabd80dcd4c2183a13144cdd3197d7816ae Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Tue, 30 Jun 2026 09:15:53 +0200 Subject: [PATCH 14/15] =?UTF-8?q?N=C2=B09639=20-=20Clarify=20comment=20on?= =?UTF-8?q?=20deletion=20plan=20to=20indicate=20prevention=20of=20infinite?= =?UTF-8?q?=20loops?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Service/StaticDeletionPlan.php | 1 + 1 file changed, 1 insertion(+) 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 3356b77856..82cf651570 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 @@ -111,6 +111,7 @@ private function DeletionPlanForReferencingClasses(string $sClass): void // Delete entries in Remote Class if (count($aRemoteIdsToRemove) !== 0) { $oDeletionPlanEntity->oDelete->Merge(new DeletionPlanItem($aRemoteIdsToRemove)); + // Infinite loops do not occurs due to the datamodel structure $this->DeletionPlanForReferencingClasses($sRemoteClass); } } From 7ddc0905142bb9b8c97fe6d368a0dd508b80ee10 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Tue, 30 Jun 2026 09:36:29 +0200 Subject: [PATCH 15/15] =?UTF-8?q?N=C2=B09639=20-=20Remove=20unused=20delet?= =?UTF-8?q?ion=20plan=20from=20DataFeatureRemovalController?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Controller/DataFeatureRemovalController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php index 248b7bc1d6..53c67d856d 100644 --- a/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php +++ b/datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php @@ -169,7 +169,6 @@ public function OperationAnalysisResult(): void $oSetupAudit = new SetupAudit($sSourceEnv); $aGetRemovedClasses = array_keys($oSetupAudit->RunDataAudit()); DataFeatureRemovalLog::Debug(__METHOD__, null, ['aGetRemovedClasses' => $aGetRemovedClasses]); - $aDeletionPlan = (new StaticDeletionPlan())->GetStaticDeletionPlan($aGetRemovedClasses); $aParams['aClasses'] = $aGetRemovedClasses;