From bc1643135cdf8ba6845f3ee6a1068fee395f1e29 Mon Sep 17 00:00:00 2001
From: sam marshall <s.marshall@open.ac.uk>
Date: Thu, 22 Dec 2016 15:31:25 +0000
Subject: [PATCH] MDL-57515 Resource: 'not very efficient' with a large number
 of files

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.)
---
 mod/resource/lib.php            |  8 +++--
 mod/resource/tests/lib_test.php | 62 +++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/mod/resource/lib.php b/mod/resource/lib.php
index 6296f2e6d4f8c..3c3d1f65458dd 100644
--- a/mod/resource/lib.php
+++ b/mod/resource/lib.php
@@ -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();
     }
diff --git a/mod/resource/tests/lib_test.php b/mod/resource/tests/lib_test.php
index 13855c3a1fd9e..0c507adffff82 100644
--- a/mod/resource/tests/lib_test.php
+++ b/mod/resource/tests/lib_test.php
@@ -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);
+    }
 }