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 2017/10/13 11:24:59 UTC

httpcomponents-client git commit: Fixed regression in cache validity logic for file backed cache entries

Repository: httpcomponents-client
Updated Branches:
  refs/heads/master bb96781e5 -> 6076f5542


Fixed regression in cache validity logic for file backed cache entries


Project: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/repo
Commit: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/commit/6076f554
Tree: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/tree/6076f554
Diff: http://git-wip-us.apache.org/repos/asf/httpcomponents-client/diff/6076f554

Branch: refs/heads/master
Commit: 6076f5542bf2cd6296e3c87fa246eaab729da103
Parents: bb96781
Author: Oleg Kalnichevski <ol...@apache.org>
Authored: Fri Oct 13 12:16:08 2017 +0200
Committer: Oleg Kalnichevski <ol...@apache.org>
Committed: Fri Oct 13 13:22:23 2017 +0200

----------------------------------------------------------------------
 .../http/impl/cache/CacheValidityPolicy.java    | 37 +++++++++-----------
 .../client5/http/impl/cache/FileResource.java   | 12 +++----
 .../impl/cache/TestCacheValidityPolicy.java     |  7 ----
 .../impl/cache/TestHttpCacheJiraNumber1147.java |  2 +-
 4 files changed, 23 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/6076f554/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
----------------------------------------------------------------------
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
index d468f26..0173a3e 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
@@ -31,8 +31,8 @@ import java.util.Iterator;
 
 import org.apache.hc.client5.http.cache.HeaderConstants;
 import org.apache.hc.client5.http.cache.HttpCacheEntry;
+import org.apache.hc.client5.http.cache.Resource;
 import org.apache.hc.client5.http.utils.DateUtils;
-import org.apache.hc.core5.http.message.MessageSupport;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.Header;
@@ -40,6 +40,7 @@ import org.apache.hc.core5.http.HeaderElement;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.MessageHeaders;
+import org.apache.hc.core5.http.message.MessageSupport;
 
 /**
  * @since 4.1
@@ -180,23 +181,6 @@ class CacheValidityPolicy {
         return DateUtils.parseDate(dateHdr.getValue());
     }
 
-    protected long getContentLengthValue(final HttpCacheEntry entry) {
-        final Header cl = entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
-        if (cl == null) {
-            return -1;
-        }
-
-        try {
-            return Long.parseLong(cl.getValue());
-        } catch (final NumberFormatException ex) {
-            return -1;
-        }
-    }
-
-    protected boolean hasContentLengthHeader(final HttpCacheEntry entry) {
-        return null != entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
-    }
-
     /**
      * 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
@@ -205,8 +189,21 @@ class CacheValidityPolicy {
      * @return boolean indicating whether actual length matches Content-Length
      */
     protected boolean contentLengthHeaderMatchesActualLength(final HttpCacheEntry entry) {
-        return !hasContentLengthHeader(entry) ||
-                (entry.getResource() != null && getContentLengthValue(entry) == entry.getResource().length());
+        final Header h = entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH);
+        if (h != null) {
+            try {
+                final long responseLen = Long.parseLong(h.getValue());
+                final Resource resource = entry.getResource();
+                if (resource == null) {
+                    return false;
+                }
+                final long resourceLen = resource.length();
+                return responseLen == resourceLen;
+            } catch (final NumberFormatException ex) {
+                return false;
+            }
+        }
+        return true;
     }
 
     protected long getApparentAgeSecs(final HttpCacheEntry entry) {

http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/6076f554/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/FileResource.java
----------------------------------------------------------------------
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/FileResource.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/FileResource.java
index 321e200..fa0e2c8 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/FileResource.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/FileResource.java
@@ -51,10 +51,13 @@ public class FileResource extends Resource {
     private static final long serialVersionUID = 4132244415919043397L;
 
     private final AtomicReference<File> fileRef;
+    private final long len;
 
     public FileResource(final File file) {
         super();
-        this.fileRef = new AtomicReference<>(Args.notNull(file, "File"));
+        Args.notNull(file, "File");
+        this.fileRef = new AtomicReference<>(file);
+        this.len = file.length();
     }
 
     File getFile() {
@@ -96,12 +99,7 @@ public class FileResource extends Resource {
 
     @Override
     public long length() {
-        final File file = this.fileRef.get();
-        if (file != null) {
-            return file.length();
-        } else {
-            return -1;
-        }
+        return len;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/6076f554/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java
----------------------------------------------------------------------
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java
index 7f92555..935986b 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java
@@ -362,13 +362,6 @@ public class TestCacheValidityPolicy {
     }
 
     @Test
-    public void testMalformedContentLengthReturnsNegativeOne() {
-        final Header[] headers = new Header[] { new BasicHeader("Content-Length", "asdf") };
-        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(headers);
-        assertEquals(-1, impl.getContentLengthValue(entry));
-    }
-
-    @Test
     public void testNegativeAgeHeaderValueReturnsMaxAge() {
         final Header[] headers = new Header[] { new BasicHeader("Age", "-100") };
         final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(headers);

http://git-wip-us.apache.org/repos/asf/httpcomponents-client/blob/6076f554/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java
----------------------------------------------------------------------
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java
index 4ede4ba..f665654 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java
@@ -135,7 +135,7 @@ public class TestHttpCacheJiraNumber1147 {
         Assert.assertEquals(200, response2.getCode());
         EntityUtils.consume(response2.getEntity());
 
-        verify(mockExecChain, Mockito.times(2)).proceed(
+        verify(mockExecChain, Mockito.times(1)).proceed(
                 isA(ClassicHttpRequest.class),
                 isA(ExecChain.Scope.class));
         Assert.assertEquals(CacheResponseStatus.FAILURE, context.getCacheResponseStatus());