Skip to content

Commit

Permalink
MDL-72643 core: Improve display_size
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@sammarshallou
sammarshallou committed Sep 27, 2021
1 parent 214adb7 commit e332d18
Show file tree
Hide file tree
Showing 18 changed files with 160 additions and 70 deletions.
2 changes: 1 addition & 1 deletion admin/classes/local/settings/filesize.php
View file
Expand Up @@ -98,7 +98,7 @@ protected static function get_size_text(int $bytes): string {
if (empty($bytes)) {
return get_string('none');
}
return display_size($bytes);
return display_size($bytes, 0);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion course/downloadcontent.php
View file
Expand Up @@ -87,7 +87,7 @@
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);
Expand Down
4 changes: 2 additions & 2 deletions files/renderer.php
View file
Expand Up @@ -289,9 +289,9 @@ public function filemanager_js_templates() {
* @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) {
Expand Down
2 changes: 1 addition & 1 deletion h5p/ajax.php
View file
Expand Up @@ -85,7 +85,7 @@
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);
Expand Down
2 changes: 1 addition & 1 deletion lib/classes/content/export/zipwriter.php
View file
Expand Up @@ -273,7 +273,7 @@ public function add_file_from_template(
'courseshortname' => $exportedcourse->shortname,
'courselink' => $courselink,
'exportdate' => userdate(time()),
'maxfilesize' => display_size($this->maxfilesize),
'maxfilesize' => display_size($this->maxfilesize, 0),
];

$renderer = $PAGE->get_renderer('core');
Expand Down
68 changes: 45 additions & 23 deletions lib/moodlelib.php
View file
Expand Up @@ -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);
}
}

Expand All @@ -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.
}

Expand Down Expand Up @@ -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;

Expand All @@ -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];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/outputrenderers.php
View file
Expand Up @@ -2699,7 +2699,7 @@ public function render_file_picker(file_picker $fp) {
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 . '"';
Expand Down
2 changes: 1 addition & 1 deletion lib/portfoliolib.php
View file
Expand Up @@ -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,
Expand Down
116 changes: 89 additions & 27 deletions lib/tests/moodlelib_test.php
View file
Expand Up @@ -2743,7 +2743,8 @@ public function test_get_max_upload_sizes() {
$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.
Expand All @@ -2752,7 +2753,7 @@ public function test_get_max_upload_sizes() {
$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.
Expand All @@ -2761,7 +2762,7 @@ public function test_get_max_upload_sizes() {
$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.
Expand All @@ -2770,15 +2771,15 @@ public function test_get_max_upload_sizes() {
$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;
$coursebytes = 51200;
$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.
Expand Down Expand Up @@ -2821,9 +2822,9 @@ public function test_get_max_upload_sizes() {
$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.
Expand Down Expand Up @@ -4987,25 +4988,25 @@ public function test_rename_to_unused_name_failure() {
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'],
];
}

Expand All @@ -5017,6 +5018,67 @@ public function display_size_provider() {
*/
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);
}

Expand Down
3 changes: 3 additions & 0 deletions lib/upgrade.txt
View file
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion mod/forum/classes/message/inbound/reply_handler.php
View file
Expand Up @@ -220,7 +220,7 @@ public function process_message(\stdClass $record, \stdClass $messagedata) {
. "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);
}
Expand Down
4 changes: 2 additions & 2 deletions mod/resource/tests/behat/display_resource.feature
View file
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions question/type/essay/tests/behat/max_file_size.feature
View file
Expand Up @@ -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"

0 comments on commit e332d18

Please sign in to comment.