Skip to content

Commit

Permalink
New APIs to iterate the response cache.
Browse files Browse the repository at this point in the history
Closes #853
  • Loading branch information
squarejesse committed Dec 29, 2014
1 parent 36be8ff commit a409634
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 6 deletions.
128 changes: 128 additions & 0 deletions okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.NoSuchElementException;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -1931,6 +1932,133 @@ public void assertCookies(URL url, String... expectedCookies) throws Exception {
assertEquals("A", get(url).body().string());
}

@Test public void iterateCache() throws Exception {
// Put some responses in the cache.
server.enqueue(new MockResponse()
.setBody("a"));
URL urlA = server.getUrl("/a");
assertEquals("a", get(urlA).body().string());

server.enqueue(new MockResponse()
.setBody("b"));
URL urlB = server.getUrl("/b");
assertEquals("b", get(urlB).body().string());

server.enqueue(new MockResponse()
.setBody("c"));
URL urlC = server.getUrl("/c");
assertEquals("c", get(urlC).body().string());

// Confirm the iterator returns those responses...
Iterator<String> i = cache.urls();
assertTrue(i.hasNext());
assertEquals(urlA.toString(), i.next());
assertTrue(i.hasNext());
assertEquals(urlB.toString(), i.next());
assertTrue(i.hasNext());
assertEquals(urlC.toString(), i.next());

// ... and nothing else.
assertFalse(i.hasNext());
try {
i.next();
fail();
} catch (NoSuchElementException expected) {
}
}

@Test public void iteratorRemoveFromCache() throws Exception {
// Put a response in the cache.
server.enqueue(new MockResponse()
.addHeader("Cache-Control: max-age=60")
.setBody("a"));
URL url = server.getUrl("/a");
assertEquals("a", get(url).body().string());

// Remove it with iteration.
Iterator<String> i = cache.urls();
assertEquals(url.toString(), i.next());
i.remove();

// Confirm that subsequent requests suffer a cache miss.
server.enqueue(new MockResponse()
.setBody("b"));
assertEquals("b", get(url).body().string());
}

@Test public void iteratorRemoveWithoutNextThrows() throws Exception {
// Put a response in the cache.
server.enqueue(new MockResponse()
.setBody("a"));
URL url = server.getUrl("/a");
assertEquals("a", get(url).body().string());

Iterator<String> i = cache.urls();
assertTrue(i.hasNext());
try {
i.remove();
fail();
} catch (IllegalStateException expected) {
}
}

@Test public void iteratorRemoveOncePerCallToNext() throws Exception {
// Put a response in the cache.
server.enqueue(new MockResponse()
.setBody("a"));
URL url = server.getUrl("/a");
assertEquals("a", get(url).body().string());

Iterator<String> i = cache.urls();
assertEquals(url.toString(), i.next());
i.remove();

// Too many calls to remove().
try {
i.remove();
fail();
} catch (IllegalStateException expected) {
}
}

@Test public void elementEvictedBetweenHasNextAndNext() throws Exception {
// Put a response in the cache.
server.enqueue(new MockResponse()
.setBody("a"));
URL url = server.getUrl("/a");
assertEquals("a", get(url).body().string());

// The URL will remain available if hasNext() returned true...
Iterator<String> i = cache.urls();
assertTrue(i.hasNext());

// ...so even when we evict the element, we still get something back.
cache.evictAll();
assertEquals(url.toString(), i.next());

// Remove does nothing. But most importantly, it doesn't throw!
i.remove();
}

@Test public void elementEvictedBeforeHasNextIsOmitted() throws Exception {
// Put a response in the cache.
server.enqueue(new MockResponse()
.setBody("a"));
URL url = server.getUrl("/a");
assertEquals("a", get(url).body().string());

Iterator<String> i = cache.urls();
cache.evictAll();

// The URL was evicted before hasNext() made any promises.
assertFalse(i.hasNext());
try {
i.next();
fail();
} catch (NoSuchElementException expected) {
}
}

private Response get(URL url) throws IOException {
Request request = new Request.Builder()
.url(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -962,18 +962,21 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
assertEquals("a", a.key());
assertEquals("a1", a.getString(0));
assertEquals("a2", a.getString(1));
a.close();

assertTrue(iterator.hasNext());
DiskLruCache.Snapshot b = iterator.next();
assertEquals("b", b.key());
assertEquals("b1", b.getString(0));
assertEquals("b2", b.getString(1));
b.close();

assertTrue(iterator.hasNext());
DiskLruCache.Snapshot c = iterator.next();
assertEquals("c", c.key());
assertEquals("c1", c.getString(0));
assertEquals("c2", c.getString(1));
c.close();

assertFalse(iterator.hasNext());
try {
Expand All @@ -988,11 +991,16 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
set("b", "b1", "b2");
Iterator<DiskLruCache.Snapshot> iterator = cache.snapshots();

assertEquals("a", iterator.next().key());
DiskLruCache.Snapshot a = iterator.next();
assertEquals("a", a.key());
a.close();

set("c", "c1", "c2");

assertEquals("b", iterator.next().key());
DiskLruCache.Snapshot b = iterator.next();
assertEquals("b", b.key());
b.close();

assertFalse(iterator.hasNext());
}

Expand All @@ -1001,14 +1009,17 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
set("b", "b1", "b2");
Iterator<DiskLruCache.Snapshot> iterator = cache.snapshots();

assertEquals("a", iterator.next().key());
DiskLruCache.Snapshot a = iterator.next();
assertEquals("a", a.key());
a.close();

set("b", "b3", "b4");

DiskLruCache.Snapshot b = iterator.next();
assertEquals("b", b.key());
assertEquals("b3", b.getString(0));
assertEquals("b4", b.getString(1));
b.close();
}

@Test public void iteratorElementsRemovedDuringIterationAreOmitted() throws Exception {
Expand All @@ -1018,16 +1029,21 @@ private void createNewCacheWithSize(int maxSize) throws IOException {

cache.remove("b");

assertEquals("a", iterator.next().key());
DiskLruCache.Snapshot a = iterator.next();
assertEquals("a", a.key());
a.close();

assertFalse(iterator.hasNext());
}

@Test public void iteratorRemove() throws Exception {
set("a", "a1", "a2");
Iterator<DiskLruCache.Snapshot> iterator = cache.snapshots();
iterator.next();

DiskLruCache.Snapshot a = iterator.next();
a.close();
iterator.remove();

assertEquals(null, cache.get("a"));
}

Expand All @@ -1044,15 +1060,25 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
@Test public void iteratorRemoveOncePerCallToNext() throws Exception {
set("a", "a1", "a2");
Iterator<DiskLruCache.Snapshot> iterator = cache.snapshots();
iterator.next();

DiskLruCache.Snapshot a = iterator.next();
iterator.remove();
a.close();

try {
iterator.remove();
fail();
} catch (IllegalStateException expected) {
}
}

@Test public void cacheClosedTruncatesIterator() throws Exception {
set("a", "a1", "a2");
Iterator<DiskLruCache.Snapshot> iterator = cache.snapshots();
cache.close();
assertFalse(iterator.hasNext());
}

private void assertJournalEquals(String... expectedBodyLines) throws Exception {
List<String> expectedLines = new ArrayList<>();
expectedLines.add(MAGIC);
Expand Down
54 changes: 54 additions & 0 deletions okhttp/src/main/java/com/squareup/okhttp/Cache.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
import java.security.cert.CertificateFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import okio.BufferedSink;
import okio.BufferedSource;
import okio.ByteString;
Expand Down Expand Up @@ -259,6 +261,58 @@ public void evictAll() throws IOException {
cache.evictAll();
}

/**
* Returns an iterator over the URLs in this cache. This iterator doesn't throw {@code
* ConcurrentModificationException}, but if new responses are added while iterating, their URLs
* will not be returned. If existing responses are evicted during iteration, they will be absent
* (unless they were already returned).
*
* <p>The iterator supports {@linkplain Iterator#remove}. Removing a URL from the iterator evicts
* the corresponding response from the cache. Use this to evict selected responses.
*/
public Iterator<String> urls() {
return new Iterator<String>() {
final Iterator<DiskLruCache.Snapshot> delegate = cache.snapshots();

String nextUrl;
boolean canRemove;

@Override public boolean hasNext() {
if (nextUrl != null) return true;

canRemove = false; // Prevent delegate.remove() on the wrong item!
while (delegate.hasNext()) {
DiskLruCache.Snapshot snapshot = delegate.next();
try {
BufferedSource metadata = Okio.buffer(snapshot.getSource(ENTRY_METADATA));
nextUrl = metadata.readUtf8LineStrict();
return true;
} catch (IOException ignored) {
// We couldn't read the metadata for this snapshot; possibly because the host filesystem
// has disappeared! Skip it.
} finally {
snapshot.close();
}
}

return false;
}

@Override public String next() {
if (!hasNext()) throw new NoSuchElementException();
String result = nextUrl;
nextUrl = null;
canRemove = true;
return result;
}

@Override public void remove() {
if (!canRemove) throw new IllegalStateException("remove() before next()");
delegate.remove();
}
};
}

public synchronized int getWriteAbortCount() {
return writeAbortCount;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,9 @@ private static String sourceToString(Source in) throws IOException {
* the hosting filesystem becomes unreachable, the iterator will omit elements rather than
* throwing exceptions.
*
* <p><strong>The caller must {@link Snapshot#close close}</strong> each snapshot returned by
* {@link Iterator#next}. Failing to do so leaks open files!
*
* <p>The returned iterator supports {@link Iterator#remove}.
*/
public synchronized Iterator<Snapshot> snapshots() {
Expand All @@ -703,6 +706,9 @@ public synchronized Iterator<Snapshot> snapshots() {
if (nextSnapshot != null) return true;

synchronized (DiskLruCache.this) {
// If the cache is closed, truncate the iterator.
if (isClosed()) return false;

while (delegate.hasNext()) {
Entry entry = delegate.next();
Snapshot snapshot = entry.snapshot();
Expand Down

0 comments on commit a409634

Please sign in to comment.