diff --git a/.doc/contributing-guide/2023.contributing-stickers-side-by-side.png b/.doc/contributing-guide/2023.contributing-stickers-side-by-side.png new file mode 100644 index 0000000000..e477510ba6 Binary files /dev/null and b/.doc/contributing-guide/2023.contributing-stickers-side-by-side.png differ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 187069b62b..18b8c3dae0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -161,4 +161,4 @@ We have one sticker per contribution type. You might get multiple stickers with Here is the design of each stickers for year 2022: -![iTop stickers 2022](.doc/contributing-guide/2022.contributing-stickers-side-by-side.png) +![iTop stickers 2023](.doc/contributing-guide/2023.contributing-stickers-side-by-side.png) diff --git a/application/dashboard.class.inc.php b/application/dashboard.class.inc.php index 8bc91c7217..872d52f374 100644 --- a/application/dashboard.class.inc.php +++ b/application/dashboard.class.inc.php @@ -918,7 +918,7 @@ class RuntimeDashboard extends Dashboard { $bCustomized = false; - $sDashboardFileSanitized = utils::RealPath($sDashboardFile, APPROOT); + $sDashboardFileSanitized = utils::RealPath(APPROOT.$sDashboardFile, APPROOT); if (false === $sDashboardFileSanitized) { throw new SecurityException('Invalid dashboard file !'); } @@ -1141,7 +1141,7 @@ JS $oToolbar->AddSubBlock($oActionButton); $aActions = array(); - $sFile = addslashes($this->sDefinitionFile); + $sFile = addslashes(utils::LocalPath($this->sDefinitionFile)); $sJSExtraParams = json_encode($aExtraParams); if ($this->HasCustomDashboard()) { $oEdit = new JSPopupMenuItem('UI:Dashboard:Edit', Dict::S('UI:Dashboard:EditCustom'), "return EditDashboard('{$this->sId}', '$sFile', $sJSExtraParams)"); diff --git a/application/ui.extkeywidget.class.inc.php b/application/ui.extkeywidget.class.inc.php index 82e53ba80b..cd0f01e832 100644 --- a/application/ui.extkeywidget.class.inc.php +++ b/application/ui.extkeywidget.class.inc.php @@ -163,7 +163,7 @@ class UIExtKeyWidget $oPage->add_linked_script('../js/extkeywidget.js'); $oPage->add_linked_script('../js/forms-json-utils.js'); - $bCreate = (!$this->bSearchMode) && (UserRights::IsActionAllowed($this->sTargetClass, UR_ACTION_BULK_MODIFY) && $bAllowTargetCreation); + $bCreate = (!$this->bSearchMode) && (UserRights::IsActionAllowed($this->sTargetClass, UR_ACTION_MODIFY) && $bAllowTargetCreation); $bExtensions = true; $sMessage = Dict::S('UI:Message:EmptyList:UseSearchForm'); $sAttrFieldPrefix = ($this->bSearchMode) ? '' : 'attr_'; diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 9764d2598d..493c98c5fd 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -6,7 +6,9 @@ use Combodo\iTop\Core\MetaModel\FriendlyNameType; use Combodo\iTop\Service\Events\EventData; +use Combodo\iTop\Service\Events\EventException; use Combodo\iTop\Service\Events\EventService; +use Combodo\iTop\Service\Events\EventServiceLog; use Combodo\iTop\Service\TemporaryObjects\TemporaryObjectManager; /** @@ -203,6 +205,8 @@ abstract class DBObject implements iDisplay const MAX_UPDATE_LOOP_COUNT = 10; + private $aEventListeners = []; + /** * DBObject constructor. * @@ -255,6 +259,10 @@ abstract class DBObject implements iDisplay $this->RegisterEventListeners(); } + /** + * @see RegisterCRUDListener + * @see EventService::RegisterListener() + */ protected function RegisterEventListeners() { } @@ -6189,6 +6197,51 @@ abstract class DBObject implements iDisplay return OPT_ATT_NORMAL; } + public final function GetListeners(): array + { + $aListeners = []; + foreach ($this->aEventListeners as $aEventListener) { + $aListeners = array_merge($aListeners, $aEventListener); + } + return $aListeners; + } + + /** + * Register a callback for a specific event. The method to call will be saved in the object instance itself whereas calling {@see EventService::RegisterListener()} would + * save a callable (thus the method name AND the whole DBObject instance) + * + * @param string $sEvent corresponding event + * @param string $callback The callback method to call + * @param float $fPriority optional priority for callback order + * @param string $sModuleId + * + * @see EventService::RegisterListener() + * + * @since 3.1.0-3 3.1.1 3.2.0 N°6716 + */ + final protected function RegisterCRUDListener(string $sEvent, string $callback, float $fPriority = 0.0, string $sModuleId = '') + { + $aEventCallbacks = $this->aEventListeners[$sEvent] ?? []; + + $aEventCallbacks[] = array( + 'event' => $sEvent, + 'callback' => $callback, + 'priority' => $fPriority, + 'module' => $sModuleId, + ); + usort($aEventCallbacks, function ($a, $b) { + $fPriorityA = $a['priority']; + $fPriorityB = $b['priority']; + if ($fPriorityA == $fPriorityB) { + return 0; + } + + return ($fPriorityA < $fPriorityB) ? -1 : 1; + }); + + $this->aEventListeners[$sEvent] = $aEventCallbacks; + } + /** * @param string $sEvent * @param array $aEventData @@ -6200,15 +6253,53 @@ abstract class DBObject implements iDisplay */ public function FireEvent(string $sEvent, array $aEventData = array()): void { - if (EventService::IsEventRegistered($sEvent)) { - $aEventData['debug_info'] = 'from: '.get_class($this).':'.$this->GetKey(); - $aEventData['object'] = $this; - $aEventSources = [$this->m_sObjectUniqId]; - foreach (MetaModel::EnumParentClasses(get_class($this), ENUM_PARENT_CLASSES_ALL, false) as $sClass) { - $aEventSources[] = $sClass; + $aEventData['debug_info'] = 'from: '.get_class($this).':'.$this->GetKey(); + $aEventData['object'] = $this; + + // Call local listeners first + $aEventCallbacks = $this->aEventListeners[$sEvent] ?? []; + $oFirstException = null; + $sFirstExceptionMessage = ''; + foreach ($aEventCallbacks as $aEventCallback) { + $oKPI = new ExecutionKPI(); + $sCallback = $aEventCallback['callback']; + if (!method_exists($this, $sCallback)) { + EventServiceLog::Error("Callback '".get_class($this).":$sCallback' does not exist"); + continue; + } + EventServiceLog::Debug("Fire event '$sEvent' calling '".get_class($this).":$sCallback'"); + try { + call_user_func([$this, $sCallback], new EventData($sEvent, null, $aEventData)); + } + catch (EventException $e) { + EventServiceLog::Error("Event '$sEvent' for '$sCallback'} failed with blocking error: ".$e->getMessage()); + throw $e; + } + catch (Exception $e) { + $sMessage = "Event '$sEvent' for '$sCallback'} failed with non-blocking error: ".$e->getMessage(); + EventServiceLog::Error($sMessage); + if (is_null($oFirstException)) { + $sFirstExceptionMessage = $sMessage; + $oFirstException = $e; + } + } + finally { + $oKPI->ComputeStats('FireEvent', $sEvent); } - EventService::FireEvent(new EventData($sEvent, $aEventSources, $aEventData)); } + if (!is_null($oFirstException)) { + throw new Exception($sFirstExceptionMessage, $oFirstException->getCode(), $oFirstException); + } + + // Call global event listeners + if (!EventService::IsEventRegistered($sEvent)) { + return; + } + $aEventSources = []; + foreach (MetaModel::EnumParentClasses(get_class($this), ENUM_PARENT_CLASSES_ALL, false) as $sClass) { + $aEventSources[] = $sClass; + } + EventService::FireEvent(new EventData($sEvent, $aEventSources, $aEventData)); } ////////////////// diff --git a/datamodels/2.x/itop-core-update/dictionaries/hu.dict.itop-core-update.php b/datamodels/2.x/itop-core-update/dictionaries/hu.dict.itop-core-update.php index 0cb9515c4d..22f0114730 100644 --- a/datamodels/2.x/itop-core-update/dictionaries/hu.dict.itop-core-update.php +++ b/datamodels/2.x/itop-core-update/dictionaries/hu.dict.itop-core-update.php @@ -88,7 +88,7 @@ Dict::Add('HU HU', 'Hungarian', 'Magyar', array( // Errors 'iTopUpdate:Error:MissingFunction' => 'Lehetetlen elindítani a frissítést, hiányzó funkció', - 'iTopUpdate:Error:MissingFile' => 'Hiányzó fájl: %1$', + 'iTopUpdate:Error:MissingFile' => 'Hiányzó fájl: %1$s', 'iTopUpdate:Error:CorruptedFile' => 'A %1$s fájl sérült', 'iTopUpdate:Error:BadFileFormat' => 'A frissítési fájl nem zip fájl', 'iTopUpdate:Error:BadFileContent' => 'A frissítési fájl nem alkalmazás archívum', diff --git a/js/dashboard.js b/js/dashboard.js index 6441ed3179..2699e189e9 100644 --- a/js/dashboard.js +++ b/js/dashboard.js @@ -332,11 +332,14 @@ $(function() oParams.dashlet_class = sDashletClass; oParams.dashlet_id = sDashletId; oParams.dashlet_type = options.dashlet_type; + oParams.ajax_promise_id = 'ajax_promise_' + sDashletId; var me = this; $.post(this.options.render_to, oParams, function (data) { me.ajax_div.html(data); - me.add_dashlet_finalize(options, sDashletId, sDashletClass); - me.mark_as_modified(); + window[oParams.ajax_promise_id].then(function(){ + me.add_dashlet_finalize(options, sDashletId, sDashletClass); + me.mark_as_modified(); + }); }); }, on_dashlet_moved: function (oDashlet, oReceiver, bRefresh) { diff --git a/pages/schema.php b/pages/schema.php index 1705531882..a12fe92eed 100644 --- a/pages/schema.php +++ b/pages/schema.php @@ -290,7 +290,6 @@ function DisplayEvents(WebPage $oPage, $sClass) foreach (MetaModel::EnumChildClasses($sClass, ENUM_CHILD_CLASSES_ALL) as $sChildClass) { if (!MetaModel::IsAbstract($sChildClass)) { $oObject = MetaModel::NewObject($sChildClass); - $aSources[] = $oObject->GetObjectUniqId(); break; } } @@ -299,7 +298,6 @@ function DisplayEvents(WebPage $oPage, $sClass) } } else { $oObject = MetaModel::NewObject($sClass); - $aSources[] = $oObject->GetObjectUniqId(); foreach (MetaModel::EnumParentClasses($sClass, ENUM_PARENT_CLASSES_ALL, false) as $sParentClass) { $aSources[] = $sParentClass; } @@ -320,12 +318,19 @@ function DisplayEvents(WebPage $oPage, $sClass) }); $aColumns = [ 'event' => ['label' => Dict::S('UI:Schema:Events:Event')], - 'listener' => ['label' => Dict::S('UI:Schema:Events:Listener')], + 'callback' => ['label' => Dict::S('UI:Schema:Events:Listener')], 'priority' => ['label' => Dict::S('UI:Schema:Events:Rank')], 'module' => ['label' => Dict::S('UI:Schema:Events:Module')], ]; + // Get the object listeners first $aRows = []; $oReflectionClass = new ReflectionClass($sClass); + if ($oReflectionClass->isInstantiable()) { + /** @var DBObject $oClass */ + $oClass = new $sClass(); + $aRows = $oClass->GetListeners(); + } + foreach ($aListeners as $aListener) { if (is_object($aListener['callback'][0])) { $sListenerClass = $sClass; @@ -343,7 +348,7 @@ function DisplayEvents(WebPage $oPage, $sClass) } $aRows[] = [ 'event' => $aListener['event'], - 'listener' => $sListener, + 'callback' => $sListener, 'priority' => $aListener['priority'], 'module' => $aListener['module'], ]; diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index c0e20d619b..ee57fa4d7a 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -1385,17 +1385,12 @@ EOF } $sMethods .= "\n $sCallbackFct\n\n"; } - if (strpos($sCallback, '::') === false) { - $sEventListener = '[$this, \''.$sCallback.'\']'; - } else { - $sEventListener = "'$sCallback'"; - } $sListenerRank = (float)($oListener->GetChildText('rank', '0')); $sEvents .= <<m_sObjectUniqId, [], null, $sListenerRank, '$sModuleRelativeDir'); + \$this->RegisterCRUDListener("$sEventName", '$sCallback', $sListenerRank, '$sModuleRelativeDir'); PHP; } } diff --git a/sources/Service/Events/EventService.php b/sources/Service/Events/EventService.php index 197f05de4d..9f3269084d 100644 --- a/sources/Service/Events/EventService.php +++ b/sources/Service/Events/EventService.php @@ -10,6 +10,7 @@ use Closure; use Combodo\iTop\Service\Events\Description\EventDescription; use ContextTag; use CoreException; +use DBObject; use Exception; use ExecutionKPI; use ReflectionClass; @@ -53,6 +54,12 @@ final class EventService /** * Register a callback for a specific event * + * **Warning** : be ultra careful on memory footprint ! each callback will be saved in {@see aEventListeners}, and a callback is + * made of the whole object instance and the method name ({@link https://www.php.net/manual/en/language.types.callable.php}). + * For example to register on DBObject instances, you should better use {@see DBObject::RegisterCRUDListener()} + * + * @uses aEventListeners + * * @api * @param string $sEvent corresponding event * @param callable $callback The callback to call @@ -61,8 +68,12 @@ final class EventService * @param array|string|null $context context filter * @param float $fPriority optional priority for callback order * - * @return string Id of the registration + * @return string registration identifier * + * @see DBObject::RegisterCRUDListener() to register in DBObject instances instead, to reduce memory footprint (callback saving) + * + * @since 3.1.0 method creation + * @since 3.1.0-3 3.1.1 3.2.0 N°6716 PHPDoc change to warn on memory footprint, and {@see DBObject::RegisterCRUDListener()} alternative */ public static function RegisterListener(string $sEvent, callable $callback, $sEventSource = null, array $aCallbackData = [], $context = null, float $fPriority = 0.0, $sModuleId = ''): string { diff --git a/tests/php-unit-tests/integration-tests/DictionariesConsistencyAfterSetupTest.php b/tests/php-unit-tests/integration-tests/DictionariesConsistencyAfterSetupTest.php index 8b54282f65..4c64469102 100644 --- a/tests/php-unit-tests/integration-tests/DictionariesConsistencyAfterSetupTest.php +++ b/tests/php-unit-tests/integration-tests/DictionariesConsistencyAfterSetupTest.php @@ -151,6 +151,12 @@ class DictionariesConsistencyAfterSetupTest extends ItopTestCase $aMismatchedKeys = []; foreach ($aKeyArgsCountMap[$sReferenceLangCode] as $sKey => $iExpectedNbOfArgs){ + if (0 === $iExpectedNbOfArgs){ + //no arg needed in EN. + //let s assume job has been done correctly in EN to simplify + continue; + } + if (in_array($sKey, self::$aLabelCodeNotToCheck)){ //false positive: do not test continue; diff --git a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php index accbbca48b..9b9c3f6b86 100644 --- a/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CRUDEventTest.php @@ -12,6 +12,7 @@ use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use ContactType; use CoreException; use DBObject; +use DBObject\MockDBObjectWithCRUDEventListener; use DBObjectSet; use DBSearch; use lnkPersonToTeam; @@ -518,6 +519,24 @@ class CRUDEventTest extends ItopDataTestCase $this->assertEquals(2, self::$aEventCalls[EVENT_DB_LINKS_CHANGED]); } + + // Tests with MockDBObject + public function testFireCRUDEvent() + { + $this->RequireOnceUnitTestFile('DBObject/MockDBObjectWithCRUDEventListener.php'); + + // For Metamodel list of classes + MockDBObjectWithCRUDEventListener::Init(); + $oDBObject = new MockDBObjectWithCRUDEventListener(); + $oDBObject2 = new MockDBObjectWithCRUDEventListener(); + + $oDBObject->FireEvent(MockDBObjectWithCRUDEventListener::TEST_EVENT); + + $this->assertNotNull($oDBObject->oEventDataReceived); + $this->assertNull($oDBObject2->oEventDataReceived); + + //echo($oDBObject->oEventDataReceived->Get('debug_info')); + } } /** diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/MockDBObjectWithCRUDEventListener.php b/tests/php-unit-tests/unitary-tests/core/DBObject/MockDBObjectWithCRUDEventListener.php new file mode 100644 index 0000000000..f81ce637c3 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/DBObject/MockDBObjectWithCRUDEventListener.php @@ -0,0 +1,44 @@ + 'bizmodel, searchable', + 'key_type' => 'autoincrement', + 'name_attcode' => '', + 'state_attcode' => '', + 'reconc_keys' => [], + 'db_table' => 'priv_unit_tests_mock', + 'db_key_field' => 'id', + 'db_finalclass_field' => '', + 'display_template' => '', + 'indexes' => [], + ); + MetaModel::Init_Params($aParams); + } + + protected function RegisterEventListeners() + { + $this->RegisterCRUDListener(self::TEST_EVENT, 'TestEventCallback', 0, 'unit-test'); + } + + public function TestEventCallback(EventData $oEventData) + { + $this->oEventDataReceived = $oEventData; + } +} \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php index 28d92a785b..51b93a4167 100644 --- a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php +++ b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php @@ -941,4 +941,40 @@ class DBObjectTest extends ItopDataTestCase { return $this->aReloadCount[$sClass][$sKey] ?? 0; } + + /** + * @since 3.1.0-3 3.1.1 3.2.0 N°6716 test creation + */ + public function testConstructorMemoryFootprint():void + { + $idx = 0; + $fStart = microtime(true); + $fStartLoop = $fStart; + $iInitialPeak = 0; + $iMaxAllowedMemoryIncrease = 1 * 1024 * 1024; + + for ($i = 0; $i < 5000; $i++) { + /** @noinspection PhpUnusedLocalVariableInspection We intentionally use a reference that will disappear on each loop */ + $oPerson = new \Person(); + if (0 == ($idx % 100)) { + $fDuration = microtime(true) - $fStartLoop; + $iMemoryPeakUsage = memory_get_peak_usage(); + if ($iInitialPeak === 0) { + $iInitialPeak = $iMemoryPeakUsage; + $sInitialPeak = \utils::BytesToFriendlyFormat($iInitialPeak, 4); + } + + $sCurrPeak = \utils::BytesToFriendlyFormat($iMemoryPeakUsage, 4); + echo "$idx ".sprintf('%.1f ms', $fDuration * 1000)." - Peak Memory Usage: $sCurrPeak\n"; + + $this->assertTrue(($iMemoryPeakUsage - $iInitialPeak) <= $iMaxAllowedMemoryIncrease , "Peak memory changed from $sInitialPeak to $sCurrPeak after $i loops"); + + $fStartLoop = microtime(true); + } + $idx++; + } + + $fTotalDuration = microtime(true) - $fStart; + echo 'Total duration: '.sprintf('%.3f s', $fTotalDuration)."\n\n"; + } }