MDL-37641 files: Improve file name suggestion and performance

lib/filestorage/file_storage.php

@@ -180,6 +180,89 @@ class file_storage {
         return $preview;
     }
 
+    /**
+     * Return an available file name.
+     *
+     * This will return the next available file name in the area, adding/incrementing a suffix
+     * of the file, ie: file.txt > file (1).txt > file (2).txt > etc...
+     *
+     * If the file name passed is available without modification, it is returned as is.
+     *
+     * @param int $contextid context ID.
+     * @param string $component component.
+     * @param string $filearea file area.
+     * @param int $itemid area item ID.
+     * @param string $filepath the file path.
+     * @param string $filename the file name.
+     * @return string available file name.
+     * @throws coding_exception if the file name is invalid.
+     * @since 2.5
+     */
+    public function get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, $filename) {
+        global $DB;
+
+        // Do not accept '.' or an empty file name (zero is acceptable).
+        if ($filename == '.' || (empty($filename) && !is_numeric($filename))) {
+            throw new coding_exception('Invalid file name passed', $filename);
+        }
+
+        // The file does not exist, we return the same file name.
+        if (!$this->file_exists($contextid, $component, $filearea, $itemid, $filepath, $filename)) {
+            return $filename;
+        }
+
+        // Trying to locate a file name using the used pattern. We remove the used pattern from the file name first.
+        $pathinfo = pathinfo($filename);
+        $basename = $pathinfo['filename'];
+        $matches = array();
+        if (preg_match('~^(.+) \(([0-9]+)\)$~', $basename, $matches)) {
+            $basename = $matches[1];
+        }
+
+        $filenamelike = $DB->sql_like_escape($basename) . ' (%)';
+        if (isset($pathinfo['extension'])) {
+            $filenamelike .= '.' . $DB->sql_like_escape($pathinfo['extension']);
+        }
+
+        $filenamelikesql = $DB->sql_like('f.filename', ':filenamelike');
+        $filenamelen = $DB->sql_length('f.filename');
+        $sql = "SELECT filename
+                FROM {files} f
+                WHERE
+                    f.contextid = :contextid AND
+                    f.component = :component AND
+                    f.filearea = :filearea AND
+                    f.itemid = :itemid AND
+                    f.filepath = :filepath AND
+                    $filenamelikesql
+                ORDER BY
+                    $filenamelen DESC,
+                    f.filename DESC";
+        $params = array('contextid' => $contextid, 'component' => $component, 'filearea' => $filearea, 'itemid' => $itemid,
+                'filepath' => $filepath, 'filenamelike' => $filenamelike);
+        $results = $DB->get_fieldset_sql($sql, $params, IGNORE_MULTIPLE);
+
+        // Loop over the results to make sure we are working on a valid file name. Because 'file (1).txt' and 'file (copy).txt'
+        // would both be returned, but only the one only containing digits should be used.
+        $number = 1;
+        foreach ($results as $result) {
+            $resultbasename = pathinfo($result, PATHINFO_FILENAME);
+            $matches = array();
+            if (preg_match('~^(.+) \(([0-9]+)\)$~', $resultbasename, $matches)) {
+                $number = $matches[2] + 1;
+                break;
+            }
+        }
+
+        // Constructing the new filename.
+        $newfilename = $basename . ' (' . $number . ')';
+        if (isset($pathinfo['extension'])) {
+            $newfilename .= '.' . $pathinfo['extension'];
+        }
+
+        return $newfilename;
+    }
+
     /**
      * Generates a preview image for the stored file
      *

lib/form/dndupload.js

@@ -796,12 +796,12 @@ M.form_dndupload.init = function(Y, options) {
                 extension = filename.substr(dotpos, filename.length);
             }
 
-            // Look to see if the name already has _NN at the end of it.
+            // Look to see if the name already has (NN) at the end of it.
             var number = 0;
-            var hasnumber = basename.match(/^(.*)_(\d+)$/);
+            var hasnumber = basename.match(/^(.*) \((\d+)\)$/);
-            if (hasnumber != null) {
+            if (hasnumber !== null) {
                 // Note the current number & remove it from the basename.
-                number = parseInt(hasnumber[2]);
+                number = parseInt(hasnumber[2], 10);
                 basename = hasnumber[1];
             }
 
@@ -809,7 +809,7 @@ M.form_dndupload.init = function(Y, options) {
             var newname;
             do {
                 number++;
-                newname = basename + '_' + number + extension;
+                newname = basename + ' (' + number + ')' + extension;
             } while (this.has_name_clash(newname));
 
             return newname;

repository/lib.php

@@ -636,23 +636,19 @@ abstract class repository {
     }
 
     /**
-     * Check if file already exists in draft area
+     * Check if file already exists in draft area.
      *
      * @static
-     * @param int $itemid
+     * @param int $itemid of the draft area.
-     * @param string $filepath
+     * @param string $filepath path to the file.
-     * @param string $filename
+     * @param string $filename file name.
      * @return bool
      */
     public static function draftfile_exists($itemid, $filepath, $filename) {
         global $USER;
         $fs = get_file_storage();
         $usercontext = context_user::instance($USER->id);
-        if ($fs->get_file($usercontext->id, 'user', 'draft', $itemid, $filepath, $filename)) {
+        return $fs->file_exists($usercontext->id, 'user', 'draft', $itemid, $filepath, $filename);
-            return true;
-        } else {
-            return false;
-        }
     }
 
     /**
@@ -769,31 +765,34 @@ abstract class repository {
     }
 
     /**
-     * Get unused filename by appending suffix
+     * Get an unused filename from the current draft area.
+     *
+     * Will check if the file ends with ([0-9]) and increase the number.
      *
      * @static
-     * @param int $itemid
+     * @param int $itemid draft item ID.
-     * @param string $filepath
+     * @param string $filepath path to the file.
-     * @param string $filename
+     * @param string $filename name of the file.
-     * @return string
+     * @return string an unused file name.
      */
     public static function get_unused_filename($itemid, $filepath, $filename) {
         global $USER;
+        $contextid = context_user::instance($USER->id)->id;
         $fs = get_file_storage();
-        while (repository::draftfile_exists($itemid, $filepath, $filename)) {
+        return $fs->get_unused_filename($contextid, 'user', 'draft', $itemid, $filepath, $filename);
-            $filename = repository::append_suffix($filename);
-        }
-        return $filename;
     }
 
     /**
-     * Append a suffix to filename
+     * Append a suffix to filename.
      *
      * @static
      * @param string $filename
      * @return string
+     * @deprecated since 2.5
      */
     public static function append_suffix($filename) {
+        debugging('The function repository::append_suffix() has been deprecated. Use repository::get_unused_filename() instead.',
+            DEBUG_DEVELOPER);
         $pathinfo = pathinfo($filename);
         if (empty($pathinfo['extension'])) {
             return $filename . RENAME_SUFFIX;

repository/upgrade.txt

@@ -3,6 +3,11 @@ information provided here is intended especially for developers. Full
 details of the repository API are available on Moodle docs:
 http://docs.moodle.org/dev/Repository_API
 
+=== 2.5 ===
+
+* repository::append_suffix() has been deprecated, use repository::get_unused_filename() if you need
+  to get a file name which has not yet been used in the draft area.
+
 === 2.4 ===
 
 * copy_to_area() can receive a new parameter called $areamaxbytes which controls the maximum