You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2011/12/02 15:15:12 UTC

svn commit: r1209503 - in /httpcomponents/httpclient/trunk: ./ httpclient-cache/src/main/java/org/apache/http/client/cache/ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ httpclient-cache/src/test/java/org/apache/http/client/cache/ h...

Author: olegk
Date: Fri Dec  2 14:15:11 2011
New Revision: 1209503

URL: http://svn.apache.org/viewvc?rev=1209503&view=rev
Log:
HTTPCLIENT-1147: When HttpClient-Cache cannot open cache file, should act like miss
Contributed by Joe Campbell <joseph.r.campbell at gmail.com>

Added:
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java   (with props)
Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ConsumableInputStream.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRFC5861Compliance.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Dec  2 14:15:11 2011
@@ -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>
 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/Resource.java Fri Dec  2 14:15:11 2011
@@ -57,4 +57,9 @@ public interface Resource extends Serial
      */
     void dispose();
 
+    /**
+     * Is this resource still valid to be used
+     */
+    boolean isValid();
+
 }

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheValidityPolicy.java Fri Dec  2 14:15:11 2011
@@ -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 now
-     * @param coefficient
-     * @param defaultLifetime
+     * @param entry the cache entry
+     * @param now what time is it currently (When is right NOW)
+     * @param coefficient Part of the heuristic for cache entry freshness
+     * @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);

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachedResponseSuitabilityChecker.java Fri Dec  2 14:15:11 2011
@@ -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 entry
-     * @param now
+     * @param request The current httpRequest being made
+     * @param entry the cache entry
+     * @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 lastModifiedValidatorMatches = (hasLastModifiedValidator) ? lastModifiedValidatorMatches(request, entry, now) : false;
+        boolean etagValidatorMatches = (hasEtagValidator) && etagValidatorMatches(request, entry);
+        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 entry
-     * @return
+     * @param request The current httpRequest being made
+     * @param entry the cache entry
+     * @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 entry
-     * @param now
-     * @return
+     * @param request The current httpRequest being made
+     * @param entry the cache entry
+     * @param now right NOW in time
+     * @return  boolean Does the last modified header match
      */
     private boolean lastModifiedValidatorMatches(HttpRequest request, HttpCacheEntry entry, Date now) {
         Header lastModifiedHeader = entry.getFirstHeader(HeaderConstants.LAST_MODIFIED);

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/FileResource.java Fri Dec  2 14:15:11 2011
@@ -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 Res
 
     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() {
-        if (this.disposed) {
-            throw new IllegalStateException("Resource has been deallocated");
+    public boolean isValid() {
+        if (this.disposed || !file.exists()) {
+            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();
     }
 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HeapResource.java Fri Dec  2 14:15:11 2011
@@ -64,4 +64,8 @@ public class HeapResource implements Res
     public void dispose() {
     }
 
+    public boolean isValid() {
+        return true;
+    }
+
 }

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java Fri Dec  2 14:15:11 2011
@@ -84,26 +84,26 @@ class ResponseCachingPolicy {
         }
 
         switch (response.getStatusLine().getStatusCode()) {
-        case HttpStatus.SC_OK:
-        case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION:
-        case HttpStatus.SC_MULTIPLE_CHOICES:
-        case HttpStatus.SC_MOVED_PERMANENTLY:
-        case HttpStatus.SC_GONE:
-            // these response codes MAY be cached
-            cacheable = true;
-            log.debug("Response was cacheable");
-            break;
-        case HttpStatus.SC_PARTIAL_CONTENT:
-            // we don't implement Range requests and hence are not
-            // allowed to cache partial content
-            log.debug("Response was not cacheable (Partial Content)");
-            return cacheable;
-
-        default:
-            // If the status code is not one of the recognized
-            // available codes in HttpStatus Don't Cache
-            log.debug("Response was not cacheable (Unknown Status code)");
-            return cacheable;
+            case HttpStatus.SC_OK:
+            case HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION:
+            case HttpStatus.SC_MULTIPLE_CHOICES:
+            case HttpStatus.SC_MOVED_PERMANENTLY:
+            case HttpStatus.SC_GONE:
+                // these response codes MAY be cached
+                cacheable = true;
+                log.debug("Response was cacheable");
+                break;
+            case HttpStatus.SC_PARTIAL_CONTENT:
+                // we don't implement Range requests and hence are not
+                // allowed to cache partial content
+                log.debug("Response was not cacheable (Partial Content)");
+                return cacheable;
+
+            default:
+                // If the status code is not one of the recognized
+                // available codes in HttpStatus Don't Cache
+                log.debug("Response was not cacheable (Unknown Status code)");
+                return cacheable;
         }
 
         Header contentLength = response.getFirstHeader(HTTP.CONTENT_LEN);

Added: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java?rev=1209503&view=auto
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java (added)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java Fri Dec  2 14:15:11 2011
@@ -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;
+
+    }
+}

Propchange: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/client/cache/TestHttpCacheJiraNumber1147.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ConsumableInputStream.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ConsumableInputStream.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ConsumableInputStream.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ConsumableInputStream.java Fri Dec  2 14:15:11 2011
@@ -34,15 +34,15 @@ public class ConsumableInputStream exten
 
     private ByteArrayInputStream buf;
     private boolean closed = false;
-    
+
     public ConsumableInputStream(ByteArrayInputStream buf) {
         this.buf = buf;
     }
-    
+
     public int read() throws IOException {
         return buf.read();
     }
-    
+
     @Override
     public void close() {
         closed = true;

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheValidityPolicy.java Fri Dec  2 14:15:11 2011
@@ -138,12 +138,7 @@ public class TestCacheValidityPolicy {
     @Test
     public void testResidentTimeSecondsIsTimeSinceResponseTime() {
         HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(now, sixSecondsAgo);
-        impl = new CacheValidityPolicy() {
-            @Override
-            protected Date getCurrentDate() {
-                return now;
-            }
-        };
+        impl = new CacheValidityPolicy();
         assertEquals(6, impl.getResidentTimeSecs(entry, now));
     }
 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRFC5861Compliance.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRFC5861Compliance.java?rev=1209503&r1=1209502&r2=1209503&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRFC5861Compliance.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRFC5861Compliance.java Fri Dec  2 14:15:11 2011
@@ -114,7 +114,7 @@ public class TestRFC5861Compliance exten
 
         assertTrue(cis.wasClosed());
     }
-    
+
     @Test
     public void testStaleIfErrorInResponseYieldsToMustRevalidate()
             throws Exception{