Index: message/index.php =================================================================== --- message/index.php (revision 1.41) +++ message/index.php (revision ) @@ -140,9 +140,8 @@ $messageerror = get_string('userisblockingyou', 'message'); } } - $userpreferences = get_user_preferences(NULL, NULL, $user2->id); - if (!empty($userpreferences['message_blocknoncontacts'])) { // User is blocking non-contacts + if (get_user_preferences('message_blocknoncontacts', 0, $user2->id)) { // User is blocking non-contacts if (empty($contact)) { // We are not a contact! $messageerror = get_string('userisblockingyounoncontact', 'message'); } Index: lib/moodlelib.php =================================================================== --- lib/moodlelib.php (revision 1.1486) +++ lib/moodlelib.php (revision ) @@ -1236,7 +1236,7 @@ return true; } - if ($f = $DB->get_record('cache_flags', array('name'=>$name, 'flagtype'=>$type), '*', IGNORE_MULTIPLE)) { // this is a potentail problem in DEBUG_DEVELOPER + if ($f = $DB->get_record('cache_flags', array('name'=>$name, 'flagtype'=>$type), '*', IGNORE_MULTIPLE)) { // this is a potential problem in DEBUG_DEVELOPER if ($f->value == $value and $f->expiry == $expiry and $f->timemodified == $timemodified) { return true; //no need to update; helps rcache too } @@ -1284,137 +1284,154 @@ /// FUNCTIONS FOR HANDLING USER PREFERENCES //////////////////////////////////// /** - * Refresh current $USER session global variable with all their current preferences. + * Refresh user preference cache. This is used most often for $USER + * object that is stored in session, but it also helps with performance in cron script. * - * @global object - * @param mixed $time default null + * Preferences for each user are loaded on first use on every page, then again after the timeout expires. + * + * @param stdClass $user user object, preferences are preloaded into ->preference property + * @param int $cachelifetime cache life time on the current page (ins seconds) * @return void */ -function check_user_preferences_loaded($time = null) { - global $USER, $DB; - static $timenow = null; // Static cache, so we only check up-to-dateness once per request. +function check_user_preferences_loaded(stdClass $user, $cachelifetime = 120) { + global $DB; + static $loadedusers = array(); // Static cache, we need to check on each page load, not only every 2 minutes. - if (!empty($USER->preference) && isset($USER->preference['_lastloaded'])) { - // Already loaded. Are we up to date? + if (!isset($user->id)) { + throw new coding_exception('Invalid $user parameter in check_user_preferences_loaded() call, missing id field'); + } - if (is_null($timenow) || (!is_null($time) && $time != $timenow)) { - $timenow = time(); - if (!get_cache_flag('userpreferenceschanged', $USER->id, $USER->preference['_lastloaded'])) { - // We are up-to-date. - return; + if (empty($user->id) or isguestuser($user->id)) { + // No permanent storage for not-logged-in users and guest + if (!isset($user->preference)) { + $user->preference = array(); - } + } - } else { - // Already checked for up-to-date-ness. - return; - } + return; + } - } - // OK, so we have to reload. Reset preference - $USER->preference = array(); + $timenow = time(); - if (!isloggedin() or isguestuser()) { - // No permanent storage for not-logged-in user and guest + if (isset($loadedusers[$user->id]) and isset($user->preference) and isset($user->preference['_lastloaded'])) { + // Already loaded at least once on this page. Are we up to date? + if ($user->preference['_lastloaded'] + $cachelifetime > $timenow) { + // no need to reload - we are on the same page and we loaded prefs just a moment ago + return; - } else if ($preferences = $DB->get_records('user_preferences', array('userid'=>$USER->id))) { - foreach ($preferences as $preference) { - $USER->preference[$preference->name] = $preference->value; + } else if (!get_cache_flag('userpreferenceschanged', $user->id, $user->preference['_lastloaded'])) { + // no change since the lastcheck on this page + $user->preference['_lastloaded'] = $timenow; + return; } } - $USER->preference['_lastloaded'] = $timenow; + // OK, so we have to reload all preferences + $loadedusers[$user->id] = true; + $user->preference = $DB->get_records_menu('user_preferences', array('userid'=>$user->id), '', 'name,value'); // All values + $user->preference['_lastloaded'] = $timenow; } /** - * Called from set/delete_user_preferences, so that the prefs can be correctly reloaded. + * Called from set/delete_user_preferences, so that the prefs can + * be correctly reloaded in different sessions. * - * @global object - * @global object + * NOTE: internal function, do not call from other code. + * * @param integer $userid the user whose prefs were changed. + * @return void */ function mark_user_preferences_changed($userid) { - global $CFG, $USER; - if ($userid == $USER->id) { - check_user_preferences_loaded(time()); + global $CFG; + + if (empty($userid) or isguestuser($userid)) { + // no cache flags for guest and not-logged-in users + return; } + set_cache_flag('userpreferenceschanged', $userid, 1, time() + $CFG->sessiontimeout); } /** - * Sets a preference for the current user + * Sets a preference for the specified user. * - * Optionally, can set a preference for a different user object + * If user object submitted, 'preference' property contains the preferences cache. * - * @todo Add a better description and include usage examples. Add inline links to $USER and user functions in above line. - * - * @global object - * @global object * @param string $name The key to set as preference for the specified user - * @param string $value The value to set for the $name key in the specified user's record - * @param int $otheruserid A moodle user ID, default null - * @return bool + * @param string $value The value to set for the $name key in the specified user's record, + * null means delete current value + * @param stdClass|int $user A moodle user object or id, null means current user + * @return bool always true or exception */ -function set_user_preference($name, $value, $otheruserid=NULL) { +function set_user_preference($name, $value, $user = null) { global $USER, $DB; - if (empty($name)) { - return false; + if (empty($name) or is_numeric($name) or $name === '_lastloaded') { + throw new coding_exception('Invalid preference name in set_user_preference() call'); } - $nostore = false; - if (empty($otheruserid)){ - if (!isloggedin() or isguestuser()) { - $nostore = true; + if (is_null($value)) { + // null means delete current + return unset_user_preference($name, $user); + } else if (is_object($value)) { + throw new coding_exception('Invalid value in set_user_preference() call, objects are not allowed'); + } else if (is_array($value)) { + throw new coding_exception('Invalid value in set_user_preference() call, arrays are not allowed'); - } + } - $userid = $USER->id; + $value = (string)$value; + + if (is_null($user)) { + $user = $USER; + } else if (isset($user->id)) { + // $user is valid object + } else if (is_numeric($user)) { + $user = (object)array('id'=>(int)$user); } else { - if (isguestuser($otheruserid)) { - $nostore = true; + throw new coding_exception('Invalid $user parameter in set_user_preference() call'); - } + } - $userid = $otheruserid; - } - if ($nostore) { - // no permanent storage for not-logged-in user and guest + check_user_preferences_loaded($user); - } else if ($preference = $DB->get_record('user_preferences', array('userid'=>$userid, 'name'=>$name))) { - if ($preference->value === $value) { + if (empty($user->id) or isguestuser($user->id)) { + // no permanent storage for not-logged-in users and guest + $user->preference[$name] = $value; - return true; - } + return true; + } - $DB->set_field('user_preferences', 'value', (string)$value, array('id'=>$preference->id)); + if ($preference = $DB->get_record('user_preferences', array('userid'=>$user->id, 'name'=>$name))) { + if ($preference->value === $value and isset($user->preference[$name]) and $user->preference[$name] === $value) { + // preference already set to this value + return true; + } + $DB->set_field('user_preferences', 'value', $value, array('id'=>$preference->id)); + } else { $preference = new stdClass(); - $preference->userid = $userid; + $preference->userid = $user->id; $preference->name = $name; - $preference->value = (string)$value; + $preference->value = $value; $DB->insert_record('user_preferences', $preference); } - mark_user_preferences_changed($userid); - // update value in USER session if needed - if ($userid == $USER->id) { - $USER->preference[$name] = (string)$value; - $USER->preference['_lastloaded'] = time(); - } + // update value in cache + $user->preference[$name] = $value; + // set reload flag for other sessions + mark_user_preferences_changed($user->id); + return true; } /** * Sets a whole array of preferences for the current user * + * If user object submitted, 'preference' property contains the preferences cache. + * * @param array $prefarray An array of key/value pairs to be set - * @param int $otheruserid A moodle user ID - * @return bool + * @param stdClass|int $user A moodle user object or id, null means current user + * @return bool always true or exception */ -function set_user_preferences($prefarray, $otheruserid=NULL) { - - if (!is_array($prefarray) or empty($prefarray)) { - return false; - } - +function set_user_preferences(array $prefarray, $user = null) { foreach ($prefarray as $name => $value) { - set_user_preference($name, $value, $otheruserid); + set_user_preference($name, $value, $user); } return true; } @@ -1422,32 +1439,46 @@ /** * Unsets a preference completely by deleting it from the database * - * Optionally, can set a preference for a different user id + * If user object submitted, 'preference' property contains the preferences cache. * - * @global object * @param string $name The key to unset as preference for the specified user - * @param int $otheruserid A moodle user ID + * @param stdClass|int $user A moodle user object or id, null means current user + * @return bool always true or exception */ -function unset_user_preference($name, $otheruserid=NULL) { +function unset_user_preference($name, $user = null) { global $USER, $DB; - if (empty($otheruserid)){ - $userid = $USER->id; - check_user_preferences_loaded(); + if (empty($name) or is_numeric($name) or $name === '_lastloaded') { + throw new coding_exception('Invalid preference name in unset_user_preference() call'); + } + + if (is_null($user)) { + $user = $USER; + } else if (isset($user->id)) { + // $user is valid object + } else if (is_numeric($user)) { + $user = (object)array('id'=>(int)$user); } else { - $userid = $otheruserid; + throw new coding_exception('Invalid $user parameter in unset_user_preference() call'); } - //Then from DB - $DB->delete_records('user_preferences', array('userid'=>$userid, 'name'=>$name)); + check_user_preferences_loaded($user); - mark_user_preferences_changed($userid); - //Delete the preference from $USER if needed - if ($userid == $USER->id) { - unset($USER->preference[$name]); - $USER->preference['_lastloaded'] = time(); + if (empty($user->id) or isguestuser($user->id)) { + // no permanent storage for not-logged-in user and guest + unset($user->preference[$name]); + return; } + // delete from DB + $DB->delete_records('user_preferences', array('userid'=>$user->id, 'name'=>$name)); + + // delete the preference from cache + unset($user->preference[$name]); + + // set reload flag for other sessions + mark_user_preferences_changed($user->id); + return true; } @@ -1462,37 +1493,42 @@ * none is found, then the optional value $default is returned, * otherwise NULL. * - * @global object - * @global object + * If user object submitted, 'preference' property contains the preferences cache. + * * @param string $name Name of the key to use in finding a preference value - * @param string $default Value to be returned if the $name key is not set in the user preferences - * @param int $otheruserid A moodle user ID - * @return string + * @param mixed $default Value to be returned if the $name key is not set in the user preferences + * @param stdClass|int $user A moodle user object or id, null means current user + * @return mixed string value or default */ -function get_user_preferences($name=NULL, $default=NULL, $otheruserid=NULL) { - global $USER, $DB; +function get_user_preferences($name = null, $default = null, $user = null) { + global $USER; - if (empty($otheruserid) || (isloggedin() && ($USER->id == $otheruserid))){ - check_user_preferences_loaded(); + if (is_null($name)) { + // all prefs + } else if (is_numeric($name) or $name === '_lastloaded') { + throw new coding_exception('Invalid preference name in get_user_preferences() call'); + } - if (empty($name)) { - return $USER->preference; // All values - } else if (array_key_exists($name, $USER->preference)) { - return $USER->preference[$name]; // The single value + if (is_null($user)) { + $user = $USER; + } else if (isset($user->id)) { + // $user is valid object + } else if (is_numeric($user)) { + $user = (object)array('id'=>(int)$user); - } else { + } else { - return $default; // Default value (or NULL) + throw new coding_exception('Invalid $user parameter in get_user_preferences() call'); - } + } - } else { + check_user_preferences_loaded($user); + - if (empty($name)) { + if (empty($name)) { - return $DB->get_records_menu('user_preferences', array('userid'=>$otheruserid), '', 'name,value'); // All values - } else if ($value = $DB->get_field('user_preferences', 'value', array('userid'=>$otheruserid, 'name'=>$name))) { - return $value; // The single value + return $user->preference; // All values + } else if (isset($user->preference[$name])) { + return $user->preference[$name]; // The single string value - } else { + } else { - return $default; // Default value (or NULL) + return $default; // Default value (or NULL if not specified) - } - } + } +} -} /// FUNCTIONS FOR HANDLING TIME //////////////////////////////////////////// @@ -3422,7 +3458,7 @@ $DB->insert_record('user', $newuser); $user = get_complete_user_data('username', $newuser->username); - if(!empty($CFG->{'auth_'.$newuser->auth.'_forcechangepassword'})){ + if (!empty($CFG->{'auth_'.$newuser->auth.'_forcechangepassword'})){ set_user_preference('auth_forcepasswordchange', 1, $user->id); } update_internal_user_password($user, $password); @@ -3747,10 +3783,14 @@ // check enrolments, load caps and setup $USER object session_set_user($user); - // clear the preferences and invalidate caches too - unset($USER->preference); + // reload preferences from DB + unset($user->preference); + check_user_preferences_loaded($user); + // update login times update_user_login_times(); + + // extra session prefs init set_login_session_preferences(); if (isguestuser()) { @@ -3875,15 +3915,12 @@ * * Intended for setting as $USER session variable * - * @global object - * @global object - * @uses SITEID * @param string $field The user field to be checked for a given value. * @param string $value The value to match for $field. * @param int $mnethostid * @return mixed False, or A {@link $USER} object. */ -function get_complete_user_data($field, $value, $mnethostid=null) { +function get_complete_user_data($field, $value, $mnethostid = null) { global $CFG, $DB; if (!$field || !$value) { @@ -3921,9 +3958,10 @@ } } - $user->preference = get_user_preferences(null, null, $user->id); - $user->preference['_lastloaded'] = time(); + // preload preference cache + check_user_preferences_loaded($user); + // load course enrolment related stuff $user->lastcourseaccess = array(); // during last session $user->currentcourseaccess = array(); // during current session if ($lastaccesses = $DB->get_records('user_lastaccess', array('userid'=>$user->id))) { Index: message/send.php =================================================================== --- message/send.php (revision 1.46) +++ message/send.php (revision ) @@ -59,9 +59,8 @@ exit; } } - $userpreferences = get_user_preferences(NULL, NULL, $user->id); - if (!empty($userpreferences['message_blocknoncontacts'])) { // User is blocking non-contacts + if (get_user_preferences('message_blocknoncontacts', 0, $user->id)) { // User is blocking non-contacts if (empty($contact)) { // We are not a contact! echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('userisblockingyounoncontact', 'message'), 1); Index: lib/simpletest/testmoodlelib.php =================================================================== --- lib/simpletest/testmoodlelib.php (revision 1.41) +++ lib/simpletest/testmoodlelib.php (revision ) @@ -518,4 +518,165 @@ $this->assertEqual(normalize_component('whothefuck_would_come_withsuchastupidnameofcomponent'), array('mod', 'whothefuck_would_come_withsuchastupidnameofcomponent')); } + + protected function get_preference_test_user() { + global $DB; + + // we need some nonexistent user id + $id = 2147483647 - 666; + if ($DB->get_records('user', array('id'=>$id))) { + //weird! + return false; -} + } + return $id; + } + + public function test_mark_user_preferences_changed() { + if (!$otheruserid = $this->get_preference_test_user()) { + $this->fail('Can not find unused user id for the preferences test'); + return; + } + + set_cache_flag('userpreferenceschanged', $otheruserid, NULL); + mark_user_preferences_changed($otheruserid); + + $this->assertEqual(get_cache_flag('userpreferenceschanged', $otheruserid, time()-10), 1); + set_cache_flag('userpreferenceschanged', $otheruserid, NULL); + } + + public function test_check_user_preferences_loaded() { + global $DB; + + if (!$otheruserid = $this->get_preference_test_user()) { + $this->fail('Can not find unused user id for the preferences test'); + return; + } + + $DB->delete_records('user_preferences', array('userid'=>$otheruserid)); + set_cache_flag('userpreferenceschanged', $otheruserid, NULL); + + $user = new stdClass(); + $user->id = $otheruserid; + + // load + check_user_preferences_loaded($user); + $this->assertTrue(isset($user->preference)); + $this->assertTrue(is_array($user->preference)); + $this->assertTrue(isset($user->preference['_lastloaded'])); + $this->assertEqual(count($user->preference), 1); + + // add preference via direct call + $DB->insert_record('user_preferences', array('name'=>'xxx', 'value'=>'yyy', 'userid'=>$user->id)); + + // no cache reload yet + check_user_preferences_loaded($user); + $this->assertEqual(count($user->preference), 1); + + // forced reloading of cache + unset($user->preference); + check_user_preferences_loaded($user); + $this->assertEqual(count($user->preference), 2); + $this->assertEqual($user->preference['xxx'], 'yyy'); + + // add preference via direct call + $DB->insert_record('user_preferences', array('name'=>'aaa', 'value'=>'bbb', 'userid'=>$user->id)); + + // test timeouts and modifications from different session + set_cache_flag('userpreferenceschanged', $user->id, 1, time() + 1000); + $user->preference['_lastloaded'] = $user->preference['_lastloaded'] - 20; + check_user_preferences_loaded($user); + $this->assertEqual(count($user->preference), 2); + check_user_preferences_loaded($user, 10); + $this->assertEqual(count($user->preference), 3); + $this->assertEqual($user->preference['aaa'], 'bbb'); + set_cache_flag('userpreferenceschanged', $user->id, null); + } + + public function test_set_user_preference() { + global $DB, $USER; + + if (!$otheruserid = $this->get_preference_test_user()) { + $this->fail('Can not find empty unused id for the preferences test'); + return; + } + + $DB->delete_records('user_preferences', array('userid'=>$otheruserid)); + set_cache_flag('userpreferenceschanged', $otheruserid, null); + + $user = new stdClass(); + $user->id = $otheruserid; + + set_user_preference('aaa', 'bbb', $otheruserid); + $this->assertEqual('bbb', $DB->get_field('user_preferences', 'value', array('userid'=>$otheruserid, 'name'=>'aaa'))); + $this->assertEqual('bbb', get_user_preferences('aaa', null, $otheruserid)); + + set_user_preference('xxx', 'yyy', $user); + $this->assertEqual('yyy', $DB->get_field('user_preferences', 'value', array('userid'=>$otheruserid, 'name'=>'xxx'))); + $this->assertEqual('yyy', get_user_preferences('xxx', null, $otheruserid)); + $this->assertTrue(is_array($user->preference)); + $this->assertEqual($user->preference['aaa'], 'bbb'); + $this->assertEqual($user->preference['xxx'], 'yyy'); + + set_user_preference('xxx', NULL, $user); + $this->assertIdentical(false, $DB->get_field('user_preferences', 'value', array('userid'=>$otheruserid, 'name'=>'xxx'))); + $this->assertIdentical(null, get_user_preferences('xxx', null, $otheruserid)); + + set_user_preference('ooo', true, $user); + $prefs = get_user_preferences(null, null, $otheruserid); + $this->assertIdentical($prefs['aaa'], $user->preference['aaa']); + $this->assertIdentical($prefs['ooo'], $user->preference['ooo']); + $this->assertIdentical($prefs['ooo'], '1'); + + set_user_preference('null', 0, $user); + $this->assertIdentical('0', get_user_preferences('null', null, $otheruserid)); + + $this->assertIdentical('lala', get_user_preferences('undefined', 'lala', $otheruserid)); + + $DB->delete_records('user_preferences', array('userid'=>$otheruserid)); + set_cache_flag('userpreferenceschanged', $otheruserid, null); + + // test $USER default + set_user_preference('_test_user_preferences_pref', 'ok'); + $this->assertIdentical('ok', $USER->preference['_test_user_preferences_pref']); + unset_user_preference('_test_user_preferences_pref'); + $this->assertTrue(!isset($USER->preference['_test_user_preferences_pref'])); + + //test invalid params + try { + set_user_preference('_test_user_preferences_pref', array()); + $this->assertFail('Exception expected - array not valid preference value'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + try { + set_user_preference('_test_user_preferences_pref', new stdClass); + $this->assertFail('Exception expected - class not valid preference value'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + try { + set_user_preference('_test_user_preferences_pref', 1, array('xx'=>1)); + $this->assertFail('Exception expected - user instance expected'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + try { + set_user_preference('_test_user_preferences_pref', 1, 'abc'); + $this->assertFail('Exception expected - user instance expected'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + try { + set_user_preference('', 1); + $this->assertFail('Exception expected - invalid name accepted'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + try { + set_user_preference('1', 1); + $this->assertFail('Exception expected - invalid name accepted'); + } catch (Exception $ex) { + $this->assertTrue(true); + } + } +}