From a9c139c8d43bb682fd463797c2831db7a68d4fec Mon Sep 17 00:00:00 2001 From: Nathan Nguyen Date: Fri, 17 Apr 2020 13:10:26 +1000 Subject: [PATCH] MDL-39571 repository_recent: query improvement --- .../recent/lang/en/repository_recent.php | 2 + repository/recent/lib.php | 47 +++++++++++++++--- repository/recent/tests/generator/lib.php | 3 ++ repository/recent/tests/lib_test.php | 49 ++++++++++++++----- 4 files changed, 81 insertions(+), 20 deletions(-) diff --git a/repository/recent/lang/en/repository_recent.php b/repository/recent/lang/en/repository_recent.php index 5784334b98b..f28fe85cce2 100644 --- a/repository/recent/lang/en/repository_recent.php +++ b/repository/recent/lang/en/repository_recent.php @@ -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'; diff --git a/repository/recent/lib.php b/repository/recent/lib.php index 35a7a0ad3c0..adeaa8e5b49 100644 --- a/repository/recent/lib.php +++ b/repository/recent/lib.php @@ -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); } /** diff --git a/repository/recent/tests/generator/lib.php b/repository/recent/tests/generator/lib.php index ffc7e27dfc0..572570830a2 100644 --- a/repository/recent/tests/generator/lib.php +++ b/repository/recent/tests/generator/lib.php @@ -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; } diff --git a/repository/recent/tests/lib_test.php b/repository/recent/tests/lib_test.php index 2e63b81fbd9..080999b2eae 100644 --- a/repository/recent/tests/lib_test.php +++ b/repository/recent/tests/lib_test.php @@ -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); } } -- 2.17.1