N°5906 - CRUD Event - Fix listener launched twice and refactor calls to Tickets::UpdateImpactedItems()

application/cmdbabstract.class.inc.php

@@ -4533,8 +4533,6 @@ HTML;
 			}
 		} finally {
 			if (static::IsCrudStackEmpty()) {
-				// Avoid signaling the current object that links were modified
-				static::RemoveObjectAwaitingEventDbLinksChanged(get_class($this), $this->GetKey());
 				static::FireEventDbLinksChangedForAllObjects();
 			}
 		}
@@ -5811,6 +5809,7 @@ JS
 	final protected function FireEventCreateDone(): void
 	{
 		$this->NotifyAttachedObjectsOnLinkClassModification();
+		$this->FireEventDbLinksChangedForCurrentObject();
 		$this->FireEvent(EVENT_DB_CREATE_DONE);
 	}
 
@@ -5829,6 +5828,7 @@ JS
 	final protected function FireEventUpdateDone(array $aChanges): void
 	{
 		$this->NotifyAttachedObjectsOnLinkClassModification();
+		$this->FireEventDbLinksChangedForCurrentObject();
 		$this->FireEvent(EVENT_DB_UPDATE_DONE, ['changes' => $aChanges]);
 	}
 
@@ -5857,6 +5857,7 @@ JS
 	final protected function FireEventDeleteDone(): void
 	{
 		$this->NotifyAttachedObjectsOnLinkClassModification();
+		$this->FireEventDbLinksChangedForCurrentObject();
 		$this->FireEvent(EVENT_DB_DELETE_DONE);
 	}
 
@@ -5955,11 +5956,24 @@ JS
 			return;
 		}
 
-		$oObject = MetaModel::GetObject($sClass, $sId);
+		// First we are disabling firing the event to avoid reentrance
+		// For example on a Ticket :
+		// - in the Ticket CRUD stack, DBWriteLinks will generate lnkApplicationSolutionToFunctionalCI instances
+		// - therefore the $aObjectsAwaitingEventDbLinksChanged attribute will contain our Ticket
+		// - we have a EVENT_DB_LINKS_CHANGED listener on Ticket that will update impacted items, so it will create new lnkApplicationSolutionToFunctionalCI
+		// 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 !
+		/** @noinspection PhpRedundantOptionalArgumentInspection */
+		$oObject = MetaModel::GetObject($sClass, $sId, true);
+		self::SetEventDBLinksChangedBlocked(true);
+		MetaModel::StartReentranceProtection($oObject);
 		$oObject->FireEvent(EVENT_DB_LINKS_CHANGED);
-
+		MetaModel::StopReentranceProtection($oObject);
-		// The event listeners might have generated new lnk instances pointing to this object, so removing object from stack to avoid reentrance
+		if ($oObject->IsModified()) {
+			$oObject->DBUpdate();
+		}
 		self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId);
+		cmdbAbstractObject::SetEventDBLinksChangedBlocked(false);
 	}
 
 	/**

datamodels/2.x/itop-change-mgmt-itil/datamodel.itop-change-mgmt-itil.xml

@@ -1022,8 +1022,7 @@
           <code><![CDATA[
     protected function OnInsert()
 	{
-        parent::OnInsert();
+    parent::OnInsert();
-        $this->UpdateImpactedItems();
 		$this->SetIfNull('creation_date', time());
 		$this->SetIfNull('last_update', time());
 	}]]></code>
@@ -1036,11 +1035,6 @@
     protected function OnUpdate()
   	{
         parent::OnUpdate();
-        $aChanges = $this->ListChanges();
-        if (array_key_exists('functionalcis_list', $aChanges))
-        {
-            $this->UpdateImpactedItems();
-        }
   	    $this->Set('last_update', time());
   	}]]></code>
         </method>

datamodels/2.x/itop-change-mgmt/datamodel.itop-change-mgmt.xml

@@ -581,8 +581,7 @@
           <code><![CDATA[
     protected function OnInsert()
 	{
-        parent::OnInsert();
+    parent::OnInsert();
-        $this->UpdateImpactedItems();
 		$this->SetIfNull('creation_date', time());
 		$this->SetIfNull('last_update', time());
 	}]]></code>
@@ -594,12 +593,7 @@
           <code><![CDATA[
     protected function OnUpdate()
 	{
-        parent::OnUpdate();
+    parent::OnUpdate();
-        $aChanges = $this->ListChanges();
-        if (array_key_exists('functionalcis_list', $aChanges))
-        {
-            $this->UpdateImpactedItems();
-        }
 		$this->Set('last_update', time());
 	}]]></code>
         </method>

datamodels/2.x/itop-incident-mgmt-itil/datamodel.itop-incident-mgmt-itil.xml

@@ -1496,12 +1496,7 @@
           <type>Overload-DBObject</type>
           <code><![CDATA[	protected function OnUpdate()
 	{
-        parent::OnUpdate();
+      parent::OnUpdate();
-        $aChanges = $this->ListChanges();
-        if (array_key_exists('functionalcis_list', $aChanges))
-        {
-            $this->UpdateImpactedItems();
-        }
 		$this->Set('last_update', time());
 		$this->UpdateChildRequestLog();
 		$this->UpdateChildIncidentLog();

datamodels/2.x/itop-request-mgmt-itil/datamodel.itop-request-mgmt-itil.xml

@@ -1567,11 +1567,6 @@
     protected function OnUpdate()
 	{
         parent::OnUpdate();
-        $aChanges = $this->ListChanges();
-        if (array_key_exists('functionalcis_list', $aChanges))
-        {
-            $this->UpdateImpactedItems();
-        }
 		$this->Set('last_update', time());
 		$this->UpdateChildRequestLog();
 	}]]></code>

datamodels/2.x/itop-request-mgmt/datamodel.itop-request-mgmt.xml

@@ -1610,12 +1610,7 @@
           <code><![CDATA[
    	protected function OnUpdate()
 	{
-        parent::OnUpdate();
+    parent::OnUpdate();
-        $aChanges = $this->ListChanges();
-        if (array_key_exists('functionalcis_list', $aChanges))
-        {
-            $this->UpdateImpactedItems();
-        }
 		$this->Set('last_update', time());
 		$this->UpdateChildRequestLog();
 	}]]></code>

datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml

@@ -223,23 +223,11 @@
       <event_listeners>
         <listener id="UpdateImpactAnalysis">
           <event>EVENT_DB_LINKS_CHANGED</event>
-          <callback>UpdateTicketImpactedItems</callback>
+          <callback>UpdateImpactedItems</callback>
           <rank>0</rank>
         </listener>
       </event_listeners>
       <methods>
-        <method id="UpdateTicketImpactedItems">
-        <comment/>
-        <static>false</static>
-        <access>public</access>
-        <type>EventListener</type>
-        <code><![CDATA[
-public function UpdateTicketImpactedItems(Combodo\iTop\Service\Events\EventData $oEventData) {
-    $this->UpdateImpactedItems();
-    $this->DBUpdate();
-}
-            ]]></code>
-        </method>
         <method id="DBInsertNoReload">
           <static>false</static>
           <access>public</access>

tests/php-unit-tests/unitary-tests/application/cmdbAbstractObjectTest.php

@@ -195,8 +195,8 @@ class cmdbAbstractObjectTest extends ItopDataTestCase {
 		$oTeam->DBInsert();
 		$this->assertIsObject($oTeam);
 
-		// 3 links added to person (the Team side is ignored)
+		// 3 links added to person  + 1 for the Team
-		$this->assertEquals(3, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]);
+		$this->assertEquals(4, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]);
 	}
 
 	/**