Return-Path: Delivered-To: apmail-hc-commits-archive@www.apache.org Received: (qmail 40225 invoked from network); 22 Sep 2010 14:53:13 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 22 Sep 2010 14:53:13 -0000 Received: (qmail 82242 invoked by uid 500); 22 Sep 2010 14:53:13 -0000 Delivered-To: apmail-hc-commits-archive@hc.apache.org Received: (qmail 82202 invoked by uid 500); 22 Sep 2010 14:53:11 -0000 Mailing-List: contact commits-help@hc.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "HttpComponents Project" Delivered-To: mailing list commits@hc.apache.org Received: (qmail 82194 invoked by uid 99); 22 Sep 2010 14:53:10 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 Sep 2010 14:53:10 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 Sep 2010 14:53:07 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 1F1F02388A38; Wed, 22 Sep 2010 14:52:45 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1000011 [1/2] - in /httpcomponents/httpclient/trunk/httpclient-cache/src: main/java/org/apache/http/impl/client/cache/ test/java/org/apache/http/impl/client/cache/ test/java/org/apache/http/impl/client/cache/ehcache/ Date: Wed, 22 Sep 2010 14:52:45 -0000 To: commits@hc.apache.org From: olegk@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100922145246.1F1F02388A38@eris.apache.org> Author: olegk Date: Wed Sep 22 14:52:44 2010 New Revision: 1000011 URL: http://svn.apache.org/viewvc?rev=1000011&view=rev Log: HTTPCLIENT-997: cache module should handle out-of-order validations properly and unconditionally refresh Contributed by Jonathan Moore Removed: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ehcache/TestEhcacheProtcolRequirements.java Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java 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 httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/HttpTestUtils.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/ehcache/TestEhcacheProtocolRequirements.java Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java Wed Sep 22 14:52:44 2010 @@ -33,7 +33,6 @@ import java.net.URL; 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.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.annotation.ThreadSafe; @@ -145,45 +144,11 @@ class CacheInvalidator { protected boolean requestShouldNotBeCached(HttpRequest req) { String method = req.getRequestLine().getMethod(); - return notGetOrHeadRequest(method) || containsCacheControlHeader(req) - || containsPragmaHeader(req); + return notGetOrHeadRequest(method); } private boolean notGetOrHeadRequest(String method) { return !(HeaderConstants.GET_METHOD.equals(method) || HeaderConstants.HEAD_METHOD .equals(method)); } - - private boolean containsPragmaHeader(HttpRequest req) { - return req.getFirstHeader(HeaderConstants.PRAGMA) != null; - } - - private boolean containsCacheControlHeader(HttpRequest request) { - Header[] cacheControlHeaders = request.getHeaders(HeaderConstants.CACHE_CONTROL); - - if (cacheControlHeaders == null) { - return false; - } - - for (Header cacheControl : cacheControlHeaders) { - HeaderElement[] cacheControlElements = cacheControl.getElements(); - if (cacheControlElements == null) { - return false; - } - - for (HeaderElement cacheControlElement : cacheControlElements) { - if (HeaderConstants.CACHE_CONTROL_NO_CACHE.equalsIgnoreCase(cacheControlElement - .getName())) { - return true; - } - - if (HeaderConstants.CACHE_CONTROL_NO_STORE.equalsIgnoreCase(cacheControlElement - .getName())) { - return true; - } - } - } - - return false; - } } 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=1000011&r1=1000010&r2=1000011&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 Wed Sep 22 14:52:44 2010 @@ -56,6 +56,8 @@ import org.apache.http.client.cache.Http import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.impl.client.DefaultHttpClient; +import org.apache.http.impl.cookie.DateParseException; +import org.apache.http.impl.cookie.DateUtils; import org.apache.http.message.BasicHttpResponse; import org.apache.http.params.HttpParams; import org.apache.http.protocol.HttpContext; @@ -529,12 +531,34 @@ public class CachingHttpClient implement HttpContext context, HttpCacheEntry cacheEntry) throws IOException, ProtocolException { HttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequest(request, cacheEntry); - Date requestDate = getCurrentDate(); + Date requestDate = getCurrentDate(); HttpResponse backendResponse = backend.execute(target, conditionalRequest, context); - Date responseDate = getCurrentDate(); + + final Header entryDateHeader = cacheEntry.getFirstHeader("Date"); + final Header responseDateHeader = backendResponse.getFirstHeader("Date"); + if (entryDateHeader != null && responseDateHeader != null) { + try { + Date entryDate = DateUtils.parseDate(entryDateHeader.getValue()); + Date respDate = DateUtils.parseDate(responseDateHeader.getValue()); + if (respDate.before(entryDate)) { + HttpRequest unconditional = conditionalRequestBuilder + .buildUnconditionalRequest(request, cacheEntry); + requestDate = getCurrentDate(); + backendResponse = backend.execute(target, unconditional, context); + responseDate = getCurrentDate(); + } + } catch (DateParseException e) { + // either backend response or cached entry did not have a valid + // Date header, so we can't tell if they are out of order + // according to the origin clock; thus we can skip the + // unconditional retry recommended in 13.2.6 of RFC 2616. + } + } + + int statusCode = backendResponse.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) { cacheUpdates.getAndIncrement(); @@ -558,14 +582,32 @@ public class CachingHttpClient implement responseCompliance.ensureProtocolCompliance(request, backendResponse); boolean cacheable = responseCachingPolicy.isResponseCacheable(request, backendResponse); - - if (cacheable) { + if (cacheable && + !alreadyHaveNewerCacheEntry(target, request, backendResponse)) { return responseCache.cacheAndReturnResponse(target, request, backendResponse, requestDate, responseDate); } - - responseCache.flushCacheEntriesFor(target, request); + if (!cacheable) { + responseCache.flushCacheEntriesFor(target, request); + } return backendResponse; } + private boolean alreadyHaveNewerCacheEntry(HttpHost target, HttpRequest request, + HttpResponse backendResponse) throws IOException { + HttpCacheEntry existing = responseCache.getCacheEntry(target, request); + if (existing == null) return false; + Header entryDateHeader = existing.getFirstHeader("Date"); + if (entryDateHeader == null) return false; + Header responseDateHeader = backendResponse.getFirstHeader("Date"); + if (responseDateHeader == null) return false; + try { + Date entryDate = DateUtils.parseDate(entryDateHeader.getValue()); + Date responseDate = DateUtils.parseDate(responseDateHeader.getValue()); + return responseDate.before(entryDate); + } catch (DateParseException e) { + } + return false; + } + } 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=1000011&r1=1000010&r2=1000011&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 Wed Sep 22 14:52:44 2010 @@ -79,4 +79,30 @@ class ConditionalRequestBuilder { } + /** + * Returns a request to unconditionally validate a cache entry with + * the origin. In certain cases (due to multiple intervening caches) + * our cache may actually receive a response to a normal conditional + * validation where the Date header is actually older than that of + * our current cache entry. In this case, the protocol recommendation + * is to retry the validation and force syncup with the origin. + * @param request client request we are trying to satisfy + * @param entry existing cache entry we are trying to validate + * @return an unconditional validation request + * @throws ProtocolException + */ + public HttpRequest buildUnconditionalRequest(HttpRequest request, + HttpCacheEntry entry) throws ProtocolException { + RequestWrapper wrapped = new RequestWrapper(request); + wrapped.resetHeaders(); + wrapped.addHeader("Cache-Control","no-cache"); + wrapped.addHeader("Pragma","no-cache"); + wrapped.removeHeaders("If-Range"); + wrapped.removeHeaders("If-Match"); + wrapped.removeHeaders("If-None-Match"); + wrapped.removeHeaders("If-Unmodified-Since"); + wrapped.removeHeaders("If-Modified-Since"); + return wrapped; + } + } Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java Wed Sep 22 14:52:44 2010 @@ -1,18 +1,14 @@ package org.apache.http.impl.client.cache; -import java.util.Date; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; -import org.apache.http.HttpStatus; import org.apache.http.HttpVersion; import org.apache.http.client.HttpClient; import org.apache.http.client.cache.HttpCache; -import org.apache.http.impl.cookie.DateUtils; import org.apache.http.message.BasicHttpRequest; -import org.apache.http.message.BasicHttpResponse; import org.apache.http.protocol.HttpContext; import org.easymock.IExpectationSetters; import org.easymock.classextension.EasyMock; @@ -46,7 +42,7 @@ public abstract class AbstractProtocolTe request = new BasicHttpRequest("GET", "/foo", HttpVersion.HTTP_1_1); - originResponse = make200Response(); + originResponse = HttpTestUtils.make200Response(); params = new CacheConfig(); params.setMaxCacheEntries(MAX_ENTRIES); @@ -67,19 +63,6 @@ public abstract class AbstractProtocolTe EasyMock.verify(mockCache); } - protected HttpResponse make200Response() { - HttpResponse out = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); - out.setHeader("Date", DateUtils.formatDate(new Date())); - out.setHeader("Server", "MockOrigin/1.0"); - out.setHeader("Content-Length", "128"); - out.setEntity(makeBody(128)); - return out; - } - - protected HttpEntity makeBody(int nbytes) { - return HttpTestUtils.makeBody(nbytes); - } - protected IExpectationSetters backendExpectsAnyRequest() throws Exception { HttpResponse resp = mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock .isA(HttpRequest.class), (HttpContext) EasyMock.isNull()); Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/HttpTestUtils.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/HttpTestUtils.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/HttpTestUtils.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/HttpTestUtils.java Wed Sep 22 14:52:44 2010 @@ -44,6 +44,7 @@ import org.apache.http.client.cache.Http import org.apache.http.entity.ByteArrayEntity; import org.apache.http.impl.cookie.DateUtils; import org.apache.http.message.BasicHeader; +import org.apache.http.message.BasicHttpResponse; import org.apache.http.message.BasicStatusLine; public class HttpTestUtils { @@ -288,4 +289,13 @@ public class HttpTestUtils { Date now = new Date(); return makeCacheEntry(now, now); } + + public static HttpResponse make200Response() { + HttpResponse out = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + out.setHeader("Date", DateUtils.formatDate(new Date())); + out.setHeader("Server", "MockOrigin/1.0"); + out.setHeader("Content-Length", "128"); + out.setEntity(makeBody(128)); + return out; + } } \ No newline at end of file Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java Wed Sep 22 14:52:44 2010 @@ -208,16 +208,10 @@ public class TestCacheInvalidator { } @Test - public void testInvalidatesRequestsWithClientCacheControlHeaders() throws Exception { + public void testDoesNotInvalidateRequestsWithClientCacheControlHeaders() throws Exception { HttpRequest request = new BasicHttpRequest("GET","/",HTTP_1_1); request.setHeader("Cache-Control","no-cache"); - final String theUri = "http://foo.example.com:80/"; - cacheReturnsEntryForUri(theUri); - Set variantURIs = new HashSet(); - cacheEntryHasVariantURIs(variantURIs); - - entryIsRemoved(theUri); replayMocks(); impl.flushInvalidatedCacheEntries(host, request); @@ -226,16 +220,10 @@ public class TestCacheInvalidator { } @Test - public void testInvalidatesRequestsWithClientPragmaHeaders() throws Exception { + public void testDoesNotInvalidateRequestsWithClientPragmaHeaders() throws Exception { HttpRequest request = new BasicHttpRequest("GET","/",HTTP_1_1); request.setHeader("Pragma","no-cache"); - final String theUri = "http://foo.example.com:80/"; - cacheReturnsEntryForUri(theUri); - Set variantURIs = new HashSet(); - cacheEntryHasVariantURIs(variantURIs); - - entryIsRemoved(theUri); replayMocks(); impl.flushInvalidatedCacheEntries(host, request); Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java Wed Sep 22 14:52:44 2010 @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; +import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; @@ -49,6 +50,7 @@ import org.apache.http.client.methods.Ht import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.impl.cookie.DateUtils; +import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicHttpRequest; import org.apache.http.message.BasicHttpResponse; import org.apache.http.params.BasicHttpParams; @@ -99,6 +101,7 @@ public class TestCachingHttpClient { private HttpRequest request; private HttpContext context; private HttpParams params; + private HttpCacheEntry entry; @SuppressWarnings("unchecked") @Before @@ -128,6 +131,7 @@ public class TestCachingHttpClient { request = new BasicHttpRequest("GET", "/stuff", HttpVersion.HTTP_1_1); context = new BasicHttpContext(); params = new BasicHttpParams(); + entry = HttpTestUtils.makeCacheEntry(); impl = new CachingHttpClient( mockBackend, mockValidityPolicy, @@ -195,6 +199,8 @@ public class TestCachingHttpClient { responseProtocolValidationIsCalled(); + EasyMock.expect(mockCache.getCacheEntry(host, request)) + .andReturn(null); EasyMock.expect(mockCache.cacheAndReturnResponse(host, request, mockBackendResponse, requestDate, responseDate)) .andReturn(mockCachedResponse); @@ -207,6 +213,60 @@ public class TestCachingHttpClient { } @Test + public void testOlderCacheableResponsesDoNotGoIntoCache() throws Exception { + responsePolicyAllowsCaching(true); + responseProtocolValidationIsCalled(); + + Date now = new Date(); + Date fiveSecondsAgo = new Date(now.getTime() - 5 * 1000L); + Header entryDateHeader = new BasicHeader("Date", DateUtils.formatDate(now)); + Header[] headers = { entryDateHeader }; + entry = HttpTestUtils.makeCacheEntry(headers); + Header responseDateHeader = new BasicHeader("Date", DateUtils.formatDate(fiveSecondsAgo)); + + EasyMock.expect(mockCache.getCacheEntry(host, request)) + .andReturn(entry); + EasyMock.expect(mockBackendResponse.getFirstHeader("Date")) + .andReturn(responseDateHeader).anyTimes(); + + replayMocks(); + HttpResponse result = impl.handleBackendResponse(host, request, requestDate, + responseDate, mockBackendResponse); + verifyMocks(); + + Assert.assertSame(mockBackendResponse, result); + } + + @Test + public void testNewerCacheableResponsesReplaceExistingCacheEntry() throws Exception { + responsePolicyAllowsCaching(true); + responseProtocolValidationIsCalled(); + + Date now = new Date(); + Date fiveSecondsAgo = new Date(now.getTime() - 5 * 1000L); + Header entryDateHeader = new BasicHeader("Date", DateUtils.formatDate(fiveSecondsAgo)); + Header[] headers = { entryDateHeader }; + entry = HttpTestUtils.makeCacheEntry(headers); + Header responseDateHeader = new BasicHeader("Date", DateUtils.formatDate(now)); + + EasyMock.expect(mockCache.getCacheEntry(host, request)) + .andReturn(entry); + EasyMock.expect(mockBackendResponse.getFirstHeader("Date")) + .andReturn(responseDateHeader).anyTimes(); + EasyMock.expect(mockCache.cacheAndReturnResponse(host, request, + mockBackendResponse, requestDate, responseDate)) + .andReturn(mockCachedResponse); + + replayMocks(); + HttpResponse result = impl.handleBackendResponse(host, request, requestDate, + responseDate, mockBackendResponse); + verifyMocks(); + + Assert.assertSame(mockCachedResponse, result); + } + + + @Test public void testRequestThatCannotBeServedFromCacheCausesBackendRequest() throws Exception { cacheInvalidatorWasCalled(); requestPolicyAllowsCaching(false); @@ -302,54 +362,139 @@ public class TestCachingHttpClient { } @Test - public void testRevalidationCallsHandleBackEndResponseWhenNot304() throws Exception { + public void testRevalidationCallsHandleBackEndResponseWhenNot200Or304() throws Exception { mockImplMethods(GET_CURRENT_DATE, HANDLE_BACKEND_RESPONSE); - conditionalRequestBuilderCalled(); + HttpRequest validate = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse originResponse = + new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_NOT_FOUND, "Not Found"); + HttpResponse finalResponse = HttpTestUtils.make200Response(); + + conditionalRequestBuilderReturns(validate); getCurrentDateReturns(requestDate); - backendCallWasMadeWithRequest(mockConditionalRequest); + backendCall(validate, originResponse); getCurrentDateReturns(responseDate); - backendResponseCodeIs(HttpStatus.SC_OK); - EasyMock.expect(mockCache.updateCacheEntry(host, request, - mockCacheEntry, mockBackendResponse, requestDate, responseDate)) - .andReturn(mockCachedResponse); + EasyMock.expect(impl.handleBackendResponse(host, validate, + requestDate, responseDate, originResponse)) + .andReturn(finalResponse); replayMocks(); + HttpResponse result = + impl.revalidateCacheEntry(host, request, context, entry); + verifyMocks(); + + Assert.assertSame(finalResponse, result); + } + + @Test + public void testRevalidationUpdatesCacheEntryAndPutsItToCacheWhen304ReturningCachedResponse() + throws Exception { - HttpResponse result = impl.revalidateCacheEntry(host, request, context, - mockCacheEntry); + mockImplMethods(GET_CURRENT_DATE); + HttpRequest validate = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse originResponse = + new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + HttpResponse finalResponse = HttpTestUtils.make200Response(); + + conditionalRequestBuilderReturns(validate); + getCurrentDateReturns(requestDate); + backendCall(validate, originResponse); + getCurrentDateReturns(responseDate); + EasyMock.expect(mockCache.updateCacheEntry(host, request, + entry, originResponse, requestDate, responseDate)) + .andReturn(finalResponse); + + replayMocks(); + HttpResponse result = + impl.revalidateCacheEntry(host, request, context, entry); verifyMocks(); - Assert.assertEquals(mockCachedResponse, result); - Assert.assertEquals(0, impl.getCacheMisses()); - Assert.assertEquals(0, impl.getCacheHits()); - Assert.assertEquals(1, impl.getCacheUpdates()); + Assert.assertSame(finalResponse, result); } @Test - public void testRevalidationUpdatesCacheEntryAndPutsItToCacheWhen304ReturningCachedResponse() + public void testRevalidationUpdatesCacheEntryAndPutsItToCacheWhen200ReturningCachedResponse() throws Exception { + mockImplMethods(GET_CURRENT_DATE); - conditionalRequestBuilderCalled(); + + HttpRequest validate = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse originResponse = HttpTestUtils.make200Response(); + HttpResponse finalResponse = HttpTestUtils.make200Response(); + + conditionalRequestBuilderReturns(validate); getCurrentDateReturns(requestDate); - backendCallWasMadeWithRequest(mockConditionalRequest); + backendCall(validate, originResponse); getCurrentDateReturns(responseDate); - backendResponseCodeIs(HttpStatus.SC_NOT_MODIFIED); EasyMock.expect(mockCache.updateCacheEntry(host, request, - mockCacheEntry, mockBackendResponse, requestDate, responseDate)) - .andReturn(mockCachedResponse); + entry, originResponse, requestDate, responseDate)) + .andReturn(finalResponse); replayMocks(); + HttpResponse result = + impl.revalidateCacheEntry(host, request, context, entry); + verifyMocks(); + + Assert.assertSame(finalResponse, result); + } + + @Test + public void testRevalidationRetriesUnconditionallyIfOlderResponseReceived() + throws Exception { +// TODO + mockImplMethods(GET_CURRENT_DATE); + + Date now = new Date(); + Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); + Date elevenSecondsAgo = new Date(now.getTime() - 11 * 1000L); - HttpResponse result = impl.revalidateCacheEntry(host, request, context, mockCacheEntry); + Header[] headers = { + new BasicHeader("Date", DateUtils.formatDate(tenSecondsAgo)), + new BasicHeader("Cache-Control","max-age=5"), + new BasicHeader("ETag", "\"etag1\"") + }; + entry = HttpTestUtils.makeCacheEntry(headers); + + HttpRequest validate = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse originResponse1 = + new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, + "Not Modified"); + originResponse1.setHeader("Date", DateUtils.formatDate(elevenSecondsAgo)); + HttpRequest unconditional = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse originResponse2 = HttpTestUtils.make200Response(); + HttpResponse finalResponse = HttpTestUtils.make200Response(); + + conditionalRequestBuilderReturns(validate); + getCurrentDateReturns(requestDate); + backendCall(validate, originResponse1); + getCurrentDateReturns(responseDate); + EasyMock.expect(mockConditionalRequestBuilder.buildUnconditionalRequest(request, entry)) + .andReturn(unconditional); + final Date requestDate2 = new Date(); + getCurrentDateReturns(requestDate2); + backendCall(unconditional, originResponse2); + final Date responseDate2 = new Date(); + getCurrentDateReturns(responseDate2); + EasyMock.expect(mockCache.updateCacheEntry(host, request, + entry, originResponse2, requestDate2, responseDate2)) + .andReturn(finalResponse); + + replayMocks(); + HttpResponse result = + impl.revalidateCacheEntry(host, request, context, entry); verifyMocks(); - Assert.assertEquals(mockCachedResponse, result); - Assert.assertEquals(0, impl.getCacheMisses()); - Assert.assertEquals(0, impl.getCacheHits()); - Assert.assertEquals(1, impl.getCacheUpdates()); + Assert.assertSame(finalResponse, result); + } @Test @@ -1102,16 +1247,11 @@ public class TestCachingHttpClient { EasyMock.anyObject())).andReturn(b); } - private void backendResponseCodeIs(int code) { - EasyMock.expect(mockBackendResponse.getStatusLine()).andReturn(mockStatusLine); - EasyMock.expect(mockStatusLine.getStatusCode()).andReturn(code); - } - - private void conditionalRequestBuilderCalled() throws ProtocolException { - EasyMock.expect( - mockConditionalRequestBuilder.buildConditionalRequest( - EasyMock.anyObject(), - EasyMock.anyObject())).andReturn(mockConditionalRequest); + private void conditionalRequestBuilderReturns(HttpRequest validate) + throws Exception { + EasyMock.expect(mockConditionalRequestBuilder + .buildConditionalRequest(request, entry)) + .andReturn(validate); } private void getCurrentDateReturns(Date date) { @@ -1123,11 +1263,10 @@ public class TestCachingHttpClient { EasyMock.anyObject())).andReturn(allow); } - private void backendCallWasMadeWithRequest(HttpRequest request) throws IOException { - EasyMock.expect(mockBackend.execute( - EasyMock.anyObject(), - EasyMock.same(request), - EasyMock.anyObject())).andReturn(mockBackendResponse); + private void backendCall(HttpRequest req, HttpResponse resp) + throws Exception { + EasyMock.expect(mockBackend.execute(host, req, context)) + .andReturn(resp); } private void backendCallWasMade(HttpRequest request, HttpResponse response) throws IOException { Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java Wed Sep 22 14:52:44 2010 @@ -44,10 +44,14 @@ import org.junit.Test; public class TestConditionalRequestBuilder { private ConditionalRequestBuilder impl; + private HttpRequest request; + private HttpCacheEntry entry; @Before public void setUp() throws Exception { impl = new ConditionalRequestBuilder(); + request = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + entry = HttpTestUtils.makeCacheEntry(); } @Test @@ -170,4 +174,107 @@ public class TestConditionalRequestBuild } Assert.assertTrue(foundMaxAge0); } + + @Test + public void testBuildUnconditionalRequestUsesGETMethod() + throws Exception { + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertEquals("GET", result.getRequestLine().getMethod()); + } + + @Test + public void testBuildUnconditionalRequestUsesRequestUri() + throws Exception { + final String uri = "/theURI"; + request = new BasicHttpRequest("GET", uri, HttpVersion.HTTP_1_1); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertEquals(uri, result.getRequestLine().getUri()); + } + + @Test + public void testBuildUnconditionalRequestUsesHTTP_1_1() + throws Exception { + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertEquals(HttpVersion.HTTP_1_1, result.getProtocolVersion()); + } + + @Test + public void testBuildUnconditionalRequestAddsCacheControlNoCache() + throws Exception { + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + boolean ccNoCacheFound = false; + for(Header h : result.getHeaders("Cache-Control")) { + for(HeaderElement elt : h.getElements()) { + if ("no-cache".equals(elt.getName())) { + ccNoCacheFound = true; + } + } + } + Assert.assertTrue(ccNoCacheFound); + } + + @Test + public void testBuildUnconditionalRequestAddsPragmaNoCache() + throws Exception { + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + boolean ccNoCacheFound = false; + for(Header h : result.getHeaders("Pragma")) { + for(HeaderElement elt : h.getElements()) { + if ("no-cache".equals(elt.getName())) { + ccNoCacheFound = true; + } + } + } + Assert.assertTrue(ccNoCacheFound); + } + + @Test + public void testBuildUnconditionalRequestDoesNotUseIfRange() + throws Exception { + request.addHeader("If-Range","\"etag\""); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertNull(result.getFirstHeader("If-Range")); + } + + @Test + public void testBuildUnconditionalRequestDoesNotUseIfMatch() + throws Exception { + request.addHeader("If-Match","\"etag\""); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertNull(result.getFirstHeader("If-Match")); + } + + @Test + public void testBuildUnconditionalRequestDoesNotUseIfNoneMatch() + throws Exception { + request.addHeader("If-None-Match","\"etag\""); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertNull(result.getFirstHeader("If-None-Match")); + } + + @Test + public void testBuildUnconditionalRequestDoesNotUseIfUnmodifiedSince() + throws Exception { + request.addHeader("If-Unmodified-Since", DateUtils.formatDate(new Date())); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertNull(result.getFirstHeader("If-Unmodified-Since")); + } + + @Test + public void testBuildUnconditionalRequestDoesNotUseIfModifiedSince() + throws Exception { + request.addHeader("If-Modified-Since", DateUtils.formatDate(new Date())); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertNull(result.getFirstHeader("If-Modified-Since")); + } + + @Test + public void testBuildUnconditionalRequestCarriesOtherRequestHeaders() + throws Exception { + request.addHeader("User-Agent","MyBrowser/1.0"); + HttpRequest result = impl.buildUnconditionalRequest(request, entry); + Assert.assertEquals("MyBrowser/1.0", + result.getFirstHeader("User-Agent").getValue()); + } + } Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java?rev=1000011&r1=1000010&r2=1000011&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java Wed Sep 22 14:52:44 2010 @@ -26,6 +26,7 @@ */ package org.apache.http.impl.client.cache; +import static org.easymock.classextension.EasyMock.*; import static org.junit.Assert.*; import java.io.IOException; @@ -33,12 +34,16 @@ import java.util.Date; import org.apache.http.Header; import org.apache.http.HeaderElement; +import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; import org.apache.http.HttpVersion; import org.apache.http.impl.cookie.DateUtils; import org.apache.http.message.BasicHttpRequest; +import org.apache.http.message.BasicHttpResponse; +import org.apache.http.protocol.HttpContext; +import org.easymock.Capture; import org.junit.Test; /* @@ -81,7 +86,7 @@ public class TestProtocolRecommendations */ private HttpRequest requestToPopulateStaleCacheEntry() throws Exception { HttpRequest req1 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); - HttpResponse resp1 = make200Response(); + HttpResponse resp1 = HttpTestUtils.make200Response(); Date now = new Date(); Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo)); @@ -218,7 +223,7 @@ public class TestProtocolRecommendations public void testReturnsCachedResponsesAppropriatelyWhenNoOriginCommunication() throws Exception { HttpRequest req1 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); - HttpResponse resp1 = make200Response(); + HttpResponse resp1 = HttpTestUtils.make200Response(); Date now = new Date(); Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); resp1.setHeader("Cache-Control", "public, max-age=5"); @@ -299,4 +304,131 @@ public class TestProtocolRecommendations assertEquals(warning, result.getFirstHeader("Warning").getValue()); } + + /* + * "If an origin server wishes to force a semantically transparent cache + * to validate every request, it MAY assign an explicit expiration time + * in the past. This means that the response is always stale, and so the + * cache SHOULD validate it before using it for subsequent requests." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.2.1 + */ + @Test + public void testRevalidatesCachedResponseWithExpirationInThePast() + throws Exception { + Date now = new Date(); + Date oneSecondAgo = new Date(now.getTime() - 1 * 1000L); + Date oneSecondFromNow = new Date(now.getTime() + 1 * 1000L); + Date twoSecondsFromNow = new Date(now.getTime() + 2 * 1000L); + HttpRequest req1 = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("ETag","\"etag\""); + resp1.setHeader("Date", DateUtils.formatDate(now)); + resp1.setHeader("Expires",DateUtils.formatDate(oneSecondAgo)); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpRequest revalidate = + new BasicHttpRequest("GET", "/",HttpVersion.HTTP_1_1); + revalidate.setHeader("If-None-Match","\"etag\""); + + HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + resp2.setHeader("Date", DateUtils.formatDate(twoSecondsFromNow)); + resp2.setHeader("Expires", DateUtils.formatDate(oneSecondFromNow)); + resp2.setHeader("ETag","\"etag\""); + + expect(mockBackend.execute(isA(HttpHost.class), + eqRequest(revalidate), (HttpContext)isNull())) + .andReturn(resp2); + + replayMocks(); + impl.execute(host, req1); + HttpResponse result = impl.execute(host, req2); + verifyMocks(); + + assertEquals(HttpStatus.SC_OK, + result.getStatusLine().getStatusCode()); + } + + /* "When a client tries to revalidate a cache entry, and the response + * it receives contains a Date header that appears to be older than the + * one for the existing entry, then the client SHOULD repeat the + * request unconditionally, and include + * Cache-Control: max-age=0 + * to force any intermediate caches to validate their copies directly + * with the origin server, or + * Cache-Control: no-cache + * to force any intermediate caches to obtain a new copy from the + * origin server." + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.2.6 + */ + @Test + public void testRetriesValidationThatResultsInAnOlderDated304Response() + throws Exception { + Date now = new Date(); + Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); + Date elevenSecondsAgo = new Date(now.getTime() - 11 * 1000L); + HttpRequest req1 = + new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("ETag","\"etag\""); + resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo)); + resp1.setHeader("Cache-Control","max-age=5"); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + resp2.setHeader("ETag","\"etag\""); + resp2.setHeader("Date", DateUtils.formatDate(elevenSecondsAgo)); + + backendExpectsAnyRequest().andReturn(resp2); + + Capture cap = new Capture(); + HttpResponse resp3 = HttpTestUtils.make200Response(); + resp3.setHeader("ETag","\"etag2\""); + resp3.setHeader("Date", DateUtils.formatDate(now)); + resp3.setHeader("Cache-Control","max-age=5"); + + expect(mockBackend.execute(isA(HttpHost.class), capture(cap), + (HttpContext)isNull())) + .andReturn(resp3); + + replayMocks(); + impl.execute(host, req1); + impl.execute(host, req2); + verifyMocks(); + + HttpRequest captured = cap.getValue(); + boolean hasMaxAge0 = false; + boolean hasNoCache = false; + for(Header h : captured.getHeaders("Cache-Control")) { + for(HeaderElement elt : h.getElements()) { + if ("max-age".equals(elt.getName())) { + try { + int maxage = Integer.parseInt(elt.getValue()); + if (maxage == 0) { + hasMaxAge0 = true; + } + } catch (NumberFormatException nfe) { + // nop + } + } else if ("no-cache".equals(elt.getName())) { + hasNoCache = true; + } + } + } + assertTrue(hasMaxAge0 || hasNoCache); + assertNull(captured.getFirstHeader("If-None-Match")); + assertNull(captured.getFirstHeader("If-Modified-Since")); + assertNull(captured.getFirstHeader("If-Range")); + assertNull(captured.getFirstHeader("If-Match")); + assertNull(captured.getFirstHeader("If-Unmodified-Since")); + } }