Fix NPE in ParseKeyValueCache due to cache dir not exist

Author: wangmengyan95

Parse/src/main/java/com/parse/ParseKeyValueCache.java

@@ -64,14 +64,21 @@
   }
 
   private static File getKeyValueCacheDir() {
+    if (directory == null || !directory.exists()) {
+      directory.mkdir();
+    }
     return directory;
   }
 
   /**
    * How many files are in the key-value cache.
    */
   /* package */ static int size() {
-    return getKeyValueCacheDir().listFiles().length;
+    File[] files = getKeyValueCacheDir().listFiles();
+    if (files == null) {
+      return 0;
+    }
+    return files.length;
   }
 
   private static File getKeyValueCacheFile(String key) {
@@ -96,7 +103,7 @@ private static long getKeyValueCacheAge(File cacheFile) {
     }
   }
 
-  /* package */ private static File createKeyValueCacheFile(String key) {
+  private static File createKeyValueCacheFile(String key) {
     String filename = String.valueOf(new Date().getTime()) + '.' + key;
     return new File(getKeyValueCacheDir(), filename);
   }
@@ -127,9 +134,7 @@ private static long getKeyValueCacheAge(File cacheFile) {
       }
       File f = createKeyValueCacheFile(key);
       try {
-        FileOutputStream out = new FileOutputStream(f);
-        out.write(value.getBytes("UTF-8"));
-        out.close();
+        ParseFileUtils.writeByteArrayToFile(f, value.getBytes("UTF-8"));
       } catch (UnsupportedEncodingException e) {
         // do nothing
       } catch (IOException e) {
@@ -138,38 +143,47 @@ private static long getKeyValueCacheAge(File cacheFile) {
 
       // Check if we should kick out old cache entries
       File[] files = getKeyValueCacheDir().listFiles();
+      // We still need this check since dir.mkdir() may fail
+      if (files == null || files.length == 0) {
+        return;
+      }
+
       int numFiles = files.length;
       int numBytes = 0;
       for (File file : files) {
         numBytes += file.length();
       }
-      if (numFiles > maxKeyValueCacheFiles || numBytes > maxKeyValueCacheBytes) {
-        // We need to kick out some cache entries.
-        // Sort oldest-first. We touch on read so mtime is really LRU.
-        // Sometimes (i.e. tests) the time of lastModified isn't granular enough,
-        // so we resort
-        // to sorting by the file name which is always prepended with time in ms
-        Arrays.sort(files, new Comparator<File>() {
-          @Override
-          public int compare(File f1, File f2) {
-            int dateCompare = Long.valueOf(f1.lastModified()).compareTo(f2.lastModified());
-            if (dateCompare != 0) {
-              return dateCompare;
-            } else {
-              return f1.getName().compareTo(f2.getName());
-            }
-          }
-        });
 
-        for (File file : files) {
-          numFiles--;
-          numBytes -= file.length();
-          file.delete();
+      // If we do not need to clear the cache, simply return
+      if (numFiles <= maxKeyValueCacheFiles && numBytes <= maxKeyValueCacheBytes) {
+        return;
+      }
 
-          if (numFiles <= maxKeyValueCacheFiles && numBytes <= maxKeyValueCacheBytes) {
-            break;
+      // We need to kick out some cache entries.
+      // Sort oldest-first. We touch on read so mtime is really LRU.
+      // Sometimes (i.e. tests) the time of lastModified isn't granular enough,
+      // so we resort
+      // to sorting by the file name which is always prepended with time in ms
+      Arrays.sort(files, new Comparator<File>() {
+        @Override
+        public int compare(File f1, File f2) {
+          int dateCompare = Long.valueOf(f1.lastModified()).compareTo(f2.lastModified());
+          if (dateCompare != 0) {
+            return dateCompare;
+          } else {
+            return f1.getName().compareTo(f2.getName());
           }
         }
+      });
+
+      for (File file : files) {
+        numFiles--;
+        numBytes -= file.length();
+        file.delete();
+
+        if (numFiles <= maxKeyValueCacheFiles && numBytes <= maxKeyValueCacheBytes) {
+          break;
+        }
       }
     }
   }

Parse/src/test/java/com/parse/ParseKeyValueCacheTest.java

@@ -14,20 +14,29 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Callable;
 
 import bolts.Task;
 
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 public class ParseKeyValueCacheTest {
 
+  private File keyValueCacheDir;
+
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
   @Before
   public void setUp() throws Exception {
-    ParseKeyValueCache.initialize(temporaryFolder.newFolder("ParseKeyValueCache"));
+    keyValueCacheDir = temporaryFolder.newFolder("ParseKeyValueCache");
+    ParseKeyValueCache.initialize(keyValueCacheDir);
   }
 
   @After
@@ -59,4 +68,31 @@ public Void call() throws Exception {
     }
     ParseTaskUtils.wait(Task.whenAll(tasks));
   }
+
+  @Test
+  public void testSaveToKeyValueCacheWithoutCacheDir() throws Exception {
+    // Delete the cache folder(Simulate users clear the app cache)
+    assertTrue(keyValueCacheDir.exists());
+    keyValueCacheDir.delete();
+    assertFalse(keyValueCacheDir.exists());
+
+    // Save a key value pair
+    ParseKeyValueCache.saveToKeyValueCache("key", "value");
+
+    // Verify cache file is correct
+    assertEquals(1, keyValueCacheDir.listFiles().length);
+    assertArrayEquals(
+        "value".getBytes(), ParseFileUtils.readFileToByteArray(keyValueCacheDir.listFiles()[0]));
+  }
+
+  @Test
+  public void testGetSizeWithoutCacheDir() throws Exception {
+    // Delete the cache folder(Simulate users clear the app cache)
+    assertTrue(keyValueCacheDir.exists());
+    keyValueCacheDir.delete();
+    assertFalse(keyValueCacheDir.exists());
+
+    // Verify size is zero
+    assertEquals(0, ParseKeyValueCache.size());
+  }
 }