L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-65931 Web services: get_courses_by_field minor inefficiency

Browse Source 
When called with a list of course IDs, this change makes it preload
the contexts and categories before further processing. This saves a
number of queries, depending on how many courses you call it for.

(Typically the function is called with a list of all your enrolled
courses.)

The change may also improve performance for other web services that
call the validate_courses function.
This commit is contained in:
sam marshall
2019-07-10 18:27:42 +01:00
parent fac49f8f72
commit 107ab4b97b
3 changed files with 85 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
course/externallib.php
+18 -3
View File
@@ -3129,7 +3129,20 @@ class core_course_external extends external_api {
}
if ($params['field'] === 'ids') {
$courses = $DB->get_records_list('course', 'id', explode(',', $value), 'id ASC');
// Preload categories to avoid loading one at a time.
$courseids = explode(',', $value);
list ($listsql, $listparams) = $DB->get_in_or_equal($courseids);
$categoryids = $DB->get_fieldset_sql("
SELECT DISTINCT cc.id
FROM {course} c
JOIN {course_categories} cc ON cc.id = c.category
WHERE c.id $listsql", $listparams);
core_course_category::get_many($categoryids);
// Load and validate all courses. This is called because it loads the courses
// more efficiently.
list ($courses, $warnings) = external_util::validate_courses($courseids, [],
false, true);
} else {
$courses = $DB->get_records('course', array($params['field'] => $value), 'id ASC');
}
@@ -3148,9 +3161,11 @@ class core_course_external extends external_api {
// Get the public course information, even if we are not enrolled.
$courseinlist = new core_course_list_element($course);
// Now, check if we have access to the course.
// Now, check if we have access to the course, unless it was already checked.
try {
self::validate_context($context);
if (empty($course->contextvalidated)) {
self::validate_context($context);
}
} catch (Exception $e) {
// User can not access the course, check if they can see the public information about the course and return it.
if (core_course_category::can_view_course_info($course)) {
lib/externallib.php
+43 -6
View File
@@ -1334,18 +1334,53 @@ class external_util {
/**
* Validate a list of courses, returning the complete course objects for valid courses.
*
* Each course has an additional 'contextvalidated' field, this will be set to true unless
* you set $keepfails, in which case it will be false if validation fails for a course.
*
* @param array $courseids A list of course ids
* @param array $courses An array of courses already pre-fetched, indexed by course id.
* @param bool $addcontext True if the returned course object should include the full context object.
* @param bool $keepfails True to keep all the course objects even if validation fails
* @return array An array of courses and the validation warnings
*/
public static function validate_courses($courseids, $courses = array(), $addcontext = false) {
public static function validate_courses($courseids, $courses = array(), $addcontext = false,
$keepfails = false) {
global $DB;
// Delete duplicates.
$courseids = array_unique($courseids);
$warnings = array();
// Remove courses which are not even requested.
$courses = array_intersect_key($courses, array_flip($courseids));
$courses = array_intersect_key($courses, array_flip($courseids));
// For any courses NOT loaded already, get them in a single query (and preload contexts)
// for performance. Preserve ordering because some tests depend on it.
$newcourseids = [];
foreach ($courseids as $cid) {
if (!array_key_exists($cid, $courses)) {
$newcourseids[] = $cid;
}
}
if ($newcourseids) {
list ($listsql, $listparams) = $DB->get_in_or_equal($newcourseids);
// Load list of courses, and preload associated contexts.
$contextselect = context_helper::get_preload_record_columns_sql('x');
$newcourses = $DB->get_records_sql("
SELECT c.*, $contextselect
FROM {course} c
JOIN {context} x ON x.instanceid = c.id
WHERE x.contextlevel = ? AND c.id $listsql",
array_merge([CONTEXT_COURSE], $listparams));
foreach ($newcourseids as $cid) {
if (array_key_exists($cid, $newcourses)) {
$course = $newcourses[$cid];
context_helper::preload_from_record($course);
$courses[$course->id] = $course;
}
}
}
foreach ($courseids as $cid) {
// Check the user can function in this context.
@@ -1353,14 +1388,16 @@ class external_util {
$context = context_course::instance($cid);
external_api::validate_context($context);
if (!isset($courses[$cid])) {
$courses[$cid] = get_course($cid);
}
if ($addcontext) {
$courses[$cid]->context = $context;
}
$courses[$cid]->contextvalidated = true;
} catch (Exception $e) {
unset($courses[$cid]);
if ($keepfails) {
$courses[$cid]->contextvalidated = false;
} else {
unset($courses[$cid]);
}
$warnings[] = array(
'item' => 'course',
'itemid' => $cid,
lib/tests/externallib_test.php
+24
View File
@@ -481,6 +481,30 @@ class core_externallib_testcase extends advanced_testcase {
$this->assertEquals($c1->id, $courses[$c1->id]->id);
}
/**
* Validate courses, but still return courses even if they fail validation.
*/
public function test_validate_courses_keepfails() {
$this->resetAfterTest(true);
$c1 = $this->getDataGenerator()->create_course();
$c2 = $this->getDataGenerator()->create_course();
$c3 = $this->getDataGenerator()->create_course();
$u1 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($u1->id, $c1->id);
$courseids = array($c1->id, $c2->id, $c3->id);
$this->setUser($u1);
list($courses, $warnings) = external_util::validate_courses($courseids, [], false, true);
$this->assertCount(2, $warnings);
$this->assertEquals($c2->id, $warnings[0]['itemid']);
$this->assertEquals($c3->id, $warnings[1]['itemid']);
$this->assertCount(3, $courses);
$this->assertTrue($courses[$c1->id]->contextvalidated);
$this->assertFalse($courses[$c2->id]->contextvalidated);
$this->assertFalse($courses[$c3->id]->contextvalidated);
}
/**
* Validate courses can re-use an array of prefetched courses.
*/