From 9a59bc78902467ac543c12000192520dd12666d8 Mon Sep 17 00:00:00 2001 From: odain-cbd <56586767+odain-cbd@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:46:00 +0100 Subject: [PATCH 1/4] =?UTF-8?q?N=C2=B06901=20-=20Enable=20tracking=20of=20?= =?UTF-8?q?iTop=20active=20sessions=20(monitoring)=20(#568)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Molkobain Co-authored-by: Romain Quetiez --- application/menunode.class.inc.php | 30 +-- core/cmdbsource.class.inc.php | 26 +- core/config.class.inc.php | 24 ++ core/log.class.inc.php | 7 +- lib/composer/autoload_classmap.php | 2 + lib/composer/autoload_static.php | 2 + pages/ajax.render.php | 2 +- sources/Application/Helper/Session.php | 4 +- sources/SessionTracker/SessionGC.php | 32 +++ sources/SessionTracker/SessionHandler.php | 240 +++++++++++++++++ .../SessionTracker/SessionHandlerTest.php | 241 ++++++++++++++++++ webservices/cron.php | 4 +- 12 files changed, 581 insertions(+), 33 deletions(-) create mode 100644 sources/SessionTracker/SessionGC.php create mode 100644 sources/SessionTracker/SessionHandler.php create mode 100644 tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php diff --git a/application/menunode.class.inc.php b/application/menunode.class.inc.php index d76616e6e..3a5006aa9 100644 --- a/application/menunode.class.inc.php +++ b/application/menunode.class.inc.php @@ -103,7 +103,7 @@ class ApplicationMenu { self::$sFavoriteSiloQuery = $sOQL; } - + /** * Get the query used to limit the list of displayed organizations in the drop-down menu * @return string The OQL query returning a list of Organization objects @@ -536,7 +536,7 @@ EOF return -1; } - + /** * Retrieves the currently active menu (if any, otherwise the first menu is the default) * @return string The Id of the currently active menu @@ -544,7 +544,7 @@ EOF public static function GetActiveNodeId() { $oAppContext = new ApplicationContext(); - $sMenuId = $oAppContext->GetCurrentValue('menu', null); + $sMenuId = $oAppContext->GetCurrentValue('menu', null); if ($sMenuId === null) { $sMenuId = self::GetDefaultMenuId(); @@ -654,7 +654,7 @@ abstract class MenuNode /** * Stimulus to check: if the user can 'apply' this stimulus, then she/he can see this menu - */ + */ protected $m_aEnableStimuli; /** @@ -814,7 +814,7 @@ abstract class MenuNode { return false; } - + /** * Add a limiting display condition for the same menu node. The conditions will be combined with a AND * @param $oMenuNode MenuNode Another definition of the same menu node, with potentially different access restriction @@ -987,7 +987,7 @@ class TemplateMenuNode extends MenuNode * @var string */ protected $sTemplateFile; - + /** * Create a menu item based on a custom template and inserts it into the application's main menu * @param string $sMenuId Unique identifier of the menu (used to identify the menu for bookmarking, and for getting the labels from the dictionary) @@ -1058,7 +1058,7 @@ class OQLMenuNode extends MenuNode * @var bool|null */ protected $bSearchFormOpen; - + /** * Extra parameters to be passed to the display block to fine tune its appearence */ @@ -1091,7 +1091,7 @@ class OQLMenuNode extends MenuNode // Enhancement: we could set as the "enable" condition that the user has enough rights to "read" the objects // of the class specified by the OQL... } - + /** * Set some extra parameters to be passed to the display block to fine tune its appearence * @param array $aParams paramCode => value. See DisplayBlock::GetDisplay for the meaning of the parameters @@ -1111,7 +1111,7 @@ class OQLMenuNode extends MenuNode */ public function RenderContent(WebPage $oPage, $aExtraParams = array()) { - ContextTag::AddContext(ContextTag::TAG_OBJECT_SEARCH); + $oTag = new ContextTag(ContextTag::TAG_OBJECT_SEARCH); ApplicationMenu::CheckMenuIdEnabled($this->GetMenuId()); OQLMenuNode::RenderOQLSearch ( @@ -1120,7 +1120,7 @@ class OQLMenuNode extends MenuNode 'Menu_'.$this->GetMenuId(), $this->bSearch, // Search pane $this->bSearchFormOpen, // Search open - $oPage, + $oPage, array_merge($this->m_aParams, $aExtraParams), true ); @@ -1354,10 +1354,10 @@ class NewObjectMenuNode extends MenuNode { // Enable this menu, only if the current user has enough rights to create such an object, or an object of // any child class - + $aSubClasses = MetaModel::EnumChildClasses($this->sClass, ENUM_CHILD_CLASSES_ALL); // Including the specified class itself $bActionIsAllowed = false; - + foreach($aSubClasses as $sCandidateClass) { if (!MetaModel::IsAbstract($sCandidateClass) && (UserRights::IsActionAllowed($sCandidateClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES)) @@ -1366,7 +1366,7 @@ class NewObjectMenuNode extends MenuNode break; // Enough for now } } - return $bActionIsAllowed; + return $bActionIsAllowed; } /** @@ -1508,7 +1508,7 @@ class DashboardMenuNode extends MenuNode throw new Exception("Error: failed to load dashboard file: '{$this->sDashboardFile}'"); } } - + } /** @@ -1549,7 +1549,7 @@ class ShortcutContainerMenuNode extends MenuNode $sName = $this->GetMenuId().'_'.$oShortcut->GetKey(); new ShortcutMenuNode($sName, $oShortcut, $this->GetIndex(), $fRank++); } - + // Complete the tree // parent::PopulateChildMenus(); diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index e05d4e94a..5feb077ba 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -3,7 +3,7 @@ // // This file is part of iTop. // -// iTop is free software; you can redistribute it and/or modify +// iTop is free software; you can redistribute it and/or modify // it under the terms of the GNU Affero General Public License as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. @@ -380,7 +380,7 @@ class CMDBSource public static function GetDBVendor() { $sDBVendor = static::ENUM_DB_VENDOR_MYSQL; - + $sVersionComment = static::GetServerVariable('version') . ' - ' . static::GetServerVariable('version_comment'); if(preg_match('/mariadb/i', $sVersionComment) === 1) { @@ -390,7 +390,7 @@ class CMDBSource { $sDBVendor = static::ENUM_DB_VENDOR_PERCONA; } - + return $sDBVendor; } @@ -934,7 +934,7 @@ class CMDBSource { throw new MySQLException('Failed to issue SQL query', array('query' => $sSql)); } - + while ($aRow = $oResult->fetch_array($iMode)) { $aData[] = $aRow; @@ -1088,7 +1088,7 @@ class CMDBSource if (!array_key_exists($iKey, $aTableInfo["Fields"])) return false; $aFieldData = $aTableInfo["Fields"][$iKey]; if (!array_key_exists("Key", $aFieldData)) return false; - return ($aFieldData["Key"] == "PRI"); + return ($aFieldData["Key"] == "PRI"); } public static function IsAutoIncrement($sTable, $sField) @@ -1099,7 +1099,7 @@ class CMDBSource $aFieldData = $aTableInfo["Fields"][$sField]; if (!array_key_exists("Extra", $aFieldData)) return false; //MyHelpers::debug_breakpoint($aFieldData); - return (strstr($aFieldData["Extra"], "auto_increment")); + return (strstr($aFieldData["Extra"], "auto_increment")); } public static function IsField($sTable, $sField) @@ -1366,13 +1366,13 @@ class CMDBSource public static function GetTableFieldsList($sTable) { assert(!empty($sTable)); - + $aTableInfo = self::GetTableInfo($sTable); if (empty($aTableInfo)) return array(); // #@# or an error ? return array_keys($aTableInfo["Fields"]); } - + // Cache the information about existing tables, and their fields private static $m_aTablesInfo = array(); private static function _TablesInfoCacheReset($sTableName = null) @@ -1505,7 +1505,7 @@ class CMDBSource { throw new MySQLException('Failed to issue SQL query', array('query' => $sSql)); } - + $aRows = array(); while ($aRow = $oResult->fetch_array(MYSQLI_ASSOC)) { @@ -1514,7 +1514,7 @@ class CMDBSource $oResult->free(); return $aRows; } - + /** * Returns the value of the specified server variable * @param string $sVarName Name of the server variable @@ -1530,7 +1530,7 @@ class CMDBSource /** * Returns the privileges of the current user * @return string privileges in a raw format - */ + */ public static function GetRawPrivileges() { try @@ -1556,8 +1556,8 @@ class CMDBSource /** * Determine the slave status of the server - * @return bool true if the server is slave - */ + * @return bool true if the server is slave + */ public static function IsSlaveServer() { try diff --git a/core/config.class.inc.php b/core/config.class.inc.php index f23e80e2a..2f50949b4 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1193,6 +1193,30 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'sessions_tracking.enabled' => [ + 'type' => 'bool', + 'description' => 'Whether or not the whole mechanism to track active sessions is enabled. See PHP session.gc_maxlifetime setting to configure session expiration.', + 'default' => false, + 'value' => '', + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], + 'sessions_tracking.gc_threshold' => [ + 'type' => 'integer', + 'description' => 'fallback in case cron is not active: probability in percent that session files are cleanup during any itop request (100 means always)', + 'default' => 1, + 'value' => '', + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], + 'sessions_tracking.gc_duration_in_seconds' => [ + 'type' => 'integer', + 'description' => 'fallback in case cron is not active: when a cleanup is triggered cleanup duration will not exceed this duration (in seconds).', + 'default' => 1, + 'value' => '', + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], 'transaction_storage' => [ 'type' => 'string', 'description' => 'The type of mechanism to use for storing the unique identifiers for transactions (Session|File).', diff --git a/core/log.class.inc.php b/core/log.class.inc.php index 29b359689..a8b920815 100644 --- a/core/log.class.inc.php +++ b/core/log.class.inc.php @@ -3,7 +3,7 @@ // // This file is part of iTop. // -// iTop is free software; you can redistribute it and/or modify +// iTop is free software; you can redistribute it and/or modify // it under the terms of the GNU Affero General Public License as published by // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. @@ -576,6 +576,11 @@ class LogChannels public const DATATABLE = 'Datatable'; public const DEADLOCK = 'DeadLock'; + /** + * @var string Everything related to PHP sessions tracking + * @since 3.1.1 3.2.0 N°6901 + */ + public const SESSIONTRACKER = 'SessionTracker'; /** * @var string Everything related to the datamodel CRUD diff --git a/lib/composer/autoload_classmap.php b/lib/composer/autoload_classmap.php index 4989deb64..80e888e24 100644 --- a/lib/composer/autoload_classmap.php +++ b/lib/composer/autoload_classmap.php @@ -473,6 +473,8 @@ return array( 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectManager' => $baseDir . '/sources/Service/TemporaryObjects/TemporaryObjectManager.php', 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectRepository' => $baseDir . '/sources/Service/TemporaryObjects/TemporaryObjectRepository.php', 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectsEvents' => $baseDir . '/sources/Service/TemporaryObjects/TemporaryObjectsEvents.php', + 'Combodo\\iTop\\SessionTracker\\SessionGC' => $baseDir . '/sources/SessionTracker/SessionGC.php', + 'Combodo\\iTop\\SessionTracker\\SessionHandler' => $baseDir . '/sources/SessionTracker/SessionHandler.php', 'CompileCSSService' => $baseDir . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'Config' => $baseDir . '/core/config.class.inc.php', diff --git a/lib/composer/autoload_static.php b/lib/composer/autoload_static.php index da75d88ba..065c0f934 100644 --- a/lib/composer/autoload_static.php +++ b/lib/composer/autoload_static.php @@ -837,6 +837,8 @@ class ComposerStaticInit7f81b4a2a468a061c306af5e447a9a9f 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectManager' => __DIR__ . '/../..' . '/sources/Service/TemporaryObjects/TemporaryObjectManager.php', 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectRepository' => __DIR__ . '/../..' . '/sources/Service/TemporaryObjects/TemporaryObjectRepository.php', 'Combodo\\iTop\\Service\\TemporaryObjects\\TemporaryObjectsEvents' => __DIR__ . '/../..' . '/sources/Service/TemporaryObjects/TemporaryObjectsEvents.php', + 'Combodo\\iTop\\SessionTracker\\SessionGC' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionGC.php', + 'Combodo\\iTop\\SessionTracker\\SessionHandler' => __DIR__ . '/../..' . '/sources/SessionTracker/SessionHandler.php', 'CompileCSSService' => __DIR__ . '/../..' . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'Config' => __DIR__ . '/../..' . '/core/config.class.inc.php', diff --git a/pages/ajax.render.php b/pages/ajax.render.php index 2981273ab..a200cce29 100644 --- a/pages/ajax.render.php +++ b/pages/ajax.render.php @@ -68,7 +68,7 @@ try break; default: - ContextTag::AddContext(ContextTag::TAG_CONSOLE); + $oTag = new ContextTag(ContextTag::TAG_CONSOLE); } // First check if we can redirect the route to a dedicated controller diff --git a/sources/Application/Helper/Session.php b/sources/Application/Helper/Session.php index 4b708a61e..867e3ca70 100644 --- a/sources/Application/Helper/Session.php +++ b/sources/Application/Helper/Session.php @@ -7,6 +7,7 @@ namespace Combodo\iTop\Application\Helper; +use Combodo\iTop\SessionTracker\SessionHandler; use utils; /** @@ -34,6 +35,7 @@ class Session } if (!self::$bIsInitialized) { + SessionHandler::session_set_save_handler(); session_name('itop-'.md5(APPROOT)); } @@ -214,4 +216,4 @@ class Session return utils::IsModeCLI(); } -} \ No newline at end of file +} diff --git a/sources/SessionTracker/SessionGC.php b/sources/SessionTracker/SessionGC.php new file mode 100644 index 000000000..8653ea26d --- /dev/null +++ b/sources/SessionTracker/SessionGC.php @@ -0,0 +1,32 @@ + + * @package Combodo\iTop\SessionTracker + * @since 3.1.1 3.2.0 N°6901 + */ +class SessionGC implements \iBackgroundProcess +{ + /** + * @inheritDoc + */ + public function GetPeriodicity() + { + return 60 * 1; // seconds + } + + /** + * @inheritDoc + */ + public function Process($iTimeLimit) + { + $iMaxLifetime = ini_get('session.gc_maxlifetime') ?? 1440; + $oSessionHandler = new SessionHandler(); + $iProcessed = $oSessionHandler->gc_with_time_limit($iMaxLifetime, $iTimeLimit); + return "processed $iProcessed tasks"; + } +} diff --git a/sources/SessionTracker/SessionHandler.php b/sources/SessionTracker/SessionHandler.php new file mode 100644 index 000000000..5aa0c4072 --- /dev/null +++ b/sources/SessionTracker/SessionHandler.php @@ -0,0 +1,240 @@ + + * @package Combodo\iTop\SessionTracker + * @since 3.1.1 3.2.0 N°6901 + */ +class SessionHandler extends \SessionHandler +{ + /** + * @param string $session_id + * + * @return bool + */ + #[ReturnTypeWillChange] + public function destroy($session_id) : bool + { + IssueLog::Debug("Destroy PHP session", \LogChannels::SESSIONTRACKER, [ + 'session_id' => $session_id, + ]); + $bRes = parent::destroy($session_id); + + if ($bRes) { + $this->unlink_session_file($session_id); + } + + return $bRes; + } + + /** + * @param int $max_lifetime + */ + #[ReturnTypeWillChange] + public function gc($max_lifetime) : bool + { + IssueLog::Debug("Run PHP sessions garbage collector", \LogChannels::SESSIONTRACKER, [ + 'max_lifetime' => $max_lifetime, + ]); + $iRes = parent::gc($max_lifetime); + $this->gc_with_time_limit($max_lifetime); + return $iRes; + } + + /** + * @param string $save_path + * @param string $session_name + */ + #[ReturnTypeWillChange] + public function open($save_path, $session_name) : bool + { + $bRes = parent::open($save_path, $session_name); + + $session_id = session_id(); + IssueLog::Debug("Open PHP session", \LogChannels::SESSIONTRACKER, [ + 'session_id' => $session_id, + ]); + + if ($bRes) { + $this->touch_session_file($session_id); + } + + return $bRes; + } + + /** + * @param string $session_id + * @param string $data + * + * @return bool + */ + #[ReturnTypeWillChange] + public function write($session_id, $data) : bool + { + $bRes = parent::write($session_id, $data); + + IssueLog::Debug("Write PHP session", \LogChannels::SESSIONTRACKER, [ + 'session_id' => $session_id, + 'data' => $data, + ]); + + if ($bRes) { + $this->touch_session_file($session_id); + } + + return $bRes; + } + + public static function session_set_save_handler() : void + { + if (false === utils::GetConfig()->Get('sessions_tracking.enabled')){ + //feature disabled + return; + } + + $sessionhandler = new SessionHandler(); + session_set_save_handler($sessionhandler, true); + + $iThreshold = utils::GetConfig()->Get('sessions_tracking.gc_threshold'); + $iThreshold = min(100, $iThreshold); + $iThreshold = max(1, $iThreshold); + if ((100 != $iThreshold) && (rand(1, 100) > $iThreshold)) { + return; + } + + $iMaxLifetime = ini_get('session.gc_maxlifetime') ?? 60; + $iMaxDurationInSeconds = utils::GetConfig()->Get('sessions_tracking.gc_duration_in_seconds'); + $sessionhandler->gc_with_time_limit($iMaxLifetime, time() + $iMaxDurationInSeconds); + } + + private function generate_session_content(?string $sPreviousFileVersionContent) : ?string + { + try { + $sUserId = UserRights::GetUserId(); + if (null === $sUserId) { + return null; + } + + // Default value in case of + // - First time file creation + // - Data corruption (not a json / not an array / no previous creation_time key) + $iCreationTime = time(); + + if (! is_null($sPreviousFileVersionContent)) { + $aJson = json_decode($sPreviousFileVersionContent, true); + if (is_array($aJson) && array_key_exists('creation_time', $aJson)) { + $iCreationTime = $aJson['creation_time']; + } + } + + return json_encode ( + [ + 'login_mode' => Session::Get('login_mode'), + 'user_id' => $sUserId, + 'creation_time' => $iCreationTime, + 'context' => implode('|', ContextTag::GetStack()) + ] + ); + } catch(Exception $e) { + + } + + return null; + } + + private function get_file_path($session_id) : string + { + return utils::GetDataPath() . "sessions/session_$session_id"; + } + + private function touch_session_file($session_id) : ?string + { + if (strlen($session_id) == 0) { + return null; + } + + clearstatcache(); + if (! is_dir(utils::GetDataPath() . "sessions")) { + @mkdir(utils::GetDataPath() . "sessions"); + } + + $sFilePath = $this->get_file_path($session_id); + + $sPreviousFileVersionContent = null; + if (is_file($sFilePath)) { + $sPreviousFileVersionContent = file_get_contents($sFilePath); + } + $sNewContent = $this->generate_session_content($sPreviousFileVersionContent); + if (is_null($sNewContent) || ($sPreviousFileVersionContent === $sNewContent)) { + @touch($sFilePath); + } else { + file_put_contents($sFilePath, $sNewContent); + } + + return $sFilePath; + } + + private function unlink_session_file($session_id) + { + $sFilePath = $this->get_file_path($session_id); + if (is_file($sFilePath)) { + @unlink($sFilePath); + } + } + + /** + * @param int $max_lifetime + * @param int $iTimeLimit Unix timestamp of time limit not to exceed. -1 for no limit. + * + * @return int + */ + public function gc_with_time_limit(int $max_lifetime, int $iTimeLimit = -1) : int + { + $aFiles = $this->list_session_files(); + $iProcessed = 0; + $now = time(); + + foreach ($aFiles as $sFile) { + if ($now - filemtime($sFile) > $max_lifetime) { + @unlink($sFile); + $iProcessed++; + } + + if (-1 !== $iTimeLimit && time() > $iTimeLimit) { + //cleanup processing has to stop after $iTimeLimit + break; + } + } + + return $iProcessed; + } + + public function list_session_files() : array + { + clearstatcache(); + if (! is_dir(utils::GetDataPath() . "sessions")) { + @mkdir(utils::GetDataPath() . "sessions"); + } + + return glob(utils::GetDataPath() . "sessions/session_*"); + } +} diff --git a/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php new file mode 100644 index 000000000..ac89a8421 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php @@ -0,0 +1,241 @@ +aFiles=[]; + $this->oTag = new ContextTag(ContextTag::TAG_REST); + } + + protected function tearDown(): void + { + parent::tearDown(); + $this->oTag = null; + + foreach ($this->aFiles as $sFile){ + if (is_file($sFile)){ + @unlink($sFile); + } + } + } + + private function CreateUserAndLogIn() : ? string { + $_SESSION = []; + $oUser = $this->CreateContactlessUser("admin" . uniqid(), 1, "1234@Abcdefg"); + + \UserRights::Login($oUser->Get('login')); + return $oUser->GetKey(); + } + + private function GenerateSessionContent(SessionHandler $oSessionHandler, ?string $sPreviousFileVersionContent) : ?string { + return $this->InvokeNonPublicMethod(SessionHandler::class, "generate_session_content", $oSessionHandler, $aArgs = [$sPreviousFileVersionContent]); + } + + /* + * @covers SessionHandler::generate_session_content + */ + public function testGenerateSessionContentNoUserLoggedIn(){ + $oSessionHandler = new SessionHandler(); + $sContent = $this->GenerateSessionContent($oSessionHandler, null); + $this->assertNull($sContent, "Session content should be null when there is no user logged in"); + } + + public function GenerateSessionContentCorruptedPreviousFileContentProvider() { + return [ + 'not a json' => [ "not a json" ], + 'not an array' => [ json_encode("not an array") ], + 'array without creation_time key' => [ json_encode([]) ], + ]; + } + + /** + * @covers SessionHandler::generate_session_content + * @dataProvider GenerateSessionContentCorruptedPreviousFileContentProvider + */ + public function testGenerateSessionContent_SessionFileRepairment(?string $sFileContent){ + $sUserId = $this->CreateUserAndLogIn(); + + $oSessionHandler = new SessionHandler(); + Session::Set('login_mode', 'foo_login_mode'); + + $sContent = $this->GenerateSessionContent($oSessionHandler, $sFileContent); + + $this->assertNotNull($sContent, 'Should not return null'); + $aJson = json_decode($sContent, true); + $this->assertNotEquals(false, $aJson, 'Should return a valid json string, found: '.$sContent); + $this->assertEquals($sUserId, $aJson['user_id'] ?? '', "Should report the login of the logged in user in [user_id]: $sContent"); + $this->assertEquals(ContextTag::TAG_REST, $aJson['context'] ?? '', "Should report the context tag(s) in [context]: $sContent"); + $this->assertIsInt($aJson['creation_time'] ?? '', "Should report the session start timestamp in [creation_time]: $sContent"); + $this->assertEquals('foo_login_mode', $aJson['login_mode'] ?? '', "Should report the current login mode in [login_mode]: $sContent"); + } + + /* + * @covers SessionHandler::generate_session_content + */ + public function testGenerateSessionContent(){ + $sUserId = $this->CreateUserAndLogIn(); + + $oSessionHandler = new SessionHandler(); + Session::Set('login_mode', 'foo_login_mode'); + + //first time + $sFirstContent = $this->GenerateSessionContent($oSessionHandler, null); + + $this->assertNotNull($sFirstContent, 'Should not return null'); + $aJson = json_decode($sFirstContent, true); + $this->assertNotEquals(false, $aJson, 'Should return a valid json string, found: '.$sFirstContent); + $this->assertEquals($sUserId, $aJson['user_id'] ?? '', "Should report the login of the logged in user in [user_id]: $sFirstContent"); + $this->assertEquals(ContextTag::TAG_REST, $aJson['context'] ?? '', "Should report the context tag(s) in [context]: $sFirstContent"); + $this->assertIsInt($aJson['creation_time'] ?? '', "Should report the session start timestamp in [creation_time]: $sFirstContent"); + $this->assertEquals('foo_login_mode', $aJson['login_mode'] ?? '', "Should report the current login mode in [login_mode]: $sFirstContent"); + + $iFirstSessionCreationTime = $aJson['creation_time']; + + // Switch context + change user id via impersonation + // check it is still tracked in session files + $oOtherUser = $this->CreateContactlessUser("admin" . uniqid(), 1, "1234@Abcdefg"); + $this->assertTrue(\UserRights::Impersonate($oOtherUser->Get('login')), "Failed to execute impersonate on: ".$oOtherUser->Get('login')); + $oTag2 = new ContextTag(ContextTag::TAG_SYNCHRO); + $sNewContent = $this->GenerateSessionContent($oSessionHandler, $sFirstContent); + $this->assertNotNull($sNewContent, 'Should not return null'); + $aJson = json_decode($sNewContent, true); + $this->assertNotEquals(false, $aJson, 'Should return a valid json string, found: '.$sNewContent); + $this->assertEquals(ContextTag::TAG_REST . '|' . ContextTag::TAG_SYNCHRO, $aJson['context'] ?? '', "After impersonation, should report the new context tags in [context]: $sNewContent"); + $this->assertEquals($iFirstSessionCreationTime, $aJson['creation_time'] ?? '', "After impersonation, should still report the the session start timestamp in [creation_time]: $sNewContent"); + $this->assertEquals('foo_login_mode', $aJson['login_mode'] ?? '', "After impersonation, should still report the login mode in [login_mode]: $sNewContent"); + $this->assertEquals($oOtherUser->GetKey(), $aJson['user_id'] ?? '', "Should report the impersonate user in [user_id]: $sNewContent"); + } + + private function touchSessionFile(SessionHandler $oSessionHandler, $session_id) : ?string { + $sRes = $this->InvokeNonPublicMethod(SessionHandler::class, "touch_session_file", $oSessionHandler, $aArgs = [$session_id]); + if (!is_null($sRes) && is_file($sRes)) { + // Record the file for cleanup on tearDown + $this->aFiles[] = $sRes; + } + clearstatcache(); + return $sRes; + } + + /* + * @covers SessionHandler::touch_session_file + */ + public function testTouchSessionFile_NoUserLoggedIn(){ + $oSessionHandler = new SessionHandler(); + $session_id = uniqid(); + $sFile = $this->touchSessionFile($oSessionHandler, $session_id); + $this->assertEquals(true, is_file($sFile), "Should return a file name: '$sFile' is not a valid file name"); + $sContent = file_get_contents($sFile); + $this->assertEquals(null, $sContent, 'Should create an empty file, found: '.$sContent); + } + + /* + * @covers SessionHandler::touch_session_file + */ + public function testTouchSessionFile_UserLoggedIn(){ + $sUserId = $this->CreateUserAndLogIn(); + Session::Set('login_mode', 'foo_login_mode'); + + $oSessionHandler = new SessionHandler(); + $session_id = uniqid(); + $sFile = $this->touchSessionFile($oSessionHandler, $session_id); + $this->assertEquals(true, is_file($sFile), "Should return a file name: '$sFile' is not a valid file name"); + $sFirstContent = file_get_contents($sFile); + + $iFirstCTime = filectime($sFile) - 1; + // Set it in the past to check that it will be further updated (without the need to sleep...) + touch($sFile, $iFirstCTime); + + $this->assertNotNull($sFirstContent, 'Should not return null'); + $aJson = json_decode($sFirstContent, true); + $this->assertNotEquals(false, $aJson, 'Should return a valid json string, found: '.$sFirstContent); + $this->assertEquals($sUserId, $aJson['user_id'] ?? '', "Should report the login of the logged in user in [user_id]: $sFirstContent"); + $this->assertEquals(ContextTag::TAG_REST, $aJson['context'] ?? '', "Should report the context tag(s) in [context]: $sFirstContent"); + $this->assertIsInt($aJson['creation_time'] ?? '', "Should report the session start timestamp in [creation_time]: $sFirstContent"); + $this->assertEquals('foo_login_mode', $aJson['login_mode'] ?? '', "Should report the current login mode in [login_mode]: $sFirstContent"); + + $this->touchSessionFile($oSessionHandler, $session_id); + $sNewContent = file_get_contents($sFile); + $this->assertEquals($sFirstContent, $sNewContent, 'On successive calls, should not modify an existing session file'); + $this->assertGreaterThan($iFirstCTime, filectime($sFile), 'On successive calls, should have changed the file ctime'); + } + + /** + * @covers SessionHandler::touch_session_file + */ + public function testTouchSessionFileWithEmptySessionId() { + $this->CreateUserAndLogIn(); + Session::Set('login_mode', 'toto'); + + $oSessionHandler = new SessionHandler(); + $this->assertNull($this->touchSessionFile($oSessionHandler, ''), 'Should return null when session id is an empty string'); + $this->assertNull($this->touchSessionFile($oSessionHandler, false), 'Should return null when session id (boolean) false'); + } + + private function GetFilePath(SessionHandler $oSessionHandler, $session_id) : string { + $sFile = $this->InvokeNonPublicMethod(SessionHandler::class, "get_file_path", $oSessionHandler, $aArgs = [$session_id]); + // Record file for cleanup on tearDown + $this->aFiles[] = $sFile; + return $sFile; + } + + public function GgcWithTimeLimitProvider(){ + return [ + 'no cleanup time limit' => [ + 'iTimeLimit' => -1, + 'iExpectedProcessed' => 2 + ], + 'cleanup time limit in the pass => first file removed only' => [ + 'iTimeLimit' => time() - 1, + 'iExpectedProcessed' => 1 + ], + ]; + } + + /** + * @covers SessionHandler::gc_with_time_limit + * @covers SessionHandler::list_session_files + * @dataProvider GgcWithTimeLimitProvider + */ + public function testGgcWithTimeLimit($iTimeLimit, $iExpectedProcessed) { + $oSessionHandler = new SessionHandler(); + //remove all first + $oSessionHandler->gc_with_time_limit(-1); + $this->assertEquals([], $oSessionHandler->list_session_files(), 'list_session_files should report no file at startup'); + + $max_lifetime = 1440; + $iNbExpiredFiles = 2; + $iNbFiles = 5; + $iExpiredTimeStamp = time() - $max_lifetime - 1; + for($i=0; $i<$iNbFiles; $i++) { + $sFile = $this->GetFilePath($oSessionHandler, uniqid()); + file_put_contents($sFile, "fakedata"); + + if ($iNbExpiredFiles > 0){ + $iNbExpiredFiles--; + touch($sFile, $iExpiredTimeStamp); + } + } + + $aFoundSessionFiles = $oSessionHandler->list_session_files(); + $this->assertEquals($iNbFiles, sizeof($aFoundSessionFiles), 'list_session_files should reports all files'); + foreach ($aFoundSessionFiles as $sFile){ + $this->assertTrue(is_file($sFile), 'list_session_files should return a valid file paths, found: '.$sFile); + } + + $iProcessed = $oSessionHandler->gc_with_time_limit($max_lifetime, $iTimeLimit); + $this->assertEquals($iExpectedProcessed, $iProcessed, 'gc_with_time_limit should report the count of expired files'); + $this->assertEquals($iNbFiles - $iExpectedProcessed, sizeof($oSessionHandler->list_session_files()), 'gc_with_time_limit should actually remove all processed files'); + } +} diff --git a/webservices/cron.php b/webservices/cron.php index b39ab8efe..9e8c4bec5 100644 --- a/webservices/cron.php +++ b/webservices/cron.php @@ -108,7 +108,7 @@ function RunTask(BackgroundTask $oTask, $iTimeLimit) // Time in seconds allowed to the task $iCurrTimeLimit = $iTimeLimit; // Compute allowed time - if ($oRefClass->implementsInterface('iScheduledProcess') === false) + if ($oRefClass->implementsInterface('iScheduledProcess') === false) { // Periodic task, allow only X times ($iMaxTaskExecutionTime) its periodicity (GetPeriodicity()) $iMaxTaskExecutionTime = MetaModel::GetConfig()->Get('cron_task_max_execution_time'); @@ -148,7 +148,7 @@ function RunTask(BackgroundTask $oTask, $iTimeLimit) $oTask->Set('first_run_date', $oDateStarted->format('Y-m-d H:i:s')); } $oTask->ComputeDurations($fDuration); // does increment the counter and compute statistics - + // Update the timestamp since we want to be able to re-order the tasks based on the time they finished $oDateEnded = new DateTime(); $oTask->Set('latest_run_date', $oDateEnded->format('Y-m-d H:i:s')); From ea845dc6ebd60783adfbe2b68364d7b3f4e2fcd0 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Tue, 14 Nov 2023 12:02:45 +0100 Subject: [PATCH 2/4] =?UTF-8?q?N=C2=B06228=20-=20CheckToWrite()=20propagat?= =?UTF-8?q?ion=20to=20target=20objects=20based=20on=20with=5Fphp=5Fconstra?= =?UTF-8?q?int=20property?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/cmdbabstract.class.inc.php | 54 ++- core/attributedef.class.inc.php | 9 + core/cmdbobject.class.inc.php | 2 +- core/datamodel.core.xml | 6 + core/dbobject.class.php | 257 ++++++++++--- core/metamodel.class.php | 20 +- core/userrights.class.inc.php | 36 ++ .../itop-tickets/datamodel.itop-tickets.xml | 2 + dictionaries/en.dictionary.itop.ui.php | 1 + dictionaries/fr.dictionary.itop.ui.php | 1 + setup/compiler.class.inc.php | 2 + .../application/cmdbAbstractObjectTest.php | 350 ----------------- .../application/query/QueryTest.php | 1 + .../core/AttributeDefinitionTest.php | 29 ++ .../unitary-tests/core/CRUDEventTest.php | 14 +- .../DBObject/CheckToWritePropagationTest.php | 359 ++++++++++++++++++ .../unitary-tests/core/UserRightsTest.php | 25 -- 17 files changed, 703 insertions(+), 465 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index 6daccf162..eb52af98e 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -5354,7 +5354,7 @@ EOF $aErrors = $oObj->UpdateObjectFromPostedForm(''); $bResult = (count($aErrors) == 0); if ($bResult) { - list($bResult, $aErrors) = $oObj->CheckToWrite(); + [$bResult, $aErrors] = $oObj->CheckToWrite(); } if ($bPreview) { $sStatus = $bResult ? Dict::S('UI:BulkModifyStatusOk') : Dict::S('UI:BulkModifyStatusError'); @@ -5958,42 +5958,62 @@ JS } /** - * If the passed object is an instance of a link class, then will register each remote object for modification using {@see static::RegisterObjectAwaitingEventDbLinksChanged()} + * Possibility for linked classes to be notified of current class modification + * * If an external key was modified, register also the previous object that was linked previously. * - * @throws \ArchivedObjectException - * @throws \CoreException - * @throws \Exception + * @uses static::RegisterObjectAwaitingEventDbLinksChanged() * - * @since 3.1.0 N°5906 + * @throws ArchivedObjectException + * @throws CoreException + * @throws Exception + * + * @since 3.1.0 N°5906 method creation + * @since 3.1.1 3.2.0 N°6228 now just notify attributes having `with_php_computation` */ final protected function NotifyAttachedObjectsOnLinkClassModification(): void { - $sClass = get_class($this); - if (false === MetaModel::IsLinkClass($sClass)) { - return; - } // previous values in case of link change $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); + $sClass = get_class($this); + $aClassExtKeyAttCodes = MetaModel::GetAttributesList($sClass, [AttributeExternalKey::class]); + foreach ($aClassExtKeyAttCodes as $sExternalKeyAttCode) { + /** @var AttributeExternalKey $oAttDef */ + $oAttDef = MetaModel::GetAttributeDef($sClass, $sExternalKeyAttCode); - $aLnkClassExternalKeys = MetaModel::GetAttributesList($sClass, [AttributeExternalKey::class]); - foreach ($aLnkClassExternalKeys as $sExternalKeyAttCode) { - /** @var \AttributeExternalKey $oExternalKeyAttDef */ - $oExternalKeyAttDef = MetaModel::GetAttributeDef($sClass, $sExternalKeyAttCode); - $sRemoteClassName = $oExternalKeyAttDef->GetTargetClass(); + if (false === $this->DoesRemoteObjectHavePhpComputation($oAttDef)) { + continue; + } $sRemoteObjectId = $this->Get($sExternalKeyAttCode); if ($sRemoteObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($sRemoteClassName, $sRemoteObjectId); + self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sRemoteObjectId); } $sPreviousRemoteObjectId = $aPreviousValues[$sExternalKeyAttCode] ?? 0; if ($sPreviousRemoteObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($sRemoteClassName, $sPreviousRemoteObjectId); + self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sPreviousRemoteObjectId); } } } + private function DoesRemoteObjectHavePhpComputation(AttributeExternalKey $oAttDef): bool + { + $sRemoteObjectClass = $oAttDef->GetTargetClass(); + + if (utils::IsNullOrEmptyString($sRemoteObjectClass)) { + return false; + } + + /** @var AttributeLinkedSet $oAttDefMirrorLink */ + $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); + if (is_null($oAttDefMirrorLink) || false === $oAttDefMirrorLink->GetHasComputation()){ + return false; + } + + return true; + } + /** * Register one object for later EVENT_DB_LINKS_CHANGED event. * diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index 40797d270..ed3495d78 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -1744,6 +1744,15 @@ class AttributeLinkedSet extends AttributeDefinition return $this->GetOptional('with_php_constraint', false); } + /** + * @return bool true if Attribute has computation (DB_LINKS_CHANGED event propagation, `with_php_computation` attribute xml property), false otherwise + * @since 3.1.1 3.2.0 N°6228 + */ + public function GetHasComputation() + { + return $this->GetOptional('with_php_computation', false); + } + public function GetLinkedClass() { return $this->Get('linked_class'); diff --git a/core/cmdbobject.class.inc.php b/core/cmdbobject.class.inc.php index b9df6d05f..c9ccb436f 100644 --- a/core/cmdbobject.class.inc.php +++ b/core/cmdbobject.class.inc.php @@ -474,7 +474,7 @@ abstract class CMDBObject extends DBObject public function DBDelete(&$oDeletionPlan = null) { $this->LogCRUDEnter(__METHOD__); - $oDeletionPlan = $this->DBDeleteTracked_Internal($oDeletionPlan); + $oDeletionPlan = parent::DBDelete($oDeletionPlan); $this->LogCRUDExit(__METHOD__); return $oDeletionPlan; } diff --git a/core/datamodel.core.xml b/core/datamodel.core.xml index a2c798165..112045683 100644 --- a/core/datamodel.core.xml +++ b/core/datamodel.core.xml @@ -502,6 +502,12 @@ boolean false + + with_php_computation + false + boolean + false + create_temporary_object false diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 2770566c3..898d8b33f 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -187,12 +187,13 @@ abstract class DBObject implements iDisplay protected $m_oLinkHostObject = null; /** - * @var array List all the CRUD stack in progress - * - * The array contains instances of - * ['type' => 'type of CRUD operation (INSERT, UPDATE, DELETE)', - * 'class' => 'class of the object in the CRUD process', - * 'id' => 'id of the object in the CRUD process'] + * @var array{array{ + * type: string, + * class: string, + * id: string, + * }} List all the CRUD stack in progress, with : + * - type: CRUD operation (INSERT, UPDATE, DELETE)', + * - class: class of the object in the CRUD process, leaf (object finalclass) if we have a hierarchy * * @since 3.1.0 N°5906 */ @@ -2461,6 +2462,110 @@ abstract class DBObject implements iDisplay } } + /** + * @since 3.1.1 3.2.0 N°6228 method creation + */ + final protected function CheckPhpConstraint(bool $bIsCheckToDelete = false): void + { + $aChanges = $this->ListChanges(); + + $aClassExtKeyAttCodes = MetaModel::GetAttributesList(get_class($this), [AttributeExternalKey::class]); + foreach ($aClassExtKeyAttCodes as $sExtKeyWithMirrorLinkAttCode) { + /** @var AttributeExternalKey $oExtKeyWithMirrorLinkAttDef */ + $oExtKeyWithMirrorLinkAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyWithMirrorLinkAttCode); + + $oRemoteObject = $this->GetRemoteObjectWithPhpConstraint($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode)); + if (is_null($oRemoteObject)) { + continue; + } + + /** @var AttributeLinkedSet $oAttDefMirrorLink */ + $oAttDefMirrorLink = $oExtKeyWithMirrorLinkAttDef->GetMirrorLinkAttribute(); + if (is_null($oAttDefMirrorLink)) { + continue; + } + $sAttCodeMirrorLink = $oAttDefMirrorLink->GetCode(); + + if ($this->IsNew()) { + $this->CheckRemotePhpConstraintOnObject('add', $oRemoteObject, $sAttCodeMirrorLink, false); + } else if ($bIsCheckToDelete) { + $this->CheckRemotePhpConstraintOnObject('remove', $oRemoteObject, $sAttCodeMirrorLink, true); + } else { + if (array_key_exists($sExtKeyWithMirrorLinkAttCode, $aChanges)) { + // need to update remote old + new + $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); + $sPreviousRemoteObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; + $oPreviousRemoteObject = $this->GetRemoteObjectWithPhpConstraint($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey); + if (false === is_null($oPreviousRemoteObject)) { + $this->CheckRemotePhpConstraintOnObject('remove', $oPreviousRemoteObject, $sAttCodeMirrorLink, false); + } + $this->CheckRemotePhpConstraintOnObject('add', $oRemoteObject, $sAttCodeMirrorLink, false); + } else { + $this->CheckRemotePhpConstraintOnObject('modify', $oRemoteObject, $sAttCodeMirrorLink, false); // we need to update remote with current lnk instance + } + } + } + } + + private function CheckRemotePhpConstraintOnObject(string $sAction, DBObject $oRemoteObject, string $sAttCodeMirrorLink, bool $bIsCheckToDelete): void + { + $this->LogCRUDDebug(__METHOD__, "action: $sAction ".get_class($oRemoteObject).'::'.$oRemoteObject->GetKey()." ($sAttCodeMirrorLink)"); + + /** @var \ormLinkSet $oRemoteValue */ + $oRemoteValue = $oRemoteObject->Get($sAttCodeMirrorLink); + switch ($sAction) { + case 'add': + $oRemoteValue->AddItem($this); + break; + case 'remove': + $oRemoteValue->RemoveItem($this->GetKey()); + break; + case 'modify': + $oRemoteValue->ModifyItem($this); + break; + } + $oRemoteObject->Set($sAttCodeMirrorLink, $oRemoteValue); + [$bCheckStatus, $aCheckIssues, $bSecurityIssue] = $oRemoteObject->CheckToWrite(); + if (false === $bCheckStatus) { + if ($bIsCheckToDelete) { + $this->m_aDeleteIssues = array_merge($this->m_aDeleteIssues ?? [], $aCheckIssues); + } else { + $this->m_aCheckIssues = array_merge($this->m_aCheckIssues ?? [], $aCheckIssues); + } + $this->m_bSecurityIssue = $this->m_bSecurityIssue || $bSecurityIssue; + } + $aRemoteCheckWarnings = $oRemoteObject->GetCheckWarnings(); + if (is_array($aRemoteCheckWarnings)) { + $this->m_aCheckWarnings = array_merge($this->m_aCheckWarnings ?? [], $aRemoteCheckWarnings); + } + } + + private function GetRemoteObjectWithPhpConstraint(AttributeExternalKey $oAttDef, $sRemoteObjectKey) + { + $sRemoteObjectClass = $oAttDef->GetTargetClass(); + + /** @noinspection NotOptimalIfConditionsInspection */ + /** @noinspection TypeUnsafeComparisonInspection */ + if (utils::IsNullOrEmptyString($sRemoteObjectClass) + || utils::IsNullOrEmptyString($sRemoteObjectKey) + || ($sRemoteObjectKey == 0) // non-strict comparison as we might have bad surprises + ) { + return null; + } + + /** @var AttributeLinkedSet $oAttDefMirrorLink */ + $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); + if (is_null($oAttDefMirrorLink) || false === $oAttDefMirrorLink->GetHasConstraint()) { + return null; + } + + if (DBObject::IsObjectCurrentlyInCrud($sRemoteObjectClass, $sRemoteObjectKey)) { + return null; + } + + return MetaModel::GetObject($sRemoteObjectClass, $sRemoteObjectKey, false); + } + /** * @api * @api-advanced @@ -2484,6 +2589,7 @@ abstract class DBObject implements iDisplay { return array(true, array()); } + if (is_null($this->m_bCheckStatus)) { $this->m_aCheckIssues = array(); @@ -2500,6 +2606,9 @@ abstract class DBObject implements iDisplay $oKPI = new ExecutionKPI(); $this->DoCheckToWrite(); $oKPI->ComputeStatsForExtension($this, 'DoCheckToWrite'); + + $this->CheckPhpConstraint(); + if (count($this->m_aCheckIssues) == 0) { $this->m_bCheckStatus = true; @@ -2509,6 +2618,7 @@ abstract class DBObject implements iDisplay $this->m_bCheckStatus = false; } } + return array($this->m_bCheckStatus, $this->m_aCheckIssues, $this->m_bSecurityIssue); } @@ -2661,6 +2771,7 @@ abstract class DBObject implements iDisplay { $this->MakeDeletionPlan($oDeletionPlan); $oDeletionPlan->ComputeResults(); + return (!$oDeletionPlan->FoundStopper()); } @@ -3214,7 +3325,7 @@ abstract class DBObject implements iDisplay } } - list($bRes, $aIssues) = $this->CheckToWrite(false); + [$bRes, $aIssues] = $this->CheckToWrite(false); if (!$bRes) { throw new CoreCannotSaveObjectException(array('issues' => $aIssues, 'class' => get_class($this), 'id' => $this->GetKey())); } @@ -3454,7 +3565,7 @@ abstract class DBObject implements iDisplay return $this->m_iKey; } - list($bRes, $aIssues) = $this->CheckToWrite(false); + [$bRes, $aIssues] = $this->CheckToWrite(false); if (!$bRes) { throw new CoreCannotSaveObjectException(['issues' => $aIssues, 'class' => $sClass, 'id' => $this->GetKey()]); } @@ -3921,6 +4032,8 @@ abstract class DBObject implements iDisplay */ protected function DBDeleteSingleObject() { + $this->LogCRUDEnter(__METHOD__); + if (MetaModel::DBIsReadOnly()) { $this->LogCRUDExit(__METHOD__, 'DB is read-only'); @@ -4043,6 +4156,7 @@ abstract class DBObject implements iDisplay $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) // Thought the key is not reset, using DBInsert or DBWrite will create an object having the same characteristics and a new ID. DBUpdate is protected @@ -4073,74 +4187,70 @@ abstract class DBObject implements iDisplay public function DBDelete(&$oDeletionPlan = null) { $this->LogCRUDEnter(__METHOD__); + $this->AddCurrentObjectInCrudStack('DELETE'); + try { - static $iLoopTimeLimit = null; - if ($iLoopTimeLimit == null) - { - $iLoopTimeLimit = MetaModel::GetConfig()->Get('max_execution_time_per_loop'); - } - if (is_null($oDeletionPlan)) - { - $oDeletionPlan = new DeletionPlan(); - } - $this->MakeDeletionPlan($oDeletionPlan); - $oDeletionPlan->ComputeResults(); + static $iLoopTimeLimit = null; + if ($iLoopTimeLimit == null) { + $iLoopTimeLimit = MetaModel::GetConfig()->Get('max_execution_time_per_loop'); + } + if (is_null($oDeletionPlan)) { + $oDeletionPlan = new DeletionPlan(); + } + $this->MakeDeletionPlan($oDeletionPlan); + $oDeletionPlan->ComputeResults(); - if ($oDeletionPlan->FoundStopper()) - { - $aIssues = $oDeletionPlan->GetIssues(); - $this->LogCRUDError(__METHOD__, ' Errors: '.implode(', ', $aIssues)); - throw new DeleteException('Found issue(s)', array('target_class' => get_class($this), 'target_id' => $this->GetKey(), 'issues' => implode(', ', $aIssues))); - } + if ($oDeletionPlan->FoundStopper()) { + $aIssues = $oDeletionPlan->GetIssues(); + $this->LogCRUDError(__METHOD__, ' Errors: '.implode(', ', $aIssues)); + throw new DeleteException('Found issue(s)', array('target_class' => get_class($this), 'target_id' => $this->GetKey(), 'issues' => implode(', ', $aIssues))); + } - // Getting and setting time limit are not symetric: - // www.php.net/manual/fr/function.set-time-limit.php#72305 - $iPreviousTimeLimit = ini_get('max_execution_time'); + // Getting and setting time limit are not symetric: + // www.php.net/manual/fr/function.set-time-limit.php#72305 + $iPreviousTimeLimit = ini_get('max_execution_time'); - foreach ($oDeletionPlan->ListDeletes() as $sClass => $aToDelete) - { - 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 - // As a temporary fix: delete only the objects that are still to be deleted... - if ($oToDelete->m_bIsInDB) - { - set_time_limit(intval($iLoopTimeLimit)); + foreach ($oDeletionPlan->ListDeletes() as $sClass => $aToDelete) { + 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 + // As a temporary fix: delete only the objects that are still to be deleted... + if ($oToDelete->m_bIsInDB) { + set_time_limit(intval($iLoopTimeLimit)); - $oToDelete->AddCurrentObjectInCrudStack('DELETE'); - try { - $oToDelete->DBDeleteSingleObject(); - } - finally { - $oToDelete->RemoveCurrentObjectInCrudStack(); + $oToDelete->AddCurrentObjectInCrudStack('DELETE'); + try { + $oToDelete->DBDeleteSingleObject(); + } + finally { + $oToDelete->RemoveCurrentObjectInCrudStack(); + } } } } - } - foreach ($oDeletionPlan->ListUpdates() as $sClass => $aToUpdate) - { - foreach ($aToUpdate as $aData) - { - $oToUpdate = $aData['to_reset']; - /** @var \DBObject $oToUpdate */ - foreach ($aData['attributes'] as $sRemoteExtKey => $aRemoteAttDef) - { - $oToUpdate->Set($sRemoteExtKey, $aData['values'][$sRemoteExtKey]); - set_time_limit(intval($iLoopTimeLimit)); - $oToUpdate->DBUpdate(); + foreach ($oDeletionPlan->ListUpdates() as $sClass => $aToUpdate) { + foreach ($aToUpdate as $aData) { + $oToUpdate = $aData['to_reset']; + /** @var \DBObject $oToUpdate */ + foreach ($aData['attributes'] as $sRemoteExtKey => $aRemoteAttDef) { + $oToUpdate->Set($sRemoteExtKey, $aData['values'][$sRemoteExtKey]); + set_time_limit(intval($iLoopTimeLimit)); + $oToUpdate->DBUpdate(); + } } } + + set_time_limit(intval($iPreviousTimeLimit)); + } finally { + $this->LogCRUDExit(__METHOD__); + $this->RemoveCurrentObjectInCrudStack(); } - set_time_limit(intval($iPreviousTimeLimit)); - - $this->LogCRUDExit(__METHOD__); return $oDeletionPlan; } @@ -5246,6 +5356,7 @@ abstract class DBObject implements iDisplay $this->m_aDeleteIssues = array(); // Ok $this->FireEventCheckToDelete($oDeletionPlan); $this->DoCheckToDelete($oDeletionPlan); + $this->CheckPhpConstraint(true); $oDeletionPlan->SetDeletionIssues($this, $this->m_aDeleteIssues, $this->m_bSecurityIssue); $aDependentObjects = $this->GetReferencingObjects(true /* allow all data */); @@ -6236,6 +6347,18 @@ abstract class DBObject implements iDisplay $this->m_aCheckWarnings[] = $sWarning; } + /** + * + * @api + * + * @return string[]|null + * @since 3.1.1 3.2.0 + */ + public function GetCheckWarnings(): ?array + { + return $this->m_aCheckWarnings; + } + /** * @api * @@ -6484,6 +6607,11 @@ abstract class DBObject implements iDisplay // so we need to handle null values (will give empty string after conversion) $sConvertedId = (string)$sId; + if (((int)$sId) > 0) { + // When having a class hierarchy, we are saving the leaf class in the stack + $sClass = MetaModel::GetFinalClassName($sClass, $sId); + } + foreach (self::$m_aCrudStack as $aCrudStackEntry) { if (($sClass === $aCrudStackEntry['class']) && ($sConvertedId === $aCrudStackEntry['id'])) { @@ -6523,6 +6651,7 @@ abstract class DBObject implements iDisplay */ private function AddCurrentObjectInCrudStack(string $sCrudType): void { + $this->LogCRUDDebug(__METHOD__); self::$m_aCrudStack[] = [ 'type' => $sCrudType, 'class' => get_class($this), @@ -6539,6 +6668,7 @@ abstract class DBObject implements iDisplay */ private function UpdateCurrentObjectInCrudStack(): void { + $this->LogCRUDDebug(__METHOD__); $aCurrentCrudStack = array_pop(self::$m_aCrudStack); $aCurrentCrudStack['id'] = (string)$this->GetKey(); self::$m_aCrudStack[] = $aCurrentCrudStack; @@ -6552,7 +6682,8 @@ abstract class DBObject implements iDisplay */ private function RemoveCurrentObjectInCrudStack(): void { - array_pop(self::$m_aCrudStack); + $aRemoved = array_pop(self::$m_aCrudStack); + $this->LogCRUDDebug(__METHOD__, $aRemoved['class'].':'.$aRemoved['id']); } /** diff --git a/core/metamodel.class.php b/core/metamodel.class.php index 9af99c36f..e725c31d5 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -5155,7 +5155,7 @@ abstract class MetaModel */ protected static function DBCreateTables($aCallback = null) { - list($aErrors, $aSugFix, $aCondensedQueries) = self::DBCheckFormat(); + [$aErrors, $aSugFix, $aCondensedQueries] = self::DBCheckFormat(); //$sSQL = implode('; ', $aCondensedQueries); Does not work - multiple queries not allowed foreach($aCondensedQueries as $sQuery) @@ -5177,7 +5177,7 @@ abstract class MetaModel */ protected static function DBCreateViews() { - list($aErrors, $aSugFix) = self::DBCheckViews(); + [$aErrors, $aSugFix] = self::DBCheckViews(); foreach($aSugFix as $sClass => $aTarget) { @@ -6926,6 +6926,22 @@ abstract class MetaModel return $iCount === 1; } + public static function GetFinalClassName(string $sClass, int $iKey): string + { + $sFinalClassField = Metamodel::DBGetClassField($sClass); + if (utils::IsNullOrEmptyString($sFinalClassField)) { + return $sClass; + } + + $sRootClass = MetaModel::GetRootClass($sClass); + $sTable = MetaModel::DBGetTable($sRootClass); + $sKeyCol = MetaModel::DBGetKey($sRootClass); + $sEscapedKey = CMDBSource::Quote($iKey); + + $sQuery = "SELECT `{$sFinalClassField}` FROM `{$sTable}` WHERE `{$sKeyCol}` = {$sEscapedKey}"; + return CMDBSource::QueryToScalar($sQuery); + } + /** * Search for the specified class and id. If the object is archived it will be returned anyway (this is for pre-2.4 * module compatibility, see N.1108) diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index aea8658e4..a099e96fd 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -1,6 +1,8 @@ ListChanges())) { + return; + } + + $oProfileLinkSet = $this->Get('profile_list'); + if ($oProfileLinkSet->Count() > 1) { + return; + } + $oProfileLinkSet->Rewind(); + $iPowerPortalCount = 0; + $iTotalCount = 0; + while ($oUserProfile = $oProfileLinkSet->Fetch()) { + $sProfile = $oUserProfile->Get('profile'); + if ($sProfile === 'Portal power user') { + $iPowerPortalCount = 1; + } + $iTotalCount++; + } + if ($iTotalCount === $iPowerPortalCount) { + $this->AddCheckIssue(Dict::S('Class:User/Error:PortalPowerUserHasInsufficientRights')); + } + } + /** * @inheritDoc * @since 3.0.0 diff --git a/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml b/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml index 1ee54b548..69183e3b4 100755 --- a/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml +++ b/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml @@ -202,6 +202,7 @@ 0 0 contact_id + true @@ -210,6 +211,7 @@ 0 0 functionalci_id + true diff --git a/dictionaries/en.dictionary.itop.ui.php b/dictionaries/en.dictionary.itop.ui.php index 8309affd4..02f940528 100644 --- a/dictionaries/en.dictionary.itop.ui.php +++ b/dictionaries/en.dictionary.itop.ui.php @@ -182,6 +182,7 @@ Dict::Add('EN US', 'English', 'English', array( 'Class:User/Error:StatusChangeIsNotAllowed' => 'Changing status is not allowed for your own User', 'Class:User/Error:AllowedOrgsMustContainUserOrg' => 'Allowed organizations must contain User organization', 'Class:User/Error:CurrentProfilesHaveInsufficientRights' => 'The current list of profiles does not give sufficient access rights (Users are not modifiable anymore)', + 'Class:User/Error:PortalPowerUserHasInsufficientRights' => 'The Portal power user profile does not give sufficient access rights (another profile must be added)', 'Class:User/Error:AtLeastOneOrganizationIsNeeded' => 'At least one organization must be assigned to this user.', 'Class:User/Error:OrganizationNotAllowed' => 'Organization not allowed.', 'Class:User/Error:UserOrganizationNotAllowed' => 'The user account does not belong to your allowed organizations.', diff --git a/dictionaries/fr.dictionary.itop.ui.php b/dictionaries/fr.dictionary.itop.ui.php index 664deeb07..a5a4c7afb 100644 --- a/dictionaries/fr.dictionary.itop.ui.php +++ b/dictionaries/fr.dictionary.itop.ui.php @@ -166,6 +166,7 @@ Dict::Add('FR FR', 'French', 'Français', array( 'Class:User/Error:StatusChangeIsNotAllowed' => 'Impossible de changer l\'état de son propre utilisateur', 'Class:User/Error:AllowedOrgsMustContainUserOrg' => 'Les organisations permises doivent contenir l\'organisation de l\'utilisateur', 'Class:User/Error:CurrentProfilesHaveInsufficientRights' => 'Les profils existants ne permettent pas de modifier les utilisateurs', + 'Class:User/Error:PortalPowerUserHasInsufficientRights' => 'Le profil Portal power user ne donne pas suffisamment de droits à l\'utilisateur (un autre profil doit être ajouté)', 'Class:User/Error:AtLeastOneOrganizationIsNeeded' => 'L\'utilisateur doit avoir au moins une organisation.', 'Class:User/Error:OrganizationNotAllowed' => 'Organisation non autorisée.', 'Class:User/Error:UserOrganizationNotAllowed' => 'L\'utilisateur n\'appartient pas à vos organisations.', diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index 92e5a9229..f87590766 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -2081,6 +2081,7 @@ EOF $this->CompileCommonProperty('edit_when', $oField, $aParameters, $sModuleRelativeDir); $this->CompileCommonProperty('filter', $oField, $aParameters, $sModuleRelativeDir); $this->CompileCommonProperty('with_php_constraint', $oField, $aParameters, $sModuleRelativeDir, false); + $this->CompileCommonProperty('with_php_computation', $oField, $aParameters, $sModuleRelativeDir, false); $aParameters['depends_on'] = $sDependencies; } elseif ($sAttType == 'AttributeLinkedSet') { $this->CompileCommonProperty('linked_class', $oField, $aParameters, $sModuleRelativeDir); @@ -2092,6 +2093,7 @@ EOF $this->CompileCommonProperty('edit_when', $oField, $aParameters, $sModuleRelativeDir); $this->CompileCommonProperty('filter', $oField, $aParameters, $sModuleRelativeDir); $this->CompileCommonProperty('with_php_constraint', $oField, $aParameters, $sModuleRelativeDir, false); + $this->CompileCommonProperty('with_php_computation', $oField, $aParameters, $sModuleRelativeDir, false); $aParameters['depends_on'] = $sDependencies; } elseif ($sAttType == 'AttributeExternalKey') { $this->CompileCommonProperty('target_class', $oField, $aParameters, $sModuleRelativeDir); diff --git a/tests/php-unit-tests/unitary-tests/application/cmdbAbstractObjectTest.php b/tests/php-unit-tests/unitary-tests/application/cmdbAbstractObjectTest.php index 8c64f17f3..4a727cb45 100644 --- a/tests/php-unit-tests/unitary-tests/application/cmdbAbstractObjectTest.php +++ b/tests/php-unit-tests/unitary-tests/application/cmdbAbstractObjectTest.php @@ -1,82 +1,16 @@ GetObjectsAwaitingFireEventDbLinksChanged(); - $this->assertSame([], $aLinkModificationsStack); - - // retain events - cmdbAbstractObject::SetEventDBLinksChangedBlocked(true); - - // Create the person - $oPerson = $this->CreatePerson(1); - $this->assertIsObject($oPerson); - // Create the team - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - // contact types - $oContactType1 = MetaModel::NewObject(ContactType::class, ['name' => 'test_'.rand(10000, 99999)]); - $oContactType1->DBInsert(); - $oContactType2 = MetaModel::NewObject(ContactType::class, ['name' => 'test_'.rand(10000, 99999)]); - $oContactType2->DBInsert(); - - // Prepare the link for the insertion with the team - - $aValues = [ - 'person_id' => $oPerson->GetKey(), - 'role_id' => $oContactType1->GetKey(), - 'team_id' => $oTeam->GetKey(), - ]; - $oLinkPersonToTeam1 = MetaModel::NewObject(lnkPersonToTeam::class, $aValues); - $oLinkPersonToTeam1->DBInsert(); - - $aLinkModificationsStack = $this->GetObjectsAwaitingFireEventDbLinksChanged(); - self::assertCount(3, $aLinkModificationsStack); - $aExpectedLinkStack = [ - 'Team' => [$oTeam->GetKey() => 1], - 'Person' => [$oPerson->GetKey() => 1], - 'ContactType' => [$oContactType1->GetKey() => 1], - ]; - self::assertSame($aExpectedLinkStack, $aLinkModificationsStack); - - $oLinkPersonToTeam1->Set('role_id', $oContactType2->GetKey()); - $oLinkPersonToTeam1->DBWrite(); - $aLinkModificationsStack = $this->GetObjectsAwaitingFireEventDbLinksChanged(); - self::assertCount(3, $aLinkModificationsStack); - $aExpectedLinkStack = [ - 'Team' => [$oTeam->GetKey() => 2], - 'Person' => [$oPerson->GetKey() => 2], - 'ContactType' => [ - $oContactType1->GetKey() => 2, - $oContactType2->GetKey() => 1, - ], - ]; - self::assertSame($aExpectedLinkStack, $aLinkModificationsStack); - } - public function testProcessClassIdDeferredUpdate() { // Create the team @@ -152,288 +86,4 @@ class cmdbAbstractObjectTest extends ItopDataTestCase { { $this->SetNonPublicStaticProperty(cmdbAbstractObject::class, 'aObjectsAwaitingEventDbLinksChanged', $aObjects); } - - /** - * Check that EVENT_DB_LINKS_CHANGED events are not sent to the current updated/created object (Team) - * the events are sent to the other side (Person) - * - * @return void - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException - * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \CoreWarning - * @throws \MySQLException - * @throws \OQLException - */ - public function testDBInsertTeam() - { - // Prepare the link set - $sLinkedClass = lnkPersonToTeam::class; - $aLinkedObjectsArray = []; - $oSet = DBObjectSet::FromArray($sLinkedClass, $aLinkedObjectsArray); - $oLinkSet = new ormLinkSet(Team::class, 'persons_list', $oSet); - - // Create the 3 persons - for ($i = 0; $i < 3; $i++) { - $oPerson = $this->CreatePerson($i); - $this->assertIsObject($oPerson); - // Add the person to the link - $oLink = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson->GetKey()]); - $oLinkSet->AddItem($oLink); - } - - $this->debug("\n-------------> Test Starts HERE\n"); - - $oEventReceiver = new LinksEventReceiver($this); - $oEventReceiver->RegisterCRUDListeners(); - - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'persons_list' => $oLinkSet, 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - $this->assertIsObject($oTeam); - - // 3 links added to person + 1 for the Team - $this->assertEquals(4, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - } - - /** - * Check that EVENT_DB_LINKS_CHANGED events are sent to all the linked objects when creating a new lnk object - * - * @return void - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException - * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \CoreWarning - * @throws \MySQLException - * @throws \OQLException - */ - public function testAddLinkToTeam() - { - // Create a person - $oPerson = $this->CreatePerson(1); - $this->assertIsObject($oPerson); - - // Create a Team - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - $this->assertIsObject($oTeam); - - $this->debug("\n-------------> Test Starts HERE\n"); - $oEventReceiver = new LinksEventReceiver($this); - $oEventReceiver->RegisterCRUDListeners(); - - // The link creation will signal both the Person an the Team - $oLink = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson->GetKey(), 'team_id' => $oTeam->GetKey()]); - $oLink->DBInsert(); - - // 2 events one for Person and One for Team - $this->assertEquals(2, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - } - - /** - * Check that EVENT_DB_LINKS_CHANGED events are sent to all the linked objects when updating an existing lnk object - * - * @return void - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException - * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \CoreWarning - * @throws \MySQLException - * @throws \OQLException - */ - public function testUpdateLinkRole() - { - // Create a person - $oPerson = $this->CreatePerson(1); - $this->assertIsObject($oPerson); - - // Create a Team - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - $this->assertIsObject($oTeam); - - // Create the link - $oLink = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson->GetKey(), 'team_id' => $oTeam->GetKey()]); - $oLink->DBInsert(); - - $this->debug("\n-------------> Test Starts HERE\n"); - $oEventReceiver = new LinksEventReceiver($this); - $oEventReceiver->RegisterCRUDListeners(); - - // The link update will signal both the Person, the Team and the ContactType - // Change the role - $oContactType = MetaModel::NewObject(ContactType::class, ['name' => 'test_'.$oLink->GetKey()]); - $oContactType->DBInsert(); - $oLink->Set('role_id', $oContactType->GetKey()); - $oLink->DBUpdate(); - - // 3 events one for Person, one for Team and one for ContactType - $this->assertEquals(3, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - } - - /** - * Check that when a link changes from an object to another, then both objects are notified - * - * @return void - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException - * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \CoreWarning - * @throws \MySQLException - * @throws \OQLException - */ - public function testUpdateLinkPerson() - { - // Create 2 person - $oPerson1 = $this->CreatePerson(1); - $this->assertIsObject($oPerson1); - - $oPerson2 = $this->CreatePerson(2); - $this->assertIsObject($oPerson2); - - // Create a Team - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - $this->assertIsObject($oTeam); - - // Create the link between Person1 and Team - $oLink = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson1->GetKey(), 'team_id' => $oTeam->GetKey()]); - $oLink->DBInsert(); - - $this->debug("\n-------------> Test Starts HERE\n"); - $oEventReceiver = new LinksEventReceiver($this); - $oEventReceiver->RegisterCRUDListeners(); - - // The link update will signal both the Persons and the Team - // Change the person - $oLink->Set('person_id', $oPerson2->GetKey()); - $oLink->DBUpdate(); - - // 3 events 2 for Person, one for Team - $this->assertEquals(3, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - } - - /** - * Check that EVENT_DB_LINKS_CHANGED events are sent to all the linked objects when deleting an existing lnk object - * - * @return void - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException - * @throws \CoreException - * @throws \CoreUnexpectedValue - * @throws \CoreWarning - * @throws \MySQLException - * @throws \OQLException - */ - public function testDeleteLink() - { - // Create a person - $oPerson = $this->CreatePerson(1); - $this->assertIsObject($oPerson); - - // Create a Team - $oTeam = MetaModel::NewObject(Team::class, ['name' => 'TestTeam1', 'org_id' => $this->getTestOrgId()]); - $oTeam->DBInsert(); - $this->assertIsObject($oTeam); - - // Create the link - $oLink = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson->GetKey(), 'team_id' => $oTeam->GetKey()]); - $oLink->DBInsert(); - - $this->debug("\n-------------> Test Starts HERE\n"); - $oEventReceiver = new LinksEventReceiver($this); - $oEventReceiver->RegisterCRUDListeners(); - - // The link delete will signal both the Person an the Team - $oLink->DBDelete(); - - // 3 events one for Person, one for Team - $this->assertEquals(2, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - } - - /** - * Debug called by event receivers - * - * @param $sMsg - * - * @return void - */ - public static function DebugStatic($sMsg) - { - if (static::$DEBUG_UNIT_TEST) { - if (is_string($sMsg)) { - echo "$sMsg\n"; - } else { - print_r($sMsg); - } - } - } -} - - -/** - * Count events received - * And allow callbacks on events - */ -class LinksEventReceiver -{ - private $oTestCase; - private $aCallbacks = []; - - public static $bIsObjectInCrudStack; - - public function __construct(ItopDataTestCase $oTestCase) - { - $this->oTestCase = $oTestCase; - } - - public function AddCallback(string $sEvent, string $sClass, string $sFct, int $iCount = 1): void - { - $this->aCallbacks[$sEvent][$sClass] = [ - 'callback' => [$this, $sFct], - 'count' => $iCount, - ]; - } - - public function CleanCallbacks() - { - $this->aCallbacks = []; - } - - // Event callbacks - public function OnEvent(EventData $oData) - { - $sEvent = $oData->GetEvent(); - $oObject = $oData->Get('object'); - $sClass = get_class($oObject); - $iKey = $oObject->GetKey(); - $this->Debug(__METHOD__.": received event '$sEvent' for $sClass::$iKey"); - cmdbAbstractObjectTest::IncrementCallCount($sEvent); - - if (isset($this->aCallbacks[$sEvent][$sClass])) { - $aCallBack = $this->aCallbacks[$sEvent][$sClass]; - if ($aCallBack['count'] > 0) { - $this->aCallbacks[$sEvent][$sClass]['count']--; - call_user_func($this->aCallbacks[$sEvent][$sClass]['callback'], $oObject); - } - } - } - - public function RegisterCRUDListeners(string $sEvent = null, $mEventSource = null) - { - $this->Debug('Registering Test event listeners'); - if (is_null($sEvent)) { - $this->oTestCase->EventService_RegisterListener(EVENT_DB_LINKS_CHANGED, [$this, 'OnEvent']); - return; - } - $this->oTestCase->EventService_RegisterListener($sEvent, [$this, 'OnEvent'], $mEventSource); - } - - private function Debug($msg) - { - cmdbAbstractObjectTest::DebugStatic($msg); - } } diff --git a/tests/php-unit-tests/unitary-tests/application/query/QueryTest.php b/tests/php-unit-tests/unitary-tests/application/query/QueryTest.php index b9a887e83..893b2d019 100644 --- a/tests/php-unit-tests/unitary-tests/application/query/QueryTest.php +++ b/tests/php-unit-tests/unitary-tests/application/query/QueryTest.php @@ -82,6 +82,7 @@ class QueryTest extends ItopDataTestCase } $oQuery->DBInsert(); + $this->assertFalse($oQuery->IsNew()); return $oQuery; } diff --git a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php index 4c96e6e01..418b5fef1 100644 --- a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php +++ b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php @@ -208,4 +208,33 @@ PHP $oFormFieldNoTouchedAtt = $oAttDef->MakeFormField($oPerson); $this->assertTrue($oFormFieldNoTouchedAtt->IsValidationDisabled(), 'email wasn\'t modified, we must not validate the corresponding field'); } + + /** + * @dataProvider WithConstraintParameterProvider + * + * @param string $sClass + * @param string $sAttCode + * @param bool $bConstraintExpected + * @param bool $bComputationExpected + * + * @return void + * @throws \Exception + */ + public function testWithConstraintAndComputationParameters(string $sClass, string $sAttCode, bool $bConstraintExpected, bool $bComputationExpected) + { + $oAttDef = \MetaModel::GetAttributeDef($sClass, $sAttCode); + $this->assertTrue(method_exists($oAttDef, 'GetHasConstraint')); + $this->assertEquals($bConstraintExpected, $oAttDef->GetHasConstraint()); + $this->assertEquals($bComputationExpected, $oAttDef->GetHasComputation()); + } + + public function WithConstraintParameterProvider() + { + return [ + ['User', 'profile_list', true, false], + ['User', 'allowed_org_list', true, false], + ['Person', 'team_list', false, false], + ['Ticket', 'functionalcis_list', false, true], + ]; + } } \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php index 9b9c3f6b8..ebcb2e38c 100644 --- a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php @@ -7,7 +7,6 @@ namespace Combodo\iTop\Test\UnitTest\Core\CRUD; use Combodo\iTop\Service\Events\EventData; -use Combodo\iTop\Service\Events\EventService; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use ContactType; use CoreException; @@ -21,6 +20,7 @@ use ormLinkSet; use Person; use Team; use utils; +use const EVENT_DB_LINKS_CHANGED; class CRUDEventTest extends ItopDataTestCase { @@ -339,8 +339,8 @@ class CRUDEventTest extends ItopDataTestCase $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(4, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - $this->assertEquals(20, self::$iEventCalls); + $this->assertArrayNotHasKey(EVENT_DB_LINKS_CHANGED, self::$aEventCalls, 'no relation with the with_php_compute attribute !'); + $this->assertEquals(16, self::$iEventCalls); } /** @@ -388,8 +388,8 @@ class CRUDEventTest extends ItopDataTestCase $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(3, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); - $this->assertEquals(19, self::$iEventCalls); + $this->assertArrayNotHasKey(EVENT_DB_LINKS_CHANGED, self::$aEventCalls, 'no relation with the with_php_compute attribute !'); + $this->assertEquals(16, self::$iEventCalls); // Read the object explicitly from the DB to check that the role has been set $oSet = new DBObjectSet(DBSearch::FromOQL('SELECT Team WHERE id=:id'), [], ['id' => $oTeam->GetKey()]); @@ -495,7 +495,7 @@ class CRUDEventTest extends ItopDataTestCase $oLnk = MetaModel::NewObject(lnkPersonToTeam::class, ['person_id' => $oPerson->GetKey(), 'team_id' => $oTeam->GetKey()]); $oLnk->DBInsert(); - $this->assertEquals(2, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); + $this->assertArrayNotHasKey(EVENT_DB_LINKS_CHANGED, self::$aEventCalls, 'no relation with the with_php_compute attribute !'); } public function testLinksDeleted() @@ -517,7 +517,7 @@ class CRUDEventTest extends ItopDataTestCase $oLnk->DBDelete(); - $this->assertEquals(2, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); + $this->assertArrayNotHasKey(EVENT_DB_LINKS_CHANGED, self::$aEventCalls, 'no relation with the with_php_compute attribute !'); } // Tests with MockDBObject diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php b/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php new file mode 100644 index 000000000..f4e025983 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php @@ -0,0 +1,359 @@ + [ + 'aProfilesBeforeUserCreation' => [ + ], + 'bWaitForException' => 'CoreCannotSaveObjectException', + ], + 'Portal power user' => [ + 'aProfilesBeforeUserCreation' => [ + 'Portal power user', + ], + 'bWaitForException' => 'CoreCannotSaveObjectException', + ], + 'Portal power user + Configuration Manager' => [ + 'aProfilesBeforeUserCreation' => [ + 'Portal power user', + 'Configuration Manager', + ], + 'bWaitForException' => false, + ], + 'Portal power user + Configuration Manager + Admin' => [ + 'aProfilesBeforeUserCreation' => [ + 'Portal power user', + 'Configuration Manager', + 'Administrator', + ], + 'bWaitForException' => false, + ], + ]; + } + + /** + * @dataProvider PortaPowerUserProvider + * @covers User::CheckPortalProfiles + */ + public function testUserLocalCreation($aProfilesBeforeUserCreation, $sWaitForException) + { + $oUser = new UserLocal(); + $sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin); + $oUser->Set('password', 'ABCD1234@gabuzomeu'); + $oUser->Set('language', 'EN US'); + if (false !== $sWaitForException) { + $this->expectException($sWaitForException); + } + $this->commonUserCreationTest($oUser, $aProfilesBeforeUserCreation); + } + + /** + * @dataProvider PortaPowerUserProvider + * @covers User::CheckPortalProfiles + */ + public function testUserLocalDelete($aProfilesBeforeUserCreation, $sWaitForException) + { + $oUser = new UserLocal(); + $sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin); + $oUser->Set('password', 'ABCD1234@gabuzomeu'); + $oUser->Set('language', 'EN US'); + if (false !== $sWaitForException) { + $this->expectException($sWaitForException); + } + $this->commonUserCreationTest($oUser, $aProfilesBeforeUserCreation, false); + + $oUser->DBDelete(); + } + + /** + * @dataProvider PortaPowerUserProvider + * @covers User::CheckPortalProfiles + */ + public function testUserLocalUpdate($aProfilesBeforeUserCreation, $sWaitForException) + { + $oUser = new UserLocal(); + $sLogin = 'testUserLocalUpdateWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin); + $oUser->Set('password', 'ABCD1234@gabuzomeu'); + $oUser->Set('language', 'EN US'); + if (false !== $sWaitForException) { + $this->expectException($sWaitForException); + } + $this->commonUserUpdateTest($oUser, $aProfilesBeforeUserCreation); + } + + private function commonUserCreationTest($oUserToCreate, $aProfilesBeforeUserCreation, $bTestUserItopAccess = true) + { + $sUserClass = get_class($oUserToCreate); + list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUserToCreate, $aProfilesBeforeUserCreation); + + $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aProfilesBeforeUserCreation, $bTestUserItopAccess); + } + + private function commonUserUpdateTest($oUserToCreate, $aProfilesBeforeUserCreation) + { + $sUserClass = get_class($oUserToCreate); + list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUserToCreate, ['Administrator']); + + $oUserToUpdate = MetaModel::GetObject($sUserClass, $sId); + $oProfileList = $oUserToUpdate->Get('profile_list'); + while ($oObj = $oProfileList->Fetch()) { + $oProfileList->RemoveItem($oObj->GetKey()); + } + + foreach ($aProfilesBeforeUserCreation as $sProfileName) { + $oAdminUrpProfile = new URP_UserProfile(); + $oProfile = $aProfiles[$sProfileName]; + $oAdminUrpProfile->Set('profileid', $oProfile->GetKey()); + $oAdminUrpProfile->Set('reason', 'UNIT Tests'); + $oProfileList->AddItem($oAdminUrpProfile); + } + + $oUserToUpdate->Set('profile_list', $oProfileList); + $oUserToUpdate->DBWrite(); + + $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aProfilesBeforeUserCreation); + } + + private function CreateUserForProfileTesting(User $oUserToCreate, array $aProfilesBeforeUserCreation, $bDbInsert = true): array + { + $aProfiles = []; + $oSearch = DBSearch::FromOQL('SELECT URP_Profiles'); + $oProfileSet = new DBObjectSet($oSearch); + while (($oProfile = $oProfileSet->Fetch()) != null) { + $aProfiles[$oProfile->Get('name')] = $oProfile; + } + + $this->CreateTestOrganization(); + $oContact = $this->CreatePerson('1'); + $iContactId = $oContact->GetKey(); + + $oUserToCreate->Set('contactid', $iContactId); + + $oUserProfileList = $oUserToCreate->Get('profile_list'); + foreach ($aProfilesBeforeUserCreation as $sProfileName) { + $oUserProfile = new URP_UserProfile(); + $oProfile = $aProfiles[$sProfileName]; + $oUserProfile->Set('profileid', $oProfile->GetKey()); + $oUserProfile->Set('reason', 'UNIT Tests'); + $oUserProfileList->AddItem($oUserProfile); + } + + $oUserToCreate->Set('profile_list', $oUserProfileList); + if ($bDbInsert) { + $sId = $oUserToCreate->DBInsert(); + } else { + $sId = -1; + } + + return [$sId, $aProfiles]; + } + + private function CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedAssociatedProfilesAfterUserCreation, $bTestItopConnection = true) + { + $oUser = MetaModel::GetObject($sUserClass, $sId); + $oUserProfileList = $oUser->Get('profile_list'); + $aProfilesAfterCreation = []; + while (($oProfile = $oUserProfileList->Fetch()) != null) { + $aProfilesAfterCreation[] = $oProfile->Get('profile'); + } + + foreach ($aExpectedAssociatedProfilesAfterUserCreation as $sExpectedProfileName) { + $this->assertTrue(in_array($sExpectedProfileName, $aProfilesAfterCreation), + "profile \'$sExpectedProfileName\' should be asociated to user after creation. ".var_export($aProfilesAfterCreation, true)); + } + + if (!$bTestItopConnection) { + return; + } + + $_SESSION = []; + + UserRights::Login($oUser->Get('login')); + + if (!UserRights::IsPortalUser()) { + //calling this API triggers Fatal Error on below OQL used by \User->GetContactObject() for a user with only 'portal power user' profile + /** + * Error: No result for the single row query: 'SELECT DISTINCT `Contact`.`id` AS `Contactid`, `Contact`.`name` AS `Contactname`, `Contact`.`status` AS `Contactstatus`, `Contact`.`org_id` AS `Contactorg_id`, `Organization_org_id`.`name` AS `Contactorg_name`, `Contact`.`email` AS `Contactemail`, `Contact`.`phone` AS `Contactphone`, `Contact`.`notify` AS `Contactnotify`, `Contact`.`function` AS `Contactfunction`, `Contact`.`finalclass` AS `Contactfinalclass`, IF((`Contact`.`finalclass` IN ('Team', 'Contact')), CAST(CONCAT(COALESCE(`Contact`.`name`, '')) AS CHAR), CAST(CONCAT(COALESCE(`Contact_poly_Person`.`first_name`, ''), COALESCE(' ', ''), COALESCE(`Contact`.`name`, '')) AS CHAR)) AS `Contactfriendlyname`, COALESCE((`Contact`.`status` = 'inactive'), 0) AS `Contactobsolescence_flag`, `Contact`.`obsolescence_date` AS `Contactobsolescence_date`, CAST(CONCAT(COALESCE(`Organization_org_id`.`name`, '')) AS CHAR) AS `Contactorg_id_friendlyname`, COALESCE((`Organization_org_id`.`status` = 'inactive'), 0) AS `Contactorg_id_obsolescence_flag` FROM `contact` AS `Contact` INNER JOIN `organization` AS `Organization_org_id` ON `Contact`.`org_id` = `Organization_org_id`.`id` LEFT JOIN `person` AS `Contact_poly_Person` ON `Contact`.`id` = `Contact_poly_Person`.`id` WHERE ((`Contact`.`id` = 40) AND 0) '. + */ + NavigationMenuFactory::MakeStandard(); + } + + $this->assertTrue(true, 'after fix N°5324 no exception raised'); + // logout + $_SESSION = []; + } + + /** + * @dataProvider ProfilesLinksProvider + */ + public function testProfilesLinksDBDelete(string $sProfileNameToRemove, $bRaiseException = false) + { + $aInitialProfiles = [$sProfileNameToRemove, 'Portal power user']; + + $oUser = new UserExternal(); + $sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin); + + [$sId, $aProfiles] = $this->CreateUserForProfileTesting($oUser, $aInitialProfiles); + + if ($bRaiseException) { + $this->expectException(DeleteException::class); + } + + $aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId); + if (array_key_exists($sProfileNameToRemove, $aURPUserProfileByUser)) { + $oURPUserProfile = $aURPUserProfileByUser[$sProfileNameToRemove]; + $oURPUserProfile->DBDelete(); + } + } + + /** + * @dataProvider ProfilesLinksProvider + */ + public function testProfilesLinksEdit_ChangeProfileId(string $sInitialProfile, $bRaiseException = false) + { + $oUser = new UserExternal(); + $sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin); + + $sUserClass = get_class($oUser); + list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, [$sInitialProfile]); + + $oURP_Profile = MetaModel::GetObjectByColumn('URP_Profiles', 'name', 'Portal power user'); + + $aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId); + + if ($bRaiseException) { + $this->expectException(CoreCannotSaveObjectException::class); + } + + if (array_key_exists($sInitialProfile, $aURPUserProfileByUser)) { + $oURPUserProfile = $aURPUserProfileByUser[$sInitialProfile]; + $oURPUserProfile->Set('profileid', $oURP_Profile->GetKey()); + $oURPUserProfile->DBWrite(); + } + + if (!$bRaiseException) { + $aExpectedProfilesAfterUpdate = ['Portal power user', 'Portal user']; + $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedProfilesAfterUpdate); + } + } + + public function ProfilesLinksProvider() + { + return [ + 'Administrator' => ['sProfileNameToMove' => 'Administrator', 'bRaiseException' => true], + 'Portal user' => ['sProfileNameToMove' => 'Portal user', 'bRaiseException' => true], + ]; + } + + /** + * @dataProvider ProfilesLinksProvider + */ + public function testProfilesLinksEdit_ChangeUserId($sProfileNameToMove, $bRaiseException = false) + { + $aInitialProfiles = [$sProfileNameToMove, 'Portal power user']; + + $oUser = new UserExternal(); + $sLogin1 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin1); + + $sUserClass = get_class($oUser); + list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, $aInitialProfiles); + + $oUser = new UserExternal(); + $sLogin2 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true); + $oUser->Set('login', $sLogin2); + list ($sAnotherUserId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, ['Configuration Manager']); + + if ($bRaiseException) { + $this->expectException(CoreCannotSaveObjectException::class); + } + + $aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId); + if (array_key_exists($sProfileNameToMove, $aURPUserProfileByUser)) { + $oURPUserProfile = $aURPUserProfileByUser[$sProfileNameToMove]; + $oURPUserProfile->Set('userid', $sAnotherUserId); + $oURPUserProfile->DBWrite(); + } + + if (!$bRaiseException) { + $aExpectedProfilesAfterUpdate = [$sProfileNameToMove, 'Configuration Manager']; + $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sAnotherUserId, $aExpectedProfilesAfterUpdate); + + $aExpectedProfilesAfterUpdate = ['Portal power user', 'Portal user']; + $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedProfilesAfterUpdate); + } + } + + private function GetURPUserProfileByUser($iUserId): array + { + $aRes = []; + $oSearch = DBSearch::FromOQL("SELECT URP_UserProfile WHERE userid=$iUserId"); + $oSet = new DBObjectSet($oSearch); + while (($oURPUserProfile = $oSet->Fetch()) != null) { + $aRes[$oURPUserProfile->Get('profile')] = $oURPUserProfile; + } + + return $aRes; + } + + public function CustomizedPortalsProvider() + { + return [ + 'console + customized portal' => [ + 'aPortalDispatcherData' => [ + 'customer-portal', + 'backoffice', + ], + ], + 'console + itop portal + customized portal' => [ + 'aPortalDispatcherData' => [ + 'itop-portal', + 'customer-portal', + 'backoffice', + ], + ], + ]; + } +} diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index 9e65086e9..542573c74 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -488,29 +488,4 @@ class UserRightsTest extends ItopDataTestCase 'with Admins hidden' => [true, 0], ]; } - - /** - * @dataProvider WithConstraintParameterProvider - * @param string $sClass - * @param string $sAttCode - * @param bool $bExpected - * - * @return void - * @throws \Exception - */ - public function testWithConstraintParameter(string $sClass, string $sAttCode, bool $bExpected) - { - $oAttDef = \MetaModel::GetAttributeDef($sClass, $sAttCode); - $this->assertTrue(method_exists($oAttDef, "GetHasConstraint")); - $this->assertEquals($bExpected, $oAttDef->GetHasConstraint()); - } - - public function WithConstraintParameterProvider() - { - return [ - ['User', 'profile_list', true], - ['User', 'allowed_org_list', true], - ['Person', 'team_list', false], - ]; - } } From f72d8f4955c73dd0265438817c8f1d3f23471f8c Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 17 Nov 2023 17:14:17 +0100 Subject: [PATCH 3/4] =?UTF-8?q?N=C2=B06531=20-=20Trigger=20on=20Update=20o?= =?UTF-8?q?n=20n-n=20fields=20not=20working=20when=20object=20updated=20in?= =?UTF-8?q?=20pop-up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/dbobject.class.php | 156 +++++++++++++++++++++++++++++----------- 1 file changed, 114 insertions(+), 42 deletions(-) diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 898d8b33f..412d7b88d 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -2462,6 +2462,82 @@ abstract class DBObject implements iDisplay } } + /** + * Trigger onObjectUpdate on the remote object when an object pointed by a LinkSet is modified, added or removed + * + * @since 3.1.1 3.2.0 N°6531 method creation + */ + final protected function TriggerOnRemoteObjectUpdate(): void + { + $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); + + $aClassExtKeyAttCodes = MetaModel::GetAttributesList(get_class($this), [AttributeExternalKey::class]); + foreach ($aClassExtKeyAttCodes as $sExtKeyWithMirrorLinkAttCode) { + /** @var AttributeExternalKey $oExtKeyWithMirrorLinkAttDef */ + $oExtKeyWithMirrorLinkAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyWithMirrorLinkAttCode); + + $oRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode)); + if (is_null($oRemoteObject)) { + continue; + } + + /** @var AttributeLinkedSet $oAttDefMirrorLink */ + $oAttDefMirrorLink = $oExtKeyWithMirrorLinkAttDef->GetMirrorLinkAttribute(); + if (is_null($oAttDefMirrorLink)) { + continue; + } + $sAttCodeMirrorLink = $oAttDefMirrorLink->GetCode(); + + if (array_key_exists($sExtKeyWithMirrorLinkAttCode, $aPreviousValues)) { + // need to update remote old + new + $sPreviousRemoteObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; + $oPreviousRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey); + if (false === is_null($oPreviousRemoteObject)) { + $this->TriggerRemoteObject($oPreviousRemoteObject, $sAttCodeMirrorLink); + } + } + // we need to update remote with current lnk instance + $this->TriggerRemoteObject($oRemoteObject, $sAttCodeMirrorLink); + } + } + + private function TriggerRemoteObject(DBObject $oRemoteObject, string $sAttCodeMirrorLink): void + { + // indicates that $sAttCodeMirrorLink has been modified for the trigger + $oRemoteObject->m_aPreviousValuesForUpdatedAttributes[$sAttCodeMirrorLink] = $oRemoteObject->Get($sAttCodeMirrorLink); + $this->TriggerOnObjectUpdate($oRemoteObject); + } + + private function GetRemoteObject(AttributeExternalKey $oAttDef, $sRemoteObjectKey, bool $bWithConstraintProperty = false) + { + $sRemoteObjectClass = $oAttDef->GetTargetClass(); + + /** @noinspection NotOptimalIfConditionsInspection */ + /** @noinspection TypeUnsafeComparisonInspection */ + if (utils::IsNullOrEmptyString($sRemoteObjectClass) + || utils::IsNullOrEmptyString($sRemoteObjectKey) + || ($sRemoteObjectKey == 0) // non-strict comparison as we might have bad surprises + ) { + return null; + } + + /** @var AttributeLinkedSet $oAttDefMirrorLink */ + $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); + if (is_null($oAttDefMirrorLink)) { + return null; + } + + if ($bWithConstraintProperty && (false === $oAttDefMirrorLink->GetHasConstraint())) { + return null; + } + + if (DBObject::IsObjectCurrentlyInCrud($sRemoteObjectClass, $sRemoteObjectKey)) { + return null; + } + + return MetaModel::GetObject($sRemoteObjectClass, $sRemoteObjectKey, false); + } + /** * @since 3.1.1 3.2.0 N°6228 method creation */ @@ -2474,7 +2550,7 @@ abstract class DBObject implements iDisplay /** @var AttributeExternalKey $oExtKeyWithMirrorLinkAttDef */ $oExtKeyWithMirrorLinkAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyWithMirrorLinkAttCode); - $oRemoteObject = $this->GetRemoteObjectWithPhpConstraint($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode)); + $oRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode), true); if (is_null($oRemoteObject)) { continue; } @@ -2495,7 +2571,7 @@ abstract class DBObject implements iDisplay // need to update remote old + new $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); $sPreviousRemoteObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; - $oPreviousRemoteObject = $this->GetRemoteObjectWithPhpConstraint($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey); + $oPreviousRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey, true); if (false === is_null($oPreviousRemoteObject)) { $this->CheckRemotePhpConstraintOnObject('remove', $oPreviousRemoteObject, $sAttCodeMirrorLink, false); } @@ -2540,32 +2616,6 @@ abstract class DBObject implements iDisplay } } - private function GetRemoteObjectWithPhpConstraint(AttributeExternalKey $oAttDef, $sRemoteObjectKey) - { - $sRemoteObjectClass = $oAttDef->GetTargetClass(); - - /** @noinspection NotOptimalIfConditionsInspection */ - /** @noinspection TypeUnsafeComparisonInspection */ - if (utils::IsNullOrEmptyString($sRemoteObjectClass) - || utils::IsNullOrEmptyString($sRemoteObjectKey) - || ($sRemoteObjectKey == 0) // non-strict comparison as we might have bad surprises - ) { - return null; - } - - /** @var AttributeLinkedSet $oAttDefMirrorLink */ - $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); - if (is_null($oAttDefMirrorLink) || false === $oAttDefMirrorLink->GetHasConstraint()) { - return null; - } - - if (DBObject::IsObjectCurrentlyInCrud($sRemoteObjectClass, $sRemoteObjectKey)) { - return null; - } - - return MetaModel::GetObject($sRemoteObjectClass, $sRemoteObjectKey, false); - } - /** * @api * @api-advanced @@ -3470,6 +3520,9 @@ abstract class DBObject implements iDisplay // - TriggerOnObjectMention $this->ActivateOnMentionTriggers(true); + + // - Trigger for object pointing to the current object + $this->TriggerOnRemoteObjectUpdate(); } /** @@ -3770,20 +3823,10 @@ abstract class DBObject implements iDisplay $oKPI->ComputeStatsForExtension($this, 'AfterUpdate'); // - TriggerOnObjectUpdate - $aClassList = MetaModel::EnumParentClasses(get_class($this), ENUM_PARENT_CLASSES_ALL); - $aParams = array('class_list' => $aClassList); - $oSet = new DBObjectSet(DBObjectSearch::FromOQL('SELECT TriggerOnObjectUpdate AS t WHERE t.target_class IN (:class_list)'), - array(), $aParams); - while ($oTrigger = $oSet->Fetch()) { - /** @var \TriggerOnObjectUpdate $oTrigger */ - try { - $oTrigger->DoActivate($this->ToArgs()); - } - catch (Exception $e) { - $oTrigger->LogException($e, $this); - utils::EnrichRaisedException($oTrigger, $e); - } - } + $this->TriggerOnObjectUpdate($this); + + // - Trigger for object pointing to the current object + $this->TriggerOnRemoteObjectUpdate(); $sClass = get_class($this); if (MetaModel::HasLifecycle($sClass)) @@ -3829,6 +3872,33 @@ abstract class DBObject implements iDisplay $this->ActivateOnMentionTriggers(false, $aChanges); } + /** + * @param \DBObject $oObject + * + * @return void + * @throws \CoreException + * @throws \CoreUnexpectedValue + * @throws \MySQLException + * @throws \OQLException + */ + private function TriggerOnObjectUpdate(DBObject $oObject): void + { + // - TriggerOnObjectUpdate + $aClassList = MetaModel::EnumParentClasses(get_class($oObject), ENUM_PARENT_CLASSES_ALL); + $aParams = array('class_list' => $aClassList); + $oSet = new DBObjectSet(DBObjectSearch::FromOQL('SELECT TriggerOnObjectUpdate AS t WHERE t.target_class IN (:class_list)'), + array(), $aParams); + while ($oTrigger = $oSet->Fetch()) { + /** @var \TriggerOnObjectUpdate $oTrigger */ + try { + $oTrigger->DoActivate($oObject->ToArgs()); + } + catch (Exception $e) { + $oTrigger->LogException($e, $oObject); + utils::EnrichRaisedException($oTrigger, $e); + } + } + } /** * Increment attribute with specified value. @@ -4154,6 +4224,8 @@ abstract class DBObject implements iDisplay $this->AfterDelete(); $oKPI->ComputeStatsForExtension($this, 'AfterDelete'); + // - Trigger for object pointing to the current object + $this->TriggerOnRemoteObjectUpdate(); $this->m_bIsInDB = false; $this->LogCRUDExit(__METHOD__); From 8e0d6d1f00a233b1ca46b3010ba69d0250173ba6 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 20 Nov 2023 09:22:41 +0100 Subject: [PATCH 4/4] =?UTF-8?q?N=C2=B06228=20-=20Refactor=20after=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/cmdbabstract.class.inc.php | 28 +- core/attributedef.class.inc.php | 4 +- core/dbobject.class.php | 216 +++++----- core/metamodel.class.php | 4 +- core/trigger.class.inc.php | 32 ++ core/userrights.class.inc.php | 36 -- .../src/BaseTestCase/ItopDataTestCase.php | 16 + .../core/AttributeDefinitionTest.php | 7 +- .../DBObject/CheckToWritePropagationTest.php | 388 +++--------------- .../core/DBObject/CustomCheckToWriteTest.php | 64 +++ .../unitary-tests/core/MetaModelTest.php | 23 ++ 11 files changed, 302 insertions(+), 516 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/core/DBObject/CustomCheckToWriteTest.php diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index eb52af98e..362b71d23 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -4544,7 +4544,7 @@ HTML; return $res; } - public function PostInsertActions(): void + protected function PostInsertActions(): void { parent::PostInsertActions(); @@ -4610,7 +4610,7 @@ HTML; return $res; } - public function PostUpdateActions(array $aChanges): void + protected function PostUpdateActions(array $aChanges): void { parent::PostUpdateActions($aChanges); @@ -5981,33 +5981,27 @@ JS /** @var AttributeExternalKey $oAttDef */ $oAttDef = MetaModel::GetAttributeDef($sClass, $sExternalKeyAttCode); - if (false === $this->DoesRemoteObjectHavePhpComputation($oAttDef)) { + if (false === $this->DoesTargetObjectHavePhpComputation($oAttDef)) { continue; } - $sRemoteObjectId = $this->Get($sExternalKeyAttCode); - if ($sRemoteObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sRemoteObjectId); + $sTargetObjectId = $this->Get($sExternalKeyAttCode); + if ($sTargetObjectId > 0) { + self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sTargetObjectId); } - $sPreviousRemoteObjectId = $aPreviousValues[$sExternalKeyAttCode] ?? 0; - if ($sPreviousRemoteObjectId > 0) { - self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sPreviousRemoteObjectId); + $sPreviousTargetObjectId = $aPreviousValues[$sExternalKeyAttCode] ?? 0; + if ($sPreviousTargetObjectId > 0) { + self::RegisterObjectAwaitingEventDbLinksChanged($oAttDef->GetTargetClass(), $sPreviousTargetObjectId); } } } - private function DoesRemoteObjectHavePhpComputation(AttributeExternalKey $oAttDef): bool + private function DoesTargetObjectHavePhpComputation(AttributeExternalKey $oAttDef): bool { - $sRemoteObjectClass = $oAttDef->GetTargetClass(); - - if (utils::IsNullOrEmptyString($sRemoteObjectClass)) { - return false; - } - /** @var AttributeLinkedSet $oAttDefMirrorLink */ $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); - if (is_null($oAttDefMirrorLink) || false === $oAttDefMirrorLink->GetHasComputation()){ + if (is_null($oAttDefMirrorLink) || false === $oAttDefMirrorLink->HasPHPComputation()){ return false; } diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index ed3495d78..c30a8389b 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -1739,7 +1739,7 @@ class AttributeLinkedSet extends AttributeDefinition * @return bool true if Attribute has constraints * @since 3.1.0 N°6228 */ - public function GetHasConstraint() + public function HasPHPConstraint(): bool { return $this->GetOptional('with_php_constraint', false); } @@ -1748,7 +1748,7 @@ class AttributeLinkedSet extends AttributeDefinition * @return bool true if Attribute has computation (DB_LINKS_CHANGED event propagation, `with_php_computation` attribute xml property), false otherwise * @since 3.1.1 3.2.0 N°6228 */ - public function GetHasComputation() + public function HasPHPComputation(): bool { return $this->GetOptional('with_php_computation', false); } diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 412d7b88d..8bb13c0fb 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -2463,11 +2463,11 @@ abstract class DBObject implements iDisplay } /** - * Trigger onObjectUpdate on the remote object when an object pointed by a LinkSet is modified, added or removed + * Trigger onObjectUpdate on the target object when an object pointed by a LinkSet is modified, added or removed * * @since 3.1.1 3.2.0 N°6531 method creation */ - final protected function TriggerOnRemoteObjectUpdate(): void + final protected function ActivateOnObjectUpdateTriggersForTargetObjects(): void { $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); @@ -2476,72 +2476,42 @@ abstract class DBObject implements iDisplay /** @var AttributeExternalKey $oExtKeyWithMirrorLinkAttDef */ $oExtKeyWithMirrorLinkAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyWithMirrorLinkAttCode); - $oRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode)); - if (is_null($oRemoteObject)) { - continue; - } - /** @var AttributeLinkedSet $oAttDefMirrorLink */ $oAttDefMirrorLink = $oExtKeyWithMirrorLinkAttDef->GetMirrorLinkAttribute(); if (is_null($oAttDefMirrorLink)) { + // No LinkSet pointing to me continue; } $sAttCodeMirrorLink = $oAttDefMirrorLink->GetCode(); + $sTargetObjectClass = $oExtKeyWithMirrorLinkAttDef->GetTargetClass(); if (array_key_exists($sExtKeyWithMirrorLinkAttCode, $aPreviousValues)) { - // need to update remote old + new - $sPreviousRemoteObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; - $oPreviousRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey); - if (false === is_null($oPreviousRemoteObject)) { - $this->TriggerRemoteObject($oPreviousRemoteObject, $sAttCodeMirrorLink); - } + // need to update old target also + $sPreviousTargetObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; + $oPreviousTargetObject = static::GetObjectIfNotInCRUDStack($sTargetObjectClass, $sPreviousTargetObjectKey); + $this->ActivateOnObjectUpdateTriggers($oPreviousTargetObject, [$sAttCodeMirrorLink]); } + // we need to update remote with current lnk instance - $this->TriggerRemoteObject($oRemoteObject, $sAttCodeMirrorLink); + $oTargetObject = static::GetObjectIfNotInCRUDStack($sTargetObjectClass, $this->Get($sExtKeyWithMirrorLinkAttCode)); + $this->ActivateOnObjectUpdateTriggers($oTargetObject, [$sAttCodeMirrorLink]); } } - private function TriggerRemoteObject(DBObject $oRemoteObject, string $sAttCodeMirrorLink): void + final static protected function GetObjectIfNotInCRUDStack($sClass, $sKey) { - // indicates that $sAttCodeMirrorLink has been modified for the trigger - $oRemoteObject->m_aPreviousValuesForUpdatedAttributes[$sAttCodeMirrorLink] = $oRemoteObject->Get($sAttCodeMirrorLink); - $this->TriggerOnObjectUpdate($oRemoteObject); - } - - private function GetRemoteObject(AttributeExternalKey $oAttDef, $sRemoteObjectKey, bool $bWithConstraintProperty = false) - { - $sRemoteObjectClass = $oAttDef->GetTargetClass(); - - /** @noinspection NotOptimalIfConditionsInspection */ - /** @noinspection TypeUnsafeComparisonInspection */ - if (utils::IsNullOrEmptyString($sRemoteObjectClass) - || utils::IsNullOrEmptyString($sRemoteObjectKey) - || ($sRemoteObjectKey == 0) // non-strict comparison as we might have bad surprises - ) { + if (DBObject::IsObjectCurrentlyInCrud($sClass, $sKey)) { return null; } - /** @var AttributeLinkedSet $oAttDefMirrorLink */ - $oAttDefMirrorLink = $oAttDef->GetMirrorLinkAttribute(); - if (is_null($oAttDefMirrorLink)) { - return null; - } - - if ($bWithConstraintProperty && (false === $oAttDefMirrorLink->GetHasConstraint())) { - return null; - } - - if (DBObject::IsObjectCurrentlyInCrud($sRemoteObjectClass, $sRemoteObjectKey)) { - return null; - } - - return MetaModel::GetObject($sRemoteObjectClass, $sRemoteObjectKey, false); + return MetaModel::GetObject($sClass, $sKey, false); } /** + * Cascade CheckToWrite to Target Objects With LinkSet Pointing To Me * @since 3.1.1 3.2.0 N°6228 method creation */ - final protected function CheckPhpConstraint(bool $bIsCheckToDelete = false): void + final protected function CheckToWriteForTargetObjects(bool $bIsCheckToDelete = false): void { $aChanges = $this->ListChanges(); @@ -2550,58 +2520,58 @@ abstract class DBObject implements iDisplay /** @var AttributeExternalKey $oExtKeyWithMirrorLinkAttDef */ $oExtKeyWithMirrorLinkAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyWithMirrorLinkAttCode); - $oRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $this->Get($sExtKeyWithMirrorLinkAttCode), true); - if (is_null($oRemoteObject)) { - continue; - } - /** @var AttributeLinkedSet $oAttDefMirrorLink */ $oAttDefMirrorLink = $oExtKeyWithMirrorLinkAttDef->GetMirrorLinkAttribute(); - if (is_null($oAttDefMirrorLink)) { + if (is_null($oAttDefMirrorLink) || (false === $oAttDefMirrorLink->HasPHPConstraint())) { continue; } $sAttCodeMirrorLink = $oAttDefMirrorLink->GetCode(); + $sTargetObjectClass = $oExtKeyWithMirrorLinkAttDef->GetTargetClass(); + + $oTargetObject = static::GetObjectIfNotInCRUDStack($sTargetObjectClass, $this->Get($sExtKeyWithMirrorLinkAttCode)); if ($this->IsNew()) { - $this->CheckRemotePhpConstraintOnObject('add', $oRemoteObject, $sAttCodeMirrorLink, false); + $this->CheckToWriteForSingleTargetObject_Internal('add', $oTargetObject, $sAttCodeMirrorLink, false); } else if ($bIsCheckToDelete) { - $this->CheckRemotePhpConstraintOnObject('remove', $oRemoteObject, $sAttCodeMirrorLink, true); + $this->CheckToWriteForSingleTargetObject_Internal('remove', $oTargetObject, $sAttCodeMirrorLink, true); } else { if (array_key_exists($sExtKeyWithMirrorLinkAttCode, $aChanges)) { // need to update remote old + new $aPreviousValues = $this->ListPreviousValuesForUpdatedAttributes(); - $sPreviousRemoteObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; - $oPreviousRemoteObject = $this->GetRemoteObject($oExtKeyWithMirrorLinkAttDef, $sPreviousRemoteObjectKey, true); - if (false === is_null($oPreviousRemoteObject)) { - $this->CheckRemotePhpConstraintOnObject('remove', $oPreviousRemoteObject, $sAttCodeMirrorLink, false); - } - $this->CheckRemotePhpConstraintOnObject('add', $oRemoteObject, $sAttCodeMirrorLink, false); + $sPreviousTargetObjectKey = $aPreviousValues[$sExtKeyWithMirrorLinkAttCode]; + $oPreviousTargetObject = static::GetObjectIfNotInCRUDStack($sTargetObjectClass, $sPreviousTargetObjectKey); + $this->CheckToWriteForSingleTargetObject_Internal('remove', $oPreviousTargetObject, $sAttCodeMirrorLink, false); + $this->CheckToWriteForSingleTargetObject_Internal('add', $oTargetObject, $sAttCodeMirrorLink, false); } else { - $this->CheckRemotePhpConstraintOnObject('modify', $oRemoteObject, $sAttCodeMirrorLink, false); // we need to update remote with current lnk instance + $this->CheckToWriteForSingleTargetObject_Internal('modify', $oTargetObject, $sAttCodeMirrorLink, false); // we need to update remote with current lnk instance } } } } - private function CheckRemotePhpConstraintOnObject(string $sAction, DBObject $oRemoteObject, string $sAttCodeMirrorLink, bool $bIsCheckToDelete): void + private function CheckToWriteForSingleTargetObject_Internal(string $sAction, ?DBObject $oTargetObject, string $sAttCodeMirrorLink, bool $bIsCheckToDelete): void { - $this->LogCRUDDebug(__METHOD__, "action: $sAction ".get_class($oRemoteObject).'::'.$oRemoteObject->GetKey()." ($sAttCodeMirrorLink)"); + if (is_null($oTargetObject)) { + return; + } - /** @var \ormLinkSet $oRemoteValue */ - $oRemoteValue = $oRemoteObject->Get($sAttCodeMirrorLink); + $this->LogCRUDDebug(__METHOD__, "action: $sAction ".get_class($oTargetObject).'::'.$oTargetObject->GetKey()." ($sAttCodeMirrorLink)"); + + /** @var \ormLinkSet $oTargetValue */ + $oTargetValue = $oTargetObject->Get($sAttCodeMirrorLink); switch ($sAction) { case 'add': - $oRemoteValue->AddItem($this); + $oTargetValue->AddItem($this); break; case 'remove': - $oRemoteValue->RemoveItem($this->GetKey()); + $oTargetValue->RemoveItem($this->GetKey()); break; case 'modify': - $oRemoteValue->ModifyItem($this); + $oTargetValue->ModifyItem($this); break; } - $oRemoteObject->Set($sAttCodeMirrorLink, $oRemoteValue); - [$bCheckStatus, $aCheckIssues, $bSecurityIssue] = $oRemoteObject->CheckToWrite(); + $oTargetObject->Set($sAttCodeMirrorLink, $oTargetValue); + [$bCheckStatus, $aCheckIssues, $bSecurityIssue] = $oTargetObject->CheckToWrite(); if (false === $bCheckStatus) { if ($bIsCheckToDelete) { $this->m_aDeleteIssues = array_merge($this->m_aDeleteIssues ?? [], $aCheckIssues); @@ -2610,9 +2580,9 @@ abstract class DBObject implements iDisplay } $this->m_bSecurityIssue = $this->m_bSecurityIssue || $bSecurityIssue; } - $aRemoteCheckWarnings = $oRemoteObject->GetCheckWarnings(); - if (is_array($aRemoteCheckWarnings)) { - $this->m_aCheckWarnings = array_merge($this->m_aCheckWarnings ?? [], $aRemoteCheckWarnings); + $aTargetCheckWarnings = $oTargetObject->GetCheckWarnings(); + if (is_array($aTargetCheckWarnings)) { + $this->m_aCheckWarnings = array_merge($this->m_aCheckWarnings ?? [], $aTargetCheckWarnings); } } @@ -2657,7 +2627,7 @@ abstract class DBObject implements iDisplay $this->DoCheckToWrite(); $oKPI->ComputeStatsForExtension($this, 'DoCheckToWrite'); - $this->CheckPhpConstraint(); + $this->CheckToWriteForTargetObjects(); if (count($this->m_aCheckIssues) == 0) { @@ -2754,7 +2724,7 @@ abstract class DBObject implements iDisplay * * an array of displayable error is added in {@see DBObject::$m_aDeleteIssues} * - * @internal + * @internal * * @param \DeletionPlan $oDeletionPlan * @@ -2819,8 +2789,14 @@ abstract class DBObject implements iDisplay */ public function CheckToDelete(&$oDeletionPlan) { - $this->MakeDeletionPlan($oDeletionPlan); - $oDeletionPlan->ComputeResults(); + $this->AddCurrentObjectInCrudStack('DELETE'); + try { + $this->MakeDeletionPlan($oDeletionPlan); + $oDeletionPlan->ComputeResults(); + } + finally { + $this->RemoveCurrentObjectInCrudStack(); + } return (!$oDeletionPlan->FoundStopper()); } @@ -2872,7 +2848,7 @@ abstract class DBObject implements iDisplay { // The value is a scalar, the comparison must be 100% strict if($this->m_aOrigValues[$sAtt] !== $proposedValue) - { + { //echo "$sAtt:
\n";
 					//var_dump($this->m_aOrigValues[$sAtt]);
 					//var_dump($proposedValue);
@@ -2994,7 +2970,7 @@ abstract class DBObject implements iDisplay
 
 	/**
 	 * Used only by insert, Meant to be overloaded
-     * 
+     *
      * @overwritable-hook You can extend this method in order to provide your own logic.
 	 */
 	protected function OnObjectKeyReady()
@@ -3102,7 +3078,7 @@ abstract class DBObject implements iDisplay
 		// fields in first array, values in the second
 		$aFieldsToWrite = array();
 		$aValuesToWrite = array();
-		
+
 		if (!empty($this->m_iKey) && ($this->m_iKey >= 0))
 		{
 			// Add it to the list of fields to write
@@ -3111,7 +3087,7 @@ abstract class DBObject implements iDisplay
 		}
 
 		$aHierarchicalKeys = array();
-		
+
 		foreach(MetaModel::ListAttributeDefs($sTableClass) as $sAttCode=>$oAttDef) {
 			// Skip this attribute if not defined in this table
 			if ((!MetaModel::IsAttributeOrigin($sTableClass, $sAttCode) && !$oAttDef->CopyOnAllTables())
@@ -3121,7 +3097,7 @@ abstract class DBObject implements iDisplay
 			$aAttColumns = $oAttDef->GetSQLValues($this->m_aCurrValues[$sAttCode]);
 			foreach($aAttColumns as $sColumn => $sValue)
 			{
-				$aFieldsToWrite[] = "`$sColumn`"; 
+				$aFieldsToWrite[] = "`$sColumn`";
 				$aValuesToWrite[] = CMDBSource::Quote($sValue);
 			}
 			if ($oAttDef->IsHierarchicalKey())
@@ -3145,7 +3121,7 @@ abstract class DBObject implements iDisplay
 					self::$m_aBulkInsertCols[$sClass][$sTable] = implode(', ', $aFieldsToWrite);
 				}
 				self::$m_aBulkInsertItems[$sClass][$sTable][] = '('.implode (', ', $aValuesToWrite).')';
-				
+
 				$iNewKey = 999999; // TODO - compute next id....
 			}
 			else
@@ -3230,7 +3206,7 @@ abstract class DBObject implements iDisplay
 		// fields in first array, values in the second
 		$aFieldsToWrite = array();
 		$aValuesToWrite = array();
-		
+
 		if (!empty($this->m_iKey) && ($this->m_iKey >= 0))
 		{
 			// Add it to the list of fields to write
@@ -3265,7 +3241,7 @@ abstract class DBObject implements iDisplay
 			$aAttColumns = $oAttDef->GetSQLValues($value);
 			foreach($aAttColumns as $sColumn => $sValue)
 			{
-				$aFieldsToWrite[] = "`$sColumn`"; 
+				$aFieldsToWrite[] = "`$sColumn`";
 				$aValuesToWrite[] = CMDBSource::Quote($sValue);
 			}
 			if ($oAttDef->IsHierarchicalKey())
@@ -3496,7 +3472,7 @@ abstract class DBObject implements iDisplay
 	 * @throws \MySQLException
 	 * @throws \OQLException
 	 */
-	public function PostInsertActions(): void
+	protected function PostInsertActions(): void
 	{
 		$this->FireEventAfterWrite([], true);
 		$oKPI = new ExecutionKPI();
@@ -3522,7 +3498,7 @@ abstract class DBObject implements iDisplay
 		$this->ActivateOnMentionTriggers(true);
 
 		// - Trigger for object pointing to the current object
-		$this->TriggerOnRemoteObjectUpdate();
+		$this->ActivateOnObjectUpdateTriggersForTargetObjects();
 	}
 
     /**
@@ -3550,7 +3526,7 @@ abstract class DBObject implements iDisplay
 		$this->RecordObjCreation();
 		return $ret;
 	}
-	
+
 	/**
 	 * This function is automatically called after cloning an object with the "clone" PHP language construct
 	 * The purpose of this method is to reset the appropriate attributes of the object in
@@ -3815,7 +3791,7 @@ abstract class DBObject implements iDisplay
 	 * @throws \MySQLException
 	 * @throws \OQLException
 	 */
-	public function PostUpdateActions(array $aChanges): void
+	protected function PostUpdateActions(array $aChanges): void
 	{
 		$this->FireEventAfterWrite($aChanges, false);
 		$oKPI = new ExecutionKPI();
@@ -3823,10 +3799,10 @@ abstract class DBObject implements iDisplay
 		$oKPI->ComputeStatsForExtension($this, 'AfterUpdate');
 
 		// - TriggerOnObjectUpdate
-		$this->TriggerOnObjectUpdate($this);
+		$this->ActivateOnObjectUpdateTriggers($this);
 
 		// - Trigger for object pointing to the current object
-		$this->TriggerOnRemoteObjectUpdate();
+		$this->ActivateOnObjectUpdateTriggersForTargetObjects();
 
 		$sClass = get_class($this);
 		if (MetaModel::HasLifecycle($sClass))
@@ -3874,15 +3850,19 @@ abstract class DBObject implements iDisplay
 
 	/**
 	 * @param \DBObject $oObject
+	 * @param array|null $aAttributes
 	 *
-	 * @return void
 	 * @throws \CoreException
 	 * @throws \CoreUnexpectedValue
 	 * @throws \MySQLException
 	 * @throws \OQLException
 	 */
-	private function TriggerOnObjectUpdate(DBObject $oObject): void
+	private function ActivateOnObjectUpdateTriggers(?DBObject $oObject, array $aAttributes = null): void
 	{
+		if (is_null($oObject)) {
+			return;
+		}
+
 		// - TriggerOnObjectUpdate
 		$aClassList = MetaModel::EnumParentClasses(get_class($oObject), ENUM_PARENT_CLASSES_ALL);
 		$aParams = array('class_list' => $aClassList);
@@ -3891,7 +3871,7 @@ abstract class DBObject implements iDisplay
 		while ($oTrigger = $oSet->Fetch()) {
 			/** @var \TriggerOnObjectUpdate $oTrigger */
 			try {
-				$oTrigger->DoActivate($oObject->ToArgs());
+				$oTrigger->DoActivateForSpecificAttributes($oObject->ToArgs(), $aAttributes);
 			}
 			catch (Exception $e) {
 				$oTrigger->LogException($e, $oObject);
@@ -4225,7 +4205,7 @@ abstract class DBObject implements iDisplay
 		$oKPI->ComputeStatsForExtension($this, 'AfterDelete');
 
 		// - Trigger for object pointing to the current object
-		$this->TriggerOnRemoteObjectUpdate();
+		$this->ActivateOnObjectUpdateTriggersForTargetObjects();
 
 		$this->m_bIsInDB = false;
 		$this->LogCRUDExit(__METHOD__);
@@ -4240,7 +4220,7 @@ abstract class DBObject implements iDisplay
      * First, checks if the object can be deleted regarding database integrity.
      * If the answer is yes, it performs any required cleanup (delete other objects or reset external keys) in addition to the object
      * deletion.
-     * 
+     *
      * @api
      *
      * @param \DeletionPlan $oDeletionPlan Do not use: aims at dealing with recursion
@@ -4259,9 +4239,7 @@ abstract class DBObject implements iDisplay
 	public function DBDelete(&$oDeletionPlan = null)
 	{
 		$this->LogCRUDEnter(__METHOD__);
-		$this->AddCurrentObjectInCrudStack('DELETE');
 		try {
-
 			static $iLoopTimeLimit = null;
 			if ($iLoopTimeLimit == null) {
 				$iLoopTimeLimit = MetaModel::GetConfig()->Get('max_execution_time_per_loop');
@@ -4269,17 +4247,15 @@ abstract class DBObject implements iDisplay
 			if (is_null($oDeletionPlan)) {
 				$oDeletionPlan = new DeletionPlan();
 			}
-			$this->MakeDeletionPlan($oDeletionPlan);
-			$oDeletionPlan->ComputeResults();
 
-			if ($oDeletionPlan->FoundStopper()) {
+			if (false === $this->CheckToDelete($oDeletionPlan)) {
 				$aIssues = $oDeletionPlan->GetIssues();
 				$this->LogCRUDError(__METHOD__, ' Errors: '.implode(', ', $aIssues));
 				throw new DeleteException('Found issue(s)', array('target_class' => get_class($this), 'target_id' => $this->GetKey(), 'issues' => implode(', ', $aIssues)));
 			}
 
 
-			// Getting and setting time limit are not symetric:
+			// Getting and setting time limit are not symmetric:
 			// www.php.net/manual/fr/function.set-time-limit.php#72305
 			$iPreviousTimeLimit = ini_get('max_execution_time');
 
@@ -4320,7 +4296,6 @@ abstract class DBObject implements iDisplay
 			set_time_limit(intval($iPreviousTimeLimit));
 		} finally {
 			$this->LogCRUDExit(__METHOD__);
-			$this->RemoveCurrentObjectInCrudStack();
 		}
 
 		return $oDeletionPlan;
@@ -4629,7 +4604,7 @@ abstract class DBObject implements iDisplay
      *
      * @api
      *
-	 */	 	
+	 */
 	public function Reset($sAttCode)
 	{
 		$this->Set($sAttCode, $this->GetDefaultValue($sAttCode));
@@ -4641,7 +4616,7 @@ abstract class DBObject implements iDisplay
      * Suitable for use as a lifecycle action
      *
      * @api
-	 */	 	
+	 */
 	public function Copy($sDestAttCode, $sSourceAttCode)
 	{
 		$oTypeValueToCopy = MetaModel::GetAttributeDef(get_class($this), $sSourceAttCode);
@@ -4971,7 +4946,7 @@ abstract class DBObject implements iDisplay
 			{
 				throw new CoreException("Unknown attribute '$sExtKeyAttCode' for the class ".get_class($this));
 			}
-			
+
 			$oKeyAttDef = MetaModel::GetAttributeDef(get_class($this), $sExtKeyAttCode);
 			if (!$oKeyAttDef instanceof AttributeExternalKey)
 			{
@@ -4989,14 +4964,14 @@ abstract class DBObject implements iDisplay
 				$ret  = $oRemoteObj->GetForTemplate($sRemoteAttCode);
 			}
 		}
-		else 
+		else
 		{
 			switch($sPlaceholderAttCode)
 			{
 				case 'id':
 				$ret = $this->GetKey();
 				break;
-				
+
 				case 'name()':
 				$ret = $this->GetName();
 				break;
@@ -5183,7 +5158,7 @@ abstract class DBObject implements iDisplay
 		if ($oOwner)
 		{
 			$sLinkSetOwnerClass = get_class($oOwner);
-			
+
 			$oMyChangeOp = MetaModel::NewObject($sChangeOpClass);
 			$oMyChangeOp->Set("objclass", $sLinkSetOwnerClass);
 			$oMyChangeOp->Set("objkey", $iLinkSetOwnerId);
@@ -5210,7 +5185,7 @@ abstract class DBObject implements iDisplay
 		{
 			/** @var \AttributeLinkedSet $oLinkSet */
 			if (($oLinkSet->GetTrackingLevel() & LINKSET_TRACKING_LIST) == 0) continue;
-			
+
 			$iLinkSetOwnerId  = $this->Get($sExtKeyAttCode);
 			$oMyChangeOp = $this->PrepareChangeOpLinkSet($iLinkSetOwnerId, $oLinkSet, 'CMDBChangeOpSetAttributeLinksAddRemove');
 			if ($oMyChangeOp)
@@ -5428,9 +5403,9 @@ abstract class DBObject implements iDisplay
 		$this->m_aDeleteIssues = array(); // Ok
 		$this->FireEventCheckToDelete($oDeletionPlan);
 		$this->DoCheckToDelete($oDeletionPlan);
-		$this->CheckPhpConstraint(true);
+		$this->CheckToWriteForTargetObjects(true);
 		$oDeletionPlan->SetDeletionIssues($this, $this->m_aDeleteIssues, $this->m_bSecurityIssue);
-	
+
 		$aDependentObjects = $this->GetReferencingObjects(true /* allow all data */);
 
 		// Getting and setting time limit are not symmetric:
@@ -5612,7 +5587,7 @@ abstract class DBObject implements iDisplay
 				$aSynchroClasses[] = $sTarget;
 			}
 		}
-		
+
 		foreach($aSynchroClasses as $sClass)
 		{
 			if ($this instanceof $sClass)
@@ -6678,14 +6653,10 @@ abstract class DBObject implements iDisplay
 		// during insert key is reset from -1 to null
 		// so we need to handle null values (will give empty string after conversion)
 		$sConvertedId = (string)$sId;
-
-		if (((int)$sId) > 0) {
-			// When having a class hierarchy, we are saving the leaf class in the stack
-			$sClass = MetaModel::GetFinalClassName($sClass, $sId);
-		}
+		$oRootClass = MetaModel::GetRootClass($sClass);
 
 		foreach (self::$m_aCrudStack as $aCrudStackEntry) {
-			if (($sClass === $aCrudStackEntry['class'])
+			if (($oRootClass === $aCrudStackEntry['class'])
 				&& ($sConvertedId === $aCrudStackEntry['id'])) {
 				return true;
 			}
@@ -6700,12 +6671,14 @@ abstract class DBObject implements iDisplay
 	 * @param string $sClass
 	 *
 	 * @return bool
+	 * @throws \CoreException
 	 * @since 3.1.0 N°5609
 	 */
 	final public static function IsClassCurrentlyInCrud(string $sClass): bool
 	{
+		$sRootClass = MetaModel::GetRootClass($sClass);
 		foreach (self::$m_aCrudStack as $aCrudStackEntry) {
-			if ($sClass === $aCrudStackEntry['class']) {
+			if ($sRootClass === $aCrudStackEntry['class']) {
 				return true;
 			}
 		}
@@ -6724,9 +6697,10 @@ abstract class DBObject implements iDisplay
 	private function AddCurrentObjectInCrudStack(string $sCrudType): void
 	{
 		$this->LogCRUDDebug(__METHOD__);
+		$sRootClass = MetaModel::GetRootClass(get_class($this));
 		self::$m_aCrudStack[] = [
 			'type'  => $sCrudType,
-			'class' => get_class($this),
+			'class' => $sRootClass,
 			'id'    => (string)$this->GetKey(), // GetKey() doesn't have type hinting, so forcing type to avoid getting an int
 		];
 	}
diff --git a/core/metamodel.class.php b/core/metamodel.class.php
index e725c31d5..82de32529 100644
--- a/core/metamodel.class.php
+++ b/core/metamodel.class.php
@@ -6928,8 +6928,7 @@ abstract class MetaModel
 
 	public static function GetFinalClassName(string $sClass, int $iKey): string
 	{
-		$sFinalClassField = Metamodel::DBGetClassField($sClass);
-		if (utils::IsNullOrEmptyString($sFinalClassField)) {
+		if (MetaModel::IsStandaloneClass($sClass)) {
 			return $sClass;
 		}
 
@@ -6937,6 +6936,7 @@ abstract class MetaModel
 		$sTable = MetaModel::DBGetTable($sRootClass);
 		$sKeyCol = MetaModel::DBGetKey($sRootClass);
 		$sEscapedKey = CMDBSource::Quote($iKey);
+		$sFinalClassField = Metamodel::DBGetClassField($sRootClass);
 
 		$sQuery = "SELECT `{$sFinalClassField}` FROM `{$sTable}` WHERE `{$sKeyCol}` = {$sEscapedKey}";
 		return  CMDBSource::QueryToScalar($sQuery);
diff --git a/core/trigger.class.inc.php b/core/trigger.class.inc.php
index b0222184c..7cc3a1d80 100644
--- a/core/trigger.class.inc.php
+++ b/core/trigger.class.inc.php
@@ -546,6 +546,38 @@ class TriggerOnObjectUpdate extends TriggerOnObject
 		MetaModel::Init_SetZListItems('standard_search', array('description', 'target_class')); // Criteria of the std search form
 	}
 
+	/**
+	 * Activate trigger based on attribute list given instead of changed attributes
+	 *
+	 * @param array $aContextArgs
+	 * @param array|null $aAttributes if null default to changed attributes
+	 *
+	 * @throws \ArchivedObjectException
+	 * @throws \CoreException
+	 * @throws \MissingQueryArgument
+	 * @throws \MySQLException
+	 * @throws \MySQLHasGoneAwayException
+	 * @throws \OQLException
+	 * @since 3.1.1 3.2.0 N°6228
+	 */
+	public function DoActivateForSpecificAttributes(array $aContextArgs, ?array $aAttributes)
+	{
+		if (isset($aContextArgs['this->object()']))
+		{
+			/** @var \DBObject $oObject */
+			$oObject = $aContextArgs['this->object()'];
+			if (is_null($aAttributes)) {
+				$aChanges = $oObject->ListPreviousValuesForUpdatedAttributes();
+			} else {
+				$aChanges = array_fill_keys($aAttributes, true);
+			}
+			if (false === $this->IsTargetObject($oObject->GetKey(), $aChanges)) {
+				return;
+			}
+		}
+		parent::DoActivate($aContextArgs);
+	}
+
 	public function IsTargetObject($iObjectId, $aChanges = array())
 	{
 		if (!parent::IsTargetObject($iObjectId, $aChanges))
diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php
index a099e96fd..aea8658e4 100644
--- a/core/userrights.class.inc.php
+++ b/core/userrights.class.inc.php
@@ -1,8 +1,6 @@
 ListChanges())) {
-			return;
-		}
-
-		$oProfileLinkSet = $this->Get('profile_list');
-		if ($oProfileLinkSet->Count() > 1) {
-			return;
-		}
-		$oProfileLinkSet->Rewind();
-		$iPowerPortalCount = 0;
-		$iTotalCount = 0;
-		while ($oUserProfile = $oProfileLinkSet->Fetch()) {
-			$sProfile = $oUserProfile->Get('profile');
-			if ($sProfile === 'Portal power user') {
-				$iPowerPortalCount = 1;
-			}
-			$iTotalCount++;
-		}
-		if ($iTotalCount === $iPowerPortalCount) {
-			$this->AddCheckIssue(Dict::S('Class:User/Error:PortalPowerUserHasInsufficientRights'));
-		}
-	}
-
 	/**
 	 * @inheritDoc
 	 * @since 3.0.0
diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
index c14340cbc..29429f0fe 100644
--- a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
+++ b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
@@ -79,6 +79,22 @@ abstract class ItopDataTestCase extends ItopTestCase
 	const USE_TRANSACTION = true;
 	const CREATE_TEST_ORG = false;
 
+	protected static $aURP_Profiles = [
+		'Administrator'         => 1,
+		'Portal user'           => 2,
+		'Configuration Manager' => 3,
+		'Service Desk Agent'    => 4,
+		'Support Agent'         => 5,
+		'Problem Manager'       => 6,
+		'Change Implementor'    => 7,
+		'Change Supervisor'     => 8,
+		'Change Approver'       => 9,
+		'Service Manager'       => 10,
+		'Document author'       => 11,
+		'Portal power user'     => 12,
+		'REST Services User'    => 1024,
+	];
+
 	/**
 	 * This method is called before the first test of this test class is run (in the current process).
 	 */
diff --git a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php
index 418b5fef1..254c9789e 100644
--- a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php
+++ b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php
@@ -223,9 +223,10 @@ PHP
 	public function testWithConstraintAndComputationParameters(string $sClass, string $sAttCode, bool $bConstraintExpected, bool $bComputationExpected)
 	{
 		$oAttDef = \MetaModel::GetAttributeDef($sClass, $sAttCode);
-		$this->assertTrue(method_exists($oAttDef, 'GetHasConstraint'));
-		$this->assertEquals($bConstraintExpected, $oAttDef->GetHasConstraint());
-		$this->assertEquals($bComputationExpected, $oAttDef->GetHasComputation());
+		$sConstraintExpected = $bConstraintExpected ? 'true' : 'false';
+		$sComputationExpected = $bComputationExpected ? 'true' : 'false';
+		$this->assertEquals($bConstraintExpected, $oAttDef->HasPHPConstraint(), "Standard DataModel should be configured with property 'has_php_constraint'=$sConstraintExpected for $sClass:$sAttCode");
+		$this->assertEquals($bComputationExpected, $oAttDef->HasPHPComputation(), "Standard DataModel should be configured with property 'has_php_computation'=$sComputationExpected for $sClass:$sAttCode");
 	}
 
 	public function WithConstraintParameterProvider()
diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php b/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php
index f4e025983..8ab191336 100644
--- a/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php
+++ b/tests/php-unit-tests/unitary-tests/core/DBObject/CheckToWritePropagationTest.php
@@ -4,356 +4,74 @@
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
-/**
- * Created by PhpStorm.
- * Date: 25/01/2018
- * Time: 11:12
- */
-
 namespace Combodo\iTop\Test\UnitTest\Core\DBObject;
 
-use Combodo\iTop\Application\UI\Base\Layout\NavigationMenu\NavigationMenuFactory;
+use Combodo\iTop\Service\Events\EventData;
 use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
-use CoreCannotSaveObjectException;
-use DBObjectSet;
-use DBSearch;
-use DeleteException;
-use MetaModel;
 use URP_UserProfile;
-use User;
-use UserExternal;
 use UserLocal;
-use UserRights;
 
-/**
- * @group itopRequestMgmt
- * @group userRights
- * @group defaultProfiles
- *
- * @runTestsInSeparateProcesses
- * @preserveGlobalState disabled
- * @backupGlobals disabled
- */
 class CheckToWritePropagationTest extends ItopDataTestCase
 {
-	public function PortaPowerUserProvider()
+	private static array $aEventCalls;
+
+	public function testCascadeCheckToWrite()
 	{
-		return [
-			'No profile' => [
-				'aProfilesBeforeUserCreation'        => [
-				],
-				'bWaitForException'                         => 'CoreCannotSaveObjectException',
-			],
-			'Portal power user'                 => [
-				'aProfilesBeforeUserCreation'        => [
-					'Portal power user',
-				],
-				'bWaitForException'                         => 'CoreCannotSaveObjectException',
-			],
-			'Portal power user + Configuration Manager'                 => [
-				'aProfilesBeforeUserCreation'        => [
-					'Portal power user',
-					'Configuration Manager',
-				],
-				'bWaitForException'                         => false,
-			],
-			'Portal power user + Configuration Manager + Admin'                 => [
-				'aProfilesBeforeUserCreation'        => [
-					'Portal power user',
-					'Configuration Manager',
-					'Administrator',
-				],
-				'bWaitForException'                         => false,
-			],
-		];
+		$sLogin = 'testCascadeCheckToWrite-'.uniqid('', true);
+		$oUser1 = $this->CreateUser($sLogin, self::$aURP_Profiles['Administrator'], 'ABCD1234@gabuzomeu');
+		$sUserId1 = $oUser1->GetKey();
+		$sLogin = 'testCascadeCheckToWrite-'.uniqid('', true);
+		$oUser2 = $this->CreateUser($sLogin, self::$aURP_Profiles['Administrator'], 'ABCD1234@gabuzomeu');
+		$sUserId2 = $oUser2->GetKey();
+
+		$this->EventService_RegisterListener(EVENT_DB_CHECK_TO_WRITE, [$this, 'CheckToWriteEventListener'], 'User');
+		$sEventKeyUser1 = $this->GetEventKey(EVENT_DB_CHECK_TO_WRITE, UserLocal::class, $sUserId1);
+		$sEventKeyUser2 = $this->GetEventKey(EVENT_DB_CHECK_TO_WRITE, UserLocal::class, $sUserId2);
+
+		// Add URP_UserProfile
+		self::$aEventCalls = [];
+		$oURPUserProfile = new URP_UserProfile();
+		$oURPUserProfile->Set('profileid', self::$aURP_Profiles['Support Agent']);
+		$oURPUserProfile->Set('userid', $sUserId1);
+		$oURPUserProfile->Set('reason', 'UNIT Tests');
+		$oURPUserProfile->DBInsert();
+		$this->assertArrayHasKey($sEventKeyUser1, self::$aEventCalls, 'User checkToWrite should be called when a URP_UserProfile is created');
+
+		// Update URP_UserProfile (change profile)
+		self::$aEventCalls = [];
+		$oURPUserProfile->Set('profileid', self::$aURP_Profiles['Problem Manager']);
+		$oURPUserProfile->DBUpdate();
+		$this->assertArrayHasKey($sEventKeyUser1, self::$aEventCalls, 'User checkToWrite should be called when a URP_UserProfile is updated');
+
+		// Update URP_UserProfile (move from User1 to User2)
+		self::$aEventCalls = [];
+		$oURPUserProfile->Set('userid', $sUserId2);
+		$oURPUserProfile->DBUpdate();
+		$this->assertCount(2, self::$aEventCalls, 'Previous User and new User checkToWrite should be called when a URP_UserProfile is moved from a User to another');
+		$this->assertArrayHasKey($sEventKeyUser1, self::$aEventCalls, 'Previous User checkToWrite should be called when a URP_UserProfile is moved from a User to another');
+		$this->assertArrayHasKey($sEventKeyUser2, self::$aEventCalls, 'New User checkToWrite should be called when a URP_UserProfile is moved from a User to another');
+
+		// Delete URP_UserProfile from User2
+		self::$aEventCalls = [];
+		$oURPUserProfile->DBDelete();
+		$this->assertArrayHasKey($sEventKeyUser2, self::$aEventCalls, 'User checkToWrite should be called when a URP_UserProfile is deleted');
+
+		$oUser1->DBDelete();
+		$oUser2->DBDelete();
 	}
 
-	/**
-	 * @dataProvider PortaPowerUserProvider
-	 * @covers User::CheckPortalProfiles
-	 */
-	public function testUserLocalCreation($aProfilesBeforeUserCreation, $sWaitForException)
+	public function CheckToWriteEventListener(EventData $oEventData)
 	{
-		$oUser = new UserLocal();
-		$sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin);
-		$oUser->Set('password', 'ABCD1234@gabuzomeu');
-		$oUser->Set('language', 'EN US');
-		if (false !== $sWaitForException) {
-			$this->expectException($sWaitForException);
-		}
-		$this->commonUserCreationTest($oUser, $aProfilesBeforeUserCreation);
+		$oObject = $oEventData->GetEventData()['object'];
+		$sEvent = $oEventData->GetEvent();
+		$sClass = get_class($oObject);
+		$sId = $oObject->GetKey();
+		self::$aEventCalls[$this->GetEventKey($sEvent, $sClass, $sId)] = true;
 	}
 
-	/**
-	 * @dataProvider PortaPowerUserProvider
-	 * @covers User::CheckPortalProfiles
-	 */
-	public function testUserLocalDelete($aProfilesBeforeUserCreation, $sWaitForException)
+	private function GetEventKey($sEvent, $sClass, $sId)
 	{
-		$oUser = new UserLocal();
-		$sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin);
-		$oUser->Set('password', 'ABCD1234@gabuzomeu');
-		$oUser->Set('language', 'EN US');
-		if (false !== $sWaitForException) {
-			$this->expectException($sWaitForException);
-		}
-		$this->commonUserCreationTest($oUser, $aProfilesBeforeUserCreation, false);
-
-		$oUser->DBDelete();
+		return "event: $sEvent, class: $sClass, id: $sId";
 	}
 
-	/**
-	 * @dataProvider PortaPowerUserProvider
-	 * @covers User::CheckPortalProfiles
-	 */
-	public function testUserLocalUpdate($aProfilesBeforeUserCreation, $sWaitForException)
-	{
-		$oUser = new UserLocal();
-		$sLogin = 'testUserLocalUpdateWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin);
-		$oUser->Set('password', 'ABCD1234@gabuzomeu');
-		$oUser->Set('language', 'EN US');
-		if (false !== $sWaitForException) {
-			$this->expectException($sWaitForException);
-		}
-		$this->commonUserUpdateTest($oUser, $aProfilesBeforeUserCreation);
-	}
-
-	private function commonUserCreationTest($oUserToCreate, $aProfilesBeforeUserCreation, $bTestUserItopAccess = true)
-	{
-		$sUserClass = get_class($oUserToCreate);
-		list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUserToCreate, $aProfilesBeforeUserCreation);
-
-		$this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aProfilesBeforeUserCreation, $bTestUserItopAccess);
-	}
-
-	private function commonUserUpdateTest($oUserToCreate, $aProfilesBeforeUserCreation)
-	{
-		$sUserClass = get_class($oUserToCreate);
-		list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUserToCreate, ['Administrator']);
-
-		$oUserToUpdate = MetaModel::GetObject($sUserClass, $sId);
-		$oProfileList = $oUserToUpdate->Get('profile_list');
-		while ($oObj = $oProfileList->Fetch()) {
-			$oProfileList->RemoveItem($oObj->GetKey());
-		}
-
-		foreach ($aProfilesBeforeUserCreation as $sProfileName) {
-			$oAdminUrpProfile = new URP_UserProfile();
-			$oProfile = $aProfiles[$sProfileName];
-			$oAdminUrpProfile->Set('profileid', $oProfile->GetKey());
-			$oAdminUrpProfile->Set('reason', 'UNIT Tests');
-			$oProfileList->AddItem($oAdminUrpProfile);
-		}
-
-		$oUserToUpdate->Set('profile_list', $oProfileList);
-		$oUserToUpdate->DBWrite();
-
-		$this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aProfilesBeforeUserCreation);
-	}
-
-	private function CreateUserForProfileTesting(User $oUserToCreate, array $aProfilesBeforeUserCreation, $bDbInsert = true): array
-	{
-		$aProfiles = [];
-		$oSearch = DBSearch::FromOQL('SELECT URP_Profiles');
-		$oProfileSet = new DBObjectSet($oSearch);
-		while (($oProfile = $oProfileSet->Fetch()) != null) {
-			$aProfiles[$oProfile->Get('name')] = $oProfile;
-		}
-
-		$this->CreateTestOrganization();
-		$oContact = $this->CreatePerson('1');
-		$iContactId = $oContact->GetKey();
-
-		$oUserToCreate->Set('contactid', $iContactId);
-
-		$oUserProfileList = $oUserToCreate->Get('profile_list');
-		foreach ($aProfilesBeforeUserCreation as $sProfileName) {
-			$oUserProfile = new URP_UserProfile();
-			$oProfile = $aProfiles[$sProfileName];
-			$oUserProfile->Set('profileid', $oProfile->GetKey());
-			$oUserProfile->Set('reason', 'UNIT Tests');
-			$oUserProfileList->AddItem($oUserProfile);
-		}
-
-		$oUserToCreate->Set('profile_list', $oUserProfileList);
-		if ($bDbInsert) {
-			$sId = $oUserToCreate->DBInsert();
-		} else {
-			$sId = -1;
-		}
-
-		return [$sId, $aProfiles];
-	}
-
-	private function CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedAssociatedProfilesAfterUserCreation, $bTestItopConnection = true)
-	{
-		$oUser = MetaModel::GetObject($sUserClass, $sId);
-		$oUserProfileList = $oUser->Get('profile_list');
-		$aProfilesAfterCreation = [];
-		while (($oProfile = $oUserProfileList->Fetch()) != null) {
-			$aProfilesAfterCreation[] = $oProfile->Get('profile');
-		}
-
-		foreach ($aExpectedAssociatedProfilesAfterUserCreation as $sExpectedProfileName) {
-			$this->assertTrue(in_array($sExpectedProfileName, $aProfilesAfterCreation),
-				"profile \'$sExpectedProfileName\' should be asociated to user after creation. ".var_export($aProfilesAfterCreation, true));
-		}
-
-		if (!$bTestItopConnection) {
-			return;
-		}
-
-		$_SESSION = [];
-
-		UserRights::Login($oUser->Get('login'));
-
-		if (!UserRights::IsPortalUser()) {
-			//calling this API triggers Fatal Error on below OQL used by \User->GetContactObject() for a user with only 'portal power user' profile
-			/**
-			 * Error: No result for the single row query: 'SELECT DISTINCT `Contact`.`id` AS `Contactid`, `Contact`.`name` AS `Contactname`, `Contact`.`status` AS `Contactstatus`, `Contact`.`org_id` AS `Contactorg_id`, `Organization_org_id`.`name` AS `Contactorg_name`, `Contact`.`email` AS `Contactemail`, `Contact`.`phone` AS `Contactphone`, `Contact`.`notify` AS `Contactnotify`, `Contact`.`function` AS `Contactfunction`, `Contact`.`finalclass` AS `Contactfinalclass`, IF((`Contact`.`finalclass` IN ('Team', 'Contact')), CAST(CONCAT(COALESCE(`Contact`.`name`, '')) AS CHAR), CAST(CONCAT(COALESCE(`Contact_poly_Person`.`first_name`, ''), COALESCE(' ', ''), COALESCE(`Contact`.`name`, '')) AS CHAR)) AS `Contactfriendlyname`, COALESCE((`Contact`.`status` = 'inactive'), 0) AS `Contactobsolescence_flag`, `Contact`.`obsolescence_date` AS `Contactobsolescence_date`, CAST(CONCAT(COALESCE(`Organization_org_id`.`name`, '')) AS CHAR) AS `Contactorg_id_friendlyname`, COALESCE((`Organization_org_id`.`status` = 'inactive'), 0) AS `Contactorg_id_obsolescence_flag` FROM `contact` AS `Contact` INNER JOIN `organization` AS `Organization_org_id` ON `Contact`.`org_id` = `Organization_org_id`.`id` LEFT JOIN `person` AS `Contact_poly_Person` ON `Contact`.`id` = `Contact_poly_Person`.`id` WHERE ((`Contact`.`id` = 40) AND 0) '.
-			 */
-			NavigationMenuFactory::MakeStandard();
-		}
-
-		$this->assertTrue(true, 'after fix N°5324 no exception raised');
-		// logout
-		$_SESSION = [];
-	}
-
-	/**
-	 * @dataProvider ProfilesLinksProvider
-	 */
-	public function testProfilesLinksDBDelete(string $sProfileNameToRemove, $bRaiseException = false)
-	{
-		$aInitialProfiles = [$sProfileNameToRemove, 'Portal power user'];
-
-		$oUser = new UserExternal();
-		$sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin);
-
-		[$sId, $aProfiles] = $this->CreateUserForProfileTesting($oUser, $aInitialProfiles);
-
-		if ($bRaiseException) {
-			$this->expectException(DeleteException::class);
-		}
-
-		$aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId);
-		if (array_key_exists($sProfileNameToRemove, $aURPUserProfileByUser)) {
-			$oURPUserProfile = $aURPUserProfileByUser[$sProfileNameToRemove];
-			$oURPUserProfile->DBDelete();
-		}
-	}
-
-	/**
-	 * @dataProvider ProfilesLinksProvider
-	 */
-	public function testProfilesLinksEdit_ChangeProfileId(string $sInitialProfile, $bRaiseException = false)
-	{
-		$oUser = new UserExternal();
-		$sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin);
-
-		$sUserClass = get_class($oUser);
-		list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, [$sInitialProfile]);
-
-		$oURP_Profile = MetaModel::GetObjectByColumn('URP_Profiles', 'name', 'Portal power user');
-
-		$aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId);
-
-		if ($bRaiseException) {
-			$this->expectException(CoreCannotSaveObjectException::class);
-		}
-
-		if (array_key_exists($sInitialProfile, $aURPUserProfileByUser)) {
-			$oURPUserProfile = $aURPUserProfileByUser[$sInitialProfile];
-			$oURPUserProfile->Set('profileid', $oURP_Profile->GetKey());
-			$oURPUserProfile->DBWrite();
-		}
-
-		if (!$bRaiseException) {
-			$aExpectedProfilesAfterUpdate = ['Portal power user', 'Portal user'];
-			$this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedProfilesAfterUpdate);
-		}
-	}
-
-	public function ProfilesLinksProvider()
-	{
-		return [
-			'Administrator' => ['sProfileNameToMove' => 'Administrator', 'bRaiseException' => true],
-			'Portal user'   => ['sProfileNameToMove' => 'Portal user', 'bRaiseException' => true],
-		];
-	}
-
-	/**
-	 * @dataProvider ProfilesLinksProvider
-	 */
-	public function testProfilesLinksEdit_ChangeUserId($sProfileNameToMove, $bRaiseException = false)
-	{
-		$aInitialProfiles = [$sProfileNameToMove, 'Portal power user'];
-
-		$oUser = new UserExternal();
-		$sLogin1 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin1);
-
-		$sUserClass = get_class($oUser);
-		list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, $aInitialProfiles);
-
-		$oUser = new UserExternal();
-		$sLogin2 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid('', true);
-		$oUser->Set('login', $sLogin2);
-		list ($sAnotherUserId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, ['Configuration Manager']);
-
-		if ($bRaiseException) {
-			$this->expectException(CoreCannotSaveObjectException::class);
-		}
-
-		$aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId);
-		if (array_key_exists($sProfileNameToMove, $aURPUserProfileByUser)) {
-			$oURPUserProfile = $aURPUserProfileByUser[$sProfileNameToMove];
-			$oURPUserProfile->Set('userid', $sAnotherUserId);
-			$oURPUserProfile->DBWrite();
-		}
-
-		if (!$bRaiseException) {
-			$aExpectedProfilesAfterUpdate = [$sProfileNameToMove, 'Configuration Manager'];
-			$this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sAnotherUserId, $aExpectedProfilesAfterUpdate);
-
-			$aExpectedProfilesAfterUpdate = ['Portal power user', 'Portal user'];
-			$this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedProfilesAfterUpdate);
-		}
-	}
-
-	private function GetURPUserProfileByUser($iUserId): array
-	{
-		$aRes = [];
-		$oSearch = DBSearch::FromOQL("SELECT URP_UserProfile WHERE userid=$iUserId");
-		$oSet = new DBObjectSet($oSearch);
-		while (($oURPUserProfile = $oSet->Fetch()) != null) {
-			$aRes[$oURPUserProfile->Get('profile')] = $oURPUserProfile;
-		}
-
-		return $aRes;
-	}
-
-	public function CustomizedPortalsProvider()
-	{
-		return [
-			'console + customized portal'               => [
-				'aPortalDispatcherData' => [
-					'customer-portal',
-					'backoffice',
-				],
-			],
-			'console + itop portal + customized portal' => [
-				'aPortalDispatcherData' => [
-					'itop-portal',
-					'customer-portal',
-					'backoffice',
-				],
-			],
-		];
-	}
 }
diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/CustomCheckToWriteTest.php b/tests/php-unit-tests/unitary-tests/core/DBObject/CustomCheckToWriteTest.php
new file mode 100644
index 000000000..93cea9f58
--- /dev/null
+++ b/tests/php-unit-tests/unitary-tests/core/DBObject/CustomCheckToWriteTest.php
@@ -0,0 +1,64 @@
+ [
+				'aProfiles'            => [],
+				'bExpectedCheckStatus' => false,
+			],
+			'Portal power user'                                 => [
+				'aProfiles'            => ['Portal power user',],
+				'bExpectedCheckStatus' => true,
+			],
+			'Portal power user + Configuration Manager'         => [
+				'aProfiles'            => ['Portal power user', 'Configuration Manager',],
+				'bExpectedCheckStatus' => true,
+			],
+			'Portal power user + Configuration Manager + Admin' => [
+				'aProfiles'            => ['Portal power user', 'Configuration Manager', 'Administrator',],
+				'bExpectedCheckStatus' => true,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider PortaPowerUserProvider
+	 * @covers       User::CheckPortalProfiles
+	 */
+	public function testUserLocalCheckPortalProfiles($aProfiles, $bExpectedCheckStatus)
+	{
+		$oUser = new UserLocal();
+		$sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid('', true);
+		$oUser->Set('login', $sLogin);
+		$oUser->Set('password', 'ABCD1234@gabuzomeu');
+		$oUser->Set('language', 'EN US');
+		$oProfileList = $oUser->Get('profile_list');
+
+		foreach ($aProfiles as $sProfileName) {
+			$oAdminUrpProfile = new URP_UserProfile();
+			$oAdminUrpProfile->Set('profileid', self::$aURP_Profiles[$sProfileName]);
+			$oAdminUrpProfile->Set('reason', 'UNIT Tests');
+			$oProfileList->AddItem($oAdminUrpProfile);
+		}
+
+		$oUser->Set('profile_list', $oProfileList);
+
+		[$bCheckStatus, $aCheckIssues, $bSecurityIssue] = $oUser->CheckToWrite();
+		$this->assertEquals($bExpectedCheckStatus, $bCheckStatus);
+	}
+
+}
diff --git a/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php b/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php
index 721d87d52..e60006e22 100644
--- a/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php
+++ b/tests/php-unit-tests/unitary-tests/core/MetaModelTest.php
@@ -33,6 +33,29 @@ class MetaModelTest extends ItopDataTestCase
 		parent::tearDown();
 	}
 
+	/**
+	 * @covers MetaModel::GetObjectByName
+	 * @return void
+	 * @throws \CoreException
+	 */
+	public function testGetFinalClassName()
+	{
+		// Standalone classes
+		$this->assertEquals('Organization', MetaModel::GetFinalClassName('Organization', 1), 'Should work with standalone classes');
+		$this->assertEquals('SynchroDataSource', MetaModel::GetFinalClassName('SynchroDataSource', 1), 'Should work with standalone classes');
+
+		// 2 levels hierarchy
+		$this->assertEquals('Person', MetaModel::GetFinalClassName('Contact', 1));
+		$this->assertEquals('Person', MetaModel::GetFinalClassName('Person', 1));
+
+		// multi-level hierarchy
+		$oServer1 = MetaModel::GetObjectByName('Server', 'Server1');
+		$sServer1Id = $oServer1->GetKey();
+		foreach (MetaModel::EnumParentClasses('Server',ENUM_PARENT_CLASSES_ALL) as $sClass) {
+			$this->assertEquals('Server', MetaModel::GetFinalClassName($sClass, $sServer1Id), 'Should return Server for all the classes in the hierarchy');
+		}
+	}
+
 	/**
      * @group itopRequestMgmt
      * @covers       MetaModel::ApplyParams()