Skip to content

Commit

Permalink
MDL-57515 Resource: 'not very efficient' with a large number of files
Browse files Browse the repository at this point in the history
Fixes bug where the resource module loads metadata for all files while
building course modinfo, even though it only needs the first file.
(This causes problems if you have ~10k files.)
  • Loading branch information
sammarshallou committed Jan 9, 2017
1 parent edab078 commit bc16431
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
8 changes: 6 additions & 2 deletions mod/resource/lib.php
Expand Up @@ -213,10 +213,14 @@ function resource_get_coursemodule_info($coursemodule) {
$info->icon ='i/invalid';
return $info;
}

// See if there is at least one file.
$fs = get_file_storage();
$files = $fs->get_area_files($context->id, 'mod_resource', 'content', 0, 'sortorder DESC, id ASC', false); // TODO: this is not very efficient!!
$files = $DB->get_records_select('files', 'contextid = ? AND component = ? AND filearea = ? AND itemid = ? AND filename != ?',
array($context->id, 'mod_resource', 'content', 0, '.'), 'sortorder DESC, id ASC', '*', 0, 1);
if (count($files) >= 1) {
$mainfile = reset($files);
$firstrecord = reset($files);
$mainfile = $fs->get_file_instance($firstrecord);
$info->icon = file_file_icon($mainfile, 24);
$resource->mainfile = $mainfile->get_filename();
}
Expand Down
62 changes: 62 additions & 0 deletions mod/resource/tests/lib_test.php
Expand Up @@ -89,4 +89,66 @@ public function test_resource_view() {
$this->assertEquals(1, $completiondata->completionstate);

}

/**
* Tests the resource_get_coursemodule_info function.
*
* Note: This currently doesn't test every aspect of the function, mainly focusing on the icon.
*/
public function test_get_coursemodule_info() {
global $DB, $USER;

$this->resetAfterTest();
$this->setAdminUser();

// Create course.
$generator = $this->getDataGenerator();
$course = $generator->create_course();

// Create a resource with no files.
$draftid = file_get_unused_draft_itemid();
$resource1 = $generator->create_module('resource', array('course' => $course->id,
'name' => 'R1', 'files' => $draftid));

// Create a resource with one file.
$draftid = file_get_unused_draft_itemid();
$contextid = context_user::instance($USER->id)->id;
$filerecord = array('component' => 'user', 'filearea' => 'draft', 'contextid' => $contextid,
'itemid' => $draftid, 'filename' => 'r2.txt', 'filepath' => '/');
$fs = get_file_storage();
$fs->create_file_from_string($filerecord, 'Test');
$resource2 = $generator->create_module('resource', array('course' => $course->id,
'name' => 'R2', 'files' => $draftid));

// Create a resource with two files.
$draftid = file_get_unused_draft_itemid();
$filerecord = array('component' => 'user', 'filearea' => 'draft', 'contextid' => $contextid,
'itemid' => $draftid, 'filename' => 'r3.txt', 'filepath' => '/', 'sortorder' => 1);
$fs->create_file_from_string($filerecord, 'Test');
$filerecord['filename'] = 'r3.doc';
$filerecord['sortorder'] = 2;
$fs->create_file_from_string($filerecord, 'Test');
$resource3 = $generator->create_module('resource', array('course' => $course->id,
'name' => 'R3', 'files' => $draftid));

// Try get_coursemodule_info for first one.
$info = resource_get_coursemodule_info(
$DB->get_record('course_modules', array('id' => $resource1->cmid)));

// The name should be set. There is no overridden icon.
$this->assertEquals('R1', $info->name);
$this->assertEmpty($info->icon);

// For second one, there should be an overridden icon.
$info = resource_get_coursemodule_info(
$DB->get_record('course_modules', array('id' => $resource2->cmid)));
$this->assertEquals('R2', $info->name);
$this->assertEquals('f/text-24', $info->icon);

// For third one, it should use the highest sortorder icon.
$info = resource_get_coursemodule_info(
$DB->get_record('course_modules', array('id' => $resource3->cmid)));
$this->assertEquals('R3', $info->name);
$this->assertEquals('f/document-24', $info->icon);
}
}

0 comments on commit bc16431

Please sign in to comment.