Merge branch 'MDL-62240-master' of git://github.com/sarjona/moodle
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 3 May 2018 10:57:28 +0000 (18:57 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 3 May 2018 10:57:28 +0000 (18:57 +0800)
lib/moodlelib.php
lib/tests/moodlelib_test.php
privacy/classes/local/request/moodle_content_writer.php
privacy/tests/moodle_content_writer_test.php

index 04b0022..e1b368a 100644 (file)
@@ -8267,9 +8267,10 @@ function shorten_text($text, $ideal=30, $exact = false, $ending='...') {
  *
  * @param string $filename file name
  * @param int $length ideal string length
+ * @param bool $includehash Whether to include a file hash in the shortened version. This ensures uniqueness.
  * @return string $shortened shortened file name
  */
-function shorten_filename($filename, $length = MAX_FILENAME_SIZE) {
+function shorten_filename($filename, $length = MAX_FILENAME_SIZE, $includehash = false) {
     $shortened = $filename;
     // Extract a part of the filename if it's char size exceeds the ideal string length.
     if (core_text::strlen($filename) > $length) {
@@ -8278,15 +8279,36 @@ function shorten_filename($filename, $length = MAX_FILENAME_SIZE) {
         $extension = pathinfo($filename, PATHINFO_EXTENSION);
         if ($extension && !empty($mimetypes[$extension])) {
             $basename = pathinfo($filename, PATHINFO_FILENAME);
-            $shortened = core_text::substr($basename, 0, $length);
+            $hash = empty($includehash) ? '' : ' - ' . substr(sha1($basename), 0, 10);
+            $shortened = core_text::substr($basename, 0, $length - strlen($hash)) . $hash;
             $shortened .= '.' . $extension;
         } else {
-            $shortened = core_text::substr($filename, 0, $length);
+            $hash = empty($includehash) ? '' : ' - ' . substr(sha1($filename), 0, 10);
+            $shortened = core_text::substr($filename, 0, $length - strlen($hash)) . $hash;
         }
     }
     return $shortened;
 }
 
+/**
+ * Shortens a given array of filenames by removing characters positioned after the ideal string length.
+ *
+ * @param array $path The paths to reduce the length.
+ * @param int $length Ideal string length
+ * @param bool $includehash Whether to include a file hash in the shortened version. This ensures uniqueness.
+ * @return array $result Shortened paths in array.
+ */
+function shorten_filenames(array $path, $length = MAX_FILENAME_SIZE, $includehash = false) {
+    $result = null;
+
+    $result = array_reduce($path, function($carry, $singlepath) use ($length, $includehash) {
+        $carry[] = shorten_filename($singlepath, $length, $includehash);
+        return $carry;
+    }, []);
+
+    return $result;
+}
+
 /**
  * Given dates in seconds, how many weeks is the date from startdate
  * The first week is 1, the second 2 etc ...
index 87f59e8..80dc799 100644 (file)
@@ -1007,27 +1007,238 @@ class core_moodlelib_testcase extends advanced_testcase {
                 shorten_text($text, 1));
     }
 
-    public function test_shorten_filename() {
-        // Test filename that contains more than 100 characters.
+    /**
+     * Provider for long filenames and its expected result, with and without hash.
+     *
+     * @return array of ($filename, $length, $expected, $includehash)
+     */
+    public function shorten_filename_provider() {
         $filename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium totam rem';
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot',
-            shorten_filename($filename));
-        // Filename contains extension.
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip',
-            shorten_filename($filename . '.zip'));
-        // Limit filename to 50 chars.
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error si',
-            shorten_filename($filename, 50));
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error si.zip',
-            shorten_filename($filename . '.zip', 50));
-
-        // Test filename that contains less than 100 characters.
-        $filename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque';
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque',
-            shorten_filename($filename));
-        // Filename contains extension.
-        $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque.zip',
-            shorten_filename($filename . '.zip'));
+        $shortfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque';
+
+        return [
+            'More than 100 characters' => [
+                $filename,
+                null,
+                'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot',
+                false,
+            ],
+            'More than 100 characters with hash' => [
+                $filename,
+                null,
+                'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8',
+                true,
+            ],
+            'More than 100 characters with extension' => [
+                "{$filename}.zip",
+                null,
+                'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip',
+                false,
+            ],
+            'More than 100 characters with extension and hash' => [
+                "{$filename}.zip",
+                null,
+                'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8.zip',
+                true,
+            ],
+            'Limit filename to 50 chars' => [
+                $filename,
+                50,
+                'sed ut perspiciatis unde omnis iste natus error si',
+                false,
+            ],
+            'Limit filename to 50 chars with hash' => [
+                $filename,
+                50,
+                'sed ut perspiciatis unde omnis iste n - 3bec1da8b8',
+                true,
+            ],
+            'Limit filename to 50 chars with extension' => [
+                "{$filename}.zip",
+                50,
+                'sed ut perspiciatis unde omnis iste natus error si.zip',
+                false,
+            ],
+            'Limit filename to 50 chars with extension and hash' => [
+                "{$filename}.zip",
+                50,
+                'sed ut perspiciatis unde omnis iste n - 3bec1da8b8.zip',
+                true,
+            ],
+            'Test filename that contains less than 100 characters' => [
+                $shortfilename,
+                null,
+                $shortfilename,
+                false,
+            ],
+            'Test filename that contains less than 100 characters and hash' => [
+                $shortfilename,
+                null,
+                $shortfilename,
+                true,
+            ],
+            'Test filename that contains less than 100 characters with extension' => [
+                "{$shortfilename}.zip",
+                null,
+                "{$shortfilename}.zip",
+                false,
+            ],
+            'Test filename that contains less than 100 characters with extension and hash' => [
+                "{$shortfilename}.zip",
+                null,
+                "{$shortfilename}.zip",
+                true,
+            ],
+        ];
+    }
+
+    /**
+     * Test the {@link shorten_filename()} method.
+     *
+     * @dataProvider shorten_filename_provider
+     *
+     * @param string $filename
+     * @param int $length
+     * @param string $expected
+     * @param boolean $includehash
+     */
+    public function test_shorten_filename($filename, $length, $expected, $includehash) {
+        if (null === $length) {
+            $length = MAX_FILENAME_SIZE;
+        }
+
+        $this->assertSame($expected, shorten_filename($filename, $length, $includehash));
+    }
+
+    /**
+     * Provider for long filenames and its expected result, with and without hash.
+     *
+     * @return array of ($filename, $length, $expected, $includehash)
+     */
+    public function shorten_filenames_provider() {
+        $shortfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque';
+        $longfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium totam rem';
+        $extfilename = $longfilename.'.zip';
+        $expected = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot';
+        $expectedwithhash = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8';
+        $expectedext = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip';
+        $expectedextwithhash = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8.zip';
+        $expected50 = 'sed ut perspiciatis unde omnis iste natus error si';
+        $expected50withhash = 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8';
+        $expected50ext = 'sed ut perspiciatis unde omnis iste natus error si.zip';
+        $expected50extwithhash = 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8.zip';
+        $expected50short = 'sed ut perspiciatis unde omnis iste n - 5fb6543490';
+
+        return [
+            'Empty array without hash' => [
+                [],
+                null,
+                [],
+                false,
+            ],
+            'Empty array with hash' => [
+                [],
+                null,
+                [],
+                true,
+            ],
+            'Array with less than 100 characters' => [
+                [$shortfilename, $shortfilename, $shortfilename],
+                null,
+                [$shortfilename, $shortfilename, $shortfilename],
+                false,
+            ],
+            'Array with more than 100 characters without hash' => [
+                [$longfilename, $longfilename, $longfilename],
+                null,
+                [$expected, $expected, $expected],
+                false,
+            ],
+            'Array with more than 100 characters with hash' => [
+                [$longfilename, $longfilename, $longfilename],
+                null,
+                [$expectedwithhash, $expectedwithhash, $expectedwithhash],
+                true,
+            ],
+            'Array with more than 100 characters with extension' => [
+                [$extfilename, $extfilename, $extfilename],
+                null,
+                [$expectedext, $expectedext, $expectedext],
+                false,
+            ],
+            'Array with more than 100 characters with extension and hash' => [
+                [$extfilename, $extfilename, $extfilename],
+                null,
+                [$expectedextwithhash, $expectedextwithhash, $expectedextwithhash],
+                true,
+            ],
+            'Array with more than 100 characters mix (short, long, with extension) without hash' => [
+                [$shortfilename, $longfilename, $extfilename],
+                null,
+                [$shortfilename, $expected, $expectedext],
+                false,
+            ],
+            'Array with more than 100 characters mix (short, long, with extension) with hash' => [
+                [$shortfilename, $longfilename, $extfilename],
+                null,
+                [$shortfilename, $expectedwithhash, $expectedextwithhash],
+                true,
+            ],
+            'Array with less than 50 characters without hash' => [
+                [$longfilename, $longfilename, $longfilename],
+                50,
+                [$expected50, $expected50, $expected50],
+                false,
+            ],
+            'Array with less than 50 characters with hash' => [
+                [$longfilename, $longfilename, $longfilename],
+                50,
+                [$expected50withhash, $expected50withhash, $expected50withhash],
+                true,
+            ],
+            'Array with less than 50 characters with extension' => [
+                [$extfilename, $extfilename, $extfilename],
+                50,
+                [$expected50ext, $expected50ext, $expected50ext],
+                false,
+            ],
+            'Array with less than 50 characters with extension and hash' => [
+                [$extfilename, $extfilename, $extfilename],
+                50,
+                [$expected50extwithhash, $expected50extwithhash, $expected50extwithhash],
+                true,
+            ],
+            'Array with less than 50 characters mix (short, long, with extension) without hash' => [
+                [$shortfilename, $longfilename, $extfilename],
+                50,
+                [$expected50, $expected50, $expected50ext],
+                false,
+            ],
+            'Array with less than 50 characters mix (short, long, with extension) with hash' => [
+                [$shortfilename, $longfilename, $extfilename],
+                50,
+                [$expected50short, $expected50withhash, $expected50extwithhash],
+                true,
+            ],
+        ];
+    }
+
+    /**
+     * Test the {@link shorten_filenames()} method.
+     *
+     * @dataProvider shorten_filenames_provider
+     *
+     * @param string $filenames
+     * @param int $length
+     * @param string $expected
+     * @param boolean $includehash
+     */
+    public function test_shorten_filenames($filenames, $length, $expected, $includehash) {
+        if (null === $length) {
+            $length = MAX_FILENAME_SIZE;
+        }
+
+        $this->assertSame($expected, shorten_filenames($filenames, $length, $includehash));
     }
 
     public function test_usergetdate() {
index de607cc..4e8f100 100644 (file)
@@ -244,7 +244,7 @@ class moodle_content_writer implements content_writer {
         $path = [];
         $contexts = array_reverse($this->context->get_parent_contexts(true));
         foreach ($contexts as $context) {
-            $path[] = clean_param($context->get_context_name(), PARAM_FILE);
+            $path[] = shorten_filename(clean_param($context->get_context_name(), PARAM_FILE), MAX_FILENAME_SIZE, true);
         }
 
         return $path;
@@ -258,6 +258,9 @@ class moodle_content_writer implements content_writer {
      * @return  string                      The fully-qualfiied file path.
      */
     protected function get_path(array $subcontext, string $name) : string {
+        $subcontext = shorten_filenames($subcontext, MAX_FILENAME_SIZE, true);
+        $name = shorten_filename($name, MAX_FILENAME_SIZE, true);
+
         // Combine the context path, and the subcontext data.
         $path = array_merge(
             $this->get_context_path(),
index b683348..a045829 100644 (file)
@@ -913,6 +913,151 @@ class moodle_content_writer_test extends advanced_testcase {
         ];
     }
 
+    /**
+     * Test that exported data is shortened when exceeds the limit.
+     *
+     * @dataProvider long_filename_provider
+     * @param string $longtext
+     * @param string $expected
+     * @param string $text
+     */
+    public function test_export_data_long_filename($longtext, $expected, $text) {
+        $context = \context_system::instance();
+        $subcontext = [$longtext];
+        $data = (object) ['key' => $text];
+
+        $writer = $this->get_writer_instance()
+                ->set_context($context)
+                ->export_data($subcontext, $data);
+
+        $fileroot = $this->fetch_exported_content($writer);
+
+        $contextpath = $this->get_context_path($context, $subcontext, 'data.json');
+        $expectedpath = 'System/'.$expected.'/data.json';
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $json = $fileroot->getChild($contextpath)->getContent();
+        $this->assertRegExp("/$text/", $json);
+
+        $expanded = json_decode($json);
+        $this->assertEquals($data, $expanded);
+    }
+
+    /**
+     * Test that exported related data is shortened when exceeds the limit.
+     *
+     * @dataProvider long_filename_provider
+     * @param string $longtext
+     * @param string $expected
+     * @param string $text
+     */
+    public function test_export_related_data_long_filename($longtext, $expected, $text) {
+        $context = \context_system::instance();
+        $subcontext = [$longtext];
+        $data = (object) ['key' => $text];
+
+        $writer = $this->get_writer_instance()
+                ->set_context($context)
+                ->export_related_data($subcontext, 'name', $data);
+
+        $fileroot = $this->fetch_exported_content($writer);
+
+        $contextpath = $this->get_context_path($context, $subcontext, 'name.json');
+        $expectedpath = 'System/'.$expected.'/name.json';
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $json = $fileroot->getChild($contextpath)->getContent();
+        $this->assertRegExp("/$text/", $json);
+
+        $expanded = json_decode($json);
+        $this->assertEquals($data, $expanded);
+    }
+
+    /**
+     * Test that exported metadata is shortened when exceeds the limit.
+     *
+     * @dataProvider long_filename_provider
+     * @param string $longtext
+     * @param string $expected
+     * @param string $text
+     */
+    public function test_export_metadata_long_filename($longtext, $expected, $text) {
+        $context = \context_system::instance();
+        $subcontext = [$longtext];
+        $data = (object) ['key' => $text];
+
+        $writer = $this->get_writer_instance()
+                ->set_context($context)
+                ->export_metadata($subcontext, $text, $text, $text);
+
+        $fileroot = $this->fetch_exported_content($writer);
+
+        $contextpath = $this->get_context_path($context, $subcontext, 'metadata.json');
+        $expectedpath = 'System/'.$expected.'/metadata.json';
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $json = $fileroot->getChild($contextpath)->getContent();
+        $this->assertRegExp("/$text.*$text.*$text/", $json);
+
+        $expanded = json_decode($json);
+        $this->assertTrue(isset($expanded->$text));
+        $this->assertEquals($text, $expanded->$text->value);
+        $this->assertEquals($text, $expanded->$text->description);
+    }
+
+    /**
+     * Test that exported user preference is shortened when exceeds the limit.
+     *
+     * @dataProvider long_filename_provider
+     * @param string $longtext
+     * @param string $expected
+     * @param string $text
+     */
+    public function test_export_user_preference_long_filename($longtext, $expected, $text) {
+        $this->resetAfterTest();
+
+        if (!array_key_exists('json', core_filetypes::get_types())) {
+            // Add json as mime type to avoid lose the extension when shortening filenames.
+            core_filetypes::add_type('json', 'application/json', 'archive', [], '', 'JSON file archive');
+        }
+        $expectedpath = 'System/User preferences/'.$expected.'.json';
+
+        $context = \context_system::instance();
+        $component = $longtext;
+
+        $writer = $this->get_writer_instance()
+                ->set_context($context)
+                ->export_user_preference($component, $text, $text, $text);
+
+        $fileroot = $this->fetch_exported_content($writer);
+
+        $contextpath = $this->get_context_path($context, [get_string('userpreferences')], "{$component}.json");
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $json = $fileroot->getChild($contextpath)->getContent();
+        $this->assertRegExp("/$text.*$text.*$text/", $json);
+
+        $expanded = json_decode($json);
+        $this->assertTrue(isset($expanded->$text));
+        $this->assertEquals($text, $expanded->$text->value);
+        $this->assertEquals($text, $expanded->$text->description);
+    }
+
+    /**
+     * Provider for long filenames.
+     *
+     * @return array
+     */
+    public function long_filename_provider() {
+        return [
+            'More than 100 characters' => [
+                'Etiam sit amet dui vel leo blandit viverra. Proin viverra suscipit velit. Aenean efficitur suscipit nibh nec suscipit',
+                'Etiam sit amet dui vel leo blandit viverra. Proin viverra suscipit velit. Aenean effici - 22f7a5030d',
+                'value',
+            ],
+        ];
+    }
+
     /**
      * Get a fresh content writer.
      *