MDL-54035 accesslib: separate role definition cache clear

Thanks to MDL-49398, we can separate the combined user session and role
definition cache clearing function into two separate functions. At the
same time, we want to identify and remove mark_dirty() calls that were
added for role definition changes but were incorrectly left behind.

Change highlights:
 - Remove unnecessary mark_dirty() calls performed after
   assign_capability(), unassign_capability(), delete_role(),
   deleted contexts, brand new contexts
 - Move role definition cache clear from the user-centric
   accesslib_clear_all_caches() to the newly created,
   role-dedicated accesslib_reset_role_cache()

admin/roles/allow.php

@@ -46,7 +46,6 @@
 
 if (optional_param('submit', false, PARAM_BOOL) && data_submitted() && confirm_sesskey()) {
     $controller->process_submission();
-    $syscontext->mark_dirty();
     $event = null;
     // Create event depending on mode.
     switch ($mode) {

admin/roles/classes/capability_table_with_risks.php

@@ -123,9 +123,6 @@ public function save_changes() {
             assign_capability($changedcap, $this->permissions[$changedcap],
                 $this->roleid, $this->context->id, true);
         }
-
-        // Force accessinfo refresh for users visiting this context.
-        $this->context->mark_dirty();
     }
 
     public function display() {

admin/roles/manage.php

@@ -85,12 +85,10 @@
             die;
         }
         if (!delete_role($roleid)) {
-            // The delete failed, but mark the context dirty in case.
-            $systemcontext->mark_dirty();
+            // The delete failed.
             print_error('cannotdeleterolewithid', 'error', $baseurl, $roleid);
         }
         // Deleted a role sitewide...
-        $systemcontext->mark_dirty();
         redirect($baseurl);
         break;
 

course/classes/category.php

@@ -445,9 +445,6 @@ public static function create($data, $editoroptions = null) {
         $path = $parent->path . '/' . $newcategory->id;
         $DB->set_field('course_categories', 'path', $path, array('id' => $newcategory->id));
 
-        // We should mark the context as dirty.
-        context_coursecat::instance($newcategory->id)->mark_dirty();
-
         fix_course_sortorder();
 
         // If this is data from form results, save embedded files and update description.

course/lib.php

@@ -2450,9 +2450,6 @@ function create_course($data, $editoroptions = NULL) {
     // purge appropriate caches in case fix_course_sortorder() did not change anything
     cache_helper::purge_by_event('changesincourse');
 
-    // new context created - better mark it as dirty
-    $context->mark_dirty();
-
     // Trigger a course created event.
     $event = \core\event\course_created::create(array(
         'objectid' => $course->id,

course/tests/courselib_test.php

@@ -985,7 +985,6 @@ public function test_course_can_delete_section() {
         $modulecontext = context_module::instance($assign1->cmid);
         assign_capability('moodle/course:manageactivities', CAP_PROHIBIT, $roleids['editingteacher'],
             $modulecontext);
-        $modulecontext->mark_dirty();
         $this->assertFalse(course_can_delete_section($courseweeks, 1));
         $this->assertTrue(course_can_delete_section($courseweeks, 2));
 
@@ -3131,7 +3130,6 @@ public function test_course_get_user_navigation_options_for_students() {
         $CFG->enableblogs = 0;
         // Disable view participants capability.
         assign_capability('moodle/course:viewparticipants', CAP_PROHIBIT, $roleid, $context);
-        $context->mark_dirty();
 
         $navoptions = course_get_user_navigation_options($context);
         $this->assertFalse($navoptions->blogs);

grade/tests/report_graderlib_test.php

@@ -287,7 +287,6 @@ public function test_get_right_rows() {
         $context = context_course::instance($course->id);
         $managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager'));
         assign_capability('moodle/grade:viewhidden', CAP_PROHIBIT, $managerroleid, $context->id, true);
-        $context->mark_dirty();
         $this->assertFalse(has_capability('moodle/grade:viewhidden', $context));
 
         // Recreate the report. Confirm it returns successfully still.

lib/accesslib.php

@@ -62,6 +62,8 @@
  * <b>Name conventions</b>
  *
  * "ctx" means context
+ * "ra" means role assignment
+ * "rdef" means role definition
  *
  * <b>accessdata</b>
  *
@@ -72,9 +74,7 @@
  * against userid in $ACCESSLIB_PRIVATE->accessdatabyuser).
  *
  * $accessdata is a multidimensional array, holding
- * role assignments (RAs), role-capabilities-perm sets
- * (role defs) and a list of courses we have loaded
- * data for.
+ * role assignments (RAs), role switches and initialization time.
  *
  * Things are keyed on "contextpaths" (the path field of
  * the context table) for fast walking up/down the tree.
@@ -201,6 +201,7 @@ function accesslib_clear_all_caches_for_unit_testing() {
     }
 
     accesslib_clear_all_caches(true);
+    accesslib_reset_role_cache();
 
     unset($USER->access);
 }
@@ -219,16 +220,32 @@ function accesslib_clear_all_caches($resetcontexts) {
 
     $ACCESSLIB_PRIVATE->dirtycontexts    = null;
     $ACCESSLIB_PRIVATE->accessdatabyuser = array();
-    $ACCESSLIB_PRIVATE->cacheroledefs    = array();
-
-    $cache = cache::make('core', 'roledefs');
-    $cache->purge();
 
     if ($resetcontexts) {
         context_helper::reset_caches();
     }
 }
 
+/**
+ * Full reset of accesslib's private role cache. ONLY TO BE USED FROM THIS LIBRARY FILE!
+ *
+ * This reset does not touch global $USER.
+ *
+ * Note: Only use this when the roles that need a refresh are unknown.
+ *
+ * @see accesslib_clear_role_cache()
+ *
+ * @access private
+ * @return void
+ */
+function accesslib_reset_role_cache() {
+    global $ACCESSLIB_PRIVATE;
+
+    $ACCESSLIB_PRIVATE->cacheroledefs = array();
+    $cache = cache::make('core', 'roledefs');
+    $cache->purge();
+}
+
 /**
  * Clears accesslib's private cache of a specific role or roles. ONLY BE USED FROM THIS LIBRARY FILE!
  *
@@ -1280,8 +1297,6 @@ function delete_role($roleid) {
 /**
  * Function to write context specific overrides, or default capabilities.
  *
- * NOTE: use $context->mark_dirty() after this
- *
  * @param string $capability string name
  * @param int $permission CAP_ constants
  * @param int $roleid role id
@@ -1335,8 +1350,6 @@ function assign_capability($capability, $permission, $roleid, $contextid, $overw
 /**
  * Unassign a capability from a role.
  *
- * NOTE: use $context->mark_dirty() after this
- *
  * @param string $capability the name of the capability
  * @param int $roleid the role id
  * @param int|context $contextid null means all contexts
@@ -2097,9 +2110,6 @@ function reset_role_capabilities($roleid) {
 
     // Reset any cache of this role, including MUC.
     accesslib_clear_role_cache($roleid);
-
-    // Mark the system context dirty.
-    context_system::instance()->mark_dirty();
 }
 
 /**
@@ -2217,7 +2227,7 @@ function update_capabilities($component = 'moodle') {
     capabilities_cleanup($component, $filecaps);
 
     // reset static caches
-    accesslib_clear_all_caches(false);
+    accesslib_reset_role_cache();
 
     // Flush the cached again, as we have changed DB.
     cache::make('core', 'capabilities')->delete('core_capabilities');
@@ -2381,8 +2391,6 @@ function is_inside_frontpage(context $context) {
  * @return stdClass or null if capability not found
  */
 function get_capability_info($capabilityname) {
-    global $ACCESSLIB_PRIVATE, $DB; // one request per page only
-
     $caps = get_all_capabilities();
 
     if (!isset($caps[$capabilityname])) {
@@ -4592,7 +4600,6 @@ function role_change_permission($roleid, $context, $capname, $permission) {
 
     if ($permission == CAP_INHERIT) {
         unassign_capability($capname, $roleid, $context->id);
-        $context->mark_dirty();
         return;
     }
 
@@ -4625,7 +4632,6 @@ function role_change_permission($roleid, $context, $capname, $permission) {
                 // permission already set in parent context or parent - just unset in this context
                 // we do this because we want as few overrides as possible for performance reasons
                 unassign_capability($capname, $roleid, $context->id);
-                $context->mark_dirty();
                 return;
             }
         }
@@ -4639,9 +4645,6 @@ function role_change_permission($roleid, $context, $capname, $permission) {
 
     // assign the needed capability
     assign_capability($capname, $permission, $roleid, $context->id, true);
-
-    // force cap reloading
-    $context->mark_dirty();
 }
 
 
@@ -5066,8 +5069,6 @@ public function update_moved(context $newparent) {
 
         $trans = $DB->start_delegated_transaction();
 
-        $this->mark_dirty();
-
         $setdepth = '';
         if (($newparent->depth +1) != $this->_depth) {
             $diff = $newparent->depth - $this->_depth + 1;
@@ -5189,11 +5190,6 @@ public function delete() {
         $DB->delete_records('context', array('id'=>$this->_id));
         // purge static context cache if entry present
         context::cache_remove($this);
-
-        // do not mark dirty contexts if parents unknown
-        if (!is_null($this->_path) and $this->_depth > 0) {
-            $this->mark_dirty();
-        }
     }
 
     // ====== context level related methods ======

lib/behat/classes/util.php

@@ -354,6 +354,7 @@ public static function reset_all_data() {
 
         // Reset all static caches.
         accesslib_clear_all_caches(true);
+        accesslib_reset_role_cache();
         // Reset the nasty strings list used during the last test.
         nasty_strings::reset_used_strings();
 

lib/moodlelib.php

@@ -5362,8 +5362,6 @@ function reset_course_userdata($data) {
         foreach ($children as $child) {
             role_unassign_all(array('contextid' => $child->id));
         }
-        // Force refresh for logged in users.
-        $context->mark_dirty();
         $status[] = array('component' => $componentstr, 'item' => get_string('deletelocalroles', 'role'), 'error' => false);
     }
 

lib/phpunit/classes/util.php

@@ -208,6 +208,7 @@ public static function reset_all_data($detectchanges = false) {
         // reset all static caches
         \core\event\manager::phpunit_reset();
         accesslib_clear_all_caches(true);
+        accesslib_reset_role_cache();
         get_string_manager()->reset_caches(true);
         reset_text_filters_cache(true);
         core_text::reset_caches();

lib/tests/accesslib_test.php

@@ -196,7 +196,6 @@ public function test_is_enrolled() {
 
         // Prevent the capability for this user role.
         assign_capability($capability, CAP_PROHIBIT, $role->id, $coursecontext);
-        $coursecontext->mark_dirty();
         $this->assertFalse(has_capability($capability, $coursecontext, $user->id));
 
         // Again, we seed the cache first by checking initial enrolment,
@@ -3212,7 +3211,7 @@ public function test_permission_evaluation() {
         $context = context_course::instance($course->id);
         $this->assertEquals($categorycontext, $context->get_parent_context());
         $dirty = get_cache_flags('accesslib/dirtycontexts', time()-2);
-        $this->assertTrue(isset($dirty[$oldpath]));
+        $this->assertFalse(isset($dirty[$oldpath]));
         $this->assertTrue(isset($dirty[$context->path]));
 
 
@@ -3238,7 +3237,7 @@ public function test_permission_evaluation() {
         $DB->delete_records('cache_flags', array());
         $context->delete(); // Should delete also linked blocks.
         $dirty = get_cache_flags('accesslib/dirtycontexts', time()-2);
-        $this->assertTrue(isset($dirty[$context->path]));
+        $this->assertFalse(isset($dirty[$context->path]));
         $this->assertFalse($DB->record_exists('context', array('id'=>$context->id)));
         $this->assertFalse($DB->record_exists('context', array('id'=>$bicontext->id)));
         $this->assertFalse($DB->record_exists('context', array('contextlevel'=>CONTEXT_MODULE, 'instanceid'=>$testpages[4])));
@@ -3258,7 +3257,7 @@ public function test_permission_evaluation() {
         $DB->delete_records('cache_flags', array());
         context_helper::delete_instance(CONTEXT_COURSE, $lastcourse);
         $dirty = get_cache_flags('accesslib/dirtycontexts', time()-2);
-        $this->assertTrue(isset($dirty[$coursecontext->path]));
+        $this->assertFalse(isset($dirty[$coursecontext->path]));
         $this->assertEquals(0, context_inspection::test_context_cache_size());
         $this->assertFalse($DB->record_exists('context', array('contextlevel'=>CONTEXT_COURSE, 'instanceid'=>$lastcourse)));
         context_course::instance($lastcourse);

lib/tests/questionlib_test.php

@@ -1669,7 +1669,6 @@ public function test_question_has_capability_on_using_stdclass($capabilities, $c
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $context->id);
         }
-        $context->mark_dirty();
 
         $this->setUser($user);
 
@@ -1719,7 +1718,6 @@ public function test_question_has_capability_on_using_question_definition($capab
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $context->id);
         }
-        $context->mark_dirty();
 
         // Create the question.
         $qtype = 'truefalse';
@@ -1771,7 +1769,6 @@ public function test_question_has_capability_on_using_question_id($capabilities,
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $context->id);
         }
-        $context->mark_dirty();
 
         // Create the question.
         $qtype = 'truefalse';
@@ -1823,7 +1820,6 @@ public function test_question_has_capability_on_using_question_string_id($capabi
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $context->id);
         }
-        $context->mark_dirty();
 
         // Create the question.
         $qtype = 'truefalse';
@@ -1881,8 +1877,6 @@ public function test_question_has_capability_on_using_moved_question($capabiliti
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $newcontext->id);
         }
-        $context->mark_dirty();
-        $newcontext->mark_dirty();
 
         // Create the question.
         $qtype = 'truefalse';
@@ -1938,7 +1932,6 @@ public function test_question_has_capability_on_using_question($capabilities, $c
         foreach ($capabilities as $capname => $capvalue) {
             assign_capability($capname, $capvalue, $roleid, $context->id);
         }
-        $context->mark_dirty();
 
         // Create the question.
         $question = $questiongenerator->create_question('truefalse', null, [

message/tests/api_test.php

@@ -254,7 +254,6 @@ public function test_search_users() {
         // Remove the viewparticipants capability from one of the courses.
         $course5context = context_course::instance($course5->id);
         assign_capability('moodle/course:viewparticipants', CAP_PROHIBIT, $role->id, $course5context->id);
-        $course5context->mark_dirty();
 
         // Perform a search.
         list($contacts, $courses, $noncontacts) = \core_message\api::search_users($user1->id, 'search');

mod/feedback/tests/lib_test.php

@@ -279,7 +279,6 @@ public function test_feedback_core_calendar_provide_event_action_can_not_submit(
 
         $this->setUser($user);
         assign_capability('mod/feedback:complete', CAP_PROHIBIT, $studentrole->id, $context);
-        $context->mark_dirty();
 
         $factory = new \core_calendar\action_factory();
         $action = mod_feedback_core_calendar_provide_event_action($event, $factory);

mod/forum/tests/subscriptions_test.php

@@ -155,7 +155,6 @@ public function test_unsubscribable_forums() {
         $roleids = $DB->get_records_menu('role', null, '', 'shortname, id');
         $context = \context_course::instance($course->id);
         assign_capability('moodle/course:viewhiddenactivities', CAP_ALLOW, $roleids['student'], $context);
-        $context->mark_dirty();
 
         // All of the unsubscribable forums should now be listed.
         $result = \mod_forum\subscriptions::get_unsubscribable_forums();
@@ -906,7 +905,6 @@ public function test_force_subscribed_to_forum() {
         $cm = get_coursemodule_from_instance('forum', $forum->id);
         $context = \context_module::instance($cm->id);
         assign_capability('mod/forum:allowforcesubscribe', CAP_PROHIBIT, $roleids['student'], $context);
-        $context->mark_dirty();
         $this->assertFalse(has_capability('mod/forum:allowforcesubscribe', $context, $user->id));
 
         // Check that the user is no longer subscribed to the forum.

tag/tests/external_test.php

@@ -102,7 +102,6 @@ public function test_update_tags() {
         // User with editing and manage cap can also change the tag name,
         // make it standard and reset flag.
         assign_capability('moodle/tag:manage', CAP_ALLOW, $roleid, $context->id);
-        $context->mark_dirty();
         $this->assertTrue(has_capability('moodle/tag:manage', $context));
         $result = core_tag_external::update_tags(array($updatetag));
         $result = external_api::clean_returnvalue(core_tag_external::update_tags_returns(), $result);