diff --git a/lib/externallib.php b/lib/externallib.php index 7fd7e52abe8..edbfabc62e9 100644 --- a/lib/externallib.php +++ b/lib/externallib.php @@ -964,21 +964,28 @@ class external_util { * Validate a list of courses, returning the complete course objects for valid courses. * * @param array $courseids A list of course ids + * @param array $courses An array of courses already pre-fetched. * @return array An array of courses and the validation warnings */ - public static function validate_courses($courseids) { + public static function validate_courses($courseids, $courses = array()) { // Delete duplicates. $courseids = array_unique($courseids); - $courses = array(); $warnings = array(); + // Remove courses which are not even requested. + $courses = array_intersect_key($courses, array_flip($courseids)); + foreach ($courseids as $cid) { // Check the user can function in this context. try { $context = context_course::instance($cid); external_api::validate_context($context); - $courses[$cid] = get_course($cid); + + if (!isset($courses[$cid])) { + $courses[$cid] = get_course($cid); + } } catch (Exception $e) { + unset($courses[$cid]); $warnings[] = array( 'item' => 'course', 'itemid' => $cid, diff --git a/lib/tests/externallib_test.php b/lib/tests/externallib_test.php index f67a1205c60..b3b0198046c 100644 --- a/lib/tests/externallib_test.php +++ b/lib/tests/externallib_test.php @@ -293,6 +293,67 @@ class core_externallib_testcase extends advanced_testcase { $this->assertInstanceOf('external_description', $desc->returns_desc); } } + + public function test_validate_courses() { + $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->setAdminUser(); + list($courses, $warnings) = external_util::validate_courses($courseids); + $this->assertEmpty($warnings); + $this->assertCount(3, $courses); + $this->assertArrayHasKey($c1->id, $courses); + $this->assertArrayHasKey($c2->id, $courses); + $this->assertArrayHasKey($c3->id, $courses); + $this->assertEquals($c1->id, $courses[$c1->id]->id); + $this->assertEquals($c2->id, $courses[$c2->id]->id); + $this->assertEquals($c3->id, $courses[$c3->id]->id); + + $this->setUser($u1); + list($courses, $warnings) = external_util::validate_courses($courseids); + $this->assertCount(2, $warnings); + $this->assertEquals($c2->id, $warnings[0]['itemid']); + $this->assertEquals($c3->id, $warnings[1]['itemid']); + $this->assertCount(1, $courses); + $this->assertArrayHasKey($c1->id, $courses); + $this->assertArrayNotHasKey($c2->id, $courses); + $this->assertArrayNotHasKey($c3->id, $courses); + $this->assertEquals($c1->id, $courses[$c1->id]->id); + } + + /** + * Validate courses can re-use an array of prefetched courses. + */ + public function test_validate_courses_prefetch() { + $this->resetAfterTest(true); + + $c1 = $this->getDataGenerator()->create_course(); + $c2 = $this->getDataGenerator()->create_course(); + $c3 = $this->getDataGenerator()->create_course(); + $c4 = $this->getDataGenerator()->create_course(); + $u1 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($u1->id, $c1->id); + $this->getDataGenerator()->enrol_user($u1->id, $c2->id); + + $courseids = array($c1->id, $c2->id, $c3->id); + $courses = array($c2->id => $c2, $c3->id => $c3, $c4->id => $c4); + + $this->setUser($u1); + list($courses, $warnings) = external_util::validate_courses($courseids, $courses); + $this->assertCount(2, $courses); + $this->assertCount(1, $warnings); + $this->assertArrayHasKey($c1->id, $courses); + $this->assertSame($c2, $courses[$c2->id]); + $this->assertArrayNotHasKey($c3->id, $courses); + // The extra course passed is not returned. + $this->assertArrayNotHasKey($c4->id, $courses); + } } /* diff --git a/mod/book/classes/external.php b/mod/book/classes/external.php index 6ae937dc7f2..34d1493d515 100644 --- a/mod/book/classes/external.php +++ b/mod/book/classes/external.php @@ -181,14 +181,16 @@ class mod_book_external extends external_api { $params = self::validate_parameters(self::get_books_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the books in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls. diff --git a/mod/chat/classes/external.php b/mod/chat/classes/external.php index 45000f38ae2..8405efbd203 100644 --- a/mod/chat/classes/external.php +++ b/mod/chat/classes/external.php @@ -523,14 +523,16 @@ class mod_chat_external extends external_api { $params = self::validate_parameters(self::get_chats_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the chats in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls. diff --git a/mod/choice/classes/external.php b/mod/choice/classes/external.php index 818201362d3..7eb5a168f01 100644 --- a/mod/choice/classes/external.php +++ b/mod/choice/classes/external.php @@ -480,14 +480,16 @@ class mod_choice_external extends external_api { $params = self::validate_parameters(self::get_choices_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the choices in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls. diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 20c7890206d..69965c050a7 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -60,8 +60,10 @@ class mod_forum_external extends external_api { $params = self::validate_parameters(self::get_forums_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Array to store the forums to return. @@ -71,7 +73,7 @@ class mod_forum_external extends external_api { // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the forums in this course. This function checks users visibility permissions. $forums = get_all_instances_in_courses("forum", $courses); diff --git a/mod/imscp/classes/external.php b/mod/imscp/classes/external.php index 5a092dea773..4ae2fdda8c3 100644 --- a/mod/imscp/classes/external.php +++ b/mod/imscp/classes/external.php @@ -136,14 +136,16 @@ class mod_imscp_external extends external_api { $params = self::validate_parameters(self::get_imscps_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the imscps in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls. diff --git a/mod/scorm/classes/external.php b/mod/scorm/classes/external.php index 87f2617d2c8..a67cb7469cf 100644 --- a/mod/scorm/classes/external.php +++ b/mod/scorm/classes/external.php @@ -640,14 +640,16 @@ class mod_scorm_external extends external_api { $params = self::validate_parameters(self::get_scorms_by_courses_parameters(), array('courseids' => $courseids)); + $courses = array(); if (empty($params['courseids'])) { - $params['courseids'] = array_keys(enrol_get_my_courses()); + $courses = enrol_get_my_courses(); + $params['courseids'] = array_keys($courses); } // Ensure there are courseids to loop through. if (!empty($params['courseids'])) { - list($courses, $warnings) = external_util::validate_courses($params['courseids']); + list($courses, $warnings) = external_util::validate_courses($params['courseids'], $courses); // Get the scorms in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls.