MDL-39571 repository_recent: query improvement
authorNathan Nguyen <nathannguyen@catalyst-au.net>
Fri, 17 Apr 2020 03:10:26 +0000 (13:10 +1000)
committerNathan Nguyen <nathannguyen@catalyst-au.net>
Wed, 20 May 2020 03:11:51 +0000 (13:11 +1000)
repository/recent/lang/en/repository_recent.php
repository/recent/lib.php
repository/recent/tests/generator/lib.php
repository/recent/tests/lib_test.php

index 5784334..f28fe85 100644 (file)
@@ -31,3 +31,5 @@ $string['recent:view'] = 'View recent files repository plugin';
 $string['pluginname_help'] = 'Files recently used by current user';
 $string['pluginname'] = 'Recent files';
 $string['privacy:metadata'] = 'The Recent files repository plugin does not store or transmit any personal data.';
+$string['timelimit'] = 'Time limit';
+$string['timelimit_help'] = 'Only retrieve recent files within the time limit';
index 35a7a0a..adeaa8e 100644 (file)
@@ -34,8 +34,16 @@ require_once($CFG->dirroot . '/repository/lib.php');
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 define('DEFAULT_RECENT_FILES_NUM', 50);
+
+/**
+ * DEFAULT_RECENT_FILES_TIME_LIMIT - default time limit.
+ */
+define('DEFAULT_RECENT_FILES_TIME_LIMIT', 6 * 4 * WEEKSECS);
 class repository_recent extends repository {
 
+    /** @var int only retrieve files within the time limit */
+    protected $timelimit;
+
     /**
      * Initialize recent plugin
      * @param int $repositoryid
@@ -51,6 +59,8 @@ class repository_recent extends repository {
         } else {
             $this->number = $number;
         }
+        $timelimit = get_config('recent', 'recentfilestimelimit');
+        $this->timelimit = (int)$timelimit;
     }
 
     /**
@@ -61,31 +71,47 @@ class repository_recent extends repository {
         return $this->get_listing();
     }
 
-    private function get_recent_files($limitfrom = 0, $limit = DEFAULT_RECENT_FILES_NUM) {
+    /**
+     * Only return files within the time limit
+     *
+     * @param int $limitfrom retrieve the files from
+     * @param int $limit limit number of the files
+     * @param int $timelimit only return files with the time limit
+     * @return array list of recent files
+     */
+    private function get_recent_files($limitfrom = 0, $limit = DEFAULT_RECENT_FILES_NUM, $timelimit = 0) {
         // XXX: get current itemid
         global $USER, $DB, $itemid;
+        $timelimitsql = '';
+        if ($timelimit > 0) {
+            $timelimitsql = "AND timemodified >= :timelimit";
+            $timelimitparam = ['timelimit' => time() - $timelimit];
+        }
         // This SQL will ignore draft files if not owned by current user.
         // Ignore all file references.
-        $sql = 'SELECT files1.*
+        $sql = "SELECT files1.id, files1.contextid, files1.component, files1.filearea,
+                       files1.itemid, files1.filepath, files1.filename, files1.pathnamehash
                   FROM {files} files1
-             LEFT JOIN {files_reference} r
-                       ON files1.referencefileid = r.id
                   JOIN (
                       SELECT contenthash, filename, MAX(id) AS id
                         FROM {files}
                        WHERE userid = :userid
+                         AND referencefileid is NULL
                          AND filename != :filename
                          AND ((filearea = :filearea1 AND itemid = :itemid) OR filearea != :filearea2)
+                         $timelimitsql
                     GROUP BY contenthash, filename
                   ) files2 ON files1.id = files2.id
-                 WHERE r.repositoryid is NULL
-              ORDER BY files1.timemodified DESC';
+              ORDER BY files1.timemodified DESC";
         $params = array(
             'userid' => $USER->id,
             'filename' => '.',
             'filearea1' => 'draft',
             'itemid' => $itemid,
             'filearea2' => 'draft');
+        if (isset($timelimitparam)) {
+            $params = array_merge($params, $timelimitparam);
+        }
         $rs = $DB->get_recordset_sql($sql, $params, $limitfrom, $limit);
         $result = array();
         foreach ($rs as $file_record) {
@@ -116,7 +142,7 @@ class repository_recent extends repository {
         $ret['nosearch'] = true;
         $ret['nologin'] = true;
         $list = array();
-        $files = $this->get_recent_files(0, $this->number);
+        $files = $this->get_recent_files(0, $this->number, $this->timelimit);
 
         try {
             foreach ($files as $file) {
@@ -156,7 +182,7 @@ class repository_recent extends repository {
     }
 
     public static function get_type_option_names() {
-        return array('recentfilesnumber', 'pluginname');
+        return array('recentfilesnumber', 'recentfilestimelimit', 'pluginname');
     }
 
     public static function type_config_form($mform, $classname = 'repository') {
@@ -168,6 +194,11 @@ class repository_recent extends repository {
         $mform->addElement('text', 'recentfilesnumber', get_string('recentfilesnumber', 'repository_recent'));
         $mform->setType('recentfilesnumber', PARAM_INT);
         $mform->setDefault('recentfilesnumber', $number);
+
+        $mform->addElement('duration', 'recentfilestimelimit',
+            get_string('timelimit', 'repository_recent'), ['units' => [DAYSECS, WEEKSECS], 'optional' => true]);
+        $mform->addHelpButton('recentfilestimelimit', 'timelimit', 'repository_recent');
+        $mform->setDefault('recentfilestimelimit', DEFAULT_RECENT_FILES_TIME_LIMIT);
     }
 
     /**
index ffc7e27..5725708 100644 (file)
@@ -44,6 +44,9 @@ class repository_recent_generator extends testing_repository_generator {
         if (!isset($record['recentfilesnumber'])) {
             $record['recentfilesnumber'] = '';
         }
+        if (!isset($record['recentfilestimelimit'])) {
+            $record['recentfilestimelimit'] = '';
+        }
         return $record;
     }
 
index 2e63b81..080999b 100644 (file)
@@ -50,15 +50,11 @@ class repository_recent_lib_testcase extends advanced_testcase {
      * SetUp to create an repository instance.
      */
     protected function setUp() {
-        global $USER, $itemid;
-
-        $this->resetAfterTest(true);
+        global $USER;
         $this->setAdminUser();
         $this->usercontext = context_user::instance($USER->id);
         $repoid = $this->getDataGenerator()->create_repository('recent')->id;
         $this->repo = repository::get_repository_by_id($repoid, $this->usercontext);
-        // Set global itemid for draft file (file manager mockup).
-        $itemid = file_get_unused_draft_itemid();
     }
 
     /**
@@ -66,23 +62,27 @@ class repository_recent_lib_testcase extends advanced_testcase {
      */
     public function test_get_listing_with_duplicate_file() {
         global $itemid;
+        $this->resetAfterTest(true);
+
+        // Set global itemid for draft file (file manager mockup).
+        $itemid = file_get_unused_draft_itemid();
 
         // No recent file.
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(0, $filelist);
 
         // Create test file 1.
-        self::create_test_file('TestFile1', 'draft', $itemid);
+        $this->create_test_file('TestFile1', 'draft', $itemid);
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(1, $filelist);
 
         // Due to create_test_file function, same filename means same content as the content is the filename hash.
-        self::create_test_file('TestFile1', 'private');
+        $this->create_test_file('TestFile1', 'private');
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(1, $filelist);
 
         // Create test file 2, different area.
-        self::create_test_file('TestFile2', 'private');
+        $this->create_test_file('TestFile2', 'private');
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(2, $filelist);
     }
@@ -91,13 +91,14 @@ class repository_recent_lib_testcase extends advanced_testcase {
      * Test get listing reference file
      */
     public function test_get_listing_with_reference_file() {
+        $this->resetAfterTest(true);
         // Create test file 1.
-        $file1 = self::create_test_file('TestFile1', 'private');
+        $file1 = $this->create_test_file('TestFile1', 'private');
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(1, $filelist);
 
         // Create reference file.
-        $file2 = self::create_reference_file($file1, 'TestFile2', 'private');
+        $file2 = $this->create_reference_file($file1, 'TestFile2', 'private');
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(1, $filelist);
 
@@ -111,7 +112,8 @@ class repository_recent_lib_testcase extends advanced_testcase {
      * Test number limit
      */
     public function test_get_listing_number_limit() {
-        self::create_multiple_test_files('private', 75);
+        $this->resetAfterTest(true);
+        $this->create_multiple_test_files('private', 75);
         $filelist = $this->repo->get_listing()['list'];
         $this->assertCount(50, $filelist);
 
@@ -123,6 +125,29 @@ class repository_recent_lib_testcase extends advanced_testcase {
         $this->assertCount(75, $filelist);
     }
 
+    /**
+     * Test time limit
+     */
+    public function test_get_listing_time_limit() {
+        $this->resetAfterTest(true);
+        $this->create_multiple_test_files('private', 25);
+        $file1 = $this->create_test_file('TestFileTimeLimit', 'private');
+        // Set time modified back to a year ago.
+        $file1->set_timemodified(time() - YEARSECS);
+
+        // There is no time limit by default.
+        $filelist = $this->repo->get_listing()['list'];
+        $this->assertCount(26, $filelist);
+
+        // The time limit is set as property of the repo, so we need to create new repo instance.
+        set_config('recentfilestimelimit', 3600, 'recent');
+        $repoid = $this->getDataGenerator()->create_repository('recent')->id;
+        $repo = repository::get_repository_by_id($repoid, $this->usercontext);
+        $filelist = $repo->get_listing()['list'];
+        // Only get the recent files in the last hour.
+        $this->assertCount(25, $filelist);
+    }
+
     /**
      * Create multiple test file
      *
@@ -132,7 +157,7 @@ class repository_recent_lib_testcase extends advanced_testcase {
     private function create_multiple_test_files($filearea, $numberoffiles) {
         for ($i = 0; $i < $numberoffiles; ++$i) {
             $filename = "TestFile$i" . time();
-            self::create_test_file($filename, $filearea);
+            $this->create_test_file($filename, $filearea);
         }
     }