diff --git a/lib/classes/cron.php b/lib/classes/cron.php index 34d3294f9a7..3c57aef68de 100644 --- a/lib/classes/cron.php +++ b/lib/classes/cron.php @@ -30,6 +30,12 @@ use stdClass; */ class cron { + /** @var ?stdClass A copy of the standard cron 'user' */ + protected static ?stdClass $cronuser = null; + + /** @var ?stdClass The cron user's session data */ + protected static ?stdClass $cronsession = null; + /** * Use a default value of 3 minutes. * The recommended cron frequency is every minute, and the default adhoc concurrency is 3. @@ -539,13 +545,64 @@ class cron { * Sets up a user and course environment in cron. * Do not use outside of cron script! * - * @param stdClass $user full user object, null means default cron user (admin), - * value 'reset' means reset internal static caches. - * @param stdClass $course full course record, null means $SITE - * @param bool $leavepagealone If specified, stops it messing with global page object - * @return void + * Please note that this function stores cache data statically. + * @see reset_user_cache() to reset this cache. + * + * @param null|stdClass $user full user object, null means default cron user (admin) + * @param null|stdClass $course full course record, null means $SITE + * @param null|bool $leavepagealone If specified, stops it messing with global page object */ public static function setup_user(?stdClass $user = null, ?stdClass $course = null, bool $leavepagealone = false): void { - cron_setup_user($user, $course, $leavepagealone); + // This function uses the $GLOBALS super global. Disable the VariableNameLowerCase sniff for this function. + // phpcs:disable moodle.NamingConventions.ValidVariableName.VariableNameLowerCase + global $CFG, $SITE, $PAGE; + + if (!CLI_SCRIPT && !$leavepagealone) { + throw new coding_exception('It is not possible to use \core\cron\setup_user() in normal requests!'); + } + + if (empty(self::$cronuser)) { + // The cron user is essentially the admin user, but with some value removed. + // We ginore the timezone language, and locale preferences - use the site default instead. + self::$cronuser = get_admin(); + self::$cronuser->timezone = $CFG->timezone; + self::$cronuser->lang = ''; + self::$cronuser->theme = ''; + unset(self::$cronuser->description); + + self::$cronsession = new stdClass(); + } + + if (!$user) { + // Cached default cron user (==modified admin for now). + \core\session\manager::init_empty_session(); + \core\session\manager::set_user(self::$cronuser); + $GLOBALS['SESSION'] = self::$cronsession; + } else { + // Emulate real user session - needed for caps in cron. + if ($GLOBALS['USER']->id != $user->id) { + \core\session\manager::init_empty_session(); + \core\session\manager::set_user($user); + } + } + + // TODO MDL-19774 relying on global $PAGE in cron is a bad idea. + // Temporary hack so that cron does not give fatal errors. + if (!$leavepagealone) { + $PAGE = new \moodle_page(); + $PAGE->set_course($course ?? $SITE); + } + + // TODO: it should be possible to improve perf by caching some limited number of users here. + // phpcs:enable + } + + /** + * Resets the cache for the cron user used by `setup_user()`. + */ + public static function reset_user_cache(): void { + self::$cronuser = null; + self::$cronsession = null; + \core\session\manager::init_empty_session(); } } diff --git a/lib/sessionlib.php b/lib/sessionlib.php index af4c0a6f6bf..9958a6d7288 100644 --- a/lib/sessionlib.php +++ b/lib/sessionlib.php @@ -176,61 +176,20 @@ function get_moodle_cookie() { * value 'reset' means reset internal static caches. * @param stdClass $course full course record, null means $SITE * @param bool $leavepagealone If specified, stops it messing with global page object + * @deprecated since 4.2. Use \core\core::setup_user() instead. * @return void */ function cron_setup_user($user = null, $course = null, $leavepagealone = false) { - global $CFG, $SITE, $PAGE; - - if (!CLI_SCRIPT && !$leavepagealone) { - throw new coding_exception('Function cron_setup_user() cannot be used in normal requests!'); - } - - static $cronuser = NULL; - static $cronsession = NULL; + debugging( + 'The cron_setup_user() function is deprecated. ' . + 'Please use \core\cron::setup_user() and reset_user_cache() as appropriate instead.', + DEBUG_DEVELOPER + ); if ($user === 'reset') { - $cronuser = null; - $cronsession = null; - \core\session\manager::init_empty_session(); + \core\cron::reset_user_cache(); return; } - if (empty($cronuser)) { - /// ignore admins timezone, language and locale - use site default instead! - $cronuser = get_admin(); - $cronuser->timezone = $CFG->timezone; - $cronuser->lang = ''; - $cronuser->theme = ''; - unset($cronuser->description); - - $cronsession = new stdClass(); - } - - if (!$user) { - // Cached default cron user (==modified admin for now). - \core\session\manager::init_empty_session(); - \core\session\manager::set_user($cronuser); - $GLOBALS['SESSION'] = $cronsession; - - } else { - // Emulate real user session - needed for caps in cron. - if ($GLOBALS['USER']->id != $user->id) { - \core\session\manager::init_empty_session(); - \core\session\manager::set_user($user); - } - } - - // TODO MDL-19774 relying on global $PAGE in cron is a bad idea. - // Temporary hack so that cron does not give fatal errors. - if (!$leavepagealone) { - $PAGE = new moodle_page(); - if ($course) { - $PAGE->set_course($course); - } else { - $PAGE->set_course($SITE); - } - } - - // TODO: it should be possible to improve perf by caching some limited number of users here ;-) - + \core\cron::setup_user($user, $course, $leavepagealone); } diff --git a/lib/tests/cron_test.php b/lib/tests/cron_test.php new file mode 100644 index 00000000000..b8760a3f4ed --- /dev/null +++ b/lib/tests/cron_test.php @@ -0,0 +1,159 @@ +. + +namespace core; + +/** + * Tests for core\cron. + * + * @package core + * @copyright 2023 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\cron + */ +class cron_test extends \advanced_testcase { + /** + * Reset relevant caches between tests. + */ + public function setUp(): void { + cron::reset_user_cache(); + } + + /** + * Test the setup_user function. + * + * @covers ::setup_user + * @covers ::reset_user_cache + */ + public function test_setup_user(): void { + // This function uses the $GLOBALS super global. Disable the VariableNameLowerCase sniff for this function. + // phpcs:disable moodle.NamingConventions.ValidVariableName.VariableNameLowerCase + global $PAGE, $USER, $SESSION, $SITE, $CFG; + $this->resetAfterTest(); + + $admin = get_admin(); + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $course = $this->getDataGenerator()->create_course(); + + cron::setup_user(); + $this->assertSame($admin->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertSame($CFG->timezone, $USER->timezone); + $this->assertSame('', $USER->lang); + $this->assertSame('', $USER->theme); + $SESSION->test1 = true; + $adminsession = $SESSION; + $adminuser = $USER; + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user(null, $course); + $this->assertSame($admin->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($course->id)); + $this->assertSame($adminsession, $SESSION); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user($user1); + $this->assertSame($user1->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertNotSame($adminsession, $SESSION); + $this->assertObjectNotHasAttribute('test1', $SESSION); + $this->assertEmpty((array)$SESSION); + $usersession1 = $SESSION; + $SESSION->test2 = true; + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user($user1); + $this->assertSame($user1->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertNotSame($adminsession, $SESSION); + $this->assertSame($usersession1, $SESSION); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user($user2); + $this->assertSame($user2->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertNotSame($adminsession, $SESSION); + $this->assertNotSame($usersession1, $SESSION); + $this->assertEmpty((array)$SESSION); + $usersession2 = $SESSION; + $usersession2->test3 = true; + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user($user2, $course); + $this->assertSame($user2->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($course->id)); + $this->assertNotSame($adminsession, $SESSION); + $this->assertNotSame($usersession1, $SESSION); + $this->assertSame($usersession2, $SESSION); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user($user1); + $this->assertSame($user1->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertNotSame($adminsession, $SESSION); + $this->assertNotSame($usersession1, $SESSION); + $this->assertEmpty((array)$SESSION); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user(); + $this->assertSame($admin->id, $USER->id); + $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); + $this->assertSame($adminsession, $SESSION); + $this->assertSame($adminuser, $USER); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::reset_user_cache(); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + cron::setup_user(); + $this->assertNotSame($adminsession, $SESSION); + $this->assertNotSame($adminuser, $USER); + $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); + $this->assertSame($GLOBALS['SESSION'], $SESSION); + $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); + $this->assertSame($GLOBALS['USER'], $USER); + + // phpcs:enable + } +} diff --git a/lib/tests/sessionlib_test.php b/lib/tests/sessionlib_test.php index 4a9e436c040..f4e40ce08ae 100644 --- a/lib/tests/sessionlib_test.php +++ b/lib/tests/sessionlib_test.php @@ -26,12 +26,20 @@ namespace core; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class sessionlib_test extends \advanced_testcase { + + /** + * @covers ::cron_setup_user + */ public function test_cron_setup_user() { + // This function uses the $GLOBALS super global. Disable the VariableNameLowerCase sniff for this function. + // phpcs:disable moodle.NamingConventions.ValidVariableName.VariableNameLowerCase + global $PAGE, $USER, $SESSION, $SITE, $CFG; $this->resetAfterTest(); // NOTE: this function contains some static caches, let's reset first. cron_setup_user('reset'); + $this->assertDebuggingCalledCount(1); $admin = get_admin(); $user1 = $this->getDataGenerator()->create_user(); @@ -39,6 +47,7 @@ class sessionlib_test extends \advanced_testcase { $course = $this->getDataGenerator()->create_course(); cron_setup_user(); + $this->assertDebuggingCalledCount(1); $this->assertSame($admin->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertSame($CFG->timezone, $USER->timezone); @@ -53,6 +62,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user(null, $course); + $this->assertDebuggingCalledCount(1); $this->assertSame($admin->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($course->id)); $this->assertSame($adminsession, $SESSION); @@ -62,6 +72,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user($user1); + $this->assertDebuggingCalledCount(1); $this->assertSame($user1->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); @@ -75,6 +86,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user($user1); + $this->assertDebuggingCalledCount(1); $this->assertSame($user1->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); @@ -85,6 +97,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user($user2); + $this->assertDebuggingCalledCount(1); $this->assertSame($user2->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); @@ -98,6 +111,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user($user2, $course); + $this->assertDebuggingCalledCount(1); $this->assertSame($user2->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($course->id)); $this->assertNotSame($adminsession, $SESSION); @@ -109,6 +123,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user($user1); + $this->assertDebuggingCalledCount(1); $this->assertSame($user1->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); @@ -120,6 +135,7 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user(); + $this->assertDebuggingCalledCount(1); $this->assertSame($admin->id, $USER->id); $this->assertSame($PAGE->context, \context_course::instance($SITE->id)); $this->assertSame($adminsession, $SESSION); @@ -130,18 +146,22 @@ class sessionlib_test extends \advanced_testcase { $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user('reset'); + $this->assertDebuggingCalledCount(1); $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); $this->assertSame($GLOBALS['SESSION'], $SESSION); $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); $this->assertSame($GLOBALS['USER'], $USER); cron_setup_user(); + $this->assertDebuggingCalledCount(1); $this->assertNotSame($adminsession, $SESSION); $this->assertNotSame($adminuser, $USER); $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); $this->assertSame($GLOBALS['SESSION'], $SESSION); $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); $this->assertSame($GLOBALS['USER'], $USER); + + // phpcs:enable } /**