From: martinlanghoff Date: Wed, 19 Sep 2007 07:03:49 +0000 (+0000) Subject: accesslib: get_my_courses() rework, new get_courses_bycap_fromsess() X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=e1d5e5c1810480a9eeb8ba33e28bc5124791951f;p=moodle.git accesslib: get_my_courses() rework, new get_courses_bycap_fromsess() get_my_courses() goes from a bazillion queries (500 in some sample cases) to 1 for the logged-in user, and 4 for a non-logged-in user. One of those queries brings a *lot* of data across (all rows from mdl_course) so there is room for serious optimisation. However, this clocks at ~300 ms on my laptop, costly, but not the end of the world. If your PHP-DB link has bandwidth probs it might be a problem. A few important changes to get_my_courses() - (Compat ALERT!) the default fields are less than before -- (will be followed by patches that fix the callers!) our defaults had grown to quite a bit because of the crazy caching scheme it had - the $fields parameter is to name _additional_ fields you need, and ideally wants them passed as an array. Will cope with old-style strings too. - the returned courses have an extra property "context" that is a fully valid context object, so if the caller needs to perform further accesslib checks, we save a query per course down the road The work is done in the newfangled get_courses_bycap_fromsess() which is brute-force but fast. I'm sure we can optimise the common cases a lot in it if we try. It'd be worthwhile to - reduce the rows we grab - that's really boneheaded - if we copy and tweak the logic of has_cap_fromsess() in it it can be made even faster --- diff --git a/lib/accesslib.php b/lib/accesslib.php index a2fc5e4955..903266c6bd 100755 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -635,6 +635,90 @@ function has_capability_including_child_contexts($context, $capabilitynames) { return false; } +/* + * Get an array of courses (with magic extra bits) + * where the access sess data shows that the cap + * requested is available. + * + * The main use is for get_my_courses(). + * + * Notes + * + * - $fields is an array of fieldnames to ADD + * so name the fields you really need, which will + * be added and uniq'd + * + * - the course records have $c->context which is a fully + * valid context object. Saves you a query per course! + * + * - current implementation is _extremely_ stupid but + * works fast enough. We can surely do much MUCH better + * for the common cases, and leave the brute-force to + * "not-easy-to-narrow-down" cases. + * + * @param string $capability - name of the capability + * @param array $sess - access session array + * @param bool $doanything - if false, ignore do anything + * @param string $sort - sorting fields - prefix each fieldname with "c." + * @param array $fields - additional fields you are interested in... + * @param int $limit - set if you want to limit the number of courses + * @return array $courses - ordered array of course objects - see notes above + * + */ +function get_courses_bycap_fromsess($cap, $sess, $doanything, $sort='c.sortorder ASC', $fields=NULL, $limit=0) { + + global $CFG; + + // Note! id is added later to ensure it's first + $basefields = array('id', 'category', 'sortorder', + 'shortname', 'fullname', 'idnumber', + 'teacher', 'teachers', 'student', 'students', + 'guest', 'startdate', 'visible'); + + if (!is_null($fields)) { + if (!is_array($fields)) { + error_log(print_r($fields,1)); + } + $fields = array_merge($basefields, $fields); + $fields = array_unique($fields); + } else { + $fields = $basefields; + } + + $coursefields = 'c.' .join(',c.', $fields); + + $sql = "SELECT $coursefields, + ctx.id AS ctxid, ctx.path AS ctxpath, ctx.depth as ctxdepth + FROM {$CFG->prefix}course c + JOIN {$CFG->prefix}context ctx + ON (c.id=ctx.instanceid AND ctx.contextlevel=".CONTEXT_COURSE.") + ORDER BY $sort; + "; + $courses = array(); + $cc = 0; // keep count + $rs = get_recordset_sql($sql); + if ($rs->RecordCount()) { + while ($c = rs_fetch_next_record($rs)) { + // build the context obj + $ctx = new StdClass; + $ctx->id = $c->ctxid; unset($c->ctxid); + $ctx->path = $c->ctxpath; unset($c->ctxpath); + $ctx->depth = $c->ctxdepth; unset($c->ctxdepth); + $ctx->instanceid = $c->id; + $ctx->contextlevel = CONTEXT_COURSE; + $c->context = $ctx; + if (has_cap_fromsess($cap, $ctx, $sess, $doanything)) { + $courses[] = $c; + if ($limit > 0 && $cc++ > $limit) { + break; + } + } + } + } + rs_close($rs); + return $courses; +} + /** * This function returns whether the current user has the capability of performing a function * For example, we can do has_capability('mod/forum:replypost',$context) in forum diff --git a/lib/datalib.php b/lib/datalib.php index 03d148f753..c8f3370563 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -585,176 +585,56 @@ function get_courses_page($categoryid="all", $sort="c.sortorder ASC", $fields="c * List of courses that a user has access to view. Note that for admins, * this usually includes every course on the system. * - * @uses $CFG + * Notes (inherited from get_courses_bycap_fromsess()): + * + * - $fields is an array of fieldnames to ADD + * so name the fields you really need, which will + * be added and uniq'd + * + * - the course records have $c->context which is a fully + * valid context object. Saves you a query per course! + * + * @uses $USER * @param int $userid The user of interest * @param string $sort the sortorder in the course table - * @param string $fields the fields to return + * @param array $fields - names of _additional_ fields to return (also accepts a string) * @param bool $doanything True if using the doanything flag * @param int $limit Maximum number of records to return, or 0 for unlimited * @return array {@link $COURSE} of course objects */ -function get_my_courses($userid, $sort=NULL, $fields=NULL, $doanything=false,$limit=0) { - - global $CFG, $USER; - - // Default parameters - $d_sort = 'visible DESC,sortorder ASC'; - $d_fields = 'id, category, sortorder, shortname, fullname, idnumber, newsitems, teacher, teachers, student, students, guest, startdate, visible, cost, enrol, summary, groupmode, groupmodeforce'; - - $usingdefaults = true; - if (is_null($sort) || $sort === $d_sort) { - $sort = $d_sort; - } else { - $usingdefaults = false; - } - if (is_null($fields) || $fields === $d_fields) { - $fields = $d_fields; - } else { - $usingdefaults = false; - } +function get_my_courses($userid, $sort='visible DESC,sortorder ASC', $fields=NULL, $doanything=false,$limit=0) { - $reallimit = 0; // this is only set if we are using a limit on the first call - - // If using default params, we may have it cached... - if (!empty($USER->id) && ($USER->id == $userid) && $usingdefaults) { - if (!empty($USER->mycourses[$doanything])) { - if ($limit && $limit < count($USER->mycourses[$doanything])) { - //silence warnings in PHP 4.x - the last param was added in PHP 5.0.2 - return @array_slice($USER->mycourses[$doanything], 0, $limit, true); - } else { - return $USER->mycourses[$doanything]; - } - } else { - // now, this is the first call, i.e. no cache, and we are using defaults, with a limit supplied, - // we need to store the limit somewhere, retrieve all, cache properly and then slice the array - // to return the proper number of entries. This is so that we don't keep missing calls like limit 20,20,20 - if ($limit) { - $reallimit = $limit; - $limit = 0; - } - } - } - - $mycourses = array(); - - // Fix fields to refer to the course table c - $fields=preg_replace('/([a-z0-9*]+)/','c.$1',$fields); - - // Attempt to filter the list of courses in order to reduce the number - // of queries in the next part. - - // Check root permissions - $sitecontext = get_context_instance(CONTEXT_SYSTEM, SITEID); + global $USER; // Guest's do not have any courses + $sitecontext = get_context_instance(CONTEXT_SYSTEM, SITEID); if (has_capability('moodle/legacy:guest',$sitecontext,$userid,false)) { return(array()); } - // we can optimise some things for true admins - $candoanything = false; - if ($doanything && has_capability('moodle/site:doanything',$sitecontext,$userid,true)) { - $candoanything = true; - } - - if ($candoanything || has_capability('moodle/course:view',$sitecontext,$userid,$doanything)) { - // User can view all courses, although there might be exceptions - // which we will filter later. - $rs = get_recordset('course c', '', '', $sort, $fields); - } else { - // The only other context level above courses that applies to moodle/course:view - // is category. So we consider: - // 1. All courses in which the user is assigned a role - // 2. All courses in categories in which the user is assigned a role - // 2BIS. All courses in subcategories in which the user gets assignment because he is assigned in one of its ascendant categories - // 3. All courses which have overrides for moodle/course:view - // Remember that this is just a filter. We check each individual course later. - // However for a typical student on a large system this can reduce the - // number of courses considered from around 2,000 to around 2, with corresponding - // reduction in the number of queries needed. - $rs=get_recordset_sql(" - SELECT $fields - FROM {$CFG->prefix}course c, ( - SELECT - c.id - FROM - {$CFG->prefix}role_assignments ra - INNER JOIN {$CFG->prefix}context x ON x.id = ra.contextid - INNER JOIN {$CFG->prefix}course c ON x.instanceid = c.id - WHERE - ra.userid = $userid AND - x.contextlevel = 50 - UNION - SELECT - c.id - FROM - {$CFG->prefix}role_assignments ra - INNER JOIN {$CFG->prefix}context x ON x.id = ra.contextid - INNER JOIN {$CFG->prefix}course_categories a ON a.path LIKE ".sql_concat("'%/'", 'x.instanceid', "'/%'")." OR x.instanceid = a.id - INNER JOIN {$CFG->prefix}course c ON c.category = a.id - WHERE - ra.userid = $userid AND - x.contextlevel = 40 - UNION - SELECT - c.id - FROM - {$CFG->prefix}role_capabilities ca - INNER JOIN {$CFG->prefix}context x ON x.id = ca.contextid - INNER JOIN {$CFG->prefix}course c ON c.id = x.instanceid - WHERE - ca.capability = 'moodle/course:view' AND - ca.contextid != {$sitecontext->id} AND - x.contextlevel = 50 - ) cids - WHERE c.id = cids.id - ORDER BY $sort" - ); - } - - if ($rs && $rs->RecordCount() > 0) { - while ($course = rs_fetch_next_record($rs)) { - if ($course->id != SITEID) { - - if ($candoanything) { // no need for further checks... - $mycourses[$course->id] = $course; - continue; - } - - // users with moodle/course:view are considered course participants - // the course needs to be visible, or user must have moodle/course:viewhiddencourses - // capability set to view hidden courses - $context = get_context_instance(CONTEXT_COURSE, $course->id); - if ( has_capability('moodle/course:view', $context, $userid, $doanything) && - !has_capability('moodle/legacy:guest', $context, $userid, false) && - ($course->visible || has_capability('moodle/course:viewhiddencourses', $context, $userid))) { - $mycourses[$course->id] = $course; - - // Only return a limited number of courses if limit is set - if($limit>0) { - $limit--; - if($limit==0) { - break; - } - } - } - } + if (!is_null($fields) && is_string($fields)) { + if (empty($fields)) { + $fields = NULL; + } else { + // turn the fields from a string to an array that + // get_courses_bycap_fromsess() will like... + $fields = explode(',',$fields); + $fields = array_map('trim', $fields); } } - // Cache if using default params... - if (!empty($USER->id) && ($USER->id == $userid) && $usingdefaults && $limit == 0) { - $USER->mycourses[$doanything] = $mycourses; - } - - if (!empty($mycourses) && $reallimit) { - return array_slice($mycourses, 0, $reallimit, true); + if ($userid === $USER->id && isset($USER->access)) { + return get_courses_bycap_fromsess('moodle/course:view', $USER->access, + $doanything, $sort, $fields, + $limit); } else { - return $mycourses; + $accessinfo = get_user_access_sitewide($userid); + return get_courses_bycap_fromsess('moodle/course:view', $accessinfo, + $doanything, $sort, $fields, + $limit); } } - /** * A list of courses that match a search *