From 2c89a4c170d05d82b81b7eba6b829aeb10aaa665 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 20 Sep 2016 18:54:46 -0600 Subject: [PATCH] Rename should behave like create: avoid conflict. When we create a file that already exists, we try attaching a suffix like "(1)" to the filename to avoid the conflict. The newly added rename method should do the same, since developers may not have access to delete the conflicting file. Test: boots, rename via UI, new unit tests Bug: 31545404 Change-Id: Ie397eebb0fbf98cf079eee3bbbb6c6b7ca627d91 --- core/java/android/os/FileUtils.java | 47 ++++++++++++++----- .../src/android/os/FileUtilsTest.java | 14 ++++++ .../ExternalStorageProvider.java | 5 +- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index fa32848e63626..8e24caf478393 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -605,6 +605,22 @@ public class FileUtils { return null; } + private static File buildUniqueFileWithExtension(File parent, String name, String ext) + throws FileNotFoundException { + File file = buildFile(parent, name, ext); + + // If conflicting file, try adding counter suffix + int n = 0; + while (file.exists()) { + if (n++ >= 32) { + throw new FileNotFoundException("Failed to create unique file"); + } + file = buildFile(parent, name + " (" + n + ")", ext); + } + + return file; + } + /** * Generates a unique file name under the given parent directory. If the display name doesn't * have an extension that matches the requested MIME type, the default extension for that MIME @@ -619,20 +635,29 @@ public class FileUtils { public static File buildUniqueFile(File parent, String mimeType, String displayName) throws FileNotFoundException { final String[] parts = splitFileName(mimeType, displayName); - final String name = parts[0]; - final String ext = parts[1]; - File file = buildFile(parent, name, ext); + return buildUniqueFileWithExtension(parent, parts[0], parts[1]); + } - // If conflicting file, try adding counter suffix - int n = 0; - while (file.exists()) { - if (n++ >= 32) { - throw new FileNotFoundException("Failed to create unique file"); - } - file = buildFile(parent, name + " (" + n + ")", ext); + /** + * Generates a unique file name under the given parent directory, keeping + * any extension intact. + */ + public static File buildUniqueFile(File parent, String displayName) + throws FileNotFoundException { + final String name; + final String ext; + + // Extract requested extension from display name + final int lastDot = displayName.lastIndexOf('.'); + if (lastDot >= 0) { + name = displayName.substring(0, lastDot); + ext = displayName.substring(lastDot + 1); + } else { + name = displayName; + ext = null; } - return file; + return buildUniqueFileWithExtension(parent, name, ext); } /** diff --git a/core/tests/coretests/src/android/os/FileUtilsTest.java b/core/tests/coretests/src/android/os/FileUtilsTest.java index ac5abadcc48a4..bd900793f87d9 100644 --- a/core/tests/coretests/src/android/os/FileUtilsTest.java +++ b/core/tests/coretests/src/android/os/FileUtilsTest.java @@ -297,6 +297,20 @@ public class FileUtilsTest extends AndroidTestCase { FileUtils.buildUniqueFile(mTarget, "image/jpeg", "test.jpg")); } + public void testBuildUniqueFile_mimeless() throws Exception { + assertNameEquals("test.jpg", FileUtils.buildUniqueFile(mTarget, "test.jpg")); + new File(mTarget, "test.jpg").createNewFile(); + assertNameEquals("test (1).jpg", FileUtils.buildUniqueFile(mTarget, "test.jpg")); + + assertNameEquals("test", FileUtils.buildUniqueFile(mTarget, "test")); + new File(mTarget, "test").createNewFile(); + assertNameEquals("test (1)", FileUtils.buildUniqueFile(mTarget, "test")); + + assertNameEquals("test.foo.bar", FileUtils.buildUniqueFile(mTarget, "test.foo.bar")); + new File(mTarget, "test.foo.bar").createNewFile(); + assertNameEquals("test.foo (1).bar", FileUtils.buildUniqueFile(mTarget, "test.foo.bar")); + } + private static void assertNameEquals(String expected, File actual) { assertEquals(expected, actual.getName()); } diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java index 7fe0d2fd074ab..3ef9b8e1b8c4e 100644 --- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java +++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java @@ -466,10 +466,7 @@ public class ExternalStorageProvider extends DocumentsProvider { displayName = FileUtils.buildValidFatFilename(displayName); final File before = getFileForDocId(docId); - final File after = new File(before.getParentFile(), displayName); - if (after.exists()) { - throw new IllegalStateException("Already exists " + after); - } + final File after = FileUtils.buildUniqueFile(before.getParentFile(), displayName); if (!before.renameTo(after)) { throw new IllegalStateException("Failed to rename to " + after); }