From 7e4d43e1a263f7003207a382251de900341db92c Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 11 May 2016 10:24:47 +0800 Subject: [PATCH] MDL-54110 repositories: Whitespace, Typo + unit test fixes --- lib/moodlelib.php | 10 ++-- lib/tests/moodlelib_test.php | 109 ++++++++++++++++++++++------------ repository/filesystem/lib.php | 2 +- repository/lib.php | 4 +- 4 files changed, 77 insertions(+), 48 deletions(-) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index cc7af3e25a1..8160f89c542 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -6253,20 +6253,17 @@ function valid_uploaded_file($newfile) { * The php.ini settings are only used if $usespost is true. This allows repositories that do not use post requests, such as * repository_filesystem, to copy in files that are larger than post_max_size if the user has permission. * - * @todo Finish documenting this function - * * @param int $sitebytes Set maximum size * @param int $coursebytes Current course $course->maxbytes (in bytes) * @param int $modulebytes Current module ->maxbytes (in bytes) * @param bool $usespost Does the upload we're getting the max size for use a post request? * @return int The maximum size for uploading files. */ -function get_max_upload_file_size($sitebytes=0, $coursebytes=0, $modulebytes=0, $usespost = true) { - +function get_max_upload_file_size($sitebytes = 0, $coursebytes = 0, $modulebytes = 0, $usespost = true) { $sizes = array(); if ($usespost) { - if (! $filesize = ini_get('upload_max_filesize')) { + if (!$filesize = ini_get('upload_max_filesize')) { $filesize = '5M'; } $sizes[] = get_real_size($filesize); @@ -6279,7 +6276,8 @@ function get_max_upload_file_size($sitebytes=0, $coursebytes=0, $modulebytes=0, $sizes[] = $sitebytes; } } else { - if ($sitebytes != 0) { // It's for possible that $sitebytes == USER_CAN_IGNORE_FILE_SIZE_LIMITS (-1). + if ($sitebytes != 0) { + // It's for possible that $sitebytes == USER_CAN_IGNORE_FILE_SIZE_LIMITS (-1). $sizes[] = $sitebytes; } } diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index ebfe794bb70..3cfb551a827 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -2216,51 +2216,82 @@ class core_moodlelib_testcase extends advanced_testcase { $this->assertArrayHasKey(get_max_upload_file_size(), $result); } - public function test_get_max_upload_file_size() { - // Get the smallest upload limit from ini settings. + /** + * Provider for get_max_upload_file_size. + * + * @return array + */ + public function get_max_upload_file_size_provider() { $inisize = min(array(get_real_size(ini_get('post_max_size')), get_real_size(ini_get('upload_max_filesize')))); + return [ + 'POST: inisize smallest' => [ + $inisize + 10, + $inisize + 20, + $inisize + 30, + true, + $inisize, + ], + 'POST: sitebytes smallest' => [ + $inisize - 30, + $inisize - 20, + $inisize - 10, + true, + $inisize - 30, + ], + 'POST: coursebytes smallest' => [ + $inisize - 20, + $inisize - 30, + $inisize - 10, + true, + $inisize - 30, + ], + 'POST: modulebytes smallest' => [ + $inisize - 20, + $inisize - 10, + $inisize - 30, + true, + $inisize - 30, + ], + 'POST: User can ignore site limit (respect ini)' => [ + USER_CAN_IGNORE_FILE_SIZE_LIMITS, + 0, + 0, + true, + $inisize, + ], + 'NOPOST: inisize smallest' => [ + $inisize + 10, + $inisize + 20, + $inisize + 30, + false, + $inisize + 10, + ], + 'NOPOST: User can ignore site limit (no limit)' => [ + USER_CAN_IGNORE_FILE_SIZE_LIMITS, + 0, + 0, + false, + USER_CAN_IGNORE_FILE_SIZE_LIMITS, + ], + ]; + } - // The inisize is the smallest. - $sitebytes = $inisize + 10; - $coursebytes = $inisize + 20; - $modulebytes = $inisize + 30; - $this->assertEquals($inisize, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes)); - - // Site limit is the smallest. - $sitebytes = $inisize - 30; - $coursebytes = $inisize - 20; - $modulebytes = $inisize - 10; - $this->assertEquals($sitebytes, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes)); - - // Course limit is the smallest. - $sitebytes = $inisize - 20; - $coursebytes = $inisize - 30; - $modulebytes = $inisize - 10; - $this->assertEquals($coursebytes, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes)); - - // Module limit is the smallest. - $sitebytes = $inisize - 20; - $coursebytes = $inisize - 10; - $modulebytes = $inisize - 30; - $this->assertEquals($modulebytes, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes)); - - // The inisize is the smallest, the upload does not use post. - $sitebytes = $inisize + 10; - $coursebytes = $inisize + 20; - $modulebytes = $inisize + 30; - $this->assertEquals($sitebytes, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes, false)); - - // The user can ignore file size limits, the upload does use post. - $this->assertEquals($inisize, get_max_upload_file_size(USER_CAN_IGNORE_FILE_SIZE_LIMITS, 0, 0)); - - // The user can ignore file size limits, the upload not does use post. - $this->assertEquals(USER_CAN_IGNORE_FILE_SIZE_LIMITS, - get_max_upload_file_size(USER_CAN_IGNORE_FILE_SIZE_LIMITS, 0, 0, false)); + /** + * Test get_max_upload_file_size with various combinations. + * + * @dataProvider get_max_upload_file_size_provider + */ + public function test_get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes, $ispost, $expectation) { + $this->assertEquals($expectation, get_max_upload_file_size($sitebytes, $coursebytes, $modulebytes, $ispost)); + } + /** + * Test that when get_max_upload_file_size is called with no sizes, and no post, an exception is thrown. + */ + public function test_get_max_upload_file_size_no_sizes() { // If not using post we have to provide at least one other limit. $this->setExpectedException('coding_exception', 'You must specify at least one filesize limit.'); get_max_upload_file_size(0, 0, 0, false); - } /** diff --git a/repository/filesystem/lib.php b/repository/filesystem/lib.php index bfcf2546634..066ad7e2c4b 100644 --- a/repository/filesystem/lib.php +++ b/repository/filesystem/lib.php @@ -812,7 +812,7 @@ class repository_filesystem extends repository { } /** - * Helper funtion to indicate if this repository uses post requests for uploading files. + * Helper function to indicate if this repository uses post requests for uploading files. * * Files are copied from the filesystem so don't rely on POST requests. * diff --git a/repository/lib.php b/repository/lib.php index 29f3fb1e576..f3e84748715 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -2777,9 +2777,9 @@ abstract class repository implements cacheable_object { } /** - * Helper funtion to indicate if this repository uses post requests for uploading files. + * Helper function to indicate if this repository uses post requests for uploading files. * - * If the respository doesn't rely on uploading via POST requests, this can be overridden to return true, + * If the respository doesn't rely on uploading via POST requests, this can be overridden to return false, * allowing users with the right permissions to upload files of any size from this repository. * * @return bool -- 2.43.0