diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index aa41c1bafe259..dfb70d84f0b7f 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -180,6 +180,89 @@ public function get_file_preview(stored_file $file, $mode) { 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 * diff --git a/lib/filestorage/tests/file_storage_test.php b/lib/filestorage/tests/file_storage_test.php index 049acfe0908fb..c634bfba4b74e 100644 --- a/lib/filestorage/tests/file_storage_test.php +++ b/lib/filestorage/tests/file_storage_test.php @@ -1352,4 +1352,67 @@ public function test_delete_reference_one_symlink_does_not_rule_them_all() { $aliasrecord->filearea, $aliasrecord->itemid, '/B/', 'symlink.txt'); $this->assertTrue($symlink2->is_external_file()); } + + public function test_get_unused_filename() { + global $USER; + $this->resetAfterTest(true); + + $fs = get_file_storage(); + $this->setAdminUser(); + $contextid = context_user::instance($USER->id)->id; + $component = 'user'; + $filearea = 'private'; + $itemid = 0; + $filepath = '/'; + + // Create some private files. + $file = new stdClass; + $file->contextid = $contextid; + $file->component = 'user'; + $file->filearea = 'private'; + $file->itemid = 0; + $file->filepath = '/'; + $file->source = 'test'; + $filenames = array('foo.txt', 'foo (1).txt', 'foo (20).txt', 'foo (999)', 'bar.jpg', 'What (a cool file).jpg', + 'Hurray! (1).php', 'Hurray! (2).php', 'Hurray! (9a).php', 'Hurray! (abc).php'); + foreach ($filenames as $key => $filename) { + $file->filename = $filename; + $userfile = $fs->create_file_from_string($file, "file $key $filename content"); + $this->assertInstanceOf('stored_file', $userfile); + } + + // Asserting new generated names. + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'unused.txt'); + $this->assertEquals('unused.txt', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo.txt'); + $this->assertEquals('foo (21).txt', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo (1).txt'); + $this->assertEquals('foo (21).txt', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo (2).txt'); + $this->assertEquals('foo (2).txt', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo (20).txt'); + $this->assertEquals('foo (21).txt', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo'); + $this->assertEquals('foo', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo (123)'); + $this->assertEquals('foo (123)', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'foo (999)'); + $this->assertEquals('foo (1000)', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'bar.png'); + $this->assertEquals('bar.png', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'bar (12).png'); + $this->assertEquals('bar (12).png', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'bar.jpg'); + $this->assertEquals('bar (1).jpg', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'bar (1).jpg'); + $this->assertEquals('bar (1).jpg', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'What (a cool file).jpg'); + $this->assertEquals('What (a cool file) (1).jpg', $newfilename); + $newfilename = $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, 'Hurray! (1).php'); + $this->assertEquals('Hurray! (3).php', $newfilename); + + $this->setExpectedException('coding_exception'); + $fs->get_unused_filename($contextid, $component, $filearea, $itemid, $filepath, ''); + } + } diff --git a/lib/form/dndupload.js b/lib/form/dndupload.js index 084f922eac478..e6a6ad37be5eb 100644 --- a/lib/form/dndupload.js +++ b/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+)$/); - if (hasnumber != null) { + var hasnumber = basename.match(/^(.*) \((\d+)\)$/); + 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; diff --git a/repository/lib.php b/repository/lib.php index 714e1b9f454e3..f28a2d7e755d2 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -636,23 +636,19 @@ public final function check_capability() { } /** - * Check if file already exists in draft area + * Check if file already exists in draft area. * * @static - * @param int $itemid - * @param string $filepath - * @param string $filename + * @param int $itemid of the draft area. + * @param string $filepath path to the file. + * @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 true; - } else { - return false; - } + return $fs->file_exists($usercontext->id, 'user', 'draft', $itemid, $filepath, $filename); } /** @@ -769,31 +765,34 @@ public function copy_to_area($source, $filerecord, $maxbytes = -1, $areamaxbytes } /** - * 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 string $filepath - * @param string $filename - * @return string + * @param int $itemid draft item ID. + * @param string $filepath path to the file. + * @param string $filename name of the file. + * @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)) { - $filename = repository::append_suffix($filename); - } - return $filename; + return $fs->get_unused_filename($contextid, 'user', 'draft', $itemid, $filepath, $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; diff --git a/repository/tests/repository_test.php b/repository/tests/repository_test.php index 46eaf44926404..ff8c754bec81a 100644 --- a/repository/tests/repository_test.php +++ b/repository/tests/repository_test.php @@ -58,4 +58,100 @@ public function test_install_repository() { $info = $repository->get_meta(); $this->assertEquals($repositorypluginname, $info->type); } + + public function test_get_unused_filename() { + global $USER; + + $this->resetAfterTest(true); + + $this->setAdminUser(); + $fs = get_file_storage(); + + $draftitemid = null; + $context = context_user::instance($USER->id); + file_prepare_draft_area($draftitemid, $context->id, 'phpunit', 'test_get_unused_filename', 1); + + $dummy = array( + 'contextid' => $context->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => $draftitemid, + 'filepath' => '/', + 'filename' => '' + ); + + // Create some files. + $existingfiles = array( + 'test', + 'test.txt', + 'test (1).txt', + 'test1.txt', + 'test1 (1).txt', + 'test1 (2).txt', + 'test1 (3).txt', + 'test1 (My name is Bob).txt', + 'test2 (555).txt', + 'test3 (1000).txt', + 'test3 (1000MB).txt', + ); + foreach ($existingfiles as $filename) { + $dummy['filename'] = $filename; + $file = $fs->create_file_from_string($dummy, 'blah! ' . $filename); + $this->assertTrue(repository::draftfile_exists($draftitemid, '/', $filename)); + } + + // Actual testing. + $this->assertEquals('free.txt', repository::get_unused_filename($draftitemid, '/', 'free.txt')); + $this->assertEquals('test (1)', repository::get_unused_filename($draftitemid, '/', 'test')); + $this->assertEquals('test (2).txt', repository::get_unused_filename($draftitemid, '/', 'test.txt')); + $this->assertEquals('test1 (4).txt', repository::get_unused_filename($draftitemid, '/', 'test1.txt')); + $this->assertEquals('test1 (8).txt', repository::get_unused_filename($draftitemid, '/', 'test1 (8).txt')); + $this->assertEquals('test1 ().txt', repository::get_unused_filename($draftitemid, '/', 'test1 ().txt')); + $this->assertEquals('test2 (556).txt', repository::get_unused_filename($draftitemid, '/', 'test2 (555).txt')); + $this->assertEquals('test3 (1001).txt', repository::get_unused_filename($draftitemid, '/', 'test3 (1000).txt')); + $this->assertEquals('test3 (1000MB) (1).txt', repository::get_unused_filename($draftitemid, '/', 'test3 (1000MB).txt')); + $this->assertEquals('test4 (1).txt', repository::get_unused_filename($draftitemid, '/', 'test4 (1).txt')); + } + + public function test_draftfile_exists() { + global $USER; + + $this->resetAfterTest(true); + + $this->setAdminUser(); + $fs = get_file_storage(); + + $draftitemid = file_get_unused_draft_itemid(); + $context = context_user::instance($USER->id); + + $dummy = array( + 'contextid' => $context->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => $draftitemid, + 'filepath' => '/', + 'filename' => '' + ); + + // Create some files. + $existingfiles = array( + 'The Matrix.movie', + 'Astalavista.txt', + 'foobar', + ); + foreach ($existingfiles as $filename) { + $dummy['filename'] = $filename; + $file = $fs->create_file_from_string($dummy, 'Content of ' . $filename); + $this->assertInstanceOf('stored_file', $file); + } + + // Doing the text. + foreach ($existingfiles as $filename) { + $this->assertTrue(repository::draftfile_exists($draftitemid, '/', $filename)); + } + foreach (array('Terminator.movie', 'Where is Wally?', 'barfoo') as $filename) { + $this->assertFalse(repository::draftfile_exists($draftitemid, '/', $filename)); + } + } + } diff --git a/repository/upgrade.txt b/repository/upgrade.txt index 7cd84ac9d0bb2..4f420d634f44f 100644 --- a/repository/upgrade.txt +++ b/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