moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-11 20:06:46 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-61519 calendar: do not iterate through all categories

Browse source 
Replace calls to \coursecat::get_all() or cache the results.
This commit is contained in:
Martin Gauk 2018-03-05 16:16:41 +00:00
parent 263e505748
commit 72901ac846
7 changed files with 146 additions and 77 deletions
Download patch file Download diff file Expand all files Collapse all files

6
calendar/classes/local/event/data_access/event_vault.php
View file

@ -201,10 +201,6 @@ class event_vault implements event_vault_interface {
event_interface $afterevent = null,
$limitnum = 20
) {
$categoryids = array_map(function($category) {
return $category->id;
}, \coursecat::get_all());
$courseids = array_map(function($course) {
return $course->id;
}, enrol_get_all_users_courses($user->id));
@ -227,7 +223,7 @@ class event_vault implements event_vault_interface {
[$user->id],
$groupids ? $groupids : null,
$courseids ? $courseids : null,
$categoryids ? $categoryids : null,
null, // All categories.
true,
true,
function ($event) {

101
calendar/externallib.php
View file

@ -181,11 +181,17 @@ class core_calendar_external extends external_api {
$funcparam = array('courses' => array(), 'groups' => array(), 'categories' => array());
$hassystemcap = has_capability('moodle/calendar:manageentries', context_system::instance());
$warnings = array();
$coursecategories = array();
// Let us findout courses that we can return events from.
// Let us find out courses and their categories that we can return events from.
if (!$hassystemcap) {
$courses = enrol_get_my_courses('id');
$courses = array_keys($courses);
$courseobjs = enrol_get_my_courses();
$courses = array_keys($courseobjs);
$coursecategories = array_flip(array_map(function($course) {
return $course->category;
}, $courseobjs));
foreach ($params['events']['courseids'] as $id) {
try {
$context = context_course::instance($id);
@ -203,6 +209,14 @@ class core_calendar_external extends external_api {
} else {
$courses = $params['events']['courseids'];
$funcparam['courses'] = $courses;
if (!empty($courses)) {
list($wheresql, $sqlparams) = $DB->get_in_or_equal($courses);
$wheresql = "id $wheresql";
$coursecategories = array_flip(array_map(function($course) {
return $course->category;
}, $DB->get_records_select('course', $wheresql, $sqlparams, '', 'category')));
}
}
// Let us findout groups that we can return events from.
@ -223,56 +237,55 @@ class core_calendar_external extends external_api {
$categories = array();
if ($hassystemcap || !empty($courses)) {
// Use the category id as the key in the following array. That way we do not have to remove duplicates and
// have a faster lookup later.
$categories = [];
$coursecategories = array();
if (!empty($courses)) {
list($wheresql, $sqlparams) = $DB->get_in_or_equal($courses);
$wheresql = "id $wheresql";
$courseswithcategory = $DB->get_records_select('course', $wheresql, $sqlparams);
// Grab the list of course categories for the requested course list.
foreach ($courseswithcategory as $course) {
if (empty($course->visible)) {
if (!has_capability('moodle/course:viewhidden', context_course::instance($course->id))) {
continue;
if (!empty($params['events']['categoryids'])) {
$catobjs = \coursecat::get_many(array_merge($params['events']['categoryids'], array_keys($coursecategories)));
foreach ($catobjs as $catobj) {
if (isset($coursecategories[$catobj->id]) ||
has_capability('moodle/category:manage', $catobj->get_context())) {
// If the user has access to a course in this category or can manage the category,
// then they can see all parent categories too.
$categories[$catobj->id] = true;
foreach ($catobj->get_parents() as $catid) {
$categories[$catid] = true;
}
}
$category = \coursecat::get($course->category);
// Fetch parent categories.
$coursecategories = array_merge($coursecategories, [$category->id], $category->get_parents());
}
}
foreach (\coursecat::get_all() as $category) {
// Skip categories not requested.
if (!empty($params['events']['categoryids'])) {
if (!in_array($category->id, $params['events']['categoryids'])) {
continue;
}
}
if (has_capability('moodle/category:manage', $category->get_context())) {
// If a user can manage a category, then they can see all child categories. as well as all parent categories.
$categories[] = $category->id;
foreach (\coursecat::get_all() as $cat) {
if (array_search($category->id, $cat->get_parents()) !== false) {
$categories[] = $cat->id;
$funcparam['categories'] = array_keys($categories);
} else {
// Fetch all categories where this user has any enrolment, and all categories that this user can manage.
$calcatcache = cache::make('core', 'calendar_categories');
// Do not use cache if the user has the system capability as $coursecategories might not represent the
// courses the user is enrolled in.
$categories = (!$hassystemcap) ? $calcatcache->get('site') : false;
if ($categories !== false) {
// The ids are stored in a list in the cache.
$funcparam['categories'] = $categories;
$categories = array_flip($categories);
} else {
$categories = [];
foreach (\coursecat::get_all() as $category) {
if (isset($coursecategories[$category->id]) ||
has_capability('moodle/category:manage', $category->get_context(), $USER, false)) {
// If the user has access to a course in this category or can manage the category,
// then they can see all parent categories too.
$categories[$category->id] = true;
foreach ($category->get_parents() as $catid) {
$categories[$catid] = true;
}
}
}
$categories = array_merge($categories, $category->get_parents());
} else if (in_array($category->id, $coursecategories)) {
// The user has access to a course in this category.
// Fetch all of the parents too.
$categories = array_merge($categories, [$category->id], $category->get_parents());
$categories[] = $category->id;
$funcparam['categories'] = array_keys($categories);
if (!$hassystemcap) {
$calcatcache->set('site', $funcparam['categories']);
}
}
}
}
$funcparam['categories'] = array_unique($categories);
// Do we need user events?
if (!empty($params['options']['userevents'])) {
$funcparam['users'] = array($USER->id);
@ -327,7 +340,7 @@ class core_calendar_external extends external_api {
// Can the user actually see this event?
$eventobj = calendar_event::load($eventobj);
if ((($eventobj->courseid == $SITE->id) && (empty($eventobj->categoryid))) ||
(!empty($eventobj->categoryid) && in_array($eventobj->categoryid, $categories)) ||
(!empty($eventobj->categoryid) && isset($categories[$eventobj->categoryid])) ||
(!empty($eventobj->groupid) && in_array($eventobj->groupid, $groups)) ||
(!empty($eventobj->courseid) && in_array($eventobj->courseid, $courses)) ||
($USER->id == $eventobj->userid) ||

47
calendar/lib.php
View file

@ -1118,7 +1118,7 @@ class calendar_information {
* course will be used.
*
* @param stdClass $course The current course being viewed.
* @param int[] $courses The list of all courses currently accessible.
* @param stdClass[] $courses The list of all courses currently accessible.
* @param stdClass $category The current category to show.
*/
public function set_sources(stdClass $course, array $courses, stdClass $category = null) {
@ -1157,17 +1157,9 @@ class calendar_information {
// Build the category list.
// This includes the current category.
$this->categories = [$category->id];
// All of its descendants.
foreach (\coursecat::get_all() as $cat) {
if (array_search($category->id, $cat->get_parents()) !== false) {
$this->categories[] = $cat->id;
}
}
// And all of its parents.
$this->categories = array_merge($this->categories, $category->get_parents());
$this->categories = $category->get_parents();
$this->categories[] = $category->id;
$this->categories = array_merge($this->categories, $category->get_all_children_ids());
} else if (SITEID === $this->courseid) {
// The site was requested.
// Fetch all categories where this user has any enrolment, and all categories that this user can manage.
@ -1177,26 +1169,25 @@ class calendar_information {
return $course->category;
}, $courses));
$categories = [];
foreach (\coursecat::get_all() as $category) {
if (has_capability('moodle/category:manage', $category->get_context(), $USER, false)) {
// If a user can manage a category, then they can see all child categories. as well as all parent categories.
$categories[] = $category->id;
foreach (\coursecat::get_all() as $cat) {
if (array_search($category->id, $cat->get_parents()) !== false) {
$categories[] = $cat->id;
$calcatcache = cache::make('core', 'calendar_categories');
$this->categories = $calcatcache->get('site');
if ($this->categories === false) {
// Use the category id as the key in the following array. That way we do not have to remove duplicates.
$categories = [];
foreach (\coursecat::get_all() as $category) {
if (isset($coursecategories[$category->id]) ||
has_capability('moodle/category:manage', $category->get_context(), $USER, false)) {
// If the user has access to a course in this category or can manage the category,
// then they can see all parent categories too.
$categories[$category->id] = true;
foreach ($category->get_parents() as $catid) {
$categories[$catid] = true;
}
}
$categories = array_merge($categories, $category->get_parents());
} else if (isset($coursecategories[$category->id])) {
// The user has access to a course in this category.
// Fetch all of the parents too.
$categories = array_merge($categories, [$category->id], $category->get_parents());
$categories[] = $category->id;
}
$this->categories = array_keys($categories);
$calcatcache->set('site', $this->categories);
}
$this->categories = array_unique($categories);
}
}

1
lang/en/cache.php
View file

@ -37,6 +37,7 @@ $string['caching'] = 'Caching';
$string['cacheadmin'] = 'Cache administration';
$string['cacheconfig'] = 'Configuration';
$string['cachedef_calendar_subscriptions'] = 'Calendar subscriptions';
$string['cachedef_calendar_categories'] = 'Calendar course categories that a user can access';
$string['cachedef_capabilities'] = 'System capabilities list';
$string['cachedef_config'] = 'Config settings';
$string['cachedef_coursecat'] = 'Course categories lists for particular user';

26
lib/coursecatlib.php
View file

@ -1171,6 +1171,32 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate {
return $rv;
}
/**
* Returns an array of ids of categories that are (direct and indirect) children
* of this category.
*
* @return int[]
*/
public function get_all_children_ids() {
$coursecattreecache = cache::make('core', 'coursecattree');
$children = $coursecattreecache->get($this->id . 'allchildren');
if ($children === false) {
$children = [];
$walk = [$this->id];
while (count($walk) > 0) {
$catid = array_pop($walk);
$directchildren = self::get_tree($catid);
if ($directchildren !== false && count($directchildren) > 0) {
$walk = array_merge($walk, $directchildren);
$children = array_merge($children, $directchildren);
}
}
$coursecattreecache->set($this->id . 'allchildren', $children);
}
return $children;
}
/**
* Returns true if the user has the manage capability on any category.
*

11
lib/db/caches.php
View file

@ -127,6 +127,17 @@ $definitions = array(
'staticacceleration' => true,
),
// Cache the course categories where the user has any enrolment and all categories that this user can manage.
'calendar_categories' => array(
'mode' => cache_store::MODE_SESSION,
'simplekeys' => true,
'simpledata' => true,
'invalidationevents' => array(
'changesincoursecat',
),
'ttl' => 900,
),
// Cache the capabilities list DB table. See get_all_capabilities in accesslib.
'capabilities' => array(
'mode' => cache_store::MODE_APPLICATION,

31
lib/tests/coursecatlib_test.php
View file

@ -381,6 +381,37 @@ class core_coursecatlib_testcase extends advanced_testcase {
$this->assertEquals(4, $category1->get_children_count());
}
/**
* Test the get_all_children_ids function.
*/
public function test_get_all_children_ids() {
$category1 = coursecat::create(array('name' => 'Cat1'));
$category2 = coursecat::create(array('name' => 'Cat2'));
$category11 = coursecat::create(array('name' => 'Cat11', 'parent' => $category1->id));
$category12 = coursecat::create(array('name' => 'Cat12', 'parent' => $category1->id));
$category13 = coursecat::create(array('name' => 'Cat13', 'parent' => $category1->id));
$category111 = coursecat::create(array('name' => 'Cat111', 'parent' => $category11->id));
$category112 = coursecat::create(array('name' => 'Cat112', 'parent' => $category11->id));
$category1121 = coursecat::create(array('name' => 'Cat1121', 'parent' => $category112->id));
$this->assertCount(0, $category2->get_all_children_ids());
$this->assertCount(6, $category1->get_all_children_ids());
$cmpchildrencat1 = array($category11->id, $category12->id, $category13->id, $category111->id, $category112->id,
$category1121->id);
$childrencat1 = $category1->get_all_children_ids();
// Order of values does not matter. Compare sorted arrays.
sort($cmpchildrencat1);
sort($childrencat1);
$this->assertEquals($cmpchildrencat1, $childrencat1);
$this->assertCount(3, $category11->get_all_children_ids());
$this->assertCount(0, $category111->get_all_children_ids());
$this->assertCount(1, $category112->get_all_children_ids());
$this->assertEquals(array($category1121->id), $category112->get_all_children_ids());
}
/**
* Test the countall function
*/