diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java index 935a5af13610..3bc114c155ac 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java @@ -23,9 +23,12 @@ import java.io.Reader; import java.io.StringWriter; import java.io.Writer; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; +import java.util.Deque; import java.util.List; +import java.util.concurrent.Executor; import okio.BufferedSink; import okio.BufferedSource; import okio.Okio; @@ -35,6 +38,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.junit.rules.Timeout; import static com.squareup.okhttp.internal.DiskLruCache.JOURNAL_FILE; import static com.squareup.okhttp.internal.DiskLruCache.JOURNAL_FILE_BACKUP; @@ -48,26 +52,39 @@ import static org.junit.Assert.fail; public final class DiskLruCacheTest { + @Rule public final TemporaryFolder tempDir = new TemporaryFolder(); + @Rule public final Timeout timeout = new Timeout(30 * 1000); + private final int appVersion = 100; private File cacheDir; private File journalFile; private File journalBkpFile; + private final TestExecutor executor = new TestExecutor(); + private DiskLruCache cache; + private final Deque toClose = new ArrayDeque<>(); - @Rule public TemporaryFolder tempDir = new TemporaryFolder(); + private void createNewCache() throws IOException { + createNewCacheWithSize(Integer.MAX_VALUE); + } + + private void createNewCacheWithSize(int maxSize) throws IOException { + cache = new DiskLruCache(cacheDir, appVersion, 2, maxSize, executor); + cache.initialize(); + toClose.add(cache); + } @Before public void setUp() throws Exception { - cacheDir = tempDir.newFolder("DiskLruCacheTest"); + cacheDir = tempDir.getRoot(); journalFile = new File(cacheDir, JOURNAL_FILE); journalBkpFile = new File(cacheDir, JOURNAL_FILE_BACKUP); - for (File file : cacheDir.listFiles()) { - file.delete(); - } - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); } @After public void tearDown() throws Exception { - cache.close(); + while (!toClose.isEmpty()) { + toClose.pop().close(); + } } @Test public void emptyCache() throws Exception { @@ -135,14 +152,6 @@ public final class DiskLruCacheTest { cache.edit(key).abort(); } - static class X { - private T t; - - public X(T t) { - this.t = t; - } - } - @Test public void writeAndReadEntry() throws Exception { DiskLruCache.Editor creator = cache.edit("k1"); creator.set(0, "ABC"); @@ -167,7 +176,7 @@ public X(T t) { creator.commit(); cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); DiskLruCache.Snapshot snapshot = cache.get("k1"); assertEquals("A", snapshot.getString(0)); assertEquals(1, snapshot.getLength(0)); @@ -183,14 +192,13 @@ public X(T t) { creator.commit(); // Simulate a dirty close of 'cache' by opening the cache directory again. - DiskLruCache cache2 = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); - DiskLruCache.Snapshot snapshot = cache2.get("k1"); + createNewCache(); + DiskLruCache.Snapshot snapshot = cache.get("k1"); assertEquals("A", snapshot.getString(0)); assertEquals(1, snapshot.getLength(0)); assertEquals("B", snapshot.getString(1)); assertEquals(1, snapshot.getLength(1)); snapshot.close(); - cache2.close(); } @Test public void journalWithEditAndPublish() throws Exception { @@ -326,7 +334,7 @@ public X(T t) { writeFile(dirtyFile0, "C"); writeFile(dirtyFile1, "D"); createJournal("CLEAN k1 1 1", "DIRTY k1"); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertFalse(cleanFile0.exists()); assertFalse(cleanFile1.exists()); assertFalse(dirtyFile0.exists()); @@ -338,7 +346,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournalWithHeader(MAGIC, "0", "100", "2", ""); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); } @@ -346,7 +354,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournalWithHeader(MAGIC, "1", "101", "2", ""); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); } @@ -354,7 +362,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournalWithHeader(MAGIC, "1", "100", "1", ""); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); } @@ -362,7 +370,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournalWithHeader(MAGIC, "1", "100", "2", "x"); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); } @@ -370,7 +378,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournal("CLEAN k1 1 1", "BOGUS"); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); assertNull(cache.get("k1")); } @@ -379,7 +387,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournal("CLEAN k1 0000x001 1"); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); assertNull(cache.get("k1")); } @@ -391,13 +399,14 @@ public X(T t) { Writer writer = new FileWriter(journalFile); writer.write(MAGIC + "\n" + VERSION_1 + "\n100\n2\n\nCLEAN k1 1 1"); // no trailing newline writer.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertNull(cache.get("k1")); // The journal is not corrupt when editing after a truncated line. set("k1", "C", "D"); + cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertValue("k1", "C", "D"); } @@ -405,7 +414,7 @@ public X(T t) { cache.close(); generateSomeGarbageFiles(); createJournal("CLEAN k1 1 1 1"); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); assertGarbageFilesAllDeleted(); assertNull(cache.get("k1")); } @@ -494,7 +503,7 @@ public X(T t) { @Test public void growMaxSize() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "a", "aaa"); // size 4 set("b", "bb", "bbbb"); // size 6 cache.setMaxSize(20); @@ -504,18 +513,17 @@ public X(T t) { @Test public void shrinkMaxSizeEvicts() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 20); + createNewCacheWithSize(20); set("a", "a", "aaa"); // size 4 set("b", "bb", "bbbb"); // size 6 set("c", "c", "c"); // size 12 cache.setMaxSize(10); - assertEquals(1, cache.executorService.getQueue().size()); - cache.executorService.purge(); + assertEquals(1, executor.jobs.size()); } @Test public void evictOnInsert() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "a", "aaa"); // size 4 set("b", "bb", "bbbb"); // size 6 @@ -551,7 +559,7 @@ public X(T t) { @Test public void evictOnUpdate() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "a", "aa"); // size 3 set("b", "b", "bb"); // size 3 @@ -569,7 +577,7 @@ public X(T t) { @Test public void evictionHonorsLruFromCurrentSession() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "a", "a"); set("b", "b", "b"); set("c", "c", "c"); @@ -601,7 +609,7 @@ public X(T t) { cache.get("b").close(); // 'B' is now least recently used. assertEquals(12, cache.size()); cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("g", "g", "g"); cache.flush(); @@ -617,7 +625,7 @@ public X(T t) { @Test public void cacheSingleEntryOfSizeGreaterThanMaxSize() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "aaaaa", "aaaaaa"); // size=11 cache.flush(); assertAbsent("a"); @@ -625,7 +633,7 @@ public X(T t) { @Test public void cacheSingleValueOfSizeGreaterThanMaxSize() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "aaaaaaaaaaa", "a"); // size=12 cache.flush(); assertAbsent("a"); @@ -661,35 +669,18 @@ public X(T t) { @Test public void rebuildJournalOnRepeatedReads() throws Exception { set("a", "a", "a"); set("b", "b", "b"); - long lastJournalLength = 0; - while (true) { - long journalLength = journalFile.length(); + while (executor.jobs.isEmpty()) { assertValue("a", "a", "a"); assertValue("b", "b", "b"); - if (journalLength < lastJournalLength) { - System.out - .printf("Journal compacted from %s bytes to %s bytes\n", lastJournalLength, - journalLength); - break; // Test passed! - } - lastJournalLength = journalLength; } } @Test public void rebuildJournalOnRepeatedEdits() throws Exception { - long lastJournalLength = 0; - while (true) { - long journalLength = journalFile.length(); + while (executor.jobs.isEmpty()) { set("a", "a", "a"); set("b", "b", "b"); - if (journalLength < lastJournalLength) { - System.out - .printf("Journal compacted from %s bytes to %s bytes\n", lastJournalLength, - journalLength); - break; - } - lastJournalLength = journalLength; } + executor.jobs.removeFirst().run(); // Sanity check that a rebuilt journal behaves normally. assertValue("a", "a", "a"); @@ -700,39 +691,21 @@ public X(T t) { @Test public void rebuildJournalOnRepeatedReadsWithOpenAndClose() throws Exception { set("a", "a", "a"); set("b", "b", "b"); - long lastJournalLength = 0; - while (true) { - long journalLength = journalFile.length(); + while (executor.jobs.isEmpty()) { assertValue("a", "a", "a"); assertValue("b", "b", "b"); cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); - if (journalLength < lastJournalLength) { - System.out - .printf("Journal compacted from %s bytes to %s bytes\n", lastJournalLength, - journalLength); - break; // Test passed! - } - lastJournalLength = journalLength; + createNewCache(); } } /** @see Issue #28 */ @Test public void rebuildJournalOnRepeatedEditsWithOpenAndClose() throws Exception { - long lastJournalLength = 0; - while (true) { - long journalLength = journalFile.length(); + while (executor.jobs.isEmpty()) { set("a", "a", "a"); set("b", "b", "b"); cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); - if (journalLength < lastJournalLength) { - System.out - .printf("Journal compacted from %s bytes to %s bytes\n", lastJournalLength, - journalLength); - break; - } - lastJournalLength = journalLength; + createNewCache(); } } @@ -746,7 +719,7 @@ public X(T t) { assertTrue(journalFile.renameTo(journalBkpFile)); assertFalse(journalFile.exists()); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); DiskLruCache.Snapshot snapshot = cache.get("k1"); assertEquals("ABC", snapshot.getString(0)); @@ -776,7 +749,7 @@ public X(T t) { assertTrue(journalFile.exists()); assertTrue(journalBkpFile.exists()); - cache = DiskLruCache.open(cacheDir, appVersion, 2, Integer.MAX_VALUE); + createNewCache(); DiskLruCache.Snapshot snapshotA = cache.get("k1"); assertEquals("ABC", snapshotA.getString(0)); @@ -842,7 +815,7 @@ public X(T t) { @Test public void editSinceEvicted() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "aa", "aaa"); // size 5 DiskLruCache.Snapshot snapshot = cache.get("a"); set("b", "bb", "bbb"); // size 5 @@ -853,7 +826,7 @@ public X(T t) { @Test public void editSinceEvictedAndRecreated() throws Exception { cache.close(); - cache = DiskLruCache.open(cacheDir, appVersion, 2, 10); + createNewCacheWithSize(10); set("a", "aa", "aaa"); // size 5 DiskLruCache.Snapshot snapshot = cache.get("a"); set("b", "bb", "bbb"); // size 5 @@ -1091,12 +1064,12 @@ private void generateSomeGarbageFiles() throws Exception { } private void assertGarbageFilesAllDeleted() throws Exception { - assertFalse((getCleanFile("g1", 0)).exists()); - assertFalse((getCleanFile("g1", 1)).exists()); - assertFalse((getCleanFile("g2", 0)).exists()); - assertFalse((getCleanFile("g2", 1)).exists()); - assertFalse((new File(cacheDir, "otherFile0")).exists()); - assertFalse((new File(cacheDir, "dir1")).exists()); + assertFalse(getCleanFile("g1", 0).exists()); + assertFalse(getCleanFile("g1", 1).exists()); + assertFalse(getCleanFile("g2", 0).exists()); + assertFalse(getCleanFile("g2", 1).exists()); + assertFalse(new File(cacheDir, "otherFile0").exists()); + assertFalse(new File(cacheDir, "dir1").exists()); } private void set(String key, String value0, String value1) throws Exception { @@ -1112,10 +1085,10 @@ private void assertAbsent(String key) throws Exception { snapshot.close(); fail(); } - assertFalse((getCleanFile(key, 0)).exists()); - assertFalse((getCleanFile(key, 1)).exists()); - assertFalse((getDirtyFile(key, 0)).exists()); - assertFalse((getDirtyFile(key, 1)).exists()); + assertFalse(getCleanFile(key, 0).exists()); + assertFalse(getCleanFile(key, 1).exists()); + assertFalse(getDirtyFile(key, 0).exists()); + assertFalse(getDirtyFile(key, 1).exists()); } private void assertValue(String key, String value0, String value1) throws Exception { @@ -1124,8 +1097,8 @@ private void assertValue(String key, String value0, String value1) throws Except assertEquals(value0.length(), snapshot.getLength(0)); assertEquals(value1, snapshot.getString(1)); assertEquals(value1.length(), snapshot.getLength(1)); - assertTrue((getCleanFile(key, 0)).exists()); - assertTrue((getCleanFile(key, 1)).exists()); + assertTrue(getCleanFile(key, 0).exists()); + assertTrue(getCleanFile(key, 1).exists()); snapshot.close(); } @@ -1136,4 +1109,12 @@ private void copyFile(File from, File to) throws IOException { source.close(); sink.close(); } + + private static class TestExecutor implements Executor { + final Deque jobs = new ArrayDeque<>(); + + @Override public void execute(Runnable command) { + jobs.addLast(command); + } + } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java index a5e5e4fbd6bd..50e455e57bbd 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.concurrent.Executor; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -154,9 +155,8 @@ public final class DiskLruCache implements Closeable { */ private long nextSequenceNumber = 0; - /** This cache uses a single background thread to evict entries. */ - final ThreadPoolExecutor executorService = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, - new LinkedBlockingQueue(), Util.threadFactory("OkHttp DiskLruCache", true)); + /** Used to run 'cleanupRunnable' for journal rebuilds. */ + private final Executor executor; private final Runnable cleanupRunnable = new Runnable() { public void run() { synchronized (DiskLruCache.this) { @@ -176,7 +176,7 @@ public void run() { } }; - private DiskLruCache(File directory, int appVersion, int valueCount, long maxSize) { + DiskLruCache(File directory, int appVersion, int valueCount, long maxSize, Executor executor) { this.directory = directory; this.appVersion = appVersion; this.journalFile = new File(directory, JOURNAL_FILE); @@ -184,6 +184,36 @@ private DiskLruCache(File directory, int appVersion, int valueCount, long maxSiz this.journalFileBackup = new File(directory, JOURNAL_FILE_BACKUP); this.valueCount = valueCount; this.maxSize = maxSize; + this.executor = executor; + } + + // Visible for testing. + void initialize() throws IOException { + // If a bkp file exists, use it instead. + if (journalFileBackup.exists()) { + // If journal file also exists just delete backup file. + if (journalFile.exists()) { + journalFileBackup.delete(); + } else { + renameTo(journalFileBackup, journalFile, false); + } + } + + // Prefer to pick up where we left off. + if (journalFile.exists()) { + try { + readJournal(); + processJournal(); + return; + } catch (IOException journalIsCorrupt) { + Platform.get().logW("DiskLruCache " + directory + " is corrupt: " + + journalIsCorrupt.getMessage() + ", removing"); + delete(); + } + } + + directory.mkdirs(); + rebuildJournal(); } /** @@ -204,36 +234,12 @@ public static DiskLruCache open(File directory, int appVersion, int valueCount, throw new IllegalArgumentException("valueCount <= 0"); } - // If a bkp file exists, use it instead. - File backupFile = new File(directory, JOURNAL_FILE_BACKUP); - if (backupFile.exists()) { - File journalFile = new File(directory, JOURNAL_FILE); - // If journal file also exists just delete backup file. - if (journalFile.exists()) { - backupFile.delete(); - } else { - renameTo(backupFile, journalFile, false); - } - } - - // Prefer to pick up where we left off. - DiskLruCache cache = new DiskLruCache(directory, appVersion, valueCount, maxSize); - if (cache.journalFile.exists()) { - try { - cache.readJournal(); - cache.processJournal(); - return cache; - } catch (IOException journalIsCorrupt) { - Platform.get().logW("DiskLruCache " + directory + " is corrupt: " - + journalIsCorrupt.getMessage() + ", removing"); - cache.delete(); - } - } + // Use a single background thread to evict entries. + Executor executor = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, + new LinkedBlockingQueue(), Util.threadFactory("OkHttp DiskLruCache", true)); - // Create a new empty cache. - directory.mkdirs(); - cache = new DiskLruCache(directory, appVersion, valueCount, maxSize); - cache.rebuildJournal(); + DiskLruCache cache = new DiskLruCache(directory, appVersion, valueCount, maxSize, executor); + cache.initialize(); return cache; } @@ -363,7 +369,7 @@ private synchronized void rebuildJournal() throws IOException { } else { writer.writeUtf8(CLEAN).writeByte(' '); writer.writeUtf8(entry.key); - writer.writeUtf8(entry.getLengths()); + entry.writeLengths(writer); writer.writeByte('\n'); } } @@ -436,7 +442,7 @@ public synchronized Snapshot get(String key) throws IOException { redundantOpCount++; journalWriter.writeUtf8(READ).writeByte(' ').writeUtf8(key).writeByte('\n'); if (journalRebuildRequired()) { - executorService.execute(cleanupRunnable); + executor.execute(cleanupRunnable); } return new Snapshot(key, entry.sequenceNumber, sources, entry.lengths); @@ -493,7 +499,7 @@ public synchronized long getMaxSize() { */ public synchronized void setMaxSize(long maxSize) { this.maxSize = maxSize; - executorService.execute(cleanupRunnable); + executor.execute(cleanupRunnable); } /** @@ -547,7 +553,7 @@ private synchronized void completeEdit(Editor editor, boolean success) throws IO entry.readable = true; journalWriter.writeUtf8(CLEAN).writeByte(' '); journalWriter.writeUtf8(entry.key); - journalWriter.writeUtf8(entry.getLengths()); + entry.writeLengths(journalWriter); journalWriter.writeByte('\n'); if (success) { entry.sequenceNumber = nextSequenceNumber++; @@ -561,7 +567,7 @@ private synchronized void completeEdit(Editor editor, boolean success) throws IO journalWriter.flush(); if (size > maxSize || journalRebuildRequired()) { - executorService.execute(cleanupRunnable); + executor.execute(cleanupRunnable); } } @@ -607,7 +613,7 @@ private boolean removeEntry(Entry entry) throws IOException { lruEntries.remove(entry.key); if (journalRebuildRequired()) { - executorService.execute(cleanupRunnable); + executor.execute(cleanupRunnable); } return true; @@ -943,14 +949,6 @@ private Entry(String key) { } } - public String getLengths() throws IOException { - StringBuilder result = new StringBuilder(); - for (long size : lengths) { - result.append(' ').append(size); - } - return result.toString(); - } - /** Set lengths using decimal numbers like "10123". */ private void setLengths(String[] strings) throws IOException { if (strings.length != valueCount) { @@ -966,6 +964,13 @@ private void setLengths(String[] strings) throws IOException { } } + /** Append space-prefixed lengths to {@code writer}. */ + void writeLengths(BufferedSink writer) throws IOException { + for (long length : lengths) { + writer.writeByte(' ').writeUtf8(Long.toString(length)); + } + } + private IOException invalidLengths(String[] strings) throws IOException { throw new IOException("unexpected journal line: " + Arrays.toString(strings)); }