diff --git a/application/menunode.class.inc.php b/application/menunode.class.inc.php index 0369c4135..ea1e3f14c 100644 --- a/application/menunode.class.inc.php +++ b/application/menunode.class.inc.php @@ -9,6 +9,7 @@ use Combodo\iTop\Application\Helper\WebResourcesHelper; require_once(APPROOT.'/application/utils.inc.php'); require_once(APPROOT.'/application/template.class.inc.php'); require_once(APPROOT."/application/user.dashboard.class.inc.php"); +require_once(APPROOT."/setup/parentmenunodecompiler.class.inc.php"); /** @@ -274,7 +275,7 @@ class ApplicationMenu } $aSubMenuNodes = static::GetSubMenuNodes($sMenuGroupIdx, $aExtraParams); - if (! MFCompiler::$bUseLegacyMenuCompilation && !($oMenuNode instanceof ShortcutMenuNode)){ + if (! ParentMenuNodeCompiler::$bUseLegacyMenuCompilation && !($oMenuNode instanceof ShortcutMenuNode)){ if (is_array($aSubMenuNodes) && 0 === sizeof($aSubMenuNodes)){ IssueLog::Error('Empty menu node not displayed', LogChannels::CONSOLE, [ 'menu_node_class' => get_class($oMenuNode), diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index 2edc2a197..da354f8eb 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -23,6 +23,7 @@ use Combodo\iTop\DesignElement; require_once(APPROOT.'setup/setuputils.class.inc.php'); require_once(APPROOT.'setup/modelfactory.class.inc.php'); +require_once(APPROOT.'setup/parentmenunodecompiler.class.inc.php'); require_once(APPROOT.'core/moduledesign.class.inc.php'); class DOMFormatException extends Exception @@ -90,8 +91,6 @@ class MFCompiler */ const REBUILD_HKEYS_NEVER = APPROOT.'data/.setup-rebuild-hkeys-never'; - public static $bUseLegacyMenuCompilation = false; - /** @var \ModelFactory */ protected $oFactory; @@ -128,10 +127,6 @@ class MFCompiler $this->aClassesCSSRules = []; } - public static function UseLegacyMenuCompilation(){ - self::$bUseLegacyMenuCompilation = true; - } - protected function Log($sText) { $this->aLog[] = $sText; @@ -332,21 +327,16 @@ class MFCompiler */ protected function DoCompile($sTempTargetDir, $sFinalTargetDir, $oP = null, $bUseSymbolicLinks = false) { - $aAllClasses = array(); // flat list of classes - $aModulesInfo = array(); // Hash array of module_name => array('version' => string, 'root_dir' => string) + $aAllClasses = []; // flat list of classes + $aModulesInfo = []; // Hash array of module_name => array('version' => string, 'root_dir' => string) // Determine the target modules for the MENUS - // - $aMenuNodes = array(); - $aMenusByModule = array(); - foreach ($this->oFactory->GetNodes('menus/menu') as $oMenuNode) - { - $sMenuId = $oMenuNode->getAttribute('id'); - $aMenuNodes[$sMenuId] = $oMenuNode; - $sModuleMenu = $oMenuNode->getAttribute('_created_in'); - $aMenusByModule[$sModuleMenu][] = $sMenuId; - } + /** + * @since 3.1 N°4762 + */ + $oParentMenuNodeCompiler = new ParentMenuNodeCompiler($this); + $oParentMenuNodeCompiler->LoadXmlMenus($this->oFactory); // Determine the target module (exactly one!) for USER RIGHTS // This used to be based solely on the module which created the user_rights node first @@ -391,6 +381,7 @@ class MFCompiler static::SetUseSymbolicLinksFlag($bUseSymbolicLinks); + $oParentMenuNodeCompiler->LoadModuleMenuInfo($aModules); foreach ($aModules as $foo => $oModule) { $sModuleName = $oModule->GetName(); $sModuleVersion = $oModule->GetVersion(); @@ -466,7 +457,7 @@ class MFCompiler } } - if (!array_key_exists($sModuleName, $aMenusByModule)) + if (is_null($oParentMenuNodeCompiler->GetMenusByModule($sModuleName))) { $this->Log("Found module without menus declared: $sModuleName"); } @@ -486,75 +477,19 @@ class $sMenuCreationClass extends ModuleHandlerAPI global \$__comp_menus__; // ensure that the global variable is indeed global ! EOF; - // Preliminary: determine parent menus not defined within the current module - $aMenusToLoad = array(); - $aParentMenus = array(); - foreach($aMenusByModule[$sModuleName] as $sMenuId) - { - $oMenuNode = $aMenuNodes[$sMenuId]; - if ($sParent = $oMenuNode->GetChildText('parent', null)) - { - $aMenusToLoad[] = $sParent; - $aParentMenus[] = $sParent; - } - // Note: the order matters: the parents must be defined BEFORE - $aMenusToLoad[] = $sMenuId; - } - $aMenusToLoad = array_unique($aMenusToLoad); - $aMenuLinesForAll = array(); - $aMenuLinesForAdmins = array(); - $aAdminMenus = array(); - foreach($aMenusToLoad as $sMenuId) - { - $oMenuNode = $aMenuNodes[$sMenuId]; - if (is_null($oMenuNode)) - { - throw new Exception("Module '{$oModule->GetId()}' (location : '$sModuleRootDir') contains an unknown menuId : '$sMenuId'"); - } - if (self::$bUseLegacyMenuCompilation && $oMenuNode->getAttribute("xsi:type") == 'MenuGroup') - { - // Note: this algorithm is wrong - // 1 - the module may appear empty in the current module, while children are defined in other modules - // 2 - check recursively that child nodes are not empty themselves - // Future algorithm: - // a- browse the modules and build the menu tree - // b- browse the tree and blacklist empty menus - // c- before compiling, discard if blacklisted - if (!in_array($oMenuNode->getAttribute("id"), $aParentMenus)) - { - // Discard empty menu groups - continue; - } - } - try - { - $aMenuLines = $this->CompileMenu($oMenuNode, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); - } - catch (DOMFormatException $e) - { - throw new Exception("Failed to process menu '$sMenuId', from '$sModuleRootDir': ".$e->getMessage()); - } - $sParent = $oMenuNode->GetChildText('parent', null); - if (($oMenuNode->GetChildText('enable_admin_only') == '1') || isset($aAdminMenus[$sParent])) - { - $aMenuLinesForAdmins = array_merge($aMenuLinesForAdmins, $aMenuLines); - $aAdminMenus[$oMenuNode->getAttribute("id")] = true; - } - else - { - $aMenuLinesForAll = array_merge($aMenuLinesForAll, $aMenuLines); - } - } + + $oParentMenuNodeCompiler->CompileModuleMenus($oModule, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); + $sIndent = "\t\t"; - foreach ($aMenuLinesForAll as $sPHPLine) + foreach ($oParentMenuNodeCompiler->GetMenuLinesForAll() as $sPHPLine) { $sCompiledCode .= $sIndent.$sPHPLine."\n"; } - if (count($aMenuLinesForAdmins) > 0) + if (count($oParentMenuNodeCompiler->GetMenuLinesForAdmins()) > 0) { $sCompiledCode .= $sIndent."if (UserRights::IsAdministrator())\n"; $sCompiledCode .= $sIndent."{\n"; - foreach ($aMenuLinesForAdmins as $sPHPLine) + foreach ($oParentMenuNodeCompiler->GetMenuLinesForAdmins() as $sPHPLine) { $sCompiledCode .= $sIndent."\t".$sPHPLine."\n"; } diff --git a/setup/parentmenunodecompiler.class.inc.php b/setup/parentmenunodecompiler.class.inc.php new file mode 100644 index 000000000..374a915ce --- /dev/null +++ b/setup/parentmenunodecompiler.class.inc.php @@ -0,0 +1,287 @@ +oMFCompiler = $oMFCompiler; + } + + public static function UseLegacyMenuCompilation(){ + self::$bUseLegacyMenuCompilation = true; + } + + /** + * @param \ModelFactory $oFactory + * Initialize menu nodes arrays + * @return void + */ + public function LoadXmlMenus(\ModelFactory $oFactory) : void { + foreach ($oFactory->GetNodes('menus/menu') as $oMenuNode) { + $sMenuId = $oMenuNode->getAttribute('id'); + $this->aMenuNodes[$sMenuId] = $oMenuNode; + + $sModuleMenu = $oMenuNode->getAttribute('_created_in'); + $this->aMenusByModule[$sModuleMenu][] = $sMenuId; + } + } + + /** + * @param $aModules + * Initialize arrays related to parent/child menus + * @return void + */ + public function LoadModuleMenuInfo($aModules) : void + { + foreach ($aModules as $foo => $oModule) { + $sModuleRootDir = $oModule->GetRootDir(); + $sModuleName = $oModule->GetName(); + + if (array_key_exists($sModuleName, $this->aMenusByModule)) { + $aMenusToLoad = []; + $aParentMenus = []; + + foreach ($this->aMenusByModule[$sModuleName] as $sMenuId) { + $oMenuNode = $this->aMenuNodes[$sMenuId]; + + if (self::$bUseLegacyMenuCompilation){ + if ($sParent = $oMenuNode->GetChildText('parent', null)) { + $aMenusToLoad[] = $sParent; + $aParentMenus[] = $sParent; + } + } else { + if ($oMenuNode->getAttribute("xsi:type") == 'MenuGroup') { + $this->aParentModuleRootDirs[$sMenuId] = $sModuleRootDir; + } + + if ($sParent = $oMenuNode->GetChildText('parent', null)) { + $aMenusToLoad[] = $sParent; + $aParentMenus[] = $sParent; + + $this->aParentModuleRootDirs[$sParent] = $sModuleRootDir; + } + + if (array_key_exists($sMenuId, $this->aParentModuleRootDirs)){ + $this->aParentMenuNodes[$sMenuId] = $oMenuNode; + } + } + + // Note: the order matters: the parents must be defined BEFORE + $aMenusToLoad[] = $sMenuId; + } + + $this->aMenusToLoadByModule[$sModuleName] = array_unique($aMenusToLoad); + $this->aParentMenusByModule[$sModuleName] = array_unique($aParentMenus); + } + } + } + + /** + * Perform the actual "Compilation" for one module at a time + * @param \MFModule $oModule + * @param string $sTempTargetDir + * @param string $sFinalTargetDir + * @param string $sRelativeDir + * @param Page $oP + * + * @return void + * @throws \Exception + */ + public function CompileModuleMenus(MFModule $oModule, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP = null) : void + { + $this->aMenuLinesForAdmins = []; + $this->aMenuLinesForAll = []; + $aAdminMenus = []; + + $sModuleRootDir = $oModule->GetRootDir(); + $sModuleName = $oModule->GetName(); + + $aParentMenus = $this->aParentMenusByModule[$sModuleName]; + foreach($this->aMenusToLoadByModule[$sModuleName] as $sMenuId) + { + $oMenuNode = $this->aMenuNodes[$sMenuId]; + if (is_null($oMenuNode)) + { + throw new Exception("Module '{$oModule->GetId()}' (location : '$sModuleRootDir') contains an unknown menuId : '$sMenuId'"); + } + + if (self::$bUseLegacyMenuCompilation) { + if ($oMenuNode->getAttribute("xsi:type") == 'MenuGroup') { + // Note: this algorithm is wrong + // 1 - the module may appear empty in the current module, while children are defined in other modules + // 2 - check recursively that child nodes are not empty themselves + // Future algorithm: + // a- browse the modules and build the menu tree + // b- browse the tree and blacklist empty menus + // c- before compiling, discard if blacklisted + if (! in_array($oMenuNode->getAttribute("id"), $aParentMenus)) { + // Discard empty menu groups + continue; + } + } + } else { + if (array_key_exists($sMenuId, $this->aParentMenuNodes)) { + // compile parent menus recursively + $this->CompileParentMenuNode($sMenuId, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); + continue; + } + } + + try + { + //both new/legacy algo: compile leaf menu + $aMenuLines = $this->oMFCompiler->CompileMenu($oMenuNode, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); + } + catch (DOMFormatException $e) + { + throw new Exception("Failed to process menu '$sMenuId', from '$sModuleRootDir': ".$e->getMessage()); + } + + $sParent = $oMenuNode->GetChildText('parent', null); + if (($oMenuNode->GetChildText('enable_admin_only') == '1') || isset($aAdminMenus[$sParent]) || isset($this->aParentAdminMenus[$sParent])) + { + $this->aMenuLinesForAdmins = array_merge($this->aMenuLinesForAdmins, $aMenuLines); + $aAdminMenus[$oMenuNode->getAttribute("id")] = true; + } + else + { + $this->aMenuLinesForAll = array_merge($this->aMenuLinesForAll, $aMenuLines); + } + } + } + + /** + * Perform parent menu compilation including its ancestrors (recursively) + * @param string $sMenuId + * @param string $sTempTargetDir + * @param string $sFinalTargetDir + * @param string $sRelativeDir + * @param Page $oP + * + * @return void + * @throws \Exception + */ + public function CompileParentMenuNode(string $sMenuId, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP = null) : void + { + $oMenuNode = $this->aParentMenuNodes[$sMenuId]; + $sStatus = array_key_exists($sMenuId, $this->aMenuProcessStatus) ? $this->aMenuProcessStatus[$sMenuId] : null; + if ($sStatus === self::COMPILED){ + //node already processed before + return; + } else if ($sStatus === self::COMPILING){ + throw new \Exception("Cyclic dependency between parent menus ($sMenuId)"); + } + + $this->aMenuProcessStatus[$sMenuId] = self::COMPILING; + + try { + $sParent = $oMenuNode->GetChildText('parent', null); + if (! empty($sParent)){ + //compile parents before (even parent of parents ... recursively) + $this->CompileParentMenuNode($sParent, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); + } + + if (! array_key_exists($sMenuId, $this->aParentModuleRootDirs)){ + throw new Exception("Failed to process parent menu '$sMenuId' that is referenced by a child but not defined"); + } + $sModuleRootDir = $this->aParentModuleRootDirs[$sMenuId]; + $aMenuLines = $this->oMFCompiler->CompileMenu($oMenuNode, $sTempTargetDir, $sFinalTargetDir, $sRelativeDir, $oP); + } catch (DOMFormatException $e) { + throw new Exception("Failed to process menu '$sMenuId', from '$sModuleRootDir': ".$e->getMessage()); + } + $sParent = $oMenuNode->GetChildText('parent', null); + if (($oMenuNode->GetChildText('enable_admin_only') == '1') || isset($this->aParentAdminMenus[$sParent])) { + $this->aMenuLinesForAdmins = array_merge($this->aMenuLinesForAdmins, $aMenuLines); + $this->aParentAdminMenus[$oMenuNode->getAttribute("id")] = true; + } else { + $this->aMenuLinesForAll = array_merge($this->aMenuLinesForAll, $aMenuLines); + } + + $this->aMenuProcessStatus[$sMenuId] = self::COMPILED; + } + + public function GetMenusByModule(string $sModuleName) : ?array + { + if (array_key_exists($sModuleName, $this->aMenusByModule)) { + return $this->aMenusByModule[$sModuleName]; + } + + return null; + } + + public function GetMenuLinesForAdmins(): array { + return $this->aMenuLinesForAdmins; + } + + public function GetMenuLinesForAll(): array { + return $this->aMenuLinesForAll; + } +} diff --git a/test/setup/MFCompilerMenuTest.php b/test/setup/MFCompilerMenuTest.php index 6926e159d..a59bda195 100644 --- a/test/setup/MFCompilerMenuTest.php +++ b/test/setup/MFCompilerMenuTest.php @@ -7,6 +7,7 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase; use Config; use MetaModel; use MFCompiler; +use ParentMenuNodeCompiler; use RunTimeEnvironment; /** @@ -14,8 +15,8 @@ use RunTimeEnvironment; * @runTestsInSeparateProcesses * @preserveGlobalState disabled * @backupGlobals disabled - * @since 3.0.x N°4762 - * @covers \MFCompiler::UseLatestPrecompiledFile + * @since 3.1 N°4762 + * @covers \MFCompiler::DoCompile */ class MFCompilerMenuTest extends ItopTestCase { private static $aPreviousEnvMenus; @@ -23,22 +24,26 @@ class MFCompilerMenuTest extends ItopTestCase { public function setUp(): void { parent::setUp(); - require_once(APPROOT.'setup/compiler.class.inc.php'); - require_once(APPROOT.'setup/modelfactory.class.inc.php'); - require_once(APPROOT.'application/utils.inc.php'); - + $this->RequireOnceItopFile('setup/compiler.class.inc.php'); + $this->RequireOnceItopFile('setup/modelfactory.class.inc.php'); + $this->RequireOnceItopFile('application/utils.inc.php'); } public function tearDown(): void { parent::tearDown(); } + private function GetCurrentEnvDeltaXmlPath(string $sEnv) : string { + return APPROOT."data/$sEnv.delta.xml"; + } + public function CompileMenusProvider(){ return [ 'legacy_algo' => [ 'sEnv' => 'legacy_algo', 'bLegacyMenuCompilation' => true ], 'menu_compilation_fix' => [ 'sEnv' => 'menu_compilation_fix', 'bLegacyMenuCompilation' => false ], ]; } + /** * @dataProvider CompileMenusProvider */ @@ -55,7 +60,7 @@ class MFCompilerMenuTest extends ItopTestCase { $oConfig = new Config($sConfigFilePath); if ($bLegacyMenuCompilation){ - MFCompiler::UseLegacyMenuCompilation(); + ParentMenuNodeCompiler::UseLegacyMenuCompilation(); } $oConfig->WriteToFile(); $oRunTimeEnvironment = new RunTimeEnvironment($sEnv); @@ -82,4 +87,53 @@ class MFCompilerMenuTest extends ItopTestCase { } static::$aPreviousEnvMenuCount = $aMenuCount; } + + public function CompileMenusWithDeltaProvider(){ + return [ + 'Menus are broken with specific delta XML using LEGACY algo' => [ 'sDeltaFile' => 'delta_broken_menus.xml', 'sEnv' => 'broken_menus', 'bLegacyMenuCompilation' => true ], + 'Menus repaired using same delta XML with NEW algo' => [ 'sDeltaFile' => 'delta_broken_menus.xml', 'sEnv' => 'fixed_menus', 'bLegacyMenuCompilation' => false ], + ]; + } + + /** + * @dataProvider CompileMenusWithDeltaProvider + */ + public function testCompileMenusWithDelta($sDeltaFile, $sEnv, $bLegacyMenuCompilation){ + $sProvidedDeltaPath = __DIR__.'/ressources/datamodels/'.$sDeltaFile; + if (is_file($sProvidedDeltaPath)){ + $sDeltaXmlPath = $this->GetCurrentEnvDeltaXmlPath($sEnv); + copy($sProvidedDeltaPath, $sDeltaXmlPath); + } + $sConfigFilePath = \utils::GetConfigFilePath($sEnv); + + //copy conf from production to phpunit context + $sDirPath = dirname($sConfigFilePath); + if (! is_dir($sDirPath)){ + mkdir($sDirPath); + } + $oConfig = new Config(\utils::GetConfigFilePath()); + $oConfig->WriteToFile($sConfigFilePath); + + $oConfig = new Config($sConfigFilePath); + if ($bLegacyMenuCompilation){ + ParentMenuNodeCompiler::UseLegacyMenuCompilation(); + } + $oConfig->WriteToFile(); + $oRunTimeEnvironment = new RunTimeEnvironment($sEnv); + $oRunTimeEnvironment->CompileFrom(\utils::GetCurrentEnvironment()); + $oConfig->WriteToFile(); + + if ($bLegacyMenuCompilation){ + /** + * PHP Notice: Undefined index: ConfigManagement in /var/www/html/iTop/env-broken_menus/itop-structure/model.itop-structure.php on line 925 + */ + error_reporting(E_ALL & ~E_NOTICE); + $this->expectErrorMessage("Call to a member function GetIndex() on null"); + } + $sConfigFile = APPCONF.\utils::GetCurrentEnvironment().'/'.ITOP_CONFIG_FILE; + MetaModel::Startup($sConfigFile, false /* $bModelOnly */, true /* $bAllowCache */, false /* $bTraceSourceFiles */, $sEnv); + + $this->assertNotEquals([], ApplicationMenu::GetMenuGroups()); + $this->assertNotEquals([], ApplicationMenu::GetMenusCount()); + } } diff --git a/tests/php-unit-tests/unitary-tests/setup/ressources/datamodels/delta_broken_menus.xml b/tests/php-unit-tests/unitary-tests/setup/ressources/datamodels/delta_broken_menus.xml new file mode 100644 index 000000000..73bbd5737 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/setup/ressources/datamodels/delta_broken_menus.xml @@ -0,0 +1,14 @@ + + + + + ConfigManagementOverview + + + ConfigManagementOverview + + + ConfigManagementOverview + + +