From f44468b7a17139164dbb72f26b1ff98679f4e955 Mon Sep 17 00:00:00 2001 From: Benjamin Dalsass <95754414+bdalsass@users.noreply.github.com> Date: Mon, 24 Mar 2025 14:05:30 +0100 Subject: [PATCH 01/18] =?UTF-8?q?N=C2=B08245=20-=20External=20key=20not=20?= =?UTF-8?q?saved=20in=20n-n=20link=20in=20edition=20(#703)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * N°8245 - External key not saved in n-n link in edition * Remove the listener from the input element itself, put it on the body with a selector that matches our input * Revert "N°8245 - External key not saved in n-n link in edition" This reverts commit a756f9b1598599d6889d725e0e15a2287e8971ea. --------- Co-authored-by: Stephen Abello --- js/links/links_widget.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/js/links/links_widget.js b/js/links/links_widget.js index eaffcae21..1dbdde0fd 100644 --- a/js/links/links_widget.js +++ b/js/links/links_widget.js @@ -390,16 +390,17 @@ function LinksWidget(id, sClass, sAttCode, iInputId, sSuffix, bDuplicates, oWizH this.RegisterChange = function () { // Listen only used inputs - $('#linkedset_'+me.id+' :input[name^="attr_'+me.sAttCode+'["]').off('change').on('change', function () { + $('body').off('change', '#linkedset_'+me.id+' :input[name^="attr_'+me.sAttCode+'["]') + .on('change', '#linkedset_'+me.id+' :input[name^="attr_'+me.sAttCode+'["]', function () { if (!($(this).hasClass('selection'))) - { - let oCheckbox = $(this).closest('tr').find('.selection'); - let iLink = oCheckbox.attr('data-link-id'); - let iUniqueId = oCheckbox.attr('data-unique-id'); - let sAttCode = $(this).closest('.attribute-edit').attr('data-attcode'); - let value = $(this).val();; - return me.OnValueChange(iLink, iUniqueId, sAttCode, value, this); - } + { + let oCheckbox = $(this).closest('tr').find('.selection'); + let iLink = oCheckbox.attr('data-link-id'); + let iUniqueId = oCheckbox.attr('data-unique-id'); + let sAttCode = $(this).closest('.attribute-edit').attr('data-attcode'); + let value = $(this).val();; + return me.OnValueChange(iLink, iUniqueId, sAttCode, value, this); + } return true; }); }; From a4a05a2579c7a71dc909835db258a1924b2a533c Mon Sep 17 00:00:00 2001 From: odain Date: Wed, 12 Mar 2025 11:25:51 +0100 Subject: [PATCH 02/18] =?UTF-8?q?N=C2=B07398=20-=20be=20able=20to=20add=20?= =?UTF-8?q?custom=20data=20in=20session=20files=20generated=20by=20Session?= =?UTF-8?q?Handler?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/config.class.inc.php | 8 + lib/composer/autoload_classmap.php | 3 + lib/composer/autoload_static.php | 3 + sources/SessionTracker/SessionHandler.php | 53 ++++++- .../iSessionHandlerExtension.php | 16 ++ .../SessionTracker/SessionHandlerTest.php | 150 +++++++++++++----- .../iSessionHandlerExtensionExamples.php | 15 ++ 7 files changed, 204 insertions(+), 44 deletions(-) create mode 100644 sources/SessionTracker/iSessionHandlerExtension.php create mode 100644 tests/php-unit-tests/unitary-tests/sources/SessionTracker/iSessionHandlerExtensionExamples.php diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 857dfbaff..66545f161 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1233,6 +1233,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'sessions_tracking.session_handler_extension' => [ + 'type' => 'string', + 'description' => 'to store more data in itop session files, set your own iSessionHandlerExtension implementation class in this variable', + 'default' => '', + '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)', diff --git a/lib/composer/autoload_classmap.php b/lib/composer/autoload_classmap.php index 270f7b0bd..095101caa 100644 --- a/lib/composer/autoload_classmap.php +++ b/lib/composer/autoload_classmap.php @@ -525,6 +525,7 @@ return array( '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', + 'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => $baseDir . '/sources/SessionTracker/iSessionHandlerExtension.php', 'CompileCSSService' => $baseDir . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'Config' => $baseDir . '/core/config.class.inc.php', @@ -1560,6 +1561,7 @@ return array( 'Sabberworm\\CSS\\Value\\URL' => $vendorDir . '/sabberworm/php-css-parser/src/Value/URL.php', 'Sabberworm\\CSS\\Value\\Value' => $vendorDir . '/sabberworm/php-css-parser/src/Value/Value.php', 'Sabberworm\\CSS\\Value\\ValueList' => $vendorDir . '/sabberworm/php-css-parser/src/Value/ValueList.php', + 'SanitizeTrait' => $baseDir . '/core/restservices.class.inc.php', 'ScalarExpression' => $baseDir . '/core/oql/expression.class.inc.php', 'ScalarOqlExpression' => $baseDir . '/core/oql/oqlquery.class.inc.php', 'ScssPhp\\ScssPhp\\Base\\Range' => $vendorDir . '/scssphp/scssphp/src/Base/Range.php', @@ -3201,6 +3203,7 @@ return array( 'iPreferencesExtension' => $baseDir . '/application/applicationextension.inc.php', 'iProcess' => $baseDir . '/core/backgroundprocess.inc.php', 'iQueryModifier' => $baseDir . '/core/querymodifier.class.inc.php', + 'iRestInputSanitizer' => $baseDir . '/application/applicationextension.inc.php', 'iRestServiceProvider' => $baseDir . '/application/applicationextension.inc.php', 'iScheduledProcess' => $baseDir . '/core/backgroundprocess.inc.php', 'iSelfRegister' => $baseDir . '/core/userrights.class.inc.php', diff --git a/lib/composer/autoload_static.php b/lib/composer/autoload_static.php index 9ecf11d57..5acd9e9b7 100644 --- a/lib/composer/autoload_static.php +++ b/lib/composer/autoload_static.php @@ -915,6 +915,7 @@ class ComposerStaticInit7f81b4a2a468a061c306af5e447a9a9f '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', + 'Combodo\\iTop\\SessionTracker\\iSessionHandlerExtension' => __DIR__ . '/../..' . '/sources/SessionTracker/iSessionHandlerExtension.php', 'CompileCSSService' => __DIR__ . '/../..' . '/application/compilecssservice.class.inc.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'Config' => __DIR__ . '/../..' . '/core/config.class.inc.php', @@ -1950,6 +1951,7 @@ class ComposerStaticInit7f81b4a2a468a061c306af5e447a9a9f 'Sabberworm\\CSS\\Value\\URL' => __DIR__ . '/..' . '/sabberworm/php-css-parser/src/Value/URL.php', 'Sabberworm\\CSS\\Value\\Value' => __DIR__ . '/..' . '/sabberworm/php-css-parser/src/Value/Value.php', 'Sabberworm\\CSS\\Value\\ValueList' => __DIR__ . '/..' . '/sabberworm/php-css-parser/src/Value/ValueList.php', + 'SanitizeTrait' => __DIR__ . '/../..' . '/core/restservices.class.inc.php', 'ScalarExpression' => __DIR__ . '/../..' . '/core/oql/expression.class.inc.php', 'ScalarOqlExpression' => __DIR__ . '/../..' . '/core/oql/oqlquery.class.inc.php', 'ScssPhp\\ScssPhp\\Base\\Range' => __DIR__ . '/..' . '/scssphp/scssphp/src/Base/Range.php', @@ -3591,6 +3593,7 @@ class ComposerStaticInit7f81b4a2a468a061c306af5e447a9a9f 'iPreferencesExtension' => __DIR__ . '/../..' . '/application/applicationextension.inc.php', 'iProcess' => __DIR__ . '/../..' . '/core/backgroundprocess.inc.php', 'iQueryModifier' => __DIR__ . '/../..' . '/core/querymodifier.class.inc.php', + 'iRestInputSanitizer' => __DIR__ . '/../..' . '/application/applicationextension.inc.php', 'iRestServiceProvider' => __DIR__ . '/../..' . '/application/applicationextension.inc.php', 'iScheduledProcess' => __DIR__ . '/../..' . '/core/backgroundprocess.inc.php', 'iSelfRegister' => __DIR__ . '/../..' . '/core/userrights.class.inc.php', diff --git a/sources/SessionTracker/SessionHandler.php b/sources/SessionTracker/SessionHandler.php index 5aa0c4072..27891a08f 100644 --- a/sources/SessionTracker/SessionHandler.php +++ b/sources/SessionTracker/SessionHandler.php @@ -139,23 +139,60 @@ class SessionHandler extends \SessionHandler // - Data corruption (not a json / not an array / no previous creation_time key) $iCreationTime = time(); + $aJson=[]; if (! is_null($sPreviousFileVersionContent)) { $aJson = json_decode($sPreviousFileVersionContent, true); - if (is_array($aJson) && array_key_exists('creation_time', $aJson)) { - $iCreationTime = $aJson['creation_time']; + if (is_array($aJson)){ + if (array_key_exists('creation_time', $aJson)) { + $iCreationTime = $aJson['creation_time']; + } + } else { + IssueLog::Warning(__METHOD__ . ': not a json due (session file corruption?)', null, [ 'sPreviousFileVersionContent' => $sPreviousFileVersionContent ]); + $aJson=[]; } } + $aData = [ + 'login_mode' => Session::Get('login_mode'), + 'user_id' => $sUserId, + 'creation_time' => $iCreationTime, + 'context' => implode('|', ContextTag::GetStack()) + ]; + + $oiSessionHandlerExtension = $this->GetSessionHandlerExtension(); + if (! is_null($oiSessionHandlerExtension)){ + $oiSessionHandlerExtension->CompleteSessionData($aJson, $aData); + } + return json_encode ( - [ - 'login_mode' => Session::Get('login_mode'), - 'user_id' => $sUserId, - 'creation_time' => $iCreationTime, - 'context' => implode('|', ContextTag::GetStack()) - ] + $aData ); } catch(Exception $e) { + IssueLog::Error(__METHOD__, null, [ 'error' => $e->getMessage() ]); + } + return null; + } + + private function GetSessionHandlerExtension() : ?iSessionHandlerExtension + { + $sSessionHandlerExtensionClass = utils::GetConfig()->Get('sessions_tracking.session_handler_extension'); + if (strlen($sSessionHandlerExtensionClass) !=0){ + try{ + if (! class_exists($sSessionHandlerExtensionClass)){ + throw new \Exception("Cannot find class"); + } + + /** @var iSessionHandlerExtension $oSessionHandlerExtension */ + $oSessionHandlerExtension = new $sSessionHandlerExtensionClass; + if ($oSessionHandlerExtension instanceof iSessionHandlerExtension){ + return $oSessionHandlerExtension; + } + + throw new \Exception("Not an instance of iSessionHandlerExtension"); + } catch(\Exception $e) { + IssueLog::Error(__METHOD__ . ': cannot instanciate iSessionHandlerExtension', null, ['sessions_tracking.session_handler_extension' => $sSessionHandlerExtensionClass]); + } } return null; diff --git a/sources/SessionTracker/iSessionHandlerExtension.php b/sources/SessionTracker/iSessionHandlerExtension.php new file mode 100644 index 000000000..edd3ef255 --- /dev/null +++ b/sources/SessionTracker/iSessionHandlerExtension.php @@ -0,0 +1,16 @@ +aFiles=[]; + $this->RequireOnceUnitTestFile('./iSessionHandlerExtensionExamples.php'); + $this->aFiles = []; $this->oTag = new ContextTag(ContextTag::TAG_REST); } @@ -24,47 +26,53 @@ class SessionHandlerTest extends ItopDataTestCase parent::tearDown(); $this->oTag = null; - foreach ($this->aFiles as $sFile){ - if (is_file($sFile)){ + foreach ($this->aFiles as $sFile) { + if (is_file($sFile)) { @unlink($sFile); } } } - private function CreateUserAndLogIn() : ? string { + private function CreateUserAndLogIn(): ?string + { $_SESSION = []; - $oUser = $this->CreateContactlessUser("admin" . uniqid(), 1, "1234@Abcdefg"); + $oUser = $this->CreateContactlessUser("admin".uniqid(), 1, "1234@Abcdefg"); \UserRights::Login($oUser->Get('login')); + return $oUser->GetKey(); } - private function GenerateSessionContent(SessionHandler $oSessionHandler, ?string $sPreviousFileVersionContent) : ?string { + 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(){ + 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() { + 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([]) ], + '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 + * @covers SessionHandler::generate_session_content * @dataProvider GenerateSessionContentCorruptedPreviousFileContentProvider */ - public function testGenerateSessionContent_SessionFileRepairment(?string $sFileContent){ + public function testGenerateSessionContent_SessionFileRepairment(?string $sFileContent) + { $sUserId = $this->CreateUserAndLogIn(); $oSessionHandler = new SessionHandler(); @@ -84,7 +92,8 @@ class SessionHandlerTest extends ItopDataTestCase /* * @covers SessionHandler::generate_session_content */ - public function testGenerateSessionContent(){ + public function testGenerateSessionContent() + { $sUserId = $this->CreateUserAndLogIn(); $oSessionHandler = new SessionHandler(); @@ -105,33 +114,36 @@ class SessionHandlerTest extends ItopDataTestCase // Switch context + change user id via impersonation // check it is still tracked in session files - $oOtherUser = $this->CreateContactlessUser("admin" . uniqid(), 1, "1234@Abcdefg"); + $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(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 { + 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(){ + public function testTouchSessionFile_NoUserLoggedIn() + { $oSessionHandler = new SessionHandler(); $session_id = uniqid(); $sFile = $this->touchSessionFile($oSessionHandler, $session_id); @@ -143,7 +155,8 @@ class SessionHandlerTest extends ItopDataTestCase /* * @covers SessionHandler::touch_session_file */ - public function testTouchSessionFile_UserLoggedIn(){ + public function testTouchSessionFile_UserLoggedIn() + { $sUserId = $this->CreateUserAndLogIn(); Session::Set('login_mode', 'foo_login_mode'); @@ -174,7 +187,8 @@ class SessionHandlerTest extends ItopDataTestCase /** * @covers SessionHandler::touch_session_file */ - public function testTouchSessionFileWithEmptySessionId() { + public function testTouchSessionFileWithEmptySessionId() + { $this->CreateUserAndLogIn(); Session::Set('login_mode', 'toto'); @@ -183,32 +197,36 @@ class SessionHandlerTest extends ItopDataTestCase $this->assertNull($this->touchSessionFile($oSessionHandler, false), 'Should return null when session id (boolean) false'); } - private function GetFilePath(SessionHandler $oSessionHandler, $session_id) : string { + 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(){ + public function GgcWithTimeLimitProvider() + { return [ - 'no cleanup time limit' => [ - 'iTimeLimit' => -1, - 'iExpectedProcessed' => 2 + 'no cleanup time limit' => [ + 'iTimeLimit' => -1, + 'iExpectedProcessed' => 2, ], 'cleanup time limit in the pass => first file removed only' => [ - 'iTimeLimit' => time() - 1, - 'iExpectedProcessed' => 1 + 'iTimeLimit' => time() - 1, + 'iExpectedProcessed' => 1, ], ]; } /** - * @covers SessionHandler::gc_with_time_limit - * @covers SessionHandler::list_session_files + * @covers SessionHandler::gc_with_time_limit + * @covers SessionHandler::list_session_files * @dataProvider GgcWithTimeLimitProvider */ - public function testGgcWithTimeLimit($iTimeLimit, $iExpectedProcessed) { + public function testGgcWithTimeLimit($iTimeLimit, $iExpectedProcessed) + { $oSessionHandler = new SessionHandler(); //remove all first $oSessionHandler->gc_with_time_limit(-1); @@ -218,11 +236,11 @@ class SessionHandlerTest extends ItopDataTestCase $iNbExpiredFiles = 2; $iNbFiles = 5; $iExpiredTimeStamp = time() - $max_lifetime - 1; - for($i=0; $i<$iNbFiles; $i++) { + for ($i = 0; $i < $iNbFiles; $i++) { $sFile = $this->GetFilePath($oSessionHandler, uniqid()); file_put_contents($sFile, "fakedata"); - if ($iNbExpiredFiles > 0){ + if ($iNbExpiredFiles > 0) { $iNbExpiredFiles--; touch($sFile, $iExpiredTimeStamp); } @@ -230,7 +248,7 @@ class SessionHandlerTest extends ItopDataTestCase $aFoundSessionFiles = $oSessionHandler->list_session_files(); $this->assertEquals($iNbFiles, sizeof($aFoundSessionFiles), 'list_session_files should reports all files'); - foreach ($aFoundSessionFiles as $sFile){ + foreach ($aFoundSessionFiles as $sFile) { $this->assertTrue(is_file($sFile), 'list_session_files should return a valid file paths, found: '.$sFile); } @@ -238,4 +256,64 @@ class SessionHandlerTest extends ItopDataTestCase $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'); } -} + + public function testGetSessionHandlerExtension_NoExtension() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', ''); + + $oSessionHandler = new SessionHandler(); + $oSessionHandlerExtension = $this->InvokeNonPublicMethod(SessionHandler::class, "GetSessionHandlerExtension", $oSessionHandler); + $this->assertNull($oSessionHandlerExtension, "by default no extension"); + } + + public function testGetSessionHandlerExtension_InvalidClassExtension() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', 'dddf'); + + $oSessionHandler = new SessionHandler(); + $oSessionHandlerExtension = $this->InvokeNonPublicMethod(SessionHandler::class, "GetSessionHandlerExtension", $oSessionHandler); + $this->assertNull($oSessionHandlerExtension); + } + + public function testGetSessionHandlerExtension_InvalidExtension() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', Session::class); + + $oSessionHandler = new SessionHandler(); + $oSessionHandlerExtension = $this->InvokeNonPublicMethod(SessionHandler::class, "GetSessionHandlerExtension", $oSessionHandler); + $this->assertNull($oSessionHandlerExtension); + } + + public function testGetSessionHandlerExtension_OK() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', 'Combodo\iTop\Test\UnitTest\SessionTracker\BasicSessionHandlerExtension'); + + $oSessionHandler = new SessionHandler(); + $oSessionHandlerExtension = $this->InvokeNonPublicMethod(SessionHandler::class, "GetSessionHandlerExtension", $oSessionHandler); + $this->assertNotNull($oSessionHandlerExtension, "by default no extension"); + $this->assertTrue($oSessionHandlerExtension instanceof iSessionHandlerExtension); + $this->assertInstanceOf(BasicSessionHandlerExtension::class, $oSessionHandlerExtension); + } + + public function testGenerateSessionContent_WithAdditionalDataProvidedBySessionHandlerExtension() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', 'Combodo\iTop\Test\UnitTest\SessionTracker\BasicSessionHandlerExtension'); + $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"); + + $this->assertEquals('gabuzomeu', $aJson['shadok'] ?? '', "Should report the current login mode in [shadok]: $sFirstContent"); + } +} \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/sources/SessionTracker/iSessionHandlerExtensionExamples.php b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/iSessionHandlerExtensionExamples.php new file mode 100644 index 000000000..b4c1a1241 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/iSessionHandlerExtensionExamples.php @@ -0,0 +1,15 @@ + Date: Tue, 1 Apr 2025 09:54:12 +0200 Subject: [PATCH 03/18] =?UTF-8?q?N=C2=B07398-log=20level=20with=20invalid?= =?UTF-8?q?=20json?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/SessionTracker/SessionHandler.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sources/SessionTracker/SessionHandler.php b/sources/SessionTracker/SessionHandler.php index 27891a08f..4548eb82e 100644 --- a/sources/SessionTracker/SessionHandler.php +++ b/sources/SessionTracker/SessionHandler.php @@ -147,8 +147,7 @@ class SessionHandler extends \SessionHandler $iCreationTime = $aJson['creation_time']; } } else { - IssueLog::Warning(__METHOD__ . ': not a json due (session file corruption?)', null, [ 'sPreviousFileVersionContent' => $sPreviousFileVersionContent ]); - $aJson=[]; + IssueLog::Debug(__METHOD__ . ': not a json due (session file corruption?)', null, [ 'sPreviousFileVersionContent' => $sPreviousFileVersionContent ]); } } From 9aa13f57d82e38266514a654069963a7170e06d3 Mon Sep 17 00:00:00 2001 From: odain Date: Wed, 21 May 2025 10:12:11 +0200 Subject: [PATCH 04/18] =?UTF-8?q?N=C2=B08413=20-=20Make=20data=20synchro?= =?UTF-8?q?=20work=20on=20DBObject?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- synchro/synchrodatasource.class.inc.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/synchro/synchrodatasource.class.inc.php b/synchro/synchrodatasource.class.inc.php index d711a31cc..56ca84776 100644 --- a/synchro/synchrodatasource.class.inc.php +++ b/synchro/synchrodatasource.class.inc.php @@ -2440,7 +2440,9 @@ class SynchroReplica extends DBObject implements iDisplay // Really modified ? if ($oDestObj->IsModified()) { - $oDestObj::SetCurrentChange($oChange); + if(method_exists(get_class($oDestObj), "SetCurrentChange")){ + $oDestObj::SetCurrentChange($oChange); + } $oDestObj->DBUpdate(); $bModified = true; $oStatLog->AddTrace('Updated object - Values: {'.implode(', ', $aValueTrace).'}', $this); @@ -2499,7 +2501,10 @@ class SynchroReplica extends DBObject implements iDisplay $aValueTrace[] = "$sAttCode: $value"; } } - $oDestObj::SetCurrentChange($oChange); + + if(method_exists(get_class($oDestObj), "SetCurrentChange")){ + $oDestObj::SetCurrentChange($oChange); + } $iNew = $oDestObj->DBInsert(); $this->Set('dest_id', $oDestObj->GetKey()); From 2ee30692ff07472eb961a70c6a0c8418a8432e62 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 22 May 2025 15:02:29 +0200 Subject: [PATCH 05/18] Change deprecated calls --- sources/Application/TwigBase/Controller/Controller.php | 2 +- sources/Application/WebPage/WebPage.php | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sources/Application/TwigBase/Controller/Controller.php b/sources/Application/TwigBase/Controller/Controller.php index 43d85f270..0f1a6a1e1 100644 --- a/sources/Application/TwigBase/Controller/Controller.php +++ b/sources/Application/TwigBase/Controller/Controller.php @@ -773,7 +773,7 @@ abstract class Controller extends AbstractController { // iTop 3.1 and older compatibility, if not an URI we don't know if its relative to app root or module root if (strpos($sLinkedScript, "://") === false) { - $this->m_oPage->add_linked_script($sLinkedScript); + $this->m_oPage->LinkScriptFromAppRoot(utils::LocalPath($sLinkedScript, APPROOT)); return; } diff --git a/sources/Application/WebPage/WebPage.php b/sources/Application/WebPage/WebPage.php index ae1e42ee5..1e2096a72 100644 --- a/sources/Application/WebPage/WebPage.php +++ b/sources/Application/WebPage/WebPage.php @@ -1280,18 +1280,19 @@ JS; protected function AddCompatibilityFiles(string $sFileType, string $sMode): void { $sConstantName = 'COMPATIBILITY_'.strtoupper($sMode).'_LINKED_'. ($sFileType === static::ENUM_COMPATIBILITY_FILE_TYPE_CSS ? 'STYLESHEETS' : 'SCRIPTS') .'_REL_PATH'; - $sMethodName = 'add_linked_'.($sFileType === static::ENUM_COMPATIBILITY_FILE_TYPE_CSS ? 'stylesheet' : 'script'); + $sMethodName = 'Link'.($sFileType === static::ENUM_COMPATIBILITY_FILE_TYPE_CSS ? 'Resource' : 'Script').'FromAppRoot'; + // Add ancestors files foreach (array_reverse(class_parents(static::class)) as $sParentClass) { foreach (constant($sParentClass.'::'.$sConstantName) as $sFile) { - $this->$sMethodName(utils::GetAbsoluteUrlAppRoot().$sFile); + $this->$sMethodName($sFile); } } // Add current class files foreach (constant('static::'.$sConstantName) as $sFile) { - $this->$sMethodName(utils::GetAbsoluteUrlAppRoot().$sFile); + $this->$sMethodName($sFile); } } From cd1c6f5ec5a1caa3b41b5ac3ab3f4861a713a075 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 16 Jun 2025 08:59:43 +0200 Subject: [PATCH 06/18] =?UTF-8?q?N=C2=B08404=20-=20Error=20when=20upgradin?= =?UTF-8?q?g=20composant=20version=20on=20Designer=20(Migration=20status?= =?UTF-8?q?=20on=20licence)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/metamodel.class.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/metamodel.class.php b/core/metamodel.class.php index f0195fc3d..f5f4f3833 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -6801,12 +6801,14 @@ abstract class MetaModel if ($bMustBeFound && empty($aRow)) { $sNotFoundErrorMessage = "No result for the single row query"; - IssueLog::Info($sNotFoundErrorMessage, LogChannels::CMDB_SOURCE, [ + $e = new CoreException($sNotFoundErrorMessage); + IssueLog::Error($sNotFoundErrorMessage, LogChannels::CMDB_SOURCE, [ 'class' => $sClass, 'key' => $iKey, 'sql_query' => $sSQL, + 'stack' => $e->getTraceAsString(), ]); - throw new CoreException($sNotFoundErrorMessage); + throw $e; } return $aRow; From 86d2a3424d0471b29e283819250aebb61907a751 Mon Sep 17 00:00:00 2001 From: odain Date: Wed, 18 Jun 2025 15:45:29 +0200 Subject: [PATCH 07/18] =?UTF-8?q?N=C2=B08306=20-=20Wrong=20line=20number?= =?UTF-8?q?=20error=20if=20XML=20is=20over=2065535=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modelfactory.class.inc.php | 86 ++++++++++++++++---------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 21b49ec58..771d6f2f7 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -749,13 +749,37 @@ class ModelFactory case 'define_if_not_exists': /** @var \MFElement $oParentNode */ $oParentNode = $oSubClassNode->parentNode; - $iLine = ModelFactory::GetXMLLineNumber($oParentNode); - $sItopNodePath = DesignDocument::GetItopNodePath($oParentNode); - throw new MFException("$sItopNodePath at line $iLine: _delta=\"$sParentDeltaSpec\" not supported for classes in hierarchy", - MFException::NOT_FOUND, $iLine, $sItopNodePath); + self::ThrowMFException("_delta=\"$sParentDeltaSpec\" not supported for classes in hierarchy", MFException::NOT_FOUND, $oParentNode); } } + /** + * @covers N°8306: try to print correct XML line number + * @param string $sMsg + * @param $code + * @param $oNode + * @param $sExtraInfo + * @param $sItopNodePath + * + * @return mixed + * @throws \MFException + */ + public static function ThrowMFException(string $sMsg, $code, $oNode, $sExtraInfo='', $sItopNodePath=null, $oParentFallbackNode=null) + { + $iLine = ModelFactory::GetXMLLineNumber($oNode); + if ($iLine==0 && ! is_null($oParentFallbackNode)){ + $iLine = ModelFactory::GetXMLLineNumber($oParentFallbackNode); + } + + if (is_null($sItopNodePath)){ + $sItopNodePath = DesignDocument::GetItopNodePath($oNode); + } + + $oMFException = new MFException("$sItopNodePath at line $iLine: $sMsg", $code, $iLine, $sItopNodePath, $sExtraInfo); + \IssueLog::Debug(__METHOD__, null, [$oMFException->getMessage(), $oMFException->getTraceAsString()]); + throw $oMFException; + } + /** * @param DesignElement $oSourceNode Delta node * @param \MFDocument $oTargetDocument Datamodel @@ -808,10 +832,7 @@ class ModelFactory // Move class after new parent class (before its next sibling) $oNodeForTargetParent = $oTargetDocument->GetNodes("/itop_design/classes/class[@id=\"$sParentClassName\"]")->item(0); if (is_null($oNodeForTargetParent)) { - $iLine = ModelFactory::GetXMLLineNumber($oSourceParentClassNode); - $sItopNodePath = DesignDocument::GetItopNodePath($oSourceParentClassNode); - throw new MFException($sItopNodePath." at line $iLine: invalid parent class '$sParentClassName'", - MFException::NOT_FOUND, $iLine, $sItopNodePath); + self::ThrowMFException("invalid parent class '$sParentClassName'", MFException::NOT_FOUND, $oSourceParentClassNode); } $oNextParentSibling = $oNodeForTargetParent->nextSibling; if ($oNextParentSibling) { @@ -839,20 +860,14 @@ class ModelFactory if (!$oTargetNode || $oTargetNode->IsRemoved()) { // The node does not exist or is marked as removed if ($bMustExist) { - $iLine = ModelFactory::GetXMLLineNumber($oSourceNode); - $sItopNodePath = DesignDocument::GetItopNodePath($oSourceNode); - throw new MFException($sItopNodePath.' at line '.$iLine.': could not be found or marked as removed', - MFException::NOT_FOUND, $iLine, $sItopNodePath); + self::ThrowMFException("could not be found or marked as removed", MFException::NOT_FOUND, $oSourceNode); } if ($bIfExists) { // Do not continue deeper $oTargetNode = null; } else { if (!$bSpecifiedMerge && $sMode === self::LOAD_DELTA_MODE_STRICT && ($sSearchId !== '' || is_null($oSourceNode->GetFirstElementChild()))) { - $iLine = ModelFactory::GetXMLLineNumber($oSourceNode); - $sItopNodePath = DesignDocument::GetItopNodePath($oSourceNode); - throw new MFException($sItopNodePath.' at line '.$iLine.': could not be found or marked as removed (strict mode)', - MFException::NOT_FOUND, $iLine, $sItopNodePath, 'strict mode'); + self::ThrowMFException("could not be found or marked as removed (strict mode)", MFException::NOT_FOUND, $oSourceNode, 'strict mode'); } // Ignore renaming non-existant node @@ -901,10 +916,7 @@ class ModelFactory if (is_null($oSourceNode->GetFirstElementChild()) && $oTargetParentNode instanceof MFElement) { // Leaf node if ($sMode === self::LOAD_DELTA_MODE_STRICT && !$oSourceNode->hasAttribute('_rename_from') && trim($oSourceNode->GetText('')) !== '') { - $iLine = ModelFactory::GetXMLLineNumber($oSourceNode); - $sItopNodePath = DesignDocument::GetItopNodePath($oSourceNode); - throw new MFException($sItopNodePath.' at line '.$iLine.': cannot be modified without _delta flag (strict mode)', - MFException::AMBIGUOUS_LEAF, $iLine, $sItopNodePath, 'strict mode'); + self::ThrowMFException("cannot be modified without _delta flag (strict mode)", MFException::AMBIGUOUS_LEAF, $oSourceNode, 'strict mode'); } else { // Lax mode: same as redefine // Replace the existing node by the given node - copy child nodes as well @@ -912,7 +924,7 @@ class ModelFactory if (trim($oSourceNode->GetText('')) !== '') { $oTargetNode = $oTargetDocument->importNode($oSourceNode, true); $sSearchId = $oSourceNode->hasAttribute('_rename_from') ? $oSourceNode->getAttribute('_rename_from') : $oSourceNode->getAttribute('id'); - $oTargetParentNode->RedefineChildNode($oTargetNode, $sSearchId); + $oTargetParentNode->RedefineChildNode($oTargetNode, $sSearchId, $oSourceNode); } } } else { @@ -956,7 +968,7 @@ class ModelFactory // Replace the existing node by the given node - copy child nodes as well /** @var \MFElement $oTargetNode */ $oTargetNode = $oTargetDocument->importNode($oSourceNode, true); - $oTargetParentNode->RedefineChildNode($oTargetNode, $sSearchId); + $oTargetParentNode->RedefineChildNode($oTargetNode, $sSearchId, $oSourceNode); break; case 'delete_if_exists': @@ -976,25 +988,18 @@ class ModelFactory case 'delete': /** @var \MFElement $oTargetNode */ $oTargetNode = $oTargetParentNode->_FindChildNode($oSourceNode, $sSearchId); - $sPath = MFDocument::GetItopNodePath($oSourceNode); - $iLine = $this->GetXMLLineNumber($oSourceNode); if ($oTargetNode == null) { - throw new MFException($sPath.' at line '.$iLine.": could not be deleted (not found)", MFException::COULD_NOT_BE_DELETED, - $iLine, $sPath); + self::ThrowMFException("could not be deleted (not found)", MFException::COULD_NOT_BE_DELETED, $oSourceNode); } if ($oTargetNode->IsRemoved()) { - throw new MFException($sPath.' at line '.$iLine.": could not be deleted (already marked as deleted)", - MFException::ALREADY_DELETED, $iLine, $sPath); + self::ThrowMFException("could not be deleted (already marked as deleted)", MFException::ALREADY_DELETED, $oSourceNode); } $oTargetNode->Delete(); break; default: - $sPath = MFDocument::GetItopNodePath($oSourceNode); - $iLine = $this->GetXMLLineNumber($oSourceNode); - throw new MFException($sPath.' at line '.$iLine.": unexpected value for attribute _delta: '".$sDeltaSpec."'", - MFException::INVALID_DELTA, $iLine, $sPath, $sDeltaSpec); + self::ThrowMFException("unexpected value for attribute _delta: '".$sDeltaSpec."'", MFException::INVALID_DELTA, $oSourceNode, $sDeltaSpec); } if ($oTargetNode && $oTargetNode->parentNode) { @@ -2201,14 +2206,12 @@ class MFElement extends Combodo\iTop\DesignElement if ($oExisting) { if (!$oExisting->IsRemoved()) { - $sPath = MFDocument::GetItopNodePath($oNode); - $iLine = ModelFactory::GetXMLLineNumber($oNode); $sExistingPath = MFDocument::GetItopNodePath($oExisting).' created_in: ['.$oExisting->getAttribute('_created_in').']'; $iExistingLine = ModelFactory::GetXMLLineNumber($oExisting); $sExceptionMessage = <<ReplaceWithSingleNode($oNode); $sFlag = 'replaced'; @@ -2229,13 +2232,14 @@ EOF; * * @param MFElement $oNode The node (including all subnodes) to set * @param string|null $sSearchId + * @param mixed $oParentFallbackNode: provided to print accurate line number in case $oNode line is 0 * * @return void * * @throws MFException * @throws \Exception */ - public function RedefineChildNode(MFElement $oNode, $sSearchId = null) + public function RedefineChildNode(MFElement $oNode, $sSearchId = null, $oParentFallbackNode=null) { // First: cleanup any flag behind the new node, and eventually add trace data $oNode->ApplyChanges(); @@ -2245,17 +2249,13 @@ EOF; if (!$oExisting) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - $iLine = ModelFactory::GetXMLLineNumber($oNode); - throw new MFException($sPath." at line $iLine: could not be modified (not found)", MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, - $sPath, $iLine); + ModelFactoryEx::ThrowMFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); } $sPrevFlag = $oExisting->GetAlteration(); $sOldId = $oExisting->getAttribute('_old_id'); if ($oExisting->IsRemoved()) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - $iLine = ModelFactory::GetXMLLineNumber($oNode); - throw new MFException($sPath." at line $iLine: could not be modified (marked as deleted)", - MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $sPath, $iLine); + ModelFactoryEx::ThrowMFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); } $oExisting->ReplaceWithSingleNode($oNode); if (!$this->IsInDefinition()) { From 83eb2b81e31d22436ebaba9cdab5386721c5c392 Mon Sep 17 00:00:00 2001 From: odain Date: Wed, 18 Jun 2025 16:25:53 +0200 Subject: [PATCH 08/18] =?UTF-8?q?N=C2=B08306=20:=20fix=20ModelFactoryEx=20?= =?UTF-8?q?class=20not=20found?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modelfactory.class.inc.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 771d6f2f7..0a54fd176 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -2211,7 +2211,7 @@ class MFElement extends Combodo\iTop\DesignElement $sExceptionMessage = <<ReplaceWithSingleNode($oNode); $sFlag = 'replaced'; @@ -2249,13 +2249,13 @@ EOF; if (!$oExisting) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - ModelFactoryEx::ThrowMFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); + ModelFactory::ThrowMFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); } $sPrevFlag = $oExisting->GetAlteration(); $sOldId = $oExisting->getAttribute('_old_id'); if ($oExisting->IsRemoved()) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - ModelFactoryEx::ThrowMFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); + ModelFactory::ThrowMFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); } $oExisting->ReplaceWithSingleNode($oNode); if (!$this->IsInDefinition()) { From 5573a222c82d3497b82c73ca1d2a899603d04848 Mon Sep 17 00:00:00 2001 From: odain Date: Thu, 19 Jun 2025 11:29:48 +0200 Subject: [PATCH 09/18] =?UTF-8?q?N=C2=B08286=20-=20fix=20call=20to=20Compl?= =?UTF-8?q?eteSessionData=20in=20case=20of=20no=20proper=20session=20conte?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/SessionTracker/SessionHandler.php | 2 +- .../sources/SessionTracker/SessionHandlerTest.php | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sources/SessionTracker/SessionHandler.php b/sources/SessionTracker/SessionHandler.php index 4548eb82e..8096e80da 100644 --- a/sources/SessionTracker/SessionHandler.php +++ b/sources/SessionTracker/SessionHandler.php @@ -147,7 +147,7 @@ class SessionHandler extends \SessionHandler $iCreationTime = $aJson['creation_time']; } } else { - IssueLog::Debug(__METHOD__ . ': not a json due (session file corruption?)', null, [ 'sPreviousFileVersionContent' => $sPreviousFileVersionContent ]); + $aJson=[]; } } diff --git a/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php index 5e51cb2a7..44a958c45 100644 --- a/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/SessionTracker/SessionHandlerTest.php @@ -316,4 +316,13 @@ class SessionHandlerTest extends ItopDataTestCase $this->assertEquals('gabuzomeu', $aJson['shadok'] ?? '', "Should report the current login mode in [shadok]: $sFirstContent"); } + + public function testGenerateSessionContentWithPreviousNonArrayContentAndWithAdditionalDataProvidedBySessionHandlerExtension() + { + \utils::GetConfig()->Set('sessions_tracking.session_handler_extension', 'Combodo\iTop\Test\UnitTest\SessionTracker\BasicSessionHandlerExtension'); + $this->CreateUserAndLogIn(); + $oSessionHandler = new SessionHandler(); + $sContent = $this->GenerateSessionContent($oSessionHandler, json_encode(null)); + $this->assertNotNull($sContent, "Call to CompleteSessionData should NOT fail due to Argument #1 (\$aJson) must be of type array, null given"); + } } \ No newline at end of file From 471422b27488ea50ae10a8d980b521386d827def Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 20 Jun 2025 08:08:36 +0200 Subject: [PATCH 10/18] =?UTF-8?q?N=C2=B08306=20:=20enhance=20MFEException?= =?UTF-8?q?=20constructor=20and=20remove=20boilerplate=20throw=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modelfactory.class.inc.php | 78 +++++++++++++++----------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 0a54fd176..4809a2db5 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -69,14 +69,37 @@ class MFException extends Exception * MFException constructor. * * @inheritDoc + * + * @param $message + * @param $code: error code + * @param $oNode: dom node + * @param $sXPath: XML xpath: if provided used in exception message. otherwise computed via $oNode + * @param $sExtraInfo: additional information stored in exception + * @param $oParentFallbackNode: fallback dom node (usually parent). in case $oNode XML line is wrong (set to 0), line number computed/displayed in error message comes from $oParentFallbackNode */ - public function __construct($message = null, $code = null, $iSourceLineNumber = 0, $sXPath = '', $sExtraInfo = '', $previous = null) + public function __construct($message, $code, $oNode, $sXPath = null, $sExtraInfo = '', $oParentFallbackNode=null) { - parent::__construct($message, $code, $previous); + $iSourceLineNumber = ModelFactory::GetXMLLineNumber($oNode); + if ($iSourceLineNumber==0 && ! is_null($oParentFallbackNode)){ + $iSourceLineNumber = ModelFactory::GetXMLLineNumber($oParentFallbackNode); + } + + if (is_null($sXPath)){ + $sXPath = DesignDocument::GetItopNodePath($oNode); + } + $this->iSourceLineNumber = $iSourceLineNumber; $this->iSourceLineOffset = 0; $this->sXPath = $sXPath; $this->sExtraInfo = $sExtraInfo; + parent::__construct("$sXPath at line $iSourceLineNumber: $message", $code); + + $aContext = [ + 'error' => $code, + 'stack' => $this->getTraceAsString(), + 'extra_info' => $sExtraInfo, + ]; + \IssueLog::Error($this->getMessage(), null, $aContext); } /** @@ -749,37 +772,10 @@ class ModelFactory case 'define_if_not_exists': /** @var \MFElement $oParentNode */ $oParentNode = $oSubClassNode->parentNode; - self::ThrowMFException("_delta=\"$sParentDeltaSpec\" not supported for classes in hierarchy", MFException::NOT_FOUND, $oParentNode); + throw new MFException("_delta=\"$sParentDeltaSpec\" not supported for classes in hierarchy", MFException::NOT_FOUND, $oParentNode); } } - /** - * @covers N°8306: try to print correct XML line number - * @param string $sMsg - * @param $code - * @param $oNode - * @param $sExtraInfo - * @param $sItopNodePath - * - * @return mixed - * @throws \MFException - */ - public static function ThrowMFException(string $sMsg, $code, $oNode, $sExtraInfo='', $sItopNodePath=null, $oParentFallbackNode=null) - { - $iLine = ModelFactory::GetXMLLineNumber($oNode); - if ($iLine==0 && ! is_null($oParentFallbackNode)){ - $iLine = ModelFactory::GetXMLLineNumber($oParentFallbackNode); - } - - if (is_null($sItopNodePath)){ - $sItopNodePath = DesignDocument::GetItopNodePath($oNode); - } - - $oMFException = new MFException("$sItopNodePath at line $iLine: $sMsg", $code, $iLine, $sItopNodePath, $sExtraInfo); - \IssueLog::Debug(__METHOD__, null, [$oMFException->getMessage(), $oMFException->getTraceAsString()]); - throw $oMFException; - } - /** * @param DesignElement $oSourceNode Delta node * @param \MFDocument $oTargetDocument Datamodel @@ -832,7 +828,7 @@ class ModelFactory // Move class after new parent class (before its next sibling) $oNodeForTargetParent = $oTargetDocument->GetNodes("/itop_design/classes/class[@id=\"$sParentClassName\"]")->item(0); if (is_null($oNodeForTargetParent)) { - self::ThrowMFException("invalid parent class '$sParentClassName'", MFException::NOT_FOUND, $oSourceParentClassNode); + throw new MFException("invalid parent class '$sParentClassName'", MFException::NOT_FOUND, $oSourceParentClassNode); } $oNextParentSibling = $oNodeForTargetParent->nextSibling; if ($oNextParentSibling) { @@ -860,14 +856,14 @@ class ModelFactory if (!$oTargetNode || $oTargetNode->IsRemoved()) { // The node does not exist or is marked as removed if ($bMustExist) { - self::ThrowMFException("could not be found or marked as removed", MFException::NOT_FOUND, $oSourceNode); + throw new MFException("could not be found or marked as removed", MFException::NOT_FOUND, $oSourceNode); } if ($bIfExists) { // Do not continue deeper $oTargetNode = null; } else { if (!$bSpecifiedMerge && $sMode === self::LOAD_DELTA_MODE_STRICT && ($sSearchId !== '' || is_null($oSourceNode->GetFirstElementChild()))) { - self::ThrowMFException("could not be found or marked as removed (strict mode)", MFException::NOT_FOUND, $oSourceNode, 'strict mode'); + throw new MFException("could not be found or marked as removed (strict mode)", MFException::NOT_FOUND, $oSourceNode, null, 'strict mode'); } // Ignore renaming non-existant node @@ -916,7 +912,7 @@ class ModelFactory if (is_null($oSourceNode->GetFirstElementChild()) && $oTargetParentNode instanceof MFElement) { // Leaf node if ($sMode === self::LOAD_DELTA_MODE_STRICT && !$oSourceNode->hasAttribute('_rename_from') && trim($oSourceNode->GetText('')) !== '') { - self::ThrowMFException("cannot be modified without _delta flag (strict mode)", MFException::AMBIGUOUS_LEAF, $oSourceNode, 'strict mode'); + throw new MFException("cannot be modified without _delta flag (strict mode)", MFException::AMBIGUOUS_LEAF, $oSourceNode, null, 'strict mode'); } else { // Lax mode: same as redefine // Replace the existing node by the given node - copy child nodes as well @@ -990,16 +986,16 @@ class ModelFactory $oTargetNode = $oTargetParentNode->_FindChildNode($oSourceNode, $sSearchId); if ($oTargetNode == null) { - self::ThrowMFException("could not be deleted (not found)", MFException::COULD_NOT_BE_DELETED, $oSourceNode); + throw new MFException("could not be deleted (not found)", MFException::COULD_NOT_BE_DELETED, $oSourceNode); } if ($oTargetNode->IsRemoved()) { - self::ThrowMFException("could not be deleted (already marked as deleted)", MFException::ALREADY_DELETED, $oSourceNode); + throw new MFException("could not be deleted (already marked as deleted)", MFException::ALREADY_DELETED, $oSourceNode); } $oTargetNode->Delete(); break; default: - self::ThrowMFException("unexpected value for attribute _delta: '".$sDeltaSpec."'", MFException::INVALID_DELTA, $oSourceNode, $sDeltaSpec); + throw new MFException("unexpected value for attribute _delta: '".$sDeltaSpec."'", MFException::INVALID_DELTA, $oSourceNode, null, $sDeltaSpec); } if ($oTargetNode && $oTargetNode->parentNode) { @@ -2129,7 +2125,7 @@ class MFElement extends Combodo\iTop\DesignElement */ public function IsInDefinition() { - // Iterate through the parents: reset the flag if any of them has a flag set + // Iterate through the parents: reset the flag if any of them has a flag set for ($oParent = $this; $oParent instanceof MFElement; $oParent = $oParent->parentNode) { if ($oParent->GetAlteration() != '') @@ -2211,7 +2207,7 @@ class MFElement extends Combodo\iTop\DesignElement $sExceptionMessage = <<ReplaceWithSingleNode($oNode); $sFlag = 'replaced'; @@ -2249,13 +2245,13 @@ EOF; if (!$oExisting) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - ModelFactory::ThrowMFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); + throw new MFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); } $sPrevFlag = $oExisting->GetAlteration(); $sOldId = $oExisting->getAttribute('_old_id'); if ($oExisting->IsRemoved()) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - ModelFactory::ThrowMFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); + throw new MFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); } $oExisting->ReplaceWithSingleNode($oNode); if (!$this->IsInDefinition()) { From 031305cfbf9bdcb6097e0cd535fe1ebd4a0ea461 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 7 Jul 2025 13:44:45 +0200 Subject: [PATCH 11/18] Add stack trace to SQL errors Reload object on modify error to display the real unmodified object --- core/cmdbsource.class.inc.php | 4 ++-- sources/Controller/Base/Layout/ObjectController.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 39817aa85..e5e59be82 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -611,12 +611,12 @@ class CMDBSource catch (mysqli_sql_exception $e) { self::LogDeadLock($e, true); - throw new MySQLException('Failed to issue SQL query', array('query' => $sSql, $e)); + throw new MySQLException('Failed to issue SQL query', ['query' => $sSql, $e, 'stack' => $e->getTraceAsString()]); } finally { $oKPI->ComputeStats('Query exec (mySQL)', $sSql); } if ($oResult === false) { - $aContext = array('query' => $sSql); + $aContext = ["\nstack" => (new Exception(''))->getTraceAsString(), "\nquery" => $sSql]; $iMySqlErrorNo = DbConnectionWrapper::GetDbConnection(true)->errno; $aMySqlHasGoneAwayErrorCodes = MySQLHasGoneAwayException::getErrorCodes(); diff --git a/sources/Controller/Base/Layout/ObjectController.php b/sources/Controller/Base/Layout/ObjectController.php index 137d7827a..bcbeb8629 100644 --- a/sources/Controller/Base/Layout/ObjectController.php +++ b/sources/Controller/Base/Layout/ObjectController.php @@ -664,6 +664,7 @@ JS; $aResult['data'] = ['error_message' => $e->getHtmlMessage()]; } else { $oPage->AddHeaderMessage($e->getHtmlMessage(), 'message_error'); + $oObj->Reload(); $oObj->DisplayModifyForm($oPage, array('wizard_container' => true)); // wizard_container: display the wizard border and the title } From 05642cdf845ba2757b22e468a57869390faaa639 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 10 Jul 2025 16:53:36 +0200 Subject: [PATCH 12/18] =?UTF-8?q?N=C2=B08306=20-=20Wrong=20line=20number?= =?UTF-8?q?=20error=20if=20XML=20is=20over=2065535=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .make/release/update.classes.inc.php | 2 +- core/designdocument.class.inc.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.make/release/update.classes.inc.php b/.make/release/update.classes.inc.php index 8f1499439..5b67edad3 100644 --- a/.make/release/update.classes.inc.php +++ b/.make/release/update.classes.inc.php @@ -199,7 +199,7 @@ class DatamodelsXmlFiles extends AbstractGlobFileVersionUpdater libxml_clear_errors(); $oFileXml->formatOutput = true; $oFileXml->preserveWhiteSpace = false; - $oFileXml->loadXML($sFileContent); + $oFileXml->loadXML($sFileContent, LIBXML_BIGLINES); $oFileItopFormat = new iTopDesignFormat($oFileXml); diff --git a/core/designdocument.class.inc.php b/core/designdocument.class.inc.php index 3965c68ec..c4a1e8e34 100644 --- a/core/designdocument.class.inc.php +++ b/core/designdocument.class.inc.php @@ -71,6 +71,11 @@ class DesignDocument extends DOMDocument $this->preserveWhiteSpace = true; // otherwise the formatOutput option would have no effect } + public function loadXML(string $source, int $options = 0) + { + parent::loadXML($source, $options | LIBXML_BIGLINES); + } + /** * Overload of the standard API * From d0f816109bd733325c9b28dc294a51f675050796 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Wed, 16 Jul 2025 13:22:50 +0200 Subject: [PATCH 13/18] =?UTF-8?q?N=C2=B07938=20-=20Cant'=20create=20OnInse?= =?UTF-8?q?rt/OnUpdate/Afterxxx=20methods=20from=20Designer=20with=20iTop?= =?UTF-8?q?=203.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/forms.class.inc.php | 1 + tests/php-unit-tests/legacy-tests/test.class.inc.php | 8 ++++---- tests/php-unit-tests/legacy-tests/testlist.inc.php | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/application/forms.class.inc.php b/application/forms.class.inc.php index 890475e4f..f0bb3830b 100644 --- a/application/forms.class.inc.php +++ b/application/forms.class.inc.php @@ -347,6 +347,7 @@ EOF { $oPage->add('
'.$sIntroduction.'
'); } + $oPage->add('
'); $this->Render($oPage); $oPage->add(''); diff --git a/tests/php-unit-tests/legacy-tests/test.class.inc.php b/tests/php-unit-tests/legacy-tests/test.class.inc.php index fb29aac13..e126d2da0 100644 --- a/tests/php-unit-tests/legacy-tests/test.class.inc.php +++ b/tests/php-unit-tests/legacy-tests/test.class.inc.php @@ -179,14 +179,14 @@ abstract class TestHandler } catch (CoreException $e) { - //$this->ReportError($e->getMessage()); - //$this->ReportError($e->__tostring()); + //$this->ReportErrors($e->getMessage()); + //$this->ReportErrors($e->__tostring()); $this->ReportError($e->getMessage().' - '.$e->getTraceAsHtml()); } catch (Exception $e) { - //$this->ReportError($e->getMessage()); - //$this->ReportError($e->__tostring()); + //$this->ReportErrors($e->getMessage()); + //$this->ReportErrors($e->__tostring()); $this->ReportError('class '.get_class($e).' --- '.$e->getMessage().' - '.$e->getTraceAsString()); } restore_error_handler(); diff --git a/tests/php-unit-tests/legacy-tests/testlist.inc.php b/tests/php-unit-tests/legacy-tests/testlist.inc.php index d6b40cea9..f7f8a4d66 100644 --- a/tests/php-unit-tests/legacy-tests/testlist.inc.php +++ b/tests/php-unit-tests/legacy-tests/testlist.inc.php @@ -397,7 +397,7 @@ class TestMyBizModel extends TestBizModel protected function DoExecute() { -// $this->ReportError("Found two different OQL expression out of the (same?) filter: $sExpr1 != $sExpr2"); +// $this->ReportErrors("Found two different OQL expression out of the (same?) filter: $sExpr1 != $sExpr2"); // $this->ReportSuccess('Found '.$oSet->Count()." objects of class $sClassName"); //$this->test_linksinfo(); //$this->test_list_attributes(); From e5129c9618be9565b301a81102567f2e179e093b Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Wed, 16 Jul 2025 15:22:10 +0200 Subject: [PATCH 14/18] =?UTF-8?q?N=C2=B07938=20-=20tooltip=20error=20style?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- js/property_field.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/property_field.js b/js/property_field.js index 59ea62c68..ac363bfcf 100644 --- a/js/property_field.js +++ b/js/property_field.js @@ -548,6 +548,7 @@ function ValidateWithPattern(sFieldId, bMandatory, sPattern, sFormId, aForbidden if (sMessage) { $('#'+sFieldId).attr('data-tooltip-content', sMessage); + $('#'+sFieldId).attr('data-tooltip-theme', 'error'); CombodoTooltip.InitTooltipFromMarkup($('#'+sFieldId), true); $('#'+sFieldId)[0]._tippy.show(); } From de54676b9fa5730077d585a0d89c92a21a6c8107 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 21 Jul 2025 16:53:13 +0200 Subject: [PATCH 15/18] =?UTF-8?q?:white=5Fcheck=5Fmark:=20N=C2=B07938=20-?= =?UTF-8?q?=20Cant'=20create=20OnInsert/OnUpdate/Afterxxx=20methods=20from?= =?UTF-8?q?=20Designer=20with=20iTop=203.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modelfactory.class.inc.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 4809a2db5..ea0211441 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -219,6 +219,10 @@ class MFModule return; } + if (!is_dir($sRootDir)) { + $sRootDir = APPROOT.$sRootDir; + } + // Scan the module's root directory to find the datamodel(*).xml files if ($hDir = opendir($sRootDir)) { From 88e0f171644fc961ca8685adf737a22045d0fe6c Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Tue, 26 Aug 2025 11:56:44 +0200 Subject: [PATCH 16/18] =?UTF-8?q?N=C2=B08306=20-=20Wrong=20line=20number?= =?UTF-8?q?=20error=20if=20XML=20is=20over=2065535=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/designdocument.class.inc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/designdocument.class.inc.php b/core/designdocument.class.inc.php index c4a1e8e34..5c792024a 100644 --- a/core/designdocument.class.inc.php +++ b/core/designdocument.class.inc.php @@ -73,7 +73,7 @@ class DesignDocument extends DOMDocument public function loadXML(string $source, int $options = 0) { - parent::loadXML($source, $options | LIBXML_BIGLINES); + return parent::loadXML($source, $options | LIBXML_BIGLINES); } /** From 5045ec4afaff5a602d6a6652fb28196ce4782871 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Thu, 25 Sep 2025 09:43:19 +0200 Subject: [PATCH 17/18] =?UTF-8?q?N=C2=B07722=20-=20Fatal=20error=20on=20xm?= =?UTF-8?q?l=20injection=20if=20xml=20entry=20"redefine"=20doesn't=20exist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modelfactory.class.inc.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index ea0211441..f515efc10 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -2249,13 +2249,13 @@ EOF; if (!$oExisting) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - throw new MFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, '', $sPath, $oParentFallbackNode); + throw new MFException('could not be modified (not found)', MFException::COULD_NOT_BE_MODIFIED_NOT_FOUND, $oNode, $sPath, $oParentFallbackNode); } $sPrevFlag = $oExisting->GetAlteration(); $sOldId = $oExisting->getAttribute('_old_id'); if ($oExisting->IsRemoved()) { $sPath = MFDocument::GetItopNodePath($this)."/".$oNode->tagName.(empty($sSearchId) ? '' : "[$sSearchId]"); - throw new MFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, '', $sPath, $oParentFallbackNode); + throw new MFException('could not be modified (marked as deleted)', MFException::COULD_NOT_BE_MODIFIED_ALREADY_DELETED, $oNode, $sPath, $oParentFallbackNode); } $oExisting->ReplaceWithSingleNode($oNode); if (!$this->IsInDefinition()) { From b5c79e1d75ffa6b1dc3a3e714a467939070dff66 Mon Sep 17 00:00:00 2001 From: odain Date: Thu, 13 Nov 2025 11:13:33 +0100 Subject: [PATCH 18/18] =?UTF-8?q?N=C2=B08912=20-=20Cannot=20work=20on=20a?= =?UTF-8?q?=20licence=20with=20legacy=20extensions=20including=20modules?= =?UTF-8?q?=20without=20explicit=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modulediscovery.class.inc.php | 26 ++++------ .../modulediscovery/ModuleDiscoveryTest.php | 51 +++++++++++++++++++ 2 files changed, 60 insertions(+), 17 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/modulediscovery/ModuleDiscoveryTest.php diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 81482c678..d50758d40 100644 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -120,10 +120,6 @@ class ModuleDiscovery $aArgs['module_file'] = $sFilePath; list($sModuleName, $sModuleVersion) = static::GetModuleName($sId); - if ($sModuleVersion == '') - { - $sModuleVersion = '1.0.0'; - } if (array_key_exists($sModuleName, self::$m_aModuleVersionByName)) { @@ -233,7 +229,7 @@ class ModuleDiscovery } ksort($aDependencies); $aOrderedModules = []; - $iLoopCount = 1; + $iLoopCount = 0; while(($iLoopCount < count($aModules)) && (count($aDependencies) > 0) ) { foreach($aDependencies as $sId => $aRemainingDeps) @@ -308,16 +304,8 @@ class ModuleDiscovery // Separate the module names from their version for an easier comparison later foreach($aOrderedModules as $sModuleId) { - $aMatches = array(); - if (preg_match('|^([^/]+)/(.*)$|', $sModuleId, $aMatches)) - { - $aModuleVersions[$aMatches[1]] = $aMatches[2]; - } - else - { - // No version number found, assume 1.0.0 - $aModuleVersions[$sModuleId] = '1.0.0'; - } + list($sModuleName, $sVersion) = self::GetModuleName($sModuleId); + $aModuleVersions[$sModuleName] = $sVersion; } if (preg_match_all('/([^\(\)&| ]+)/', $sDepString, $aMatches)) { @@ -456,13 +444,17 @@ class ModuleDiscovery { $sName = $aMatches[1]; $sVersion = $aMatches[2]; + if ($sVersion === ""){ + $sVersion = "1.0.0"; + } } else { $sName = $sModuleId; - $sVersion = ""; + $sVersion = "1.0.0"; } - return array($sName, $sVersion); + + return [$sName, $sVersion]; } /** diff --git a/tests/php-unit-tests/unitary-tests/modulediscovery/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/modulediscovery/ModuleDiscoveryTest.php new file mode 100644 index 000000000..ec573f84c --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/modulediscovery/ModuleDiscoveryTest.php @@ -0,0 +1,51 @@ + [ + 'sModuleId' => 'a/1.2.3', + 'name' => 'a', + 'version' => '1.2.3', + ], + 'develop' => [ + 'sModuleId' => 'a/1.2.3-dev', + 'name' => 'a', + 'version' => '1.2.3-dev', + ], + 'missing version => 1.0.0' => [ + 'sModuleId' => 'a/', + 'name' => 'a', + 'version' => '1.0.0', + ], + 'missing everything except name' => [ + 'sModuleId' => 'a', + 'name' => 'a', + 'version' => '1.0.0', + ], + ]; + } + + protected function setUp(): void + { + parent::setUp(); + + $this->RequireOnceItopFile('setup/modulediscovery.class.inc.php'); + } + + /** + * @dataProvider GetModuleNameProvider + */ + public function testGetModuleName($sModuleId, $expectedName, $expectedVersion) + { + $this->assertEquals([$expectedName, $expectedVersion], \ModuleDiscovery::GetModuleName($sModuleId)); + } + +}