diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index 8ed4f7eef..7c08e2500 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -4594,6 +4594,7 @@ HTML; public function DBUpdate() { $this->LogCRUDEnter(__METHOD__); + $res = 0; try { if (count($this->ListChanges()) === 0) { @@ -4654,6 +4655,7 @@ HTML; if (static::IsCrudStackEmpty()) { // Avoid signaling the current object that links were modified static::RemoveObjectAwaitingEventDbLinksChanged(get_class($this), $this->GetKey()); + $this->LogCRUDDebug(__METHOD__, var_export(self::$aObjectsAwaitingEventDbLinksChanged, true)); static::FireEventDbLinksChangedForAllObjects(); } } @@ -4662,6 +4664,11 @@ HTML; return $oDeletionPlan; } + protected function PostDeleteActions(): void + { + parent::PostDeleteActions(); + } + /** * @deprecated 3.1.1 3.2.0 N°6967 We will have only one DBDelete method in the future */ @@ -5956,7 +5963,6 @@ JS final protected function FireEventAfterDelete(): void { $this->NotifyAttachedObjectsOnLinkClassModification(); - $this->FireEventDbLinksChangedForCurrentObject(); $this->FireEvent(EVENT_DB_AFTER_DELETE); } @@ -5989,13 +5995,16 @@ JS } $sTargetObjectId = $this->Get($sExternalKeyAttCode); + $sTargetClass = $oAttDef->GetTargetClass(); if ($sTargetObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sTargetObjectId); + $this->LogCRUDDebug(__METHOD__, "Add $sTargetClass:$sTargetObjectId for DBLINKS_CHANGED"); + self::RegisterObjectAwaitingEventDbLinksChanged($sTargetClass, $sTargetObjectId); } $sPreviousTargetObjectId = $aPreviousValues[$sExternalKeyAttCode] ?? 0; if ($sPreviousTargetObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sPreviousTargetObjectId); + $this->LogCRUDDebug(__METHOD__, "Add $sTargetClass:$sPreviousTargetObjectId for DBLINKS_CHANGED"); + self::RegisterObjectAwaitingEventDbLinksChanged($sTargetClass, $sPreviousTargetObjectId); } } } @@ -6045,7 +6054,12 @@ JS $sClass = get_class($this); $sId = $this->GetKey(); - self::FireEventDbLinksChangedForClassId($sClass, $sId); + $bIsObjectAwaitingEventDbLinksChanged = self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId); + if (false === $bIsObjectAwaitingEventDbLinksChanged) { + return; + } + self::FireEventDbLinksChangedForObject($this); + self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId); } /** @@ -6077,9 +6091,15 @@ JS // We want to avoid launching the listener twice, first here, and secondly after saving the Ticket in the listener // By disabling the event to be fired, we can remove the current object from the attribute ! $oObject = MetaModel::GetObject($sClass, $sId, false); + self::FireEventDbLinksChangedForObject($oObject); + self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId); + } + + private static function FireEventDbLinksChangedForObject(DBObject $oObject) + { + self::SetEventDBLinksChangedBlocked(true); // N°6408 The object can have been deleted if (!is_null($oObject)) { - self::SetEventDBLinksChangedBlocked(true); MetaModel::StartReentranceProtection($oObject); $oObject->FireEvent(EVENT_DB_LINKS_CHANGED); MetaModel::StopReentranceProtection($oObject); @@ -6087,7 +6107,6 @@ JS $oObject->DBUpdate(); } } - self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId); cmdbAbstractObject::SetEventDBLinksChangedBlocked(false); } diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 0107ce3fe..d5db209fc 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -765,6 +765,18 @@ abstract class DBObject implements iDisplay $this->Set($sAttCode, $sValue); } + /** + * @return void + * @throws \ReflectionException + */ + protected function PostDeleteActions(): void + { + $this->FireEventAfterDelete(); + $oKPI = new ExecutionKPI(); + $this->AfterDelete(); + $oKPI->ComputeStatsForExtension($this, 'AfterDelete'); + } + /** * Compute (and optionally start) the StopWatches deadlines * @@ -4205,15 +4217,13 @@ abstract class DBObject implements iDisplay } } - $this->FireEventAfterDelete(); - $oKPI = new ExecutionKPI(); - $this->AfterDelete(); - $oKPI->ComputeStatsForExtension($this, 'AfterDelete'); + $this->m_bIsInDB = false; + + $this->PostDeleteActions(); // - Trigger for object pointing to the current object $this->ActivateOnObjectUpdateTriggersForTargetObjects(); - $this->m_bIsInDB = false; $this->LogCRUDExit(__METHOD__); // Fix for N°926: do NOT reset m_iKey as it can be used to have it for reporting purposes (see the REST service to delete // objects, reported as bug N°926) @@ -4269,6 +4279,7 @@ abstract class DBObject implements iDisplay foreach ($aToDelete as $iId => $aData) { /** @var \DBObject $oToDelete */ $oToDelete = $aData['to_delete']; + // The deletion based on a deletion plan should not be done for each object if the deletion plan is common (Trac #457) // because for each object we would try to update all the preceding ones... that are already deleted // A better approach would be to change the API to apply the DBDelete on the deletion plan itself... just once diff --git a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php index ebcb2e38c..024cfcc1b 100644 --- a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php @@ -14,12 +14,22 @@ use DBObject; use DBObject\MockDBObjectWithCRUDEventListener; use DBObjectSet; use DBSearch; +use IssueLog; +use lnkFunctionalCIToTicket; use lnkPersonToTeam; use MetaModel; use ormLinkSet; use Person; +use Server; use Team; +use UserRequest; use utils; +use const EVENT_DB_AFTER_DELETE; +use const EVENT_DB_AFTER_WRITE; +use const EVENT_DB_BEFORE_WRITE; +use const EVENT_DB_CHECK_TO_DELETE; +use const EVENT_DB_CHECK_TO_WRITE; +use const EVENT_DB_COMPUTE_VALUES; use const EVENT_DB_LINKS_CHANGED; class CRUDEventTest extends ItopDataTestCase @@ -30,12 +40,32 @@ class CRUDEventTest extends ItopDataTestCase // Count the events by name private static array $aEventCalls = []; private static int $iEventCalls = 0; + private static string $sLogFile = 'log/test_error_CRUDEventTest.log'; protected function setUp(): void { - static::$aEventCalls = []; - static::$iEventCalls = 0; + static::CleanCallCount(); parent::setUp(); + static::$DEBUG_UNIT_TEST = false; + + if (static::$DEBUG_UNIT_TEST) { + echo "--- logging in ".APPROOT.static::$sLogFile."\n\n"; + @unlink(APPROOT.static::$sLogFile); + IssueLog::Enable(APPROOT.static::$sLogFile); + $oConfig = utils::GetConfig(); + $oConfig->Set('log_level_min', ['DMCRUD' => 'Trace']); + } + } + + protected function tearDown(): void + { + if (is_file(APPROOT.static::$sLogFile)) { + $sLog = file_get_contents(APPROOT.static::$sLogFile); + echo "--- error.log\n$sLog\n\n"; + @unlink(APPROOT.static::$sLogFile); + } + + parent::tearDown(); } public static function IncrementCallCount(string $sEvent) @@ -44,6 +74,13 @@ class CRUDEventTest extends ItopDataTestCase self::$iEventCalls++; } + public static function CleanCallCount() + { + self::$aEventCalls = []; + + self::$iEventCalls = 0; + } + /** * Check that the 3 events EVENT_DB_COMPUTE_VALUES, EVENT_DB_CHECK_TO_WRITE and EVENT_DB_AFTER_WRITE are called on insert * @@ -343,6 +380,54 @@ class CRUDEventTest extends ItopDataTestCase $this->assertEquals(16, self::$iEventCalls); } + public function testDBDeleteUR() + { + // Prepare the link set + $sLinkedClass = lnkFunctionalCIToTicket::class; + $aLinkedObjectsArray = []; + $oSet = DBObjectSet::FromArray($sLinkedClass, $aLinkedObjectsArray); + $oLinkSet = new ormLinkSet(UserRequest::class, 'functionalcis_list', $oSet); + + // Create the 3 servers + for ($i = 0; $i < 3; $i++) { + $oServer = $this->CreateServer($i); + $this->assertIsObject($oServer); + // Add the person to the link + $oLink = MetaModel::NewObject(lnkFunctionalCIToTicket::class, ['functionalci_id' => $oServer->GetKey()]); + $oLinkSet->AddItem($oLink); + } + + $this->debug("\n-------------> Insert Starts HERE\n"); + + $oEventReceiver = new CRUDEventReceiver($this); + $oEventReceiver->RegisterCRUDListeners(); + + $oUserRequest = MetaModel::NewObject(UserRequest::class, array_merge($this->GetUserRequestParams(0), ['functionalcis_list' => $oLinkSet])); + $oUserRequest->DBInsert(); + $this->assertIsObject($oUserRequest); + + // 1 insert for UserRequest, 3 insert for lnkFunctionalCIToTicket + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_COMPUTE_VALUES]); + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_CHECK_TO_WRITE]); + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_BEFORE_WRITE]); + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_AFTER_WRITE]); + $this->assertEquals(1, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); + $this->assertEquals(17, self::$iEventCalls); + + $this->debug("\n-------------> Delete Starts HERE\n"); + + $oEventReceiver->CleanCallbacks(); + self::CleanCallCount(); + $oUserRequest->DBDelete(); + + // 1 delete for UserRequest, 3 delete for lnkFunctionalCIToTicket + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_CHECK_TO_DELETE]); + $this->assertEquals(4, self::$aEventCalls[EVENT_DB_AFTER_DELETE]); + $this->assertArrayNotHasKey(EVENT_DB_LINKS_CHANGED, self::$aEventCalls, 'no relation with the with_php_compute attribute !'); + $this->assertEquals(8, self::$iEventCalls); + + } + /** * The test creates a team containing one Person. * During the insert of the lnkPersonToTeam a modification is done on the link,