From f3846ef735018ee2887e0723ba3d0b6208468093 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 7 Dec 2023 21:10:26 +0000 Subject: [PATCH] MDL-80247 files: multi-byte aware filename shortening in exporter. Also correctly extract the original file extension. Co-authored-by: Andrew Nicols Co-authored-by: Mazitov Artem --- .../classes/external/stored_file_exporter.php | 21 ++- .../external/stored_file_exporter_test.php | 126 ++++++++++++++++++ 2 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 files/tests/external/stored_file_exporter_test.php diff --git a/files/classes/external/stored_file_exporter.php b/files/classes/external/stored_file_exporter.php index c491170e971..2289beaafcc 100644 --- a/files/classes/external/stored_file_exporter.php +++ b/files/classes/external/stored_file_exporter.php @@ -14,15 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Class for exporting stored_file data. - * - * @package core_files - * @copyright 2015 Frédéric Massart - FMCorz.net - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ namespace core_files\external; -defined('MOODLE_INTERNAL') || die(); use coding_exception; use core_text; @@ -40,6 +32,9 @@ use stored_file; */ class stored_file_exporter extends \core\external\exporter { + /** @var int Length of the shortened filename */ + protected const FILENAMESHORT_LENGTH = 25; + /** @var stored_file */ protected $file; @@ -142,9 +137,13 @@ class stored_file_exporter extends \core\external\exporter { protected function get_other_values(renderer_base $output) { $filename = $this->file->get_filename(); $filenameshort = $filename; - if (core_text::strlen($filename) > 25) { - $filenameshort = shorten_text(substr($filename, 0, -4), 21, true, '..'); - $filenameshort .= substr($filename, -4); + + if (core_text::strlen($filename) > static::FILENAMESHORT_LENGTH) { + $extension = pathinfo($filename, PATHINFO_EXTENSION); + $extensionlength = core_text::strlen($extension) + 1; + $filenameshort = core_text::substr($filename, 0, -$extensionlength); + $filenameshort = shorten_text($filenameshort, static::FILENAMESHORT_LENGTH - $extensionlength, true, '..') . + ".{$extension}"; } $icon = $this->file->is_directory() ? file_folder_icon() : file_file_icon($this->file); diff --git a/files/tests/external/stored_file_exporter_test.php b/files/tests/external/stored_file_exporter_test.php new file mode 100644 index 00000000000..af7549f8330 --- /dev/null +++ b/files/tests/external/stored_file_exporter_test.php @@ -0,0 +1,126 @@ +. + +namespace core_files\external; + +use advanced_testcase; +use context_user; + +/** + * Unit tests for stored file exporter + * + * @package core_files + * @covers \core_files\external\stored_file_exporter + * @copyright 2023 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class stored_file_exporter_test extends advanced_testcase { + + /** + * Test exported data structure + */ + public function test_export(): void { + global $PAGE, $USER, $CFG; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $contextuser = context_user::instance($USER->id); + + $file = get_file_storage()->create_file_from_string([ + 'contextid' => $contextuser->id, + 'userid' => $USER->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => file_get_unused_draft_itemid(), + 'filepath' => '/', + 'filename' => 'Hi.txt', + ], 'Hello'); + + $exporter = new stored_file_exporter($file, ['context' => $contextuser]); + $export = $exporter->export($PAGE->get_renderer('core')); + + $this->assertEquals((object) [ + 'contextid' => $file->get_contextid(), + 'component' => $file->get_component(), + 'filearea' => $file->get_filearea(), + 'itemid' => $file->get_itemid(), + 'filepath' => $file->get_filepath(), + 'filename' => $file->get_filename(), + 'isdir' => false, + 'isimage' => false, + 'timemodified' => $file->get_timemodified(), + 'timecreated' => $file->get_timecreated(), + 'filesize' => $file->get_filesize(), + 'author' => $file->get_author(), + 'license' => $file->get_license(), + 'filenameshort' => $file->get_filename(), + 'filesizeformatted' => display_size($file->get_filesize()), + 'icon' => 'f/text', + 'timecreatedformatted' => userdate($file->get_timecreated()), + 'timemodifiedformatted' => userdate($file->get_timemodified()), + 'url' => "{$CFG->wwwroot}/pluginfile.php/{$contextuser->id}/user/draft/{$file->get_itemid()}/Hi.txt?forcedownload=1", + ], $export); + } + + /** + * Data provider for {@see test_export_filenameshort} + * + * @return array[] + */ + public static function export_filenameshort_provider(): array { + return [ + // Long filenames (30 characters), with extensions of varying length. + ['Lorem ipsum dolor sit amet sit.c', 'Lorem ipsum dolor sit...c'], + ['Lorem ipsum dolor sit amet sit.txt', 'Lorem ipsum dolor s...txt'], + ['Lorem ipsum dolor sit amet sit.docx', 'Lorem ipsum dolor ...docx'], + // Multi-byte filenames. + ['Мазитов А.З. практика тусур.py', 'Мазитов А.З. практик...py'], + ]; + } + + /** + * Test exporting shortened filename + * + * @param string $filename + * @param string $expected + * + * @dataProvider export_filenameshort_provider + */ + public function test_export_filenameshort(string $filename, string $expected): void { + global $PAGE, $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $contextuser = context_user::instance($USER->id); + + $file = get_file_storage()->create_file_from_string([ + 'contextid' => $contextuser->id, + 'userid' => $USER->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => file_get_unused_draft_itemid(), + 'filepath' => '/', + 'filename' => $filename, + ], 'Hello'); + + $exporter = new stored_file_exporter($file, ['context' => $contextuser]); + $export = $exporter->export($PAGE->get_renderer('core')); + + $this->assertEquals($expected, $export->filenameshort); + } +}