HTTPCLIENT-1147: When HttpClient-Cache cannot open cache file, should act like miss

Contributed by Joe Campbell <joseph.r.campbell at gmail.com>

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1209503 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,9 @@
 Changes since 4.2 ALPHA1
 -------------------
 
+* [HTTPCLIENT-1147] When HttpClient-Cache cannot open cache file, should act like miss.
+  Contributed by Joe Campbell <joseph.r.campbell at gmail.com>
+
 * [HTTPCLIENT-1137] Values for the Via header are cached and reused by httpclient-cache.
   Contributed by Alin Vasile <alinachegalati at yahoo dot com>
 

httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java

@@ -57,4 +57,9 @@ public interface Resource extends Serializable {
      */
     void dispose();
 
+    /**
+     * Is this resource still valid to be used
+     */
+    boolean isValid();
+
 }

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java

@@ -81,10 +81,10 @@ class CacheValidityPolicy {
      * if last-modified and date are defined, freshness lifetime is coefficient*(date-lastModified),
      * else freshness lifetime is defaultLifetime
      *
-     * @param entry
+     * @param entry the cache entry
-     * @param now
+     * @param now what time is it currently (When is right NOW)
-     * @param coefficient
+     * @param coefficient Part of the heuristic for cache entry freshness
-     * @param defaultLifetime
+     * @param defaultLifetime How long can I assume a cache entry is default TTL
      * @return {@code true} if the response is fresh
      */
     public boolean isResponseHeuristicallyFresh(final HttpCacheEntry entry,
@@ -108,7 +108,10 @@ class CacheValidityPolicy {
     }
 
     public boolean isRevalidatable(final HttpCacheEntry entry) {
-        return entry.getFirstHeader(HeaderConstants.ETAG) != null
+        if (!entry.getResource().isValid())
+            return false;
+        else
+            return entry.getFirstHeader(HeaderConstants.ETAG) != null
                 || entry.getFirstHeader(HeaderConstants.LAST_MODIFIED) != null;
     }
 
@@ -212,6 +215,7 @@ class CacheValidityPolicy {
      * This matters for deciding whether the cache entry is valid to serve as a
      * response. If these values do not match, we might have a partial response
      *
+     * @param entry The cache entry we are currently working with
      * @return boolean indicating whether actual length matches Content-Length
      */
     protected boolean contentLengthHeaderMatchesActualLength(final HttpCacheEntry entry) {
@@ -260,10 +264,6 @@ class CacheValidityPolicy {
         return getCorrectedReceivedAgeSecs(entry) + getResponseDelaySecs(entry);
     }
 
-    protected Date getCurrentDate() {
-        return new Date();
-    }
-
     protected long getResidentTimeSecs(HttpCacheEntry entry, Date now) {
         long diff = now.getTime() - entry.getResponseDate().getTime();
         return (diff / 1000L);

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java

@@ -125,9 +125,15 @@ class CachedResponseSuitabilityChecker {
      *            {@link HttpRequest}
      * @param entry
      *            {@link HttpCacheEntry}
+     * @param now
+     *            Right now in time
      * @return boolean yes/no answer
      */
     public boolean canCachedResponseBeUsed(HttpHost host, HttpRequest request, HttpCacheEntry entry, Date now) {
+        if (!entry.getResource().isValid())  {
+            return false;
+        }
+
         if (!isFreshEnough(entry, request, now)) {
             log.trace("Cache entry was not fresh enough");
             return false;
@@ -213,7 +219,7 @@ class CachedResponseSuitabilityChecker {
 
     /**
      * Is this request the type of conditional request we support?
-     * @param request
+     * @param request The current httpRequest being made
      * @return {@code true} if the request is supported
      */
     public boolean isConditional(HttpRequest request) {
@@ -222,24 +228,26 @@ class CachedResponseSuitabilityChecker {
 
     /**
      * Check that conditionals that are part of this request match
-     * @param request
+     * @param request The current httpRequest being made
-     * @param entry
+     * @param entry the cache entry
-     * @param now
+     * @param now right NOW in time
      * @return {@code true} if the request matches all conditionals
      */
     public boolean allConditionalsMatch(HttpRequest request, HttpCacheEntry entry, Date now) {
         boolean hasEtagValidator = hasSupportedEtagValidator(request);
         boolean hasLastModifiedValidator = hasSupportedLastModifiedValidator(request);
 
-        boolean etagValidatorMatches = (hasEtagValidator) ? etagValidatorMatches(request, entry) : false;
+        boolean etagValidatorMatches = (hasEtagValidator) && etagValidatorMatches(request, entry);
-        boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) ? lastModifiedValidatorMatches(request, entry, now) : false;
+        boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) && lastModifiedValidatorMatches(request, entry, now);
 
         if ((hasEtagValidator && hasLastModifiedValidator)
             && !(etagValidatorMatches && lastModifiedValidatorMatches)) {
             return false;
         } else if (hasEtagValidator && !etagValidatorMatches) {
             return false;
-        } if (hasLastModifiedValidator && !lastModifiedValidatorMatches) {
+        }
+
+        if (hasLastModifiedValidator && !lastModifiedValidatorMatches) {
             return false;
         }
         return true;
@@ -261,9 +269,9 @@ class CachedResponseSuitabilityChecker {
 
     /**
      * Check entry against If-None-Match
-     * @param request
+     * @param request The current httpRequest being made
-     * @param entry
+     * @param entry the cache entry
-     * @return
+     * @return boolean does the etag validator match
      */
     private boolean etagValidatorMatches(HttpRequest request, HttpCacheEntry entry) {
         Header etagHeader = entry.getFirstHeader(HeaderConstants.ETAG);
@@ -286,10 +294,10 @@ class CachedResponseSuitabilityChecker {
     /**
      * Check entry against If-Modified-Since, if If-Modified-Since is in the future it is invalid as per
      * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
-     * @param request
+     * @param request The current httpRequest being made
-     * @param entry
+     * @param entry the cache entry
-     * @param now
+     * @param now right NOW in time
-     * @return
+     * @return  boolean Does the last modified header match
      */
     private boolean lastModifiedValidatorMatches(HttpRequest request, HttpCacheEntry entry, Date now) {
         Header lastModifiedHeader = entry.getFirstHeader(HeaderConstants.LAST_MODIFIED);

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java

@@ -31,6 +31,8 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.http.annotation.ThreadSafe;
 import org.apache.http.client.cache.Resource;
 
@@ -48,30 +50,34 @@ public class FileResource implements Resource {
 
     private volatile boolean disposed;
 
+    private final Log log = LogFactory.getLog(getClass());
+
     public FileResource(final File file) {
         super();
         this.file = file;
         this.disposed = false;
     }
 
-    private void ensureValid() {
+    public boolean isValid() {
-        if (this.disposed) {
+        if (this.disposed || !file.exists()) {
-            throw new IllegalStateException("Resource has been deallocated");
+            log.warn("Resource has been deallocated");
+            return false;
         }
+        return true;
     }
 
     synchronized File getFile() {
-        ensureValid();
+        isValid();
         return this.file;
     }
 
     public synchronized InputStream getInputStream() throws IOException {
-        ensureValid();
+        isValid();
         return new FileInputStream(this.file);
     }
 
     public synchronized long length() {
-        ensureValid();
+        isValid();
         return this.file.length();
     }
 

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java

@@ -64,4 +64,8 @@ public class HeapResource implements Resource {
     public void dispose() {
     }
 
+    public boolean isValid() {
+        return true;
+    }
+
 }

httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java

@@ -84,26 +84,26 @@ class ResponseCachingPolicy {
         }
 
         switch (response.getStatusLine().getStatusCode()) {
-        case HttpStatus.SC_OK:
+            case HttpStatus.SC_OK:
-        case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION:
+            case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION:
-        case HttpStatus.SC_MULTIPLE_CHOICES:
+            case HttpStatus.SC_MULTIPLE_CHOICES:
-        case HttpStatus.SC_MOVED_PERMANENTLY:
+            case HttpStatus.SC_MOVED_PERMANENTLY:
-        case HttpStatus.SC_GONE:
+            case HttpStatus.SC_GONE:
-            // these response codes MAY be cached
+                // these response codes MAY be cached
-            cacheable = true;
+                cacheable = true;
-            log.debug("Response was cacheable");
+                log.debug("Response was cacheable");
-            break;
+                break;
-        case HttpStatus.SC_PARTIAL_CONTENT:
+            case HttpStatus.SC_PARTIAL_CONTENT:
-            // we don't implement Range requests and hence are not
+                // we don't implement Range requests and hence are not
-            // allowed to cache partial content
+                // allowed to cache partial content
-            log.debug("Response was not cacheable (Partial Content)");
+                log.debug("Response was not cacheable (Partial Content)");
-            return cacheable;
+                return cacheable;
 
-        default:
+            default:
-            // If the status code is not one of the recognized
+                // If the status code is not one of the recognized
-            // available codes in HttpStatus Don't Cache
+                // available codes in HttpStatus Don't Cache
-            log.debug("Response was not cacheable (Unknown Status code)");
+                log.debug("Response was not cacheable (Unknown Status code)");
-            return cacheable;
+                return cacheable;
         }
 
         Header contentLength = response.getFirstHeader(HTTP.CONTENT_LEN);

httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java

@@ -0,0 +1,87 @@
+package org.apache.http.client.cache;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.StatusLine;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.HttpResponseException;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.cache.CacheConfig;
+import org.apache.http.impl.client.cache.CachingHttpClient;
+import org.apache.http.impl.client.cache.FileResourceFactory;
+import org.apache.http.impl.client.cache.ManagedHttpCacheStorage;
+
+import org.junit.Test;
+
+import java.io.File;
+
+public class TestHttpCacheJiraNumber1147 {
+    final String cacheDir = "/tmp/cachedir";
+    HttpClient cachingHttpClient;
+    HttpClient client = new DefaultHttpClient();
+
+    @Test
+    public void testIssue1147() throws Exception {
+        final CacheConfig cacheConfig = new CacheConfig();
+        cacheConfig.setSharedCache(true);
+        cacheConfig.setMaxObjectSize(262144); //256kb
+
+        new File(cacheDir).mkdir();
+
+        if(! new File(cacheDir, "httpclient-cache").exists()){
+            if(!new File(cacheDir, "httpclient-cache").mkdir()){
+                throw new RuntimeException("failed to create httpclient cache directory: " +
+                        new File(cacheDir, "httpclient-cache").getAbsolutePath());
+            }
+        }
+
+        final ResourceFactory resourceFactory = new FileResourceFactory(new File(cacheDir, "httpclient-cache"));
+
+        final HttpCacheStorage httpCacheStorage = new ManagedHttpCacheStorage(cacheConfig);
+
+        cachingHttpClient = new CachingHttpClient(client, resourceFactory, httpCacheStorage, cacheConfig);
+
+        final HttpGet get = new HttpGet("http://www.apache.org/js/jquery.js");
+
+        System.out.println("Calling URL First time.");
+        executeCall(get);
+
+        removeDirectory(cacheDir);
+
+        System.out.println("Calling URL Second time.");
+        executeCall(get);
+    }
+
+    private void removeDirectory(String cacheDir) {
+        File theDirectory = new File(cacheDir, "httpclient-cache");
+        File[] files = theDirectory.listFiles();
+
+        for (File cacheFile : files) {
+            cacheFile.delete();
+        }
+
+        theDirectory.delete();
+
+        new File(cacheDir).delete();
+    }
+
+    private void executeCall(HttpGet get) throws Exception {
+        final HttpResponse response = cachingHttpClient.execute(get);
+        final StatusLine statusLine = response.getStatusLine();
+        System.out.println("Status Code: " + statusLine.getStatusCode());
+
+        if (statusLine.getStatusCode() >= 300) {
+            if(statusLine.getStatusCode() == 404)
+                throw new NoResultException();
+
+            throw new HttpResponseException(statusLine.getStatusCode(), statusLine.getReasonPhrase());
+        }
+        response.getEntity().getContent();
+    }
+
+    private class NoResultException extends Exception {
+
+        private static final long serialVersionUID = 1277878788978491946L;
+
+    }
+}

httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java

@@ -138,12 +138,7 @@ public class TestCacheValidityPolicy {
     @Test
     public void testResidentTimeSecondsIsTimeSinceResponseTime() {
         HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(now, sixSecondsAgo);
-        impl = new CacheValidityPolicy() {
+        impl = new CacheValidityPolicy();
-            @Override
-            protected Date getCurrentDate() {
-                return now;
-            }
-        };
         assertEquals(6, impl.getResidentTimeSecs(entry, now));
     }