HTTPCLIENT-975: more refactoring/method-extraction

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

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

@@ -390,11 +390,7 @@ public class CachingHttpClient implements HttpClient {
             return requestCompliance.getErrorForRequest(error);
         }
 
-        try {
+        request = requestCompliance.makeRequestCompliant(request);
-            request = requestCompliance.makeRequestCompliant(request);
-        } catch (ProtocolException e) {
-            throw new ClientProtocolException(e);
-        }
         request.addHeader("Via",via);
 
         flushEntriesInvalidatedByRequest(target, request);
@@ -674,7 +670,6 @@ public class CachingHttpClient implements HttpClient {
 
         String resultEtag = resultEtagHeader.getValue();
         Variant matchingVariant = variants.get(resultEtag);
-
         if (matchingVariant == null) {
             log.debug("304 response did not contain ETag matching one sent in If-None-Match");
             return callBackend(target, request, context);
@@ -683,14 +678,43 @@ public class CachingHttpClient implements HttpClient {
         HttpCacheEntry matchedEntry = matchingVariant.getEntry();
         
         if (revalidationResponseIsTooOld(backendResponse, matchedEntry)) {
-        	HttpRequest unconditional = conditionalRequestBuilder
+        	return retryRequestUnconditionally(target, request, context,
-        		.buildUnconditionalRequest(request, matchedEntry);
+                    matchedEntry);
-        	return callBackend(target, unconditional, context);
         }
         
+        recordCacheUpdate(context);
+
+        HttpCacheEntry responseEntry = getUpdatedVariantEntry(target,
+                conditionalRequest, requestDate, responseDate, backendResponse,
+                matchingVariant, matchedEntry);
+
+        HttpResponse resp = responseGenerator.generateResponse(responseEntry);
+        tryToUpdateVariantMap(target, request, matchingVariant);
+
+        if (shouldSendNotModifiedResponse(request, responseEntry)) {
+            return responseGenerator.generateNotModifiedResponse(responseEntry);
+        }
+
+        return resp;
+    }
+
+    private HttpResponse retryRequestUnconditionally(HttpHost target,
+            HttpRequest request, HttpContext context,
+            HttpCacheEntry matchedEntry) throws IOException {
+        HttpRequest unconditional = conditionalRequestBuilder
+        	.buildUnconditionalRequest(request, matchedEntry);
+        return callBackend(target, unconditional, context);
+    }
+
+    private void recordCacheUpdate(HttpContext context) {
         cacheUpdates.getAndIncrement();
         setResponseStatus(context, CacheResponseStatus.VALIDATED);
+    }
 
+    private HttpCacheEntry getUpdatedVariantEntry(HttpHost target,
+            HttpRequest conditionalRequest, Date requestDate,
+            Date responseDate, HttpResponse backendResponse,
+            Variant matchingVariant, HttpCacheEntry matchedEntry) {
         HttpCacheEntry responseEntry = matchedEntry;
         try {
             responseEntry = responseCache.updateVariantCacheEntry(target, conditionalRequest,
@@ -698,19 +722,22 @@ public class CachingHttpClient implements HttpClient {
         } catch (IOException ioe) {
             log.warn("Could not update cache entry", ioe);
         }
+        return responseEntry;
+    }
 
-        HttpResponse resp = responseGenerator.generateResponse(responseEntry);
+    private void tryToUpdateVariantMap(HttpHost target, HttpRequest request,
+            Variant matchingVariant) {
         try {
             responseCache.reuseVariantEntryFor(target, request, matchingVariant);
         } catch (IOException ioe) {
             log.warn("Could not update cache entry to reuse variant", ioe);
         }
+    }
 
-        if (suitabilityChecker.isConditional(request) && suitabilityChecker.allConditionalsMatch(request, responseEntry, new Date())) {
+    private boolean shouldSendNotModifiedResponse(HttpRequest request,
-            return responseGenerator.generateNotModifiedResponse(responseEntry);
+            HttpCacheEntry responseEntry) {
-        }
+        return (suitabilityChecker.isConditional(request)
-
+                && suitabilityChecker.allConditionalsMatch(request, responseEntry, new Date()));
-        return resp;
     }
 
     HttpResponse revalidateCacheEntry(
@@ -737,8 +764,7 @@ public class CachingHttpClient implements HttpClient {
 
         int statusCode = backendResponse.getStatusLine().getStatusCode();
         if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) {
-            cacheUpdates.getAndIncrement();
+            recordCacheUpdate(context);
-            setResponseStatus(context, CacheResponseStatus.VALIDATED);
         }
 
         if (statusCode == HttpStatus.SC_NOT_MODIFIED) {

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

@@ -39,6 +39,7 @@ import org.apache.http.HttpVersion;
 import org.apache.http.ProtocolException;
 import org.apache.http.ProtocolVersion;
 import org.apache.http.annotation.Immutable;
+import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.cache.HeaderConstants;
 import org.apache.http.entity.AbstractHttpEntity;
 import org.apache.http.impl.client.RequestWrapper;
@@ -92,9 +93,11 @@ class RequestProtocolCompliance {
      *
      * @param request the request to check for compliance
      * @return the updated request
-     * @throws ProtocolException when we have trouble making the request compliant
+     * @throws ClientProtocolException when we have trouble making the request compliant
      */
-    public HttpRequest makeRequestCompliant(HttpRequest request) throws ProtocolException {
+    public HttpRequest makeRequestCompliant(HttpRequest request)
+        throws ClientProtocolException {
+    
         if (requestMustNotHaveEntity(request)) {
             ((HttpEntityEnclosingRequest) request).setEntity(null);
         }
@@ -212,16 +215,26 @@ class RequestProtocolCompliance {
     }
 
     private HttpRequest upgradeRequestTo(HttpRequest request, ProtocolVersion version)
-            throws ProtocolException {
+            throws ClientProtocolException {
-        RequestWrapper newRequest = new RequestWrapper(request);
+        RequestWrapper newRequest;
+        try {
+            newRequest = new RequestWrapper(request);
+        } catch (ProtocolException pe) {
+            throw new ClientProtocolException(pe);
+        }
         newRequest.setProtocolVersion(version);
 
         return newRequest;
     }
 
     private HttpRequest downgradeRequestTo(HttpRequest request, ProtocolVersion version)
-            throws ProtocolException {
+            throws ClientProtocolException {
-        RequestWrapper newRequest = new RequestWrapper(request);
+        RequestWrapper newRequest;
+        try {
+            newRequest = new RequestWrapper(request);
+        } catch (ProtocolException pe) {
+            throw new ClientProtocolException(pe);
+        }
         newRequest.setProtocolVersion(version);
 
         return newRequest;

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

@@ -868,7 +868,7 @@ public class TestCachingHttpClient {
     @Test
     public void testNonCompliantRequestWrapsAndReThrowsProtocolException() throws Exception {
 
-        ProtocolException expected = new ProtocolException("ouch");
+        ClientProtocolException expected = new ClientProtocolException("ouch");
 
         requestIsFatallyNonCompliant(null);
         requestCannotBeMadeCompliantThrows(expected);
@@ -878,7 +878,7 @@ public class TestCachingHttpClient {
         try {
             impl.execute(host, request, context);
         } catch (ClientProtocolException ex) {
-            Assert.assertTrue(ex.getCause().getMessage().equals(expected.getMessage()));
+            Assert.assertSame(expected, ex);
             gotException = true;
         }
         verifyMocks();
@@ -2039,7 +2039,7 @@ public class TestCachingHttpClient {
                         EasyMock.<HttpRequest>anyObject())).andReturn(request);
     }
 
-    private void requestCannotBeMadeCompliantThrows(ProtocolException exception) throws Exception {
+    private void requestCannotBeMadeCompliantThrows(ClientProtocolException exception) throws Exception {
         EasyMock.expect(
                 mockRequestProtocolCompliance.makeRequestCompliant(
                         EasyMock.<HttpRequest>anyObject())).andThrow(exception);