diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java index 7622aa386..6dfda9247 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java @@ -153,6 +153,10 @@ public final class HttpResponseCache extends ResponseCache { return HttpResponseCache.this.put(uri, connection); } + @Override public void maybeRemove(String requestMethod, URI uri) throws IOException { + HttpResponseCache.this.maybeRemove(requestMethod, uri); + } + @Override public void update( CacheResponse conditionalCacheHit, HttpURLConnection connection) throws IOException { HttpResponseCache.this.update(conditionalCacheHit, connection); @@ -226,17 +230,11 @@ public final class HttpResponseCache extends ResponseCache { HttpURLConnection httpConnection = (HttpURLConnection) urlConnection; String requestMethod = httpConnection.getRequestMethod(); - String key = uriToKey(uri); - if (requestMethod.equals("POST") || requestMethod.equals("PUT") || requestMethod.equals( - "DELETE")) { - try { - cache.remove(key); - } catch (IOException ignored) { - // The cache cannot be written. - } + if (maybeRemove(requestMethod, uri)) { return null; - } else if (!requestMethod.equals("GET")) { + } + if (!requestMethod.equals("GET")) { // Don't cache non-GET responses. We're technically allowed to cache // HEAD requests and some POST requests, but the complexity of doing // so is high and the benefit is low. @@ -259,7 +257,7 @@ public final class HttpResponseCache extends ResponseCache { Entry entry = new Entry(uri, varyHeaders, httpConnection); DiskLruCache.Editor editor = null; try { - editor = cache.edit(key); + editor = cache.edit(uriToKey(uri)); if (editor == null) { return null; } @@ -271,6 +269,23 @@ public final class HttpResponseCache extends ResponseCache { } } + /** + * Returns true if the supplied {@code requestMethod} potentially invalidates an entry in the + * cache. + */ + private boolean maybeRemove(String requestMethod, URI uri) { + if (requestMethod.equals("POST") || requestMethod.equals("PUT") || requestMethod.equals( + "DELETE")) { + try { + cache.remove(uriToKey(uri)); + } catch (IOException ignored) { + // The cache cannot be written. + } + return true; + } + return false; + } + private void update(CacheResponse conditionalCacheHit, HttpURLConnection httpConnection) throws IOException { HttpEngine httpEngine = getHttpEngine(httpConnection); diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java index 49e6032e8..47cba5957 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java @@ -31,6 +31,7 @@ import java.io.OutputStream; import java.net.CacheRequest; import java.net.CacheResponse; import java.net.CookieHandler; +import java.net.HttpURLConnection; import java.net.Proxy; import java.net.URI; import java.net.URISyntaxException; @@ -390,13 +391,16 @@ public class HttpEngine { return; } + HttpURLConnection connectionToCache = policy.getHttpConnectionToCache(); + // Should we cache this response for this request? if (!responseHeaders.isCacheable(requestHeaders)) { + policy.responseCache.maybeRemove(connectionToCache.getRequestMethod(), uri); return; } // Offer this request to the cache. - cacheRequest = policy.responseCache.put(uri, policy.getHttpConnectionToCache()); + cacheRequest = policy.responseCache.put(uri, connectionToCache); } /** diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCache.java index 5829f0244..97b1ea547 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCache.java @@ -39,6 +39,9 @@ public interface OkResponseCache { CacheRequest put(URI uri, URLConnection urlConnection) throws IOException; + /** Remove any cache entries for the supplied {@code uri} if the request method invalidates. */ + void maybeRemove(String requestMethod, URI uri) throws IOException; + /** * Handles a conditional request hit by updating the stored cache response * with the headers from {@code httpConnection}. The cached response body is diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCacheAdapter.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCacheAdapter.java index 2ac915a8d..fc8ffca26 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCacheAdapter.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkResponseCacheAdapter.java @@ -41,6 +41,9 @@ public final class OkResponseCacheAdapter implements OkResponseCache { return responseCache.put(uri, urlConnection); } + @Override public void maybeRemove(String requestMethod, URI uri) throws IOException { + } + @Override public void update(CacheResponse conditionalCacheHit, HttpURLConnection connection) throws IOException { } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java index 64a473848..6dd82dc02 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java @@ -773,6 +773,28 @@ public final class HttpResponseCacheTest { assertEquals("C", readAscii(openConnection(url))); } + @Test public void postInvalidatesCacheWithUncacheableResponse() throws Exception { + // 1. seed the cache + // 2. invalidate it with uncacheable response + // 3. expect a cache miss + server.enqueue( + new MockResponse().setBody("A").addHeader("Expires: " + formatDate(1, TimeUnit.HOURS))); + server.enqueue(new MockResponse().setBody("B").setResponseCode(500)); + server.enqueue(new MockResponse().setBody("C")); + server.play(); + + URL url = server.getUrl("/"); + + assertEquals("A", readAscii(openConnection(url))); + + HttpURLConnection invalidate = openConnection(url); + invalidate.setRequestMethod("POST"); + addRequestBodyIfNecessary("POST", invalidate); + assertEquals("B", readAscii(invalidate)); + + assertEquals("C", readAscii(openConnection(url))); + } + @Test public void etag() throws Exception { RecordedRequest conditionalRequest = assertConditionallyCached(new MockResponse().addHeader("ETag: v1"));