Not overwriting files during export, and instead adding a suffix to them.

Also greatly simplifying name sanitization.

MyTracks/src/com/google/android/apps/mytracks/io/TrackWriter.java

@@ -155,9 +155,14 @@ public class TrackWriter {
       return false;
     }
 
-    // Make sure the name will work on FAT
+    // Make sure the file doesn't exist yet (possibly by changing the filename)
-    String fileName =
+    String fileName = fileUtils.buildUniqueFileName(
-        fileUtils.sanitizeName(track.getName() + "." + writer.getExtension());
+        directory, track.getName(), writer.getExtension());
+    if (fileName == null) {
+      Log.e(MyTracksConstants.TAG, "Unable to get a unique filename for " + fileName);
+      return false;
+    }
+
     Log.i(MyTracksConstants.TAG, "Writing track to: " + fileName);
     try {
       writer.prepare(track, newOutputStream(fileName));

MyTracks/src/com/google/android/apps/mytracks/util/FileUtils.java

@@ -20,7 +20,7 @@ import com.google.android.apps.mytracks.MyTracksConstants;
 import android.os.Environment;
 
 import java.io.File;
-import java.util.HashSet;
+import java.util.regex.Pattern;
 
 /**
  * Utilities for dealing with files.
@@ -28,27 +28,16 @@ import java.util.HashSet;
  * @author Rodrigo Damazio
  */
 public class FileUtils {
+  /**
+   * The maximum length of a filename, as per the FAT32 specification.
+   */
+  private static final int MAX_FILENAME_LENGTH = 260;
 
   /**
    * A set of characters that are prohibited from being in file names.
    */
-  private static final HashSet<Character> PROHIBITED_CHARACTERS =
+  private static final Pattern PROHIBITED_CHAR_PATTERN =
-      new HashSet<Character>();
+      Pattern.compile("[^ A-Za-z0-9_.()]+");
-
-  static {
-    for (int i = 0; i < 48; i++) {
-      PROHIBITED_CHARACTERS.add(new Character((char) i));
-    }
-    for (int i = 58; i < 65; i++) {
-      PROHIBITED_CHARACTERS.add(new Character((char) i));
-    }
-    for (int i = 91; i < 97; i++) {
-      PROHIBITED_CHARACTERS.add(new Character((char) i));
-    }
-    for (int i = 123; i < 128; i++) {
-      PROHIBITED_CHARACTERS.add(new Character((char) i));
-    }
-  }
 
   /**
    * Builds a path inside the My Tracks directory in the SD card.
@@ -72,26 +61,33 @@ public class FileUtils {
    * Returns whether the SD card is available.
    */
   public boolean isSdCardAvailable() {
-    return Environment.MEDIA_MOUNTED.equals(Environment.getExternalStorageState());
+    return Environment.MEDIA_MOUNTED.equals(
+        Environment.getExternalStorageState());
   }
 
   /**
    * Normalizes the input string and make sure it is a valid fat32 file name.
+   *
+   * @param name the name to normalize
+   * @param overheadSize the number of additional characters that will be added
+   *        to the name after sanitization
+   * @return the sanitized name
    */
-  public String sanitizeName(String name) {
+  String sanitizeName(String name) {
-    StringBuilder cleaned = new StringBuilder();
+    String cleaned = PROHIBITED_CHAR_PATTERN.matcher(name).replaceAll("");
-    for (int i = 0; i < name.length(); i++) {
-      char c = name.charAt(i);
-      if (!PROHIBITED_CHARACTERS.contains(c)) {
-        cleaned.append(c);
-      }
-    }
 
-    return (cleaned.length() > 260)
+    return (cleaned.length() > MAX_FILENAME_LENGTH)
-        ? cleaned.substring(0, 260)
+        ? cleaned.substring(0, MAX_FILENAME_LENGTH)
         : cleaned.toString();
   }
 
+  /**
+   * Ensures the given directory exists by creating it and its parents if
+   * necessary.
+   * 
+   * @return whether the directory exists (either already existed or was
+   *         successfully created)
+   */
   public boolean ensureDirectoryExists(File dir) {
     if (dir.exists() && dir.isDirectory()) {
       return true;
@@ -103,4 +99,53 @@ public class FileUtils {
 
     return false;
   }
+
+  /**
+   * Builds a filename with the given base name (prefix) and the given
+   * extension, possibly adding a suffix to ensure the file doesn't exist.
+   *
+   * @param directory the directory the file will live in
+   * @param fileBaseName the prefix for the file name
+   * @param extension the file's extension
+   * @return the complete file name, without the directory
+   */
+  public synchronized String buildUniqueFileName(File directory,
+      String fileBaseName, String extension) {
+    return buildUniqueFileName(directory, fileBaseName, extension, 0);
+  }
+
+  /**
+   * Builds a filename with the given base name (prefix) and the given
+   * extension, possibly adding a suffix to ensure the file doesn't exist.
+   *
+   * @param directory the directory the file will live in
+   * @param fileBaseName the prefix for the file name
+   * @param extension the file's extension
+   * @param suffix the first numeric suffix to try to use, or 0 for none
+   * @return the complete file name, without the directory
+   */
+  private String buildUniqueFileName(File directory, String fileBaseName,
+      String extension, int suffix) {
+    String suffixedBaseName = fileBaseName;
+    if (suffix > 0) {
+      suffixedBaseName += " (" + Integer.toString(suffix) + ")";
+    }
+
+    String fullName = suffixedBaseName + "." + extension;
+    String sanitizedName = sanitizeName(fullName);
+    if (!fileExists(directory, sanitizedName)) {
+      return sanitizedName;
+    }
+
+    return buildUniqueFileName(directory, fileBaseName, extension, suffix + 1);
+  }
+
+  /**
+   * Checks whether a file with the given name exists in the given directory.
+   * This is isolated so it can be overridden in tests.
+   */
+  protected boolean fileExists(File directory, String fullName) {
+    File file = new File(directory, fullName);
+    return file.exists();
+  }
 }

MyTracksTest/src/com/google/android/apps/mytracks/io/TrackWriterTest.java

@@ -117,7 +117,7 @@ public class TrackWriterTest extends AndroidTestCase {
   private static final long TRACK_ID = 1234567L;
   private static final String EXTENSION = "ext";
   private static final String TRACK_NAME = "Swimming across the pacific";
-  private static final String FULL_TRACK_NAME = "Swimmingacrossthepacific.ext";
+  private static final String FULL_TRACK_NAME = "Swimming across the pacific.ext";
 
   private Track track;
   private TrackFormatWriter formatWriter;

MyTracksTest/src/com/google/android/apps/mytracks/util/FileUtilsTest.java

@@ -20,6 +20,8 @@ import com.google.android.apps.mytracks.MyTracksConstants;
 import android.os.Environment;
 
 import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
 
 import junit.framework.TestCase;
 
@@ -29,16 +31,23 @@ import junit.framework.TestCase;
  * @author Rodrigo Damazio
  */
 public class FileUtilsTest extends TestCase {
-  private static final String ORIGINAL_NAME = "Swimming across the pacific";
+  private static final String ORIGINAL_NAME = "Swim\10ming ^across: the/ pacific (ocean).";
-  private static final String SANITIZED_NAME = "Swimmingacrossthepacific";
+  private static final String SANITIZED_NAME = "Swimming across the pacific (ocean).";
 
   private FileUtils fileUtils;
+  private Set<String> existingFiles;
 
   @Override
   protected void setUp() throws Exception {
     super.setUp();
 
-    fileUtils = new FileUtils();
+    existingFiles = new HashSet<String>();
+    fileUtils = new FileUtils() {
+      @Override
+      protected boolean fileExists(File directory, String fullName) {
+        return existingFiles.contains(fullName);
+      }
+    };
   }
 
   public void testBuildExternalDirectoryPath() {
@@ -59,4 +68,28 @@ public class FileUtilsTest extends TestCase {
   public void testSanitizeName() {
     assertEquals(SANITIZED_NAME, fileUtils.sanitizeName(ORIGINAL_NAME));
   }
+
+  public void testBuildUniqueFileName_someExist() {
+    existingFiles = new HashSet<String>();
+    existingFiles.add("Filename.ext");
+    existingFiles.add("Filename (1).ext");
+    existingFiles.add("Filename (2).ext");
+    existingFiles.add("Filename (3).ext");
+    existingFiles.add("Filename (4).ext");
+
+    String filename = fileUtils.buildUniqueFileName(null, "Filename", "ext");
+    assertEquals("Filename (5).ext", filename);
+  }
+
+  public void testBuildUniqueFileName_oneExists() {
+    existingFiles.add("Filename.ext");
+
+    String filename = fileUtils.buildUniqueFileName(null, "Filename", "ext");
+    assertEquals("Filename (1).ext", filename);
+  }
+
+  public void testBuildUniqueFileName_noneExists() {
+    String filename = fileUtils.buildUniqueFileName(null, "Filename", "ext");
+    assertEquals("Filename.ext", filename);
+  }
 }