N°7080 - EVENT_DB_LINKS_CHANGED not fired when deleting a user

Fix regressions caused by deletion plan reorder

application/cmdbabstract.class.inc.php

@@ -4591,6 +4591,7 @@ HTML;
 	public function DBUpdate()
 	{
 		$this->LogCRUDEnter(__METHOD__);
+		$res = 0;
 
 		try {
 			if (count($this->ListChanges()) === 0) {
@@ -4651,6 +4652,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();
 			}
 		}
@@ -4659,6 +4661,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
 	 */
@@ -5953,7 +5960,6 @@ JS
 	final protected function FireEventAfterDelete(): void
 	{
 		$this->NotifyAttachedObjectsOnLinkClassModification();
-		$this->FireEventDbLinksChangedForCurrentObject();
 		$this->FireEvent(EVENT_DB_AFTER_DELETE);
 	}
 
@@ -5986,13 +5992,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);
 			}
 		}
 	}
@@ -6042,7 +6051,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);
 	}
 
 	/**
@@ -6074,9 +6088,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);
@@ -6084,7 +6104,6 @@ JS
 				$oObject->DBUpdate();
 			}
 		}
-		self::RemoveObjectAwaitingEventDbLinksChanged($sClass, $sId);
 		cmdbAbstractObject::SetEventDBLinksChangedBlocked(false);
 	}
 

core/dbobject.class.php

@@ -763,6 +763,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
 	 *
@@ -4203,15 +4215,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)
@@ -4267,6 +4277,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

core/userrights.class.inc.php

@@ -248,7 +248,7 @@ abstract class User extends cmdbAbstractObject
 			"depends_on"      => array(),
 		)));
 
-		MetaModel::Init_AddAttribute(new AttributeLinkedSetIndirect("profile_list",array("linked_class" => "URP_UserProfile", "ext_key_to_me" => "userid", "ext_key_to_remote" => "profileid", "allowed_values" => null, "count_min" => 1, "count_max" => 0, "depends_on" => array(), "display_style" => 'property', "with_php_constraint" => true)));
+		MetaModel::Init_AddAttribute(new AttributeLinkedSetIndirect("profile_list",array("linked_class" => "URP_UserProfile", "ext_key_to_me" => "userid", "ext_key_to_remote" => "profileid", "allowed_values" => null, "count_min" => 1, "count_max" => 0, "depends_on" => array(), "display_style" => 'property', "with_php_constraint" => true, "with_php_computation" => true)));
 		MetaModel::Init_AddAttribute(new AttributeLinkedSetIndirect("allowed_org_list", array("linked_class" => "URP_UserOrg", "ext_key_to_me" => "userid", "ext_key_to_remote" => "allowed_org_id", "allowed_values" => null, "count_min" => 1, "count_max" => 0, "depends_on" => array(), 'with_php_constraint' => true)));
 		MetaModel::Init_AddAttribute(new AttributeCaseLog("log", array("sql" => 'log', "is_null_allowed" => true, "default_value" => '', "allowed_values" => null, "depends_on" => array(), "always_load_in_tables" => false)));
 

pages/oauth.landing.php

@@ -10,6 +10,5 @@ IssueLog::Trace('----- Request: '.utils::GetRequestUri(), LogChannels::WEB_REQUE
 $sTemplates = APPROOT.'templates/pages/backoffice/oauth';
 
 $oUpdateController = new OAuthLandingController($sTemplates, 'core');
-$oUpdateController->AllowOnlyAdmin();
 $oUpdateController->SetDefaultOperation('Landing');
 $oUpdateController->HandleOperation();

tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php

@@ -232,7 +232,7 @@ PHP
 	public function WithConstraintParameterProvider()
 	{
 		return [
-			['User', 'profile_list', true, false],
+			['User', 'profile_list', true, true],
 			['User', 'allowed_org_list', true, false],
 			['Person', 'team_list', false, false],
 			['Ticket', 'functionalcis_list', false, true],

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,