MDL-72643 core: Improve display_size

Allows display_size to use a fixed unit for easy comparison of
multiple results, and fixed decimal places for the same reason.

Improves behaviour by using consistent decimal places and a
consistent space before the unit (the previous one only has a space
before 'bytes', not before 'KB').

Of existing uses, all the ones that displayed a 'maxbytes' type
configuration setting (which are likely to have an 'exact' size
and would be better shown as 512 KB rather than 512.0 KB) have been
changed to use 0 decimal places, to preserve previous behaviour.
All the uses which were showing an actual file or memory size have
been left as default (1 decimal place).
This commit is contained in:
sam marshall 2021-09-22 11:42:33 +01:00
parent 214adb7984
commit e332d1849d
18 changed files with 160 additions and 70 deletions

View File

@ -98,7 +98,7 @@ class filesize extends \admin_setting {
if (empty($bytes)) {
return get_string('none');
}
return display_size($bytes);
return display_size($bytes, 0);
}
/**

View File

@ -87,7 +87,7 @@ if ($isdownload) {
echo $OUTPUT->heading(get_string('downloadcoursecontent', 'course'));
// Prepare download confirmation information and display it.
$maxfilesize = display_size($CFG->maxsizeperdownloadcoursefile);
$maxfilesize = display_size($CFG->maxsizeperdownloadcoursefile, 0);
$downloadlink = new moodle_url('/course/downloadcontent.php', ['contextid' => $contextid, 'download' => 1]);
echo $OUTPUT->confirm(get_string('downloadcourseconfirmation', 'course', $maxfilesize), $downloadlink, $courselink);

View File

@ -289,9 +289,9 @@ class core_files_renderer extends plugin_renderer_base {
* @return string
*/
protected function fm_print_restrictions($fm) {
$maxbytes = display_size($fm->options->maxbytes);
$maxbytes = display_size($fm->options->maxbytes, 0);
$strparam = (object) array('size' => $maxbytes, 'attachments' => $fm->options->maxfiles,
'areasize' => display_size($fm->options->areamaxbytes));
'areasize' => display_size($fm->options->areamaxbytes, 0));
$hasmaxfiles = !empty($fm->options->maxfiles) && $fm->options->maxfiles > 0;
$hasarealimit = !empty($fm->options->areamaxbytes) && $fm->options->areamaxbytes != -1;
if ($hasmaxfiles && $hasarealimit) {

View File

@ -85,7 +85,7 @@ switch ($action) {
foreach ($_FILES as $uploadedfile) {
$filename = clean_param($uploadedfile['name'], PARAM_FILE);
if ($uploadedfile['size'] > $maxsize) {
H5PCore::ajaxError(get_string('maxbytesfile', 'error', ['file' => $filename, 'size' => display_size($maxsize)]));
H5PCore::ajaxError(get_string('maxbytesfile', 'error', ['file' => $filename, 'size' => display_size($maxsize, 0)]));
return;
}
\core\antivirus\manager::scan_file($uploadedfile['tmp_name'], $filename, true);

View File

@ -273,7 +273,7 @@ class zipwriter {
'courseshortname' => $exportedcourse->shortname,
'courselink' => $courselink,
'exportdate' => userdate(time()),
'maxfilesize' => display_size($this->maxfilesize),
'maxfilesize' => display_size($this->maxfilesize, 0),
];
$renderer = $PAGE->get_renderer('core');

View File

@ -6802,7 +6802,7 @@ function get_max_upload_sizes($sitebytes = 0, $coursebytes = 0, $modulebytes = 0
foreach ($sizelist as $sizebytes) {
if ($sizebytes < $maxsize && $sizebytes > 0) {
$filesize[(string)intval($sizebytes)] = display_size($sizebytes);
$filesize[(string)intval($sizebytes)] = display_size($sizebytes, 0);
}
}
@ -6812,17 +6812,17 @@ function get_max_upload_sizes($sitebytes = 0, $coursebytes = 0, $modulebytes = 0
(($modulebytes < $coursebytes || $coursebytes == 0) &&
($modulebytes < $sitebytes || $sitebytes == 0))) {
$limitlevel = get_string('activity', 'core');
$displaysize = display_size($modulebytes);
$displaysize = display_size($modulebytes, 0);
$filesize[$modulebytes] = $displaysize; // Make sure the limit is also included in the list.
} else if ($coursebytes && ($coursebytes < $sitebytes || $sitebytes == 0)) {
$limitlevel = get_string('course', 'core');
$displaysize = display_size($coursebytes);
$displaysize = display_size($coursebytes, 0);
$filesize[$coursebytes] = $displaysize; // Make sure the limit is also included in the list.
} else if ($sitebytes) {
$limitlevel = get_string('site', 'core');
$displaysize = display_size($sitebytes);
$displaysize = display_size($sitebytes, 0);
$filesize[$sitebytes] = $displaysize; // Make sure the limit is also included in the list.
}
@ -6954,14 +6954,12 @@ function get_directory_size($rootdir, $excludefile='') {
/**
* Converts bytes into display form
*
* @static string $gb Localized string for size in gigabytes
* @static string $mb Localized string for size in megabytes
* @static string $kb Localized string for size in kilobytes
* @static string $b Localized string for size in bytes
* @param int $size The size to convert to human readable form
* @return string
* @param int $decimalplaces If specified, uses fixed number of decimal places
* @param string $fixedunits If specified, uses fixed units (e.g. 'KB')
* @return string Display version of size
*/
function display_size($size) {
function display_size($size, int $decimalplaces = 1, string $fixedunits = ''): string {
static $units;
@ -6978,20 +6976,44 @@ function display_size($size) {
$units[] = get_string('sizepb');
}
if ($size >= 1024 ** 5) {
$size = round($size / 1024 ** 5 * 10) / 10 . $units[5];
} else if ($size >= 1024 ** 4) {
$size = round($size / 1024 ** 4 * 10) / 10 . $units[4];
} else if ($size >= 1024 ** 3) {
$size = round($size / 1024 ** 3 * 10) / 10 . $units[3];
} else if ($size >= 1024 ** 2) {
$size = round($size / 1024 ** 2 * 10) / 10 . $units[2];
} else if ($size >= 1024 ** 1) {
$size = round($size / 1024 ** 1 * 10) / 10 . $units[1];
} else {
$size = intval($size) .' '. $units[0]; // File sizes over 2GB can not work in 32bit PHP anyway.
switch ($fixedunits) {
case 'PB' :
$magnitude = 5;
break;
case 'TB' :
$magnitude = 4;
break;
case 'GB' :
$magnitude = 3;
break;
case 'MB' :
$magnitude = 2;
break;
case 'KB' :
$magnitude = 1;
break;
case 'B' :
$magnitude = 0;
break;
case '':
$magnitude = floor(log($size, 1024));
$magnitude = max(0, min(5, $magnitude));
break;
default:
throw new coding_exception('Unknown fixed units value: ' . $fixedunits);
}
return $size;
// Special case for magnitude 0 (bytes) - never use decimal places.
$nbsp = "\xc2\xa0";
if ($magnitude === 0) {
return round($size) . $nbsp . $units[$magnitude];
}
// Convert to specified units.
$sizeinunit = $size / 1024 ** $magnitude;
// Fixed decimal places.
return sprintf('%.' . $decimalplaces . 'f', $sizeinunit) . $nbsp . $units[$magnitude];
}
/**

View File

@ -2699,7 +2699,7 @@ class core_renderer extends renderer_base {
if ($size == -1) {
$maxsize = '';
} else {
$maxsize = get_string('maxfilesize', 'moodle', display_size($size));
$maxsize = get_string('maxfilesize', 'moodle', display_size($size, 0));
}
if ($options->buttonname) {
$buttonname = ' name="' . $options->buttonname . '"';

View File

@ -1009,7 +1009,7 @@ function portfolio_filesize_info() {
$filesizes = array();
$sizelist = array(10240, 51200, 102400, 512000, 1048576, 2097152, 5242880, 10485760, 20971520, 52428800);
foreach ($sizelist as $size) {
$filesizes[$size] = display_size($size);
$filesizes[$size] = display_size($size, 0);
}
return array(
'options' => $filesizes,

View File

@ -2743,7 +2743,8 @@ EOF;
$modulebytes = 10240;
$result = get_max_upload_sizes($sitebytes, $coursebytes, $modulebytes);
$this->assertSame('Activity upload limit (10KB)', $result['0']);
$nbsp = "\xc2\xa0";
$this->assertSame("Activity upload limit (10{$nbsp}KB)", $result['0']);
$this->assertCount(2, $result);
// Test course limit smallest.
@ -2752,7 +2753,7 @@ EOF;
$modulebytes = 51200;
$result = get_max_upload_sizes($sitebytes, $coursebytes, $modulebytes);
$this->assertSame('Course upload limit (10KB)', $result['0']);
$this->assertSame("Course upload limit (10{$nbsp}KB)", $result['0']);
$this->assertCount(2, $result);
// Test site limit smallest.
@ -2761,7 +2762,7 @@ EOF;
$modulebytes = 51200;
$result = get_max_upload_sizes($sitebytes, $coursebytes, $modulebytes);
$this->assertSame('Site upload limit (10KB)', $result['0']);
$this->assertSame("Site upload limit (10{$nbsp}KB)", $result['0']);
$this->assertCount(2, $result);
// Test site limit not set.
@ -2770,7 +2771,7 @@ EOF;
$modulebytes = 51200;
$result = get_max_upload_sizes($sitebytes, $coursebytes, $modulebytes);
$this->assertSame('Activity upload limit (50KB)', $result['0']);
$this->assertSame("Activity upload limit (50{$nbsp}KB)", $result['0']);
$this->assertCount(3, $result);
$sitebytes = 0;
@ -2778,7 +2779,7 @@ EOF;
$modulebytes = 102400;
$result = get_max_upload_sizes($sitebytes, $coursebytes, $modulebytes);
$this->assertSame('Course upload limit (50KB)', $result['0']);
$this->assertSame("Course upload limit (50{$nbsp}KB)", $result['0']);
$this->assertCount(3, $result);
// Test custom bytes in range.
@ -2821,9 +2822,9 @@ EOF;
$sitebytes = 51200;
$result = get_max_upload_sizes($sitebytes);
$this->assertSame('Site upload limit (50KB)', $result['0']);
$this->assertSame('50KB', $result['51200']);
$this->assertSame('10KB', $result['10240']);
$this->assertSame("Site upload limit (50{$nbsp}KB)", $result['0']);
$this->assertSame("50{$nbsp}KB", $result['51200']);
$this->assertSame("10{$nbsp}KB", $result['10240']);
$this->assertCount(3, $result);
// Test no limit.
@ -4987,25 +4988,25 @@ EOF;
public function display_size_provider() {
return [
[0, '0 bytes' ],
[1, '1 bytes' ],
[1023, '1023 bytes' ],
[1024, '1KB' ],
[2222, '2.2KB' ],
[33333, '32.6KB' ],
[444444, '434KB' ],
[5555555, '5.3MB' ],
[66666666, '63.6MB' ],
[777777777, '741.7MB'],
[8888888888, '8.3GB' ],
[99999999999, '93.1GB' ],
[111111111111, '103.5GB'],
[2222222222222, '2TB' ],
[33333333333333, '30.3TB' ],
[444444444444444, '404.2TB'],
[5555555555555555, '4.9PB' ],
[66666666666666666, '59.2PB' ],
[777777777777777777, '690.8PB'],
[0, '0 bytes'],
[1, '1 bytes'],
[1023, '1023 bytes'],
[1024, '1.0 KB'],
[2222, '2.2 KB'],
[33333, '32.6 KB'],
[444444, '434.0 KB'],
[5555555, '5.3 MB'],
[66666666, '63.6 MB'],
[777777777, '741.7 MB'],
[8888888888, '8.3 GB'],
[99999999999, '93.1 GB'],
[111111111111, '103.5 GB'],
[2222222222222, '2.0 TB'],
[33333333333333, '30.3 TB'],
[444444444444444, '404.2 TB'],
[5555555555555555, '4.9 PB'],
[66666666666666666, '59.2 PB'],
[777777777777777777, '690.8 PB'],
];
}
@ -5017,6 +5018,67 @@ EOF;
*/
public function test_display_size($size, $expected) {
$result = display_size($size);
$expected = str_replace(' ', "\xc2\xa0", $expected); // Should be non-breaking space.
$this->assertEquals($expected, $result);
}
/**
* Provider for display_size using fixed units.
*
* @return array of ($size, $units, $expected)
*/
public function display_size_fixed_provider(): array {
return [
[0, 'KB', '0.0 KB'],
[1, 'MB', '0.0 MB'],
[777777777, 'GB', '0.7 GB'],
[8888888888, 'PB', '0.0 PB'],
[99999999999, 'TB', '0.1 TB'],
[99999999999, 'B', '99999999999 bytes'],
];
}
/**
* Test display_size using fixed units.
*
* @dataProvider display_size_fixed_provider
* @param int $size Size in bytes
* @param string $units Fixed units
* @param string $expected Expected string.
*/
public function test_display_size_fixed(int $size, string $units, string $expected): void {
$result = display_size($size, 1, $units);
$expected = str_replace(' ', "\xc2\xa0", $expected); // Should be non-breaking space.
$this->assertEquals($expected, $result);
}
/**
* Provider for display_size using specified decimal places.
*
* @return array of ($size, $decimalplaces, $units, $expected)
*/
public function display_size_dp_provider(): array {
return [
[0, 1, 'KB', '0.0 KB'],
[1, 6, 'MB', '0.000001 MB'],
[777777777, 0, 'GB', '1 GB'],
[777777777, 0, '', '742 MB'],
[42, 6, '', '42 bytes'],
];
}
/**
* Test display_size using specified decimal places.
*
* @dataProvider display_size_dp_provider
* @param int $size Size in bytes
* @param int $places Number of decimal places
* @param string $units Fixed units
* @param string $expected Expected string.
*/
public function test_display_size_dp(int $size, int $places, string $units, string $expected): void {
$result = display_size($size, $places, $units);
$expected = str_replace(' ', "\xc2\xa0", $expected); // Should be non-breaking space.
$this->assertEquals($expected, $result);
}

View File

@ -91,6 +91,9 @@ information provided here is intended especially for developers.
completely removed from Moodle core too.
* The SWF media player has been completely removed (The Flash Player was deprecated in 2017 and officially discontinued
on 31 December 2020).
* The display_size function has been improved to add new optional parameters (decimal places,
fixed units), to always include a non-breaking space between the number and unit, and to use
consistent rounding (always 1 decimal place by default).
=== 3.11.2 ===
* For security reasons, filelib has been updated so all requests now use emulated redirects.

View File

@ -220,7 +220,7 @@ class reply_handler extends \core\message\inbound\handler {
. "Rejecting e-mail.");
$data = new \stdClass();
$data->forum = $forum;
$data->maxbytes = display_size($forum->maxbytes);
$data->maxbytes = display_size($forum->maxbytes, 0);
$data->filesize = display_size($filesize);
throw new \core\message\inbound\processing_failed_exception('messageinboundfilesizeexceeded', 'mod_forum', $data);
}

View File

@ -45,11 +45,11 @@ Feature: Teacher can specify different display options for the resource
| Show upload/modified date | <showdate> |
And I upload "mod/resource/tests/fixtures/samplefile.txt" file to "Select files" filemanager
And I press "Save and display"
Then I <seesize> see "6 bytes" in the ".resourcedetails" "css_element"
Then I <seesize> see "6 bytes" in the ".resourcedetails" "css_element"
And I <seetype> see "Text file" in the ".resourcedetails" "css_element"
And I <seedate> see "Uploaded" in the ".resourcedetails" "css_element"
And I am on "Course 1" course homepage
And I <seesize> see "6 bytes" in the ".activity.resource .resourcelinkdetails" "css_element"
And I <seesize> see "6 bytes" in the ".activity.resource .resourcelinkdetails" "css_element"
And I <seetype> see "Text file" in the ".activity.resource .resourcelinkdetails" "css_element"
And I <seedate> see "Uploaded" in the ".activity.resource .resourcelinkdetails" "css_element"
And I log out

View File

@ -28,12 +28,12 @@ I need to choose the appropriate maxbytes for attachments
Scenario: Preview an Essay question and see the allowed maximum file sizes and number of attachments.
When I choose "Preview" action for "essay-1-512KB" in the question bank
Then I should see "Please write a story about a frog."
And I should see "Maximum file size: 512KB, maximum number of files: 1"
And I should see "Maximum file size: 512 KB, maximum number of files: 1"
And I press "Close preview"
@javascript @_switch_window
Scenario: Preview an Essay question with Course upload limit and see the allowed maximum file size.
When I choose "Preview" action for "essay-1-max" in the question bank
Then I should see "Please write a story about a frog."
And I should see "Maximum file size: 1MB, maximum number of files: 1"
And I should see "Maximum file size: 1 MB, maximum number of files: 1"
And I press "Close preview"

View File

@ -72,6 +72,9 @@ class qtype_essay_question_test extends advanced_testcase {
$essay->responserequired = $responserequired;
$essay->attachmentsrequired = $attachmentsrequired;
// The space before the number of bytes from display_size is actually a non-breaking space.
$expected = str_replace(' bytes', "\xc2\xa0bytes", $expected);
$this->assertEquals($expected, $essay->summarise_response(
['answer' => $answertext, 'answerformat' => FORMAT_HTML, 'attachments' => $attachments[$attachmentuploaded]]));
}

View File

@ -890,7 +890,7 @@ abstract class repository implements cacheable_object {
// the file needs to copied to draft area
$stored_file = self::get_moodle_file($source);
if ($maxbytes != -1 && $stored_file->get_filesize() > $maxbytes) {
$maxbytesdisplay = display_size($maxbytes);
$maxbytesdisplay = display_size($maxbytes, 0);
throw new file_exception('maxbytesfile', (object) array('file' => $filerecord['filename'],
'size' => $maxbytesdisplay));
}
@ -1735,7 +1735,7 @@ abstract class repository implements cacheable_object {
// files that are references to local files are already in moodle filepool
// just validate the size
if ($maxbytes > 0 && $file->get_filesize() > $maxbytes) {
$maxbytesdisplay = display_size($maxbytes);
$maxbytesdisplay = display_size($maxbytes, 0);
throw new file_exception('maxbytesfile', (object) array('file' => $file->get_filename(),
'size' => $maxbytesdisplay));
}
@ -1743,7 +1743,7 @@ abstract class repository implements cacheable_object {
} else {
if ($maxbytes > 0 && $file->get_filesize() > $maxbytes) {
// note that stored_file::get_filesize() also calls synchronisation
$maxbytesdisplay = display_size($maxbytes);
$maxbytesdisplay = display_size($maxbytes, 0);
throw new file_exception('maxbytesfile', (object) array('file' => $file->get_filename(),
'size' => $maxbytesdisplay));
}

View File

@ -300,7 +300,7 @@ switch ($action) {
// Check if exceed maxbytes.
if ($maxbytes != -1 && filesize($downloadedfile['path']) > $maxbytes) {
$maxbytesdisplay = display_size($maxbytes);
$maxbytesdisplay = display_size($maxbytes, 0);
throw new file_exception('maxbytesfile', (object) array('file' => $record->filename,
'size' => $maxbytesdisplay));
}

View File

@ -191,7 +191,7 @@ class repository_upload extends repository {
}
if (($maxbytes!==-1) && (filesize($_FILES[$elname]['tmp_name']) > $maxbytes)) {
$maxbytesdisplay = display_size($maxbytes);
$maxbytesdisplay = display_size($maxbytes, 0);
throw new file_exception('maxbytesfile', (object) array('file' => $record->filename,
'size' => $maxbytesdisplay));
}

View File

@ -44,7 +44,7 @@ class private_files extends \core_form\dynamic_form {
if ($fileareainfo['filecount']) {
$a = (object) [
'used' => display_size($fileareainfo['filesize_without_references']),
'total' => display_size($maxareabytes)
'total' => display_size($maxareabytes, 0)
];
$quotamsg = get_string('quotausage', 'moodle', $a);
$notification = new \core\output\notification($quotamsg, \core\output\notification::NOTIFY_INFO);
@ -187,4 +187,4 @@ class private_files extends \core_form\dynamic_form {
protected function get_page_url_for_dynamic_submission(): \moodle_url {
return new moodle_url('/user/files.php');
}
}
}