From eabbfa82d3ba42c6c55d59af98a2cfb6a1c1b0c4 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Thu, 28 Feb 2013 12:29:15 +1100 Subject: [PATCH] MDL-38147 Improvements to caching of course categories, coursecat::get_children() improvements - Course categories caches are purged by event now - session cache has additional 10 minutes ttl to clear itself and accomodate for permission changes that do not trigger event purging - additional request-level cache for coursecat::get() - We store only children of one category in one cache key - Function coursecat::get_children() can return results sorted and/or paginated. Added tests --- course/lib.php | 5 + course/manage.php | 3 + lang/en/cache.php | 4 +- lib/coursecatlib.php | 338 ++++++++++++++++++++++++-------- lib/datalib.php | 32 ++- lib/db/caches.php | 25 ++- lib/tests/coursecatlib_test.php | 42 +++- 7 files changed, 361 insertions(+), 88 deletions(-) diff --git a/course/lib.php b/course/lib.php index ac830dc783e..e2ec83c2745 100644 --- a/course/lib.php +++ b/course/lib.php @@ -2649,6 +2649,7 @@ function move_courses($courseids, $categoryid) { } } fix_course_sortorder(); + cache_helper::purge_by_event('changesincourse'); return true; } @@ -2828,6 +2829,8 @@ function create_course($data, $editoroptions = NULL) { course_create_sections_if_missing($course, 0); fix_course_sortorder(); + // 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 mark_context_dirty($context->path); @@ -2908,6 +2911,8 @@ function update_course($data, $editoroptions = NULL) { } fix_course_sortorder(); + // purge appropriate caches in case fix_course_sortorder() did not change anything + cache_helper::purge_by_event('changesincourse'); // Test for and remove blocks which aren't appropriate anymore blocks_remove_inappropriate($course); diff --git a/course/manage.php b/course/manage.php index f11a9ae7bbe..128e7a7a18a 100644 --- a/course/manage.php +++ b/course/manage.php @@ -188,6 +188,7 @@ if ($coursecat->id && $canmanage && $resort && confirm_sesskey()) { } // This should not be needed but we do it just to be safe. fix_course_sortorder(); + cache_helper::purge_by_event('changesincourse'); } } @@ -233,6 +234,7 @@ if ((!empty($hide) or !empty($show)) && confirm_sesskey()) { // Set the visibility of the course. we set the old flag when user manually changes visibility of course. $params = array('id' => $course->id, 'visible' => $visible, 'visibleold' => $visible, 'timemodified' => time()); $DB->update_record('course', $params); + cache_helper::purge_by_event('changesincourse'); add_to_log($course->id, "course", ($visible ? 'show' : 'hide'), "edit.php?id=$course->id", $course->id); } @@ -260,6 +262,7 @@ if ((!empty($moveup) or !empty($movedown)) && confirm_sesskey()) { } $DB->set_field('course', 'sortorder', $swapcourse->sortorder, array('id' => $movecourse->id)); $DB->set_field('course', 'sortorder', $movecourse->sortorder, array('id' => $swapcourse->id)); + cache_helper::purge_by_event('changesincourse'); add_to_log($movecourse->id, "course", "move", "edit.php?id=$movecourse->id", $movecourse->id); } } diff --git a/lang/en/cache.php b/lang/en/cache.php index 712d06c41f5..d081b61cb28 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -36,7 +36,9 @@ $string['cacheadmin'] = 'Cache administration'; $string['cacheconfig'] = 'Configuration'; $string['cachedef_calendar_subscriptions'] = 'Calendar subscriptions'; $string['cachedef_config'] = 'Config settings'; -$string['cachedef_coursecat'] = 'Course categories'; +$string['cachedef_coursecat'] = 'Course categories lists for particular user'; +$string['cachedef_coursecatrecords'] = 'Course categories records'; +$string['cachedef_coursecattree'] = 'Course categories tree'; $string['cachedef_databasemeta'] = 'Database meta information'; $string['cachedef_eventinvalidation'] = 'Event invalidation'; $string['cachedef_groupdata'] = 'Course group information'; diff --git a/lib/coursecatlib.php b/lib/coursecatlib.php index 2ff0a33be55..96576b42541 100644 --- a/lib/coursecatlib.php +++ b/lib/coursecatlib.php @@ -208,7 +208,6 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { * @return null|coursecat */ public static function get($id, $strictness = MUST_EXIST, $alwaysreturnhidden = false) { - global $DB; if (!$id) { if (!isset(self::$coursecat0)) { $record = new stdClass(); @@ -220,24 +219,14 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { } return self::$coursecat0; } - $coursecatcache = cache::make('core', 'coursecat'); - $coursecat = $coursecatcache->get($id); + $coursecatrecordcache = cache::make('core', 'coursecatrecords'); + $coursecat = $coursecatrecordcache->get($id); if ($coursecat === false) { - $all = self::get_all_ids(); - if (array_key_exists($id, $all)) { - // Retrieve from DB only the fields that need to be stored in cache - $fields = array_filter(array_keys(self::$coursecatfields), function ($element) - { return (self::$coursecatfields[$element] !== null); } ); - $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); - $sql = "SELECT cc.". join(',cc.', $fields). ", $ctxselect - FROM {course_categories} cc - JOIN {context} ctx ON cc.id = ctx.instanceid AND ctx.contextlevel = ? - WHERE cc.id = ?"; - if ($record = $DB->get_record_sql($sql, array(CONTEXT_COURSECAT, $id))) { - $coursecat = new coursecat($record); - // Store in cache - $coursecatcache->set($id, $coursecat); - } + if ($records = self::get_records('cc.id = :id', array('id' => $id))) { + $record = reset($records); + $coursecat = new coursecat($record); + // Store in cache + $coursecatrecordcache->set($id, $coursecat); } } if ($coursecat && ($alwaysreturnhidden || $coursecat->is_uservisible())) { @@ -262,8 +251,8 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { if ($visiblechildren = self::get(0)->get_children()) { $defcategory = reset($visiblechildren); } else { - $all = $this->get_all_ids(); - $defcategoryid = $all[0][0]; + $toplevelcategories = self::get_tree(0); + $defcategoryid = $toplevelcategories[0]; $defcategory = self::get($defcategoryid, MUST_EXIST, true); } return $defcategory; @@ -377,11 +366,10 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { // update only fields description and descriptionformat $updatedata = array_intersect_key((array)$newcategory, array('id' => 1, 'description' => 1, 'descriptionformat' => 1)); $DB->update_record('course_categories', $updatedata); - - self::purge_cache(); } add_to_log(SITEID, "category", 'add', "editcategory.php?id=$newcategory->id", $newcategory->id); + cache_helper::purge_by_event('changesincoursecat'); return self::get($newcategory->id, MUST_EXIST, true); } @@ -460,7 +448,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { if (isset($data->parent) && $data->parent != $this->parent) { if ($changes) { - self::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); } $parentcat = self::get($data->parent, MUST_EXIST, true); $this->change_parent_raw($parentcat); @@ -476,6 +464,8 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { $DB->update_record('course_categories', $newcategory); add_to_log(SITEID, "category", 'update', "editcategory.php?id=$this->id", $this->id); fix_course_sortorder(); + // purge cache even if fix_course_sortorder() did not do it + cache_helper::purge_by_event('changesincoursecat'); // update all fields in the current object $this->restore(); @@ -536,43 +526,72 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { } /** - * Returns tree of categories ids + * Returns the entry from categories tree and makes sure the application-level tree cache is built * - * Return array has categories ids as keys and list of children ids as values. - * Also there is an additional first element with key 0 with list of categories on the top level. - * Therefore the number of elements in the return array is one more than number of categories in the system. + * The following keys can be requested: * - * Also this method ensures that all categories are cached together with their contexts. - * - * @return array + * 'countall' - total number of categories in the system (always present) + * 0 - array of ids of top-level categories (always present) + * '0i' - array of ids of top-level categories that have visible=0 (always present but may be empty array) + * $id (int) - array of ids of categories that are direct children of category with id $id. If + * category with id $id does not exist returns false. If category has no children returns empty array + * $id.'i' - array of ids of children categories that have visible=0 + * + * @param int|string $id + * @return mixed */ - protected static function get_all_ids() { + protected static function get_tree($id) { global $DB; - $coursecatcache = cache::make('core', 'coursecat'); - $all = $coursecatcache->get('all'); - if ($all === false) { - $coursecatcache->purge(); // it should be empty already but to be extra sure - $sql = "SELECT cc.id, cc.parent - FROM {course_categories} cc - ORDER BY cc.sortorder"; - $rs = $DB->get_recordset_sql($sql, array()); - $all = array(0 => array()); - foreach ($rs as $record) { - $all[$record->id] = array(); - $all[$record->parent][] = $record->id; - } - $rs->close(); - if (!count($all[0])) { - // No categories found. - // This may happen after upgrade from very old moodle version. In new versions the default category is created on install. - $defcoursecat = self::create(array('name' => get_string('miscellaneous'))); - $coursecatcache->set($defcoursecat->id, $defcoursecat); - set_config('defaultrequestcategory', $defcoursecat->id); - $all[0][$defcoursecat->id] = array(); - } - $coursecatcache->set('all', $all); + $coursecattreecache = cache::make('core', 'coursecattree'); + $rv = $coursecattreecache->get($id); + if ($rv !== false) { + return $rv; } - return $all; + // We did not find the entry in cache but it also can mean that tree is not built. + // The keys 0 and 'countall' must always be present if tree is built. + if ($id !== 0 && $id !== 'countall' && $coursecattreecache->has('countall')) { + // Tree was built, it means the non-existing $id was requested. + return false; + } + // Re-build the tree. + $sql = "SELECT cc.id, cc.parent, cc.visible + FROM {course_categories} cc + ORDER BY cc.sortorder"; + $rs = $DB->get_recordset_sql($sql, array()); + $all = array(0 => array(), '0i' => array()); + $count = 0; + foreach ($rs as $record) { + $all[$record->id] = array(); + $all[$record->id. 'i'] = array(); + if (array_key_exists($record->parent, $all)) { + $all[$record->parent][] = $record->id; + if (!$record->visible) { + $all[$record->parent. 'i'][] = $record->id; + } + } else { + // parent not found. This is data consistency error but next fix_course_sortorder() should fix it + $all[0][] = $record->id; + } + $count++; + } + $rs->close(); + if (!$count) { + // No categories found. + // This may happen after upgrade from very old moodle version. In new versions the default category is created on install. + $defcoursecat = self::create(array('name' => get_string('miscellaneous'))); + set_config('defaultrequestcategory', $defcoursecat->id); + $all[0] = array($defcoursecat->id); + $all[$defcoursecat->id] = array(); + $count++; + } + foreach ($all as $key => $children) { + $coursecattreecache->set($key, $children); + } + $coursecattreecache->set('countall', $count); + if (array_key_exists($id, $all)) { + return $all[$id]; + } + return false; } /** @@ -582,37 +601,205 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { * @return int */ public static function count_all() { - $all = self::get_all_ids(); - return count($all) - 1; // do not count 0-category + return self::get_tree('countall'); + } + + /** + * Retrieves number of records from course_categories table + * + * Only cached fields are retrieved. Records are ready for preloading context + * + * @param string $whereclause + * @param array $params + * @return array array of stdClass objects + */ + protected static function get_records($whereclause, $params) { + global $DB; + // Retrieve from DB only the fields that need to be stored in cache + $fields = array_filter(array_keys(self::$coursecatfields), function ($element) + { return (self::$coursecatfields[$element] !== null); } ); + $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); + $sql = "SELECT cc.". join(',cc.', $fields). ", $ctxselect + FROM {course_categories} cc + JOIN {context} ctx ON cc.id = ctx.instanceid AND ctx.contextlevel = :contextcoursecat + WHERE ". $whereclause." ORDER BY cc.sortorder"; + return $DB->get_records_sql($sql, + array('contextcoursecat' => CONTEXT_COURSECAT) + $params); + } + + /** + * Returns array of ids of children categories that current user can not see + * + * This data is cached in user session cache + * + * @return array + */ + protected function get_not_visible_children_ids() { + global $DB; + $coursecatcache = cache::make('core', 'coursecat'); + if (($invisibleids = $coursecatcache->get('ic'. $this->id)) === false) { + // we never checked visible children before + $hidden = self::get_tree($this->id.'i'); + $invisibleids = array(); + if ($hidden) { + // preload categories contexts + list($sql, $params) = $DB->get_in_or_equal($hidden, SQL_PARAMS_NAMED, 'id'); + $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); + $contexts = $DB->get_records_sql("SELECT $ctxselect FROM {context} ctx + WHERE ctx.contextlevel = :contextcoursecat AND ctx.instanceid ".$sql, + array('contextcoursecat' => CONTEXT_COURSECAT) + $params); + foreach ($contexts as $record) { + context_helper::preload_from_record($record); + } + // check that user has 'viewhiddencategories' capability for each hidden category + foreach ($hidden as $id) { + if (!has_capability('moodle/category:viewhiddencategories', context_coursecat::instance($id))) { + $invisibleids[] = $id; + } + } + } + $coursecatcache->set('ic'. $this->id, $invisibleids); + } + return $invisibleids; + } + + /** + * Compares two records. For use in uasort() + * + * @param stdClass $a + * @param stdClass $b + * @param array $sortfields assoc array where key is the field to sort and value is 1 for asc or -1 for desc + * @return int + */ + protected static function compare_records($a, $b, $sortfields) { + foreach ($sortfields as $field => $mult) { + if ($field === 'name' || $field === 'idnumber' || $field === 'path') { + // string fields + if ($cmp = strcmp($a->$field, $b->$field)) { + // TODO textlib? + return $mult * $cmp; + } + } else { + // int fields + if ($a->$field > $b->$field) { + return $mult; + } + if ($a->$field < $b->$field) { + return -$mult; + } + } + } + return 0; } /** * Returns array of children categories visible to the current user * + * @param array $options options for retrieving children + * - sort - list of fields to sort. Example + * array('idnumber' => 1, 'name' => 1, 'id' => -1) + * will sort by idnumber asc, name asc and id desc. + * Default: array('sortorder' => 1) + * Only cached fields may be used for sorting! + * - offset + * - limit - maximum number of children to return, 0 or null for no limit * @return array of coursecat objects indexed by category id */ - public function get_children() { - $all = self::get_all_ids(); - $rv = array(); - if (!empty($all[$this->id])) { - foreach ($all[$this->id] as $id) { - if ($coursecat = self::get($id, IGNORE_MISSING)) { - // do not return invisible - $rv[$coursecat->id] = $coursecat; + public function get_children($options = array()) { + global $DB; + $coursecatcache = cache::make('core', 'coursecat'); + + // get default values for options + if (!empty($options['sort']) && is_array($options['sort'])) { + $sortfields = $options['sort']; + } else { + $sortfields = array('sortorder' => 1); + } + $limit = null; + if (!empty($options['limit']) && (int)$options['limit']) { + $limit = (int)$options['limit']; + } + $offset = 0; + if (!empty($options['offset']) && (int)$options['offset']) { + $offset = (int)$options['offset']; + } + + // first retrieve list of user-visible and sorted children ids from cache + $sortedids = $coursecatcache->get('c'. $this->id. ':'. serialize($sortfields)); + if ($sortedids === false) { + $sortfieldskeys = array_keys($sortfields); + if ($sortfieldskeys[0] === 'sortorder') { + // no DB requests required to build the list of ids sorted by sortorder. + // We can easily ignore other sort fields because sortorder is always different + $sortedids = self::get_tree($this->id); + if ($sortedids && ($invisibleids = $this->get_not_visible_children_ids())) { + $sortedids = array_diff($sortedids, $invisibleids); + if ($sortfields['sortorder'] == -1) { + $sortedids = array_reverse($sortedids, true); + } } + } else { + // we need to retrieve and sort all children. Good thing that it is done only on first request + if ($invisibleids = $this->get_not_visible_children_ids()) { + list($sql, $params) = $DB->get_in_or_equal($invisibleids, SQL_PARAMS_NAMED, 'id', false); + $records = self::get_records('cc.parent = :parent AND cc.id '. $sql, + array('parent' => $this->id) + $params); + } else { + $records = self::get_records('cc.parent = :parent', array('parent' => $this->id)); + } + uasort($records, function ($a, $b) use ($sortfields) { return self::compare_records($a, $b, $sortfields); }); + $sortedids = array_keys($records); + } + $coursecatcache->set('c'. $this->id. ':'.serialize($sortfields), $sortedids); + } + + if (empty($sortedids)) { + return array(); + } + + // now retrieive and return categories + if ($offset || $limit) { + $sortedids = array_slice($sortedids, $offset, $limit); + } + if (isset($records)) { + // easy, we have already retrieved records + if ($offset || $limit) { + $records = array_slice($records, $offset, $limit, true); + } + } else { + list($sql, $params) = $DB->get_in_or_equal($sortedids, SQL_PARAMS_NAMED, 'id'); + $records = self::get_records('cc.id '. $sql, + array('parent' => $this->id) + $params); + } + + $rv = array(); + foreach ($sortedids as $id) { + if (isset($records[$id])) { + $rv[$id] = new coursecat($records[$id]); } } return $rv; } + /** + * Returns number of subcategories visible to the current user + * + * @return int + */ + public function get_children_count() { + $sortedids = self::get_tree($this->id); + $invisibleids = $this->get_not_visible_children_ids(); + return count($sortedids) - count($invisibleids); + } + /** * Returns true if the category has ANY children, including those not visible to the user * * @return boolean */ public function has_children() { - $all = self::get_all_ids(); - return !empty($all[$this->id]); + $allchildren = self::get_tree($this->id); + return !empty($allchildren); } /** @@ -731,7 +918,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { delete_context(CONTEXT_COURSECAT, $this->id); add_to_log(SITEID, "category", "delete", "index.php", "$this->name (ID $this->id)"); - self::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); events_trigger('course_category_deleted', $this); @@ -875,7 +1062,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { events_trigger('course_category_deleted', $this); - self::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); if ($showfeedback) { echo $OUTPUT->notification(get_string('coursecategorydeleted', '', $catname), 'notifysuccess'); @@ -991,6 +1178,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { if ($newparentcat->id != $this->parent) { $this->change_parent_raw($newparentcat); fix_course_sortorder(); + cache_helper::purge_by_event('changesincoursecat'); $this->restore(); add_to_log(SITEID, "category", "move", "editcategory.php?id=$this->id", $this->id); } @@ -1056,7 +1244,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { */ public function hide() { if ($this->hide_raw(0)) { - self::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); add_to_log(SITEID, "category", "hide", "editcategory.php?id=$this->id", $this->id); } } @@ -1109,7 +1297,7 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { */ public function show() { if ($this->show_raw()) { - self::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); add_to_log(SITEID, "category", "show", "editcategory.php?id=$this->id", $this->id); } } @@ -1229,14 +1417,6 @@ class coursecat implements renderable, cacheable_object, IteratorAggregate { return $list; } - /** - * Call to reset caches after any modification of course categories - */ - public static function purge_cache() { - $coursecatcache = cache::make('core', 'coursecat'); - $coursecatcache->purge(); - } - // ====== implementing method from interface cacheable_object ====== /** diff --git a/lib/datalib.php b/lib/datalib.php index 6509a65e290..b70ad1ebd22 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -951,9 +951,14 @@ function fix_course_sortorder() { //WARNING: this is PHP5 only code! + // if there are any changes made to courses or categories we will trigger + // the cache events to purge all cached courses/categories data + $cacheevents = array(); + if ($unsorted = $DB->get_records('course_categories', array('sortorder'=>0))) { //move all categories that are not sorted yet to the end $DB->set_field('course_categories', 'sortorder', MAX_COURSES_IN_CATEGORY*MAX_COURSE_CATEGORIES, array('sortorder'=>0)); + $cacheevents['changesincoursecat'] = true; } $allcats = $DB->get_records('course_categories', null, 'sortorder, id', 'id, sortorder, parent, depth, path'); @@ -993,7 +998,9 @@ function fix_course_sortorder() { // now walk recursively the tree and fix any problems found $sortorder = 0; $fixcontexts = array(); - _fix_course_cats($topcats, $sortorder, 0, 0, '', $fixcontexts); + if (_fix_course_cats($topcats, $sortorder, 0, 0, '', $fixcontexts)) { + $cacheevents['changesincoursecat'] = true; + } // detect if there are "multiple" frontpage courses and fix them if needed $frontcourses = $DB->get_records('course', array('category'=>0), 'id'); @@ -1009,6 +1016,7 @@ function fix_course_sortorder() { $DB->set_field('course', 'category', $defaultcat->id, array('id'=>$course->id)); $context = context_course::instance($course->id); $fixcontexts[$context->id] = $context; + $cacheevents['changesincourse'] = true; } unset($frontcourses); } else { @@ -1022,6 +1030,8 @@ function fix_course_sortorder() { } context_helper::build_all_paths(false); unset($fixcontexts); + $cacheevents['changesincourse'] = true; + $cacheevents['changesincoursecat'] = true; } // release memory @@ -1032,6 +1042,7 @@ function fix_course_sortorder() { // fix frontpage course sortorder if ($frontcourse->sortorder != 1) { $DB->set_field('course', 'sortorder', 1, array('id'=>$frontcourse->id)); + $cacheevents['changesincourse'] = true; } // now fix the course counts in category records if needed @@ -1056,6 +1067,7 @@ function fix_course_sortorder() { $str = implode(', ', $categories); debugging("The number of courses (category id: $str) has reached MAX_COURSES_IN_CATEGORY (" . MAX_COURSES_IN_CATEGORY . "), it will cause a sorting performance issue, please increase the value of MAX_COURSES_IN_CATEGORY in lib/datalib.php file. See tracker issue: MDL-25669", DEBUG_DEVELOPER); } + $cacheevents['changesincoursecat'] = true; } // now make sure that sortorders in course table are withing the category sortorder ranges @@ -1072,6 +1084,7 @@ function fix_course_sortorder() { WHERE category = ?"; $DB->execute($sql, array($cat->sortorder, $cat->id)); } + $cacheevents['changesincoursecat'] = true; } unset($fixcategories); @@ -1105,6 +1118,7 @@ function fix_course_sortorder() { // it needs full resorting $fixcategories[$cat->id] = $cat; } + $cacheevents['changesincourse'] = true; } unset($gapcategories); @@ -1116,13 +1130,16 @@ function fix_course_sortorder() { if ($course->sortorder != $cat->sortorder + $i) { $course->sortorder = $cat->sortorder + $i; $DB->update_record_raw('course', $course, true); + $cacheevents['changesincourse'] = true; } $i++; } } - $coursecatcache = cache::make('core', 'coursecat'); - $coursecatcache->purge(); + // advise all caches that need to be rebuilt + foreach (array_keys($cacheevents) as $event) { + cache_helper::purge_by_event($event); + } } /** @@ -1139,12 +1156,13 @@ function fix_course_sortorder() { * @param int $depth * @param string $path * @param array $fixcontexts - * @return void + * @return bool if changes were made */ function _fix_course_cats($children, &$sortorder, $parent, $depth, $path, &$fixcontexts) { global $DB; $depth++; + $changesmade = false; foreach ($children as $cat) { $sortorder = $sortorder + MAX_COURSES_IN_CATEGORY; @@ -1165,11 +1183,15 @@ function _fix_course_cats($children, &$sortorder, $parent, $depth, $path, &$fixc } if ($update) { $DB->update_record('course_categories', $cat, true); + $changesmade = true; } if (isset($cat->children)) { - _fix_course_cats($cat->children, $sortorder, $cat->id, $cat->depth, $cat->path, $fixcontexts); + if (_fix_course_cats($cat->children, $sortorder, $cat->id, $cat->depth, $cat->path, $fixcontexts)) { + $changesmade = true; + } } } + return $changesmade; } /** diff --git a/lib/db/caches.php b/lib/db/caches.php index 3b61e5f172a..d40708d18bf 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -198,10 +198,31 @@ $definitions = array( 'persistentmaxsize' => 1, ), - // Used to store data for course categories - 'coursecat' => array( + // Used to store the full tree of course categories + 'coursecattree' => array( 'mode' => cache_store::MODE_APPLICATION, + 'persistent' => true, + 'invalidationevents' => array( + 'changesincoursecat', + ) + ), + // Used to store data for course categories visible to current user. Helps to browse list of categories + 'coursecat' => array( + 'mode' => cache_store::MODE_SESSION, + 'persistent' => true, + 'invalidationevents' => array( + 'changesincoursecat', + 'changesincourse', + ), + 'ttl' => 600, + ), + // Used to store data for course categories visible to current user. Helps to browse list of categories + 'coursecatrecords' => array( + 'mode' => cache_store::MODE_REQUEST, 'simplekeys' => true, 'persistent' => true, + 'invalidationevents' => array( + 'changesincoursecat', + ), ), ); diff --git a/lib/tests/coursecatlib_test.php b/lib/tests/coursecatlib_test.php index e453c73ff64..aa9c157dd0a 100644 --- a/lib/tests/coursecatlib_test.php +++ b/lib/tests/coursecatlib_test.php @@ -239,7 +239,7 @@ class coursecatlib_testcase extends advanced_testcase { $this->assertEquals($testdescription, $category1->description); $category1 = coursecat::get($category1->id); $this->assertEquals($testdescription, $category1->description); - coursecat::purge_cache(); + cache_helper::purge_by_event('changesincoursecat'); $category1 = coursecat::get($category1->id); $this->assertEquals($testdescription, $category1->description); @@ -319,4 +319,44 @@ class coursecatlib_testcase extends advanced_testcase { $this->assertEquals(array('id' => $course4->id, 'category' => $category1->id), (array)$DB->get_record_sql('SELECT id, category from {course} where id <> ?', array(SITEID))); } + + public function test_get_children() { + $category1 = coursecat::create(array('name' => 'Cat1')); + $category2 = coursecat::create(array('name' => 'Cat2', 'parent' => $category1->id)); + $category3 = coursecat::create(array('name' => 'Cat3', 'parent' => $category1->id, 'visible' => 0)); + $category4 = coursecat::create(array('name' => 'Cat4', 'idnumber' => '12', 'parent' => $category1->id)); + $category5 = coursecat::create(array('name' => 'Cat5', 'idnumber' => '11', 'parent' => $category1->id, 'visible' => 0)); + $category6 = coursecat::create(array('name' => 'Cat6', 'idnumber' => '10', 'parent' => $category1->id)); + $category7 = coursecat::create(array('name' => 'Cat0', 'parent' => $category1->id)); + + $children = $category1->get_children(); + // user does not have the capability to view hidden categories, so the list should be + // 2,4,6,7 + $this->assertEquals(array($category2->id, $category4->id, $category6->id, $category7->id), array_keys($children)); + $this->assertEquals(4, $category1->get_children_count()); + + $children = $category1->get_children(array('offset' => 2)); + $this->assertEquals(array($category6->id, $category7->id), array_keys($children)); + $this->assertEquals(4, $category1->get_children_count()); + + $children = $category1->get_children(array('limit' => 2)); + $this->assertEquals(array($category2->id, $category4->id), array_keys($children)); + + $children = $category1->get_children(array('offset' => 1, 'limit' => 2)); + $this->assertEquals(array($category4->id, $category6->id), array_keys($children)); + + $children = $category1->get_children(array('sort' => array('name' => 1))); + // must be 7,2,4,6 + $this->assertEquals(array($category7->id, $category2->id, $category4->id, $category6->id), array_keys($children)); + + $children = $category1->get_children(array('sort' => array('idnumber' => 1, 'name' => -1))); + // must be 2,7,6,4 + $this->assertEquals(array($category2->id, $category7->id, $category6->id, $category4->id), array_keys($children)); + + // check that everything is all right after purging the caches + cache_helper::purge_by_event('changesincoursecat'); + $children = $category1->get_children(); + $this->assertEquals(array($category2->id, $category4->id, $category6->id, $category7->id), array_keys($children)); + $this->assertEquals(4, $category1->get_children_count()); + } } \ No newline at end of file