N°9639 - Refactor deletion plan handling with new entity classes for improved structure and clarity

This commit is contained in:
Eric Espie
2026-06-25 11:17:51 +02:00
parent fc9df64eea
commit bcb42fdc81
9 changed files with 220 additions and 60 deletions

View File

@@ -274,7 +274,7 @@ class DataFeatureRemovalController extends Controller
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 = [];

View File

@@ -0,0 +1,27 @@
<?php
/*
* @copyright Copyright (C) 2010-2026 Combodo SAS
* @license http://opensource.org/licenses/AGPL-3.0
*/
namespace Combodo\iTop\DataFeatureRemoval\Entity;
class DeletionPlanEntity
{
public readonly DeletionPlanItem $oDelete;
public readonly DeletionPlanItem $oUpdate;
public readonly DeletionPlanItem $oIssue;
/**
* @param \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem $oDelete
* @param \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem $oUpdate
* @param \Combodo\iTop\DataFeatureRemoval\Entity\DeletionPlanItem $oIssue
*/
public function __construct()
{
$this->oDelete = $oDelete ?? new DeletionPlanItem();
$this->oUpdate = $oUpdate ?? new DeletionPlanItem();
$this->oIssue = $oIssue ?? new DeletionPlanItem();
}
}

View File

@@ -0,0 +1,35 @@
<?php
/*
* @copyright Copyright (C) 2010-2026 Combodo SAS
* @license http://opensource.org/licenses/AGPL-3.0
*/
namespace Combodo\iTop\DataFeatureRemoval\Entity;
class DeletionPlanItem
{
public array $aQueries = [];
public array $aIds = [];
/**
* @param array $aQueries
* @param array $aIds
*/
public function __construct(array $aQueries = [], 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));
}
public function Count(): int
{
return count($this->aIds);
}
}

View File

@@ -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<DeletionPlanEntity> */
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 @@ class StaticDeletionPlan
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 @@ class StaticDeletionPlan
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 @@ class StaticDeletionPlan
$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 @@ class StaticDeletionPlan
* @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 @@ FROM $sRemoteTable AS updated
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 = <<<SQL
UPDATE $sRemoteTable SET updated.$sExtKeyAttCode = removed.$sExtKeyAttCode
@@ -133,11 +160,14 @@ WHERE removed.id IN ($sIdsToRemoveInTargetClass)
SQL;
$aIds = CMDBSource::QueryToCol($sSQL, 'id');
return [$sUpdateSQL, $aIds];
return new DeletionPlanItem([$sExtKeyAttCode => $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 @@ SQL;
/**
* @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);
}
}

View File

@@ -8,6 +8,8 @@ $baseDir = dirname($vendorDir);
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',

View File

@@ -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',

View File

@@ -41,5 +41,5 @@
</selected_modules>
<selected_extensions type="array">
</selected_extensions>
<use_symbolic_links>off</use_symbolic_links>
<use_symbolic_links>on</use_symbolic_links>
</installation>

View File

@@ -6,7 +6,6 @@
*/
use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
class AbstractCleanup extends ItopCustomDatamodelTestCase
{
@@ -45,12 +44,13 @@ class AbstractCleanup extends ItopCustomDatamodelTestCase
$sRight = trim($sRight);
if (preg_match("/(?<name>(?<class>[^_]+)_\d+)(\s+\((?<extkey>\w+)\))?/", $sRight, $aMatches) !== false) {
$sName = $aMatches['name'];
$sChildClass = $aMatches['class'];
$sExtKey = $aMatches['extkey'] ?? 'extkey_id';
$iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sName, $sExtKey => $iLeftId]);
$this->aIdByClass[$sChildClass][] = $iRightId;
$this->aIdByObjectName[$sRight] = $iRightId;
}
[$sChildClass] = explode('_', $sRight, 2);
$iRightId = $this->GivenObjectInDB($sChildClass, ['name' => $sRight, 'extkey_id' => $iLeftId]);
$this->aIdByClass[$sChildClass][] = $iRightId;
$this->aIdByObjectName[$sRight] = $iRightId;
}
}

View File

@@ -7,6 +7,8 @@ namespace Combodo\iTop\Test\UnitTest\Module\DataFeatureRemoval;
* @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 @@ class StaticDeletionPlanTest extends \AbstractCleanup
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 @@ class StaticDeletionPlanTest extends \AbstractCleanup
// 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 @@ class StaticDeletionPlanTest extends \AbstractCleanup
$this->assertEmpty($aResult, 'Expected result to be empty array when input is null.');
}
public function testExecuteCleanup_DeleteOneObjPerClass()
public function testGetStaticDeletionPlan_DeleteObjRecursively()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRToUpdate_1
@@ -93,10 +94,73 @@ class StaticDeletionPlanTest extends \AbstractCleanup
$oService = new StaticDeletionPlan();
$aRes = $oService->GetStaticDeletionPlan($aClasses);
var_export($aRes);
self::assertArrayHasKey('DFRRemovedCollateralCascade', $aRes);
var_export($this->aIdByClass);
self::assertTrue(true);
// echo json_encode($aRes, JSON_PRETTY_PRINT)."\n";
// echo json_encode($this->aIdByClass, JSON_PRETTY_PRINT);
}
public function testGetStaticDeletionPlan_IssuesArePresent()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRToUpdate_1
DFRToRemoveLeaf_1 <- DFRRemovedCollateral_1
DFRRemovedCollateral_1 <- DFRRemovedCollateralCascade_1
DFRRemovedCollateral_1 <- DFRRemovedCollateralCascade_2
DFRToRemoveLeaf_1 <- DFRManual_1
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);
}
public function testGetStaticDeletionPlan_UpdateMultipleExtKeys()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRToUpdate_1 (extkey_id)
DFRToRemoveLeaf_2 <- DFRToUpdate_2 (extkey2_id)
DFRLeafNotToRemove_1 <- DFRToUpdate_3 (extkey_id)
EOF);
$aClasses = [ 'DFRToRemoveLeaf' ];
$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);
}
public function testGetCleanupSummary()
{
$this->GivenDFRTreeInDB(<<<EOF
DFRToRemoveLeaf_1 <- DFRToUpdate_1
DFRToRemoveLeaf_1 <- DFRRemovedCollateral_1
DFRRemovedCollateral_1 <- DFRRemovedCollateralCascade_1
DFRRemovedCollateral_1 <- DFRRemovedCollateralCascade_2
DFRToRemoveLeaf_1 <- DFRManual_1
EOF);
$aClasses = [ 'DFRToRemoveLeaf' ];
$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);
}
}