MDL-62240 file: Add includehash to shorten_filename + unit tests
authorAndrew Nicols <andrew@nicols.co.uk>
Wed, 2 May 2018 02:45:12 +0000 (10:45 +0800)
committerSara Arjona <sara@moodle.com>
Thu, 3 May 2018 06:27:12 +0000 (08:27 +0200)
lib/moodlelib.php
lib/tests/moodlelib_test.php

index 04b0022..e7d1ad2 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 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
  */
  * @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) {
     $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,10 +8279,12 @@ function shorten_filename($filename, $length = MAX_FILENAME_SIZE) {
         $extension = pathinfo($filename, PATHINFO_EXTENSION);
         if ($extension && !empty($mimetypes[$extension])) {
             $basename = pathinfo($filename, PATHINFO_FILENAME);
         $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 .= '.' . $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;
         }
     }
     return $shortened;
index 87f59e8..f49fb78 100644 (file)
@@ -1007,27 +1007,107 @@ class core_moodlelib_testcase extends advanced_testcase {
                 shorten_text($text, 1));
     }
 
                 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';
         $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));
     }
 
     public function test_usergetdate() {
     }
 
     public function test_usergetdate() {