Fix undefined $_SESSION warning when rebuilding menu cache

_build/test/Tests/Model/modMenuTest.php

@@ -0,0 +1,107 @@
+<?php
+
+/*
+ * This file is part of the MODX Revolution package.
+ *
+ * Copyright (c) MODX, LLC
+ *
+ * For complete copyright and license information, see the COPYRIGHT and LICENSE
+ * files found in the top-level directory of this distribution.
+ *
+ * @package modx-test
+ */
+namespace MODX\Revolution\Tests\Model;
+
+use MODX\Revolution\modMenu;
+use MODX\Revolution\MODxTestCase;
+use xPDO\xPDO;
+
+/**
+ * Tests related to modMenu cache rebuild and invalidation.
+ *
+ * @package modx-test
+ * @subpackage modx
+ * @group Model
+ * @group modMenu
+ */
+class modMenuTest extends MODxTestCase
+{
+    /**
+     * rebuildCache must not emit Undefined global variable $_SESSION.
+     */
+    public function testRebuildCacheWithoutSessionDoesNotWarn()
+    {
+        $hadSession = array_key_exists('_SESSION', $GLOBALS);
+        $previousSession = $hadSession ? $_SESSION : null;
+        unset($GLOBALS['_SESSION']);
+
+        $sessionWarnings = [];
+        set_error_handler(static function ($severity, $message) use (&$sessionWarnings) {
+            if (strpos($message, 'Undefined global variable $_SESSION') !== false) {
+                $sessionWarnings[] = $message;
+                return true;
+            }
+
+            return false;
+        });
+
+        try {
+            $this->modx->setOption('cultureKey', 'en');
+            $this->modx->getCacheManager();
+
+            /** @var modMenu $menu */
+            $menu = $this->modx->newObject(modMenu::class);
+            $menus = $menu->rebuildCache('topnav');
+            $this->assertIsArray($menus);
+            $this->assertSame([], $sessionWarnings);
+        } finally {
+            restore_error_handler();
+            if ($hadSession) {
+                $_SESSION = $previousSession;
+            } else {
+                unset($GLOBALS['_SESSION']);
+            }
+        }
+    }
+
+    /**
+     * clearCache (used by save/remove) must wipe language shards without $_SESSION.
+     */
+    public function testClearCacheWithoutSessionRemovesMenuKeys()
+    {
+        $hadSession = array_key_exists('_SESSION', $GLOBALS);
+        $previousSession = $hadSession ? $_SESSION : null;
+        unset($GLOBALS['_SESSION']);
+
+        $this->modx->setOption('cultureKey', 'en');
+        $cacheManager = $this->modx->getCacheManager();
+        $cacheOptions = [
+            xPDO::OPT_CACHE_KEY => $this->modx->getOption('cache_menu_key', null, 'menu'),
+            xPDO::OPT_CACHE_HANDLER => $this->modx->getOption(
+                'cache_menu_handler',
+                null,
+                $this->modx->getOption(xPDO::OPT_CACHE_HANDLER)
+            ),
+        ];
+
+        $cacheKey = 'menus/topnav/en';
+        $probe = ['probe' => true];
+        $cacheManager->set($cacheKey, $probe, 0, $cacheOptions);
+        $this->assertSame(['probe' => true], $cacheManager->get($cacheKey, $cacheOptions));
+
+        try {
+            /** @var modMenu $menu */
+            $menu = $this->modx->newObject(modMenu::class);
+            $clearCache = new \ReflectionMethod(modMenu::class, 'clearCache');
+            $clearCache->setAccessible(true);
+            $clearCache->invoke($menu);
+            $this->assertNull($cacheManager->get($cacheKey, $cacheOptions));
+        } finally {
+            if ($hadSession) {
+                $_SESSION = $previousSession;
+            } else {
+                unset($GLOBALS['_SESSION']);
+            }
+        }
+    }
+}

_build/test/Tests/Model/modXTest.php

@@ -118,6 +118,55 @@ public function testGetCacheManager() {
         $this->assertInstanceOf(modCacheManager::class,$this->modx->cacheManager, "Failed to load a modCacheManager instance");
     }
 
+    /**
+     * getManagerLanguage must not require $_SESSION and must honor session when present.
+     */
+    public function testGetManagerLanguageWithoutSession()
+    {
+        $this->withSessionFixture(null, function () {
+            $this->modx->setOption('cultureKey', 'de');
+            $this->modx->setOption('manager_language', null);
+            $this->assertSame('de', $this->modx->getManagerLanguage());
+        });
+    }
+
+    /**
+     * @depends testGetManagerLanguageWithoutSession
+     */
+    public function testGetManagerLanguagePrefersSessionValue()
+    {
+        $this->withSessionFixture(['manager_language' => 'fr'], function () {
+            $this->modx->setOption('cultureKey', 'de');
+            $this->assertSame('fr', $this->modx->getManagerLanguage());
+        });
+    }
+
+    /**
+     * @param array|null $session null unsets $_SESSION; array replaces it
+     * @param callable $callback
+     */
+    private function withSessionFixture($session, callable $callback)
+    {
+        $hadSession = array_key_exists('_SESSION', $GLOBALS);
+        $previousSession = $hadSession ? $_SESSION : null;
+
+        if ($session === null) {
+            unset($GLOBALS['_SESSION']);
+        } else {
+            $_SESSION = $session;
+        }
+
+        try {
+            $callback();
+        } finally {
+            if ($hadSession) {
+                $_SESSION = $previousSession;
+            } else {
+                unset($GLOBALS['_SESSION']);
+            }
+        }
+    }
+
     /**
      * @param string $expected
      * @param string $string

_build/test/phpunit.xml

@@ -20,6 +20,7 @@
             <file>Tests/Model/modXTest.php</file>
             <file>Tests/Model/modXLoggingTest.php</file>
             <file>Tests/Model/modParserTest.php</file>
+            <file>Tests/Model/modMenuTest.php</file>
             <directory>Tests/Model/Dashboard</directory>
             <directory>Tests/Model/Element</directory>
             <directory>Tests/Model/Error</directory>

core/src/Revolution/Processors/System/Country/GetList.php

@@ -52,7 +52,7 @@ public function getCountryList()
     {
         $_country_lang = [];
         include $this->modx->getOption('core_path') . 'lexicon/country/en.inc.php';
-        $ml = $this->modx->getOption('manager_language', $_SESSION, $this->modx->getOption('cultureKey', null, 'en'));
+        $ml = $this->modx->getManagerLanguage();
         if ($ml !== 'en' && file_exists($this->modx->getOption('core_path') . 'lexicon/country/' . $ml . '.inc.php')) {
             include $this->modx->getOption('core_path') . 'lexicon/country/' . $ml . '.inc.php';
         }

core/src/Revolution/Transport/modTransportProvider.php

@@ -474,8 +474,7 @@ protected function args(array $args = [])
             'supports' => $this->xpdo->version['code_name'] . '-' . $this->xpdo->version['full_version'],
             'http_host' => $this->xpdo->getOption('http_host'),
             'php_version' => PHP_VERSION,
-            'language' => $this->xpdo->getOption('manager_language', $_SESSION,
-                $this->xpdo->getOption('cultureKey', null, 'en')),
+            'language' => $this->xpdo->getManagerLanguage(),
         ];
 
         return array_merge($baseArgs, $args);

core/src/Revolution/modConnectorRequest.php

@@ -38,8 +38,7 @@ public function initialize()
         if ($this->modx && is_object($this->modx->context) && $this->modx->context instanceof modContext) {
             $ctx = $this->modx->context->get('key');
             if (!empty($ctx) && $ctx == 'mgr') {
-                $ml = $this->modx->getOption('manager_language', $_SESSION ?? [],
-                    $this->modx->getOption('cultureKey', null, 'en'));
+                $ml = $this->modx->getManagerLanguage();
                 if (!empty($ml)) {
                     $this->modx->setOption('cultureKey', $ml);
                 }

core/src/Revolution/modLexicon.php

@@ -194,7 +194,7 @@ public function load()
         $topics = func_get_args(); /* allow for dynamic number of lexicons to load */
 
         if ($this->modx->context && $this->modx->context->get('key') == 'mgr') {
-            $defaultLanguage = $this->modx->getOption('manager_language', $_SESSION ?? [], $this->modx->getOption('cultureKey', null, 'en'));
+            $defaultLanguage = $this->modx->getManagerLanguage();
         } else {
             $defaultLanguage = $this->modx->getOption('cultureKey', null, 'en');
         }

core/src/Revolution/modManagerRequest.php

@@ -104,7 +104,7 @@ public function initialize()
             $this->modx->getVersionData();
         }
 
-        $ml = $this->modx->getOption('manager_language', $_SESSION, $this->modx->getOption('cultureKey', null, 'en'));
+        $ml = $this->modx->getManagerLanguage();
         if (!empty($ml)) {
             $this->modx->setOption('cultureKey', $ml);
         }

core/src/Revolution/modMenu.php

@@ -24,35 +24,52 @@
 class modMenu extends modAccessibleObject
 {
     /**
-     * Overrides xPDOObject::save to cache the menus.
+     * Overrides xPDOObject::save to invalidate the menu cache.
      *
      * {@inheritdoc}
      */
     public function save($cacheFlag = null)
     {
         $saved = parent::save($cacheFlag);
         if ($saved && empty($this->xpdo->config[xPDO::OPT_SETUP])) {
-            $this->rebuildCache();
+            $this->clearCache();
         }
 
         return $saved;
     }
 
     /**
-     * Overrides xPDOObject::remove to cache the menus.
+     * Overrides xPDOObject::remove to invalidate the menu cache.
      *
      * {@inheritdoc}
      */
     public function remove(array $ancestors = [])
     {
         $removed = parent::remove($ancestors);
         if ($removed && empty($this->xpdo->config[xPDO::OPT_SETUP])) {
-            $this->rebuildCache();
+            $this->clearCache();
         }
 
         return $removed;
     }
 
+    /**
+     * Clear all language shards of the menu cache partition without firing OnCacheUpdate.
+     *
+     * Menu processors still call refresh() once so plugins see a single cache event.
+     */
+    protected function clearCache()
+    {
+        $this->xpdo->getCacheManager()->clean([
+            xPDO::OPT_CACHE_KEY => $this->xpdo->getOption('cache_menu_key', null, 'menu'),
+            xPDO::OPT_CACHE_HANDLER => $this->xpdo->getOption(
+                'cache_menu_handler',
+                null,
+                $this->xpdo->getOption(xPDO::OPT_CACHE_HANDLER)
+            ),
+        ]);
+    }
+
     /**
      * Rebuilds the menu map cache.
      *
@@ -66,8 +83,7 @@ public function rebuildCache($start = '')
         if ($start !== '') {
             $cacheKey .= "{$start}/";
         }
-        $cacheKey .= $this->xpdo->getOption('manager_language', $_SESSION,
-            $this->xpdo->getOption('cultureKey', null, 'en'));
+        $cacheKey .= $this->xpdo->getManagerLanguage();
         $menus = $this->getSubMenus($start);
         $cached = $this->xpdo->cacheManager->set($cacheKey, $menus, 0, [
             xPDO::OPT_CACHE_KEY => $this->xpdo->cacheManager->getOption('cache_menu_key', null, 'menu'),
@@ -147,45 +163,29 @@ public function getSubMenus($start = '')
         $list = [];
         /** @var modMenu $menu */
         foreach ($menus as $menu) {
+            $textKey = $menu->get('text');
             $ma = $menu->toArray();
-            $ma['id'] = $menu->get('text');
-            $action = $menu->get('action');
+            $ma['id'] = $textKey;
             $namespace = $menu->get('namespace');
 
             if ($namespace !== 'core') {
                 $this->xpdo->lexicon->load($namespace . ':default');
             }
 
-            /* if 3rd party menu item, load proper text */
-            if (!empty($action)) {
-                if (!empty($namespace) && $namespace !== 'core') {
-                    $ma['text'] = $menu->get('text') === 'user'
-                        ? $this->xpdo->lexicon($menu->get('text'), ['username' => $this->xpdo->getLoginUserName()])
-                        : $this->xpdo->lexicon($menu->get('text'));
-                } else {
-                    $ma['text'] = $menu->get('text') === 'user'
-                        ? $this->xpdo->lexicon($menu->get('text'), ['username' => $this->xpdo->getLoginUserName()])
-                        : $this->xpdo->lexicon($menu->get('text'));
-                }
-            } else {
-                $ma['text'] = $menu->get('text') === 'user'
-                    ? $this->xpdo->lexicon($menu->get('text'), ['username' => $this->xpdo->getLoginUserName()])
-                    : $this->xpdo->lexicon($menu->get('text'));
-            }
+            $ma['text'] = $this->xpdo->lexicon(
+                $textKey,
+                $textKey === 'user' ? ['username' => $this->xpdo->getLoginUserName()] : []
+            );
 
             $desc = $menu->get('description');
             $ma['description'] = !empty($desc) ? $this->xpdo->lexicon($desc) : '';
-            $ma['children'] = $menu->get('text') != '' ? $this->getSubMenus($menu->get('text')) : [];
+            $ma['children'] = $textKey != '' ? $this->getSubMenus($textKey) : [];
 
-            if ($ma['id'] === 'language') {
+            if ($textKey === 'language') {
                 $ma['children'] = $this->getLanguageMenu();
             }
 
-            if ($menu->get('controller')) {
-                $ma['controller'] = $menu->get('controller');
-            } else {
-                $ma['controller'] = '';
-            }
+            $ma['controller'] = $menu->get('controller') ?: '';
             $list[] = $ma;
         }
         unset($menu, $desc, $namespace, $ma);

core/src/Revolution/modX.php

@@ -2285,6 +2285,23 @@ public function getSessionState() {
         return $this->_sessionState;
     }
 
+    /**
+     * Resolve the manager UI language without requiring $_SESSION.
+     *
+     * Looks up session `manager_language`, then config `manager_language`,
+     * then cultureKey, then en.
+     *
+     * @return string
+     */
+    public function getManagerLanguage()
+    {
+        return (string) $this->getOption(
+            'manager_language',
+            $_SESSION ?? [],
+            $this->getOption('cultureKey', null, 'en')
+        );
+    }
+
     /**
      * Executed before parser processing of an element.
      */

manager/controllers/default/header.php

@@ -258,9 +258,7 @@ protected function getCache($name)
      */
     protected function getCacheKey($name)
     {
-        $ml = $this->modx->getOption('manager_language', $_SESSION, $this->modx->getOption('cultureKey', null, 'en'));
-
-        return "menus/{$name}/" . $ml;
+        return "menus/{$name}/" . $this->modx->getManagerLanguage();
     }
 
     /**

manager/controllers/default/security/login.class.php

@@ -205,7 +205,7 @@ public function handleLanguageChange()
 
         $ml = $this->modx->sanitizeString($this->modx->getOption('manager_language', $_REQUEST));
         if (!$ml || !in_array($ml, $languages)) {
-            $ml = $this->modx->getOption('manager_language', $_SESSION);
+            $ml = $this->modx->getOption('manager_language', $_SESSION ?? []);
             if (!$ml) {
                 // Try to detect default browser language
                 $accept_languages = strtolower($_SERVER['HTTP_ACCEPT_LANGUAGE']);