From 791bb497e28b5605f6c1de223dfbd3c9855029aa Mon Sep 17 00:00:00 2001 From: Jonathan Moore Date: Mon, 20 Dec 2010 11:58:53 +0000 Subject: [PATCH] HTTPCLIENT-975: more refactoring. Handled ProtocolExceptions from RequestWrapper instantiations further down in the call stack. git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1051074 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/client/cache/CachingHttpClient.java | 55 +++++++++++-------- .../cache/ConditionalRequestBuilder.java | 27 +++++++-- 2 files changed, 54 insertions(+), 28 deletions(-) diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java index 9dcbdfe0c..b725de5d1 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java @@ -410,39 +410,23 @@ public class CachingHttpClient implements HttpClient { 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 variants = null; - try { - variants = responseCache.getVariantCacheEntriesWithEtags(target, request); - } catch (IOException ioe) { - log.warn("Unable to retrieve variant entries from cache", ioe); - } + Map 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 implements HttpClient { return callBackend(target, request, context); } + private Map getExistingCacheVariants(HttpHost target, + HttpRequest request) { + Map 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 implements HttpClient { HttpResponse negotiateResponseFromVariants(HttpHost target, HttpRequest request, HttpContext context, - Map variants) throws IOException, ProtocolException { + Map variants) throws IOException { HttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants(request, variants); Date requestDate = getCurrentDate(); diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java index b7dc859cd..28151ef89 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ConditionalRequestBuilder.java @@ -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.RequestWrapper; @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 variants) - throws ProtocolException { - RequestWrapper wrapperRequest = new RequestWrapper(request); + Map 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");