From cd263f39e39d073284a77f07d339595ecdec6337 Mon Sep 17 00:00:00 2001 From: Jonathan Moore Date: Mon, 20 Dec 2010 12:12:55 +0000 Subject: [PATCH] HTTPCLIENT-975: more refactoring/method-extraction git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1051076 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/client/cache/CachingHttpClient.java | 60 +++++++++++++------ .../cache/RequestProtocolCompliance.java | 25 ++++++-- .../client/cache/TestCachingHttpClient.java | 6 +- 3 files changed, 65 insertions(+), 26 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 b725de5d1..5e8e4bcc4 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 @@ -390,11 +390,7 @@ public class CachingHttpClient implements HttpClient { return requestCompliance.getErrorForRequest(error); } - try { - request = requestCompliance.makeRequestCompliant(request); - } catch (ProtocolException e) { - throw new ClientProtocolException(e); - } + request = requestCompliance.makeRequestCompliant(request); 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 - .buildUnconditionalRequest(request, matchedEntry); - return callBackend(target, unconditional, context); + return retryRequestUnconditionally(target, request, context, + matchedEntry); } + 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())) { - return responseGenerator.generateNotModifiedResponse(responseEntry); - } - - return resp; + private boolean shouldSendNotModifiedResponse(HttpRequest request, + HttpCacheEntry responseEntry) { + return (suitabilityChecker.isConditional(request) + && suitabilityChecker.allConditionalsMatch(request, responseEntry, new Date())); } 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(); - setResponseStatus(context, CacheResponseStatus.VALIDATED); + recordCacheUpdate(context); } if (statusCode == HttpStatus.SC_NOT_MODIFIED) { diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java index f5c1c3e96..5ed3958eb 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java +++ b/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 { - RequestWrapper newRequest = new RequestWrapper(request); + throws ClientProtocolException { + 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 { - RequestWrapper newRequest = new RequestWrapper(request); + throws ClientProtocolException { + RequestWrapper newRequest; + try { + newRequest = new RequestWrapper(request); + } catch (ProtocolException pe) { + throw new ClientProtocolException(pe); + } newRequest.setProtocolVersion(version); return newRequest; diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java index a0d63fc76..d8840a177 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java +++ b/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.anyObject())).andReturn(request); } - private void requestCannotBeMadeCompliantThrows(ProtocolException exception) throws Exception { + private void requestCannotBeMadeCompliantThrows(ClientProtocolException exception) throws Exception { EasyMock.expect( mockRequestProtocolCompliance.makeRequestCompliant( EasyMock.anyObject())).andThrow(exception);