You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by jo...@apache.org on 2010/12/20 12:58:54 UTC

svn commit: r1051074 - in /httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache: CachingHttpClient.java ConditionalRequestBuilder.java

Author: jonm
Date: Mon Dec 20 11:58:53 2010
New Revision: 1051074

URL: http://svn.apache.org/viewvc?rev=1051074&view=rev
Log:
HTTPCLIENT-975: more refactoring. Handled ProtocolExceptions from
RequestWrapper instantiations further down in the call stack.

Modified:
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java?rev=1051074&r1=1051073&r2=1051074&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java Mon Dec 20 11:58:53 2010
@@ -410,39 +410,23 @@ public class CachingHttpClient implement
             log.warn("Unable to retrieve entries from cache", ioe);
         }
         if (entry == null) {
-            cacheMisses.getAndIncrement();
-            if (log.isDebugEnabled()) {
-                RequestLine rl = request.getRequestLine();
-                log.debug("Cache miss [host: " + target + "; uri: " + rl.getUri() + "]");
-            }
+            recordCacheMiss(target, request);
 
             if (!mayCallBackend(request)) {
                 return new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_GATEWAY_TIMEOUT,
                         "Gateway Timeout");
             }
 
-            Map<String,Variant> variants = null;
-            try {
-                variants = responseCache.getVariantCacheEntriesWithEtags(target, request);
-            } catch (IOException ioe) {
-                log.warn("Unable to retrieve variant entries from cache", ioe);
-            }
+            Map<String, Variant> variants =
+                getExistingCacheVariants(target, request);
             if (variants != null && variants.size() > 0) {
-                try {
-                    return negotiateResponseFromVariants(target, request, context, variants);
-                } catch (ProtocolException e) {
-                    throw new ClientProtocolException(e);
-                }
+                return negotiateResponseFromVariants(target, request, context, variants);
             }
 
             return callBackend(target, request, context);
         }
 
-        if (log.isDebugEnabled()) {
-            RequestLine rl = request.getRequestLine();
-            log.debug("Cache hit [host: " + target + "; uri: " + rl.getUri() + "]");
-        }
-        cacheHits.getAndIncrement();
+        recordCacheHit(target, request);
 
         Date now = getCurrentDate();
         if (suitabilityChecker.canCachedResponseBeUsed(target, request, entry, now)) {
@@ -475,6 +459,33 @@ public class CachingHttpClient implement
         return callBackend(target, request, context);
     }
 
+    private Map<String, Variant> getExistingCacheVariants(HttpHost target,
+            HttpRequest request) {
+        Map<String,Variant> variants = null;
+        try {
+            variants = responseCache.getVariantCacheEntriesWithEtags(target, request);
+        } catch (IOException ioe) {
+            log.warn("Unable to retrieve variant entries from cache", ioe);
+        }
+        return variants;
+    }
+
+    private void recordCacheMiss(HttpHost target, HttpRequest request) {
+        cacheMisses.getAndIncrement();
+        if (log.isDebugEnabled()) {
+            RequestLine rl = request.getRequestLine();
+            log.debug("Cache miss [host: " + target + "; uri: " + rl.getUri() + "]");
+        }
+    }
+
+    private void recordCacheHit(HttpHost target, HttpRequest request) {
+        if (log.isDebugEnabled()) {
+            RequestLine rl = request.getRequestLine();
+            log.debug("Cache hit [host: " + target + "; uri: " + rl.getUri() + "]");
+        }
+        cacheHits.getAndIncrement();
+    }
+
     private void flushEntriesInvalidatedByRequest(HttpHost target,
             HttpRequest request) {
         try {
@@ -642,7 +653,7 @@ public class CachingHttpClient implement
     
     HttpResponse negotiateResponseFromVariants(HttpHost target,
             HttpRequest request, HttpContext context,
-            Map<String, Variant> variants) throws IOException, ProtocolException {
+            Map<String, Variant> variants) throws IOException {
         HttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants(request, variants);
 
         Date requestDate = getCurrentDate();

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java?rev=1051074&r1=1051073&r2=1051074&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java Mon Dec 20 11:58:53 2010
@@ -27,6 +27,9 @@
 package org.apache.http.impl.client.cache;
 
 import java.util.Map;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.http.Header;
 import org.apache.http.HeaderElement;
 import org.apache.http.HttpRequest;
@@ -42,6 +45,8 @@ import org.apache.http.impl.client.Reque
 @Immutable
 class ConditionalRequestBuilder {
 
+    private static final Log log = LogFactory.getLog(ConditionalRequestBuilder.class);
+    
     /**
      * When a {@link HttpCacheEntry} is stale but 'might' be used as a response
      * to an {@link HttpRequest} we will attempt to revalidate the entry with
@@ -90,12 +95,16 @@ class ConditionalRequestBuilder {
      * @param request the original request from the caller
      * @param cacheEntry the entry that needs to be revalidated
      * @return the wrapped request
-     * @throws ProtocolException when I am unable to build a new origin request.
      */
     public HttpRequest buildConditionalRequestFromVariants(HttpRequest request,
-            Map<String, Variant> variants)
-                throws ProtocolException {
-        RequestWrapper wrapperRequest = new RequestWrapper(request);
+            Map<String, Variant> variants) {
+        RequestWrapper wrapperRequest;
+        try {
+            wrapperRequest = new RequestWrapper(request);
+        } catch (ProtocolException pe) {
+            log.warn("unable to build conditional request", pe);
+            return request;
+        }
         wrapperRequest.resetHeaders();
 
         // we do not support partial content so all etags are used
@@ -126,8 +135,14 @@ class ConditionalRequestBuilder {
      * @throws ProtocolException
      */
     public HttpRequest buildUnconditionalRequest(HttpRequest request,
-            HttpCacheEntry entry) throws ProtocolException {
-        RequestWrapper wrapped = new RequestWrapper(request);
+            HttpCacheEntry entry) { 
+        RequestWrapper wrapped;
+        try {
+            wrapped = new RequestWrapper(request);
+        } catch (ProtocolException e) {
+            log.warn("unable to build proper unconditional request", e);
+            return request;
+        }
         wrapped.resetHeaders();
         wrapped.addHeader("Cache-Control","no-cache");
         wrapped.addHeader("Pragma","no-cache");