From 6892f58f2c88f8da565e6c6549cf0ee07b8303bd Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 15 Oct 2010 19:43:18 +0000 Subject: [PATCH] HTTPCLIENT-1008: send all variants' ETags on "variant miss" Contributed by Michajlo Matijkiw and Mohammed Azeem Uddin git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1023085 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/http/client/cache/HttpCache.java | 12 + .../impl/client/cache/BasicHttpCache.java | 16 + .../impl/client/cache/CachingHttpClient.java | 107 +++++- .../cache/ConditionalRequestBuilder.java | 43 +++ .../client/cache/AbstractProtocolTest.java | 6 + .../impl/client/cache/TestBasicHttpCache.java | 45 +++ .../client/cache/TestCachingHttpClient.java | 305 +++++++++++++++++- .../cache/TestConditionalRequestBuilder.java | 63 ++++ 8 files changed, 583 insertions(+), 14 deletions(-) diff --git a/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java b/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java index 04aff3aa2..b87646233 100644 --- a/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java +++ b/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java @@ -28,6 +28,7 @@ package org.apache.http.client.cache; import java.io.IOException; import java.util.Date; +import java.util.Set; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; @@ -66,6 +67,17 @@ public interface HttpCache { HttpCacheEntry getCacheEntry(HttpHost host, HttpRequest request) throws IOException; + /** + * Retrieve all variants from the cache, if there are no variants than an empty + * {@link Set} is returned + * @param host + * @param request + * @return + * @throws IOException + */ + Set getVariantCacheEntries(HttpHost host, HttpRequest request) + throws IOException; + /** * Store a {@link HttpResponse} in the cache if possible, and return * @param host diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java index 0293b24ca..f9ee33c15 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java @@ -211,4 +211,20 @@ public class BasicHttpCache implements HttpCache { } + public Set getVariantCacheEntries(HttpHost host, HttpRequest request) + throws IOException { + Set variants = new HashSet(); + + HttpCacheEntry root = storage.getEntry(uriExtractor.getURI(host, request)); + if (root != null) { + if (root.hasVariants()) { + for(String variantUri : root.getVariantURIs()) { + variants.add(storage.getEntry(variantUri)); + } + } + } + + return variants; + } + } \ No newline at end of file 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 cc032f6da..60318c9d1 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 @@ -30,6 +30,7 @@ import java.io.IOException; import java.net.URI; import java.util.Date; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import org.apache.commons.logging.Log; @@ -407,8 +408,22 @@ public class CachingHttpClient implements HttpClient { if (log.isDebugEnabled()) { RequestLine rl = request.getRequestLine(); log.debug("Cache miss [host: " + target + "; uri: " + rl.getUri() + "]"); - } + + Set variantEntries = null; + try { + responseCache.getVariantCacheEntries(target, request); + } catch (IOException ioe) { + log.warn("Unable to retrieve variant entries from cache", ioe); + } + if (variantEntries != null && variantEntries.size() > 0) { + try { + return negotiateResponseFromVariants(target, request, context, variantEntries); + } catch (ProtocolException e) { + throw new ClientProtocolException(e); + } + } + return callBackend(target, request, context); } @@ -540,6 +555,95 @@ public class CachingHttpClient implements HttpClient { } + HttpResponse negotiateResponseFromVariants(HttpHost target, + HttpRequest request, HttpContext context, + Set variantEntries) throws IOException, ProtocolException { + HttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants(request, variantEntries); + + Date requestDate = getCurrentDate(); + HttpResponse backendResponse = backend.execute(target, conditionalRequest, context); + Date responseDate = getCurrentDate(); + + backendResponse.addHeader("Via", generateViaHeader(backendResponse)); + + if (backendResponse.getStatusLine().getStatusCode() != HttpStatus.SC_NOT_MODIFIED) { + return handleBackendResponse(target, conditionalRequest, requestDate, responseDate, backendResponse); + } + + Header resultEtagHeader = backendResponse.getFirstHeader(HeaderConstants.ETAG); + if (resultEtagHeader == null) { + log.debug("304 response did not contain ETag"); + return callBackend(target, request, context); + } + + HttpCacheEntry matchedEntry = null; + + String resultEtag = resultEtagHeader.getValue(); + for (HttpCacheEntry variantEntry : variantEntries) { + Header variantEtagHeader = variantEntry.getFirstHeader(HeaderConstants.ETAG); + if (variantEtagHeader != null) { + String variantEtag = variantEtagHeader.getValue(); + if (resultEtag.equals(variantEtag)) { + matchedEntry = variantEntry; + break; + } + } + } + + if (matchedEntry == null) { + log.debug("304 response did not contain ETag matching one sent in If-None-Match"); + return callBackend(target, request, context); + } + + // make sure this cache entry is indeed new enough to update with, + // if not force to refresh + final Header entryDateHeader = matchedEntry.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)) { + // TODO: what to do here? what if the initial request was a conditional + // request. It would get the same result whether it went through our + // cache or not... + HttpRequest unconditional = conditionalRequestBuilder + .buildUnconditionalRequest(request, matchedEntry); + return callBackend(target, unconditional, context); + } + } 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. + } + } + + cacheUpdates.getAndIncrement(); + setResponseStatus(context, CacheResponseStatus.VALIDATED); + + // SHOULD update cache entry according to rfc + HttpCacheEntry responseEntry = matchedEntry; + try { + responseEntry = responseCache.updateCacheEntry(target, conditionalRequest, matchedEntry, backendResponse, requestDate, responseDate); + } catch (IOException ioe) { + log.warn("Could not update cache entry", ioe); + } + + HttpResponse resp = responseGenerator.generateResponse(responseEntry); + try { + resp = responseCache.cacheAndReturnResponse(target, request, resp, requestDate, responseDate); + } catch (IOException ioe) { + log.warn("Could not cache entry", ioe); + } + + if (suitabilityChecker.isConditional(request) && suitabilityChecker.allConditionalsMatch(request, responseEntry, new Date())) { + return responseGenerator.generateNotModifiedResponse(responseEntry); + } + + return resp; + } + HttpResponse revalidateCacheEntry( HttpHost target, HttpRequest request, @@ -551,7 +655,6 @@ public class CachingHttpClient implements HttpClient { 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) { 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 e2842cab4..efb50211b 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 @@ -26,6 +26,8 @@ */ package org.apache.http.impl.client.cache; +import java.util.Set; + import org.apache.http.Header; import org.apache.http.HeaderElement; import org.apache.http.HttpRequest; @@ -80,6 +82,47 @@ class ConditionalRequestBuilder { } + /** + * When a {@link HttpCacheEntry} does not exist for a specific {@link HttpRequest} + * we attempt to see if an existing {@link HttpCacheEntry} is appropriate by building + * a conditional {@link HttpRequest} using the variants' ETag values. If no such values + * exist, the request is unmodified + * + * @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, Set variantEntries) + throws ProtocolException { + RequestWrapper wrapperRequest = new RequestWrapper(request); + wrapperRequest.resetHeaders(); + + // we do not support partial content so all etags are used + StringBuilder etags = new StringBuilder(); + boolean first = true; + for (HttpCacheEntry entry : variantEntries) { + Header etagHeader = entry.getFirstHeader(HeaderConstants.ETAG); + if (etagHeader != null) { + if (first) { + etags.append(etagHeader.getValue()); + first = false; + } else { + etags.append(",").append(etagHeader.getValue()); + } + } + } + // if first is still true than no variants had a cache entry, return + // unmodified wrapped request + if(first) { + return wrapperRequest; + } + + wrapperRequest.setHeader(HeaderConstants.IF_NONE_MATCH, etags.toString()); + + return wrapperRequest; + } + /** * Returns a request to unconditionally validate a cache entry with * the origin. In certain cases (due to multiple intervening caches) diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java index 32cedb20d..042c5e4be 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/AbstractProtocolTest.java @@ -1,6 +1,8 @@ package org.apache.http.impl.client.cache; +import java.util.HashSet; + import org.apache.http.HttpEntity; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; @@ -8,6 +10,7 @@ import org.apache.http.HttpResponse; import org.apache.http.HttpVersion; import org.apache.http.client.HttpClient; import org.apache.http.client.cache.HttpCache; +import org.apache.http.client.cache.HttpCacheEntry; import org.apache.http.message.BasicHttpRequest; import org.apache.http.protocol.HttpContext; import org.easymock.IExpectationSetters; @@ -77,6 +80,9 @@ public abstract class AbstractProtocolTest { EasyMock.expect(mockCache.getCacheEntry(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class))) .andReturn(null).anyTimes(); + EasyMock.expect(mockCache.getVariantCacheEntries(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class))) + .andReturn(new HashSet()).anyTimes(); + mockCache.flushCacheEntriesFor(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class)); EasyMock.expectLastCall().anyTimes(); diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java index 0963b970f..6fb083c28 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java @@ -331,5 +331,50 @@ public class TestBasicHttpCache { assertNotNull(result); } + @Test + public void testGetVariantCacheEntriesReturnsEmptySetOnNoVariants() throws Exception { + HttpHost host = new HttpHost("foo.example.com"); + HttpRequest request = new HttpGet("http://foo.example.com/bar"); + + Set variants = impl.getVariantCacheEntries(host, request); + + assertNotNull(variants); + assertEquals(0, variants.size()); + } + + @Test + public void testGetVariantCacheEntriesReturnsAllVariants() throws Exception { + HttpHost host = new HttpHost("foo.example.com"); + HttpRequest req1 = new HttpGet("http://foo.example.com/bar"); + req1.setHeader("Accept-Encoding", "gzip"); + + HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatDate(new Date())); + resp1.setHeader("Cache-Control", "max-age=3600, public"); + resp1.setHeader("ETag", "\"etag1\""); + resp1.setHeader("Vary", "Accept-Encoding"); + resp1.setHeader("Content-Encoding","gzip"); + resp1.setHeader("Vary", "Accept-Encoding"); + + HttpRequest req2 = new HttpGet("http://foo.example.com/bar"); + req2.setHeader("Accept-Encoding", "identity"); + + HttpResponse resp2 = HttpTestUtils.make200Response(); + resp2.setHeader("Date", DateUtils.formatDate(new Date())); + resp2.setHeader("Cache-Control", "max-age=3600, public"); + resp2.setHeader("ETag", "\"etag1\""); + resp2.setHeader("Vary", "Accept-Encoding"); + resp2.setHeader("Content-Encoding","gzip"); + resp2.setHeader("Vary", "Accept-Encoding"); + + impl.cacheAndReturnResponse(host, req1, resp1, new Date(), new Date()); + impl.cacheAndReturnResponse(host, req2, resp2, new Date(), new Date()); + + Set variants = impl.getVariantCacheEntries(host, req1); + + assertNotNull(variants); + assertEquals(2, variants.size()); + + } } 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 659215ea6..6bacd6452 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 @@ -30,7 +30,9 @@ import java.io.IOException; import java.net.URI; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.http.Header; import org.apache.http.HttpHost; @@ -44,6 +46,7 @@ import org.apache.http.client.ClientProtocolException; import org.apache.http.client.HttpClient; import org.apache.http.client.ResponseHandler; import org.apache.http.client.cache.CacheResponseStatus; +import org.apache.http.client.cache.HeaderConstants; import org.apache.http.client.cache.HttpCache; import org.apache.http.client.cache.HttpCacheEntry; import org.apache.http.client.methods.HttpGet; @@ -299,6 +302,7 @@ public class TestCachingHttpClient { cacheInvalidatorWasCalled(); requestPolicyAllowsCaching(true); getCacheEntryReturns(null); + getVariantCacheEntriesReturns(new HashSet()); requestProtocolValidationIsCalled(); requestIsFatallyNonCompliant(null); @@ -1066,7 +1070,7 @@ public class TestCachingHttpClient { impl.execute(host, req1); HttpResponse result = impl.execute(host, req2); verifyMocks(); - Assert.assertEquals(304, result.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } @@ -1107,7 +1111,7 @@ public class TestCachingHttpClient { impl.execute(host, req1); HttpResponse result = impl.execute(host, req2); verifyMocks(); - Assert.assertEquals(200, result.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode()); } @@ -1142,7 +1146,7 @@ public class TestCachingHttpClient { impl.execute(host, req1); HttpResponse result = impl.execute(host, req2); verifyMocks(); - Assert.assertEquals(200, result.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode()); } @@ -1170,7 +1174,7 @@ public class TestCachingHttpClient { impl.execute(host, req1); HttpResponse result = impl.execute(host, req2); verifyMocks(); - Assert.assertEquals(304, result.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } @@ -1239,7 +1243,7 @@ public class TestCachingHttpClient { impl.execute(host, req1); HttpResponse result = impl.execute(host, req2); verifyMocks(); - Assert.assertEquals(304, result.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } @@ -1451,6 +1455,7 @@ public class TestCachingHttpClient { verifyMocks(); Assert.assertNotNull(result.getFirstHeader("Via")); } + @Test public void testReturns304ForIfNoneMatchPassesIfRequestServedFromOrigin() throws Exception { @@ -1473,8 +1478,6 @@ public class TestCachingHttpClient { req2.addHeader("If-None-Match", "\"etag\""); HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not Modified"); - resp2.setEntity(HttpTestUtils.makeBody(128)); - resp2.setHeader("Content-Length", "128"); resp2.setHeader("ETag", "\"etag\""); resp2.setHeader("Date", DateUtils.formatDate(now)); resp2.setHeader("Cache-Control", "public, max-age=5"); @@ -1562,8 +1565,6 @@ public class TestCachingHttpClient { req2.addHeader("If-Modified-Since", DateUtils.formatDate(tenSecondsAgo)); HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not Modified"); - resp2.setEntity(HttpTestUtils.makeBody(128)); - resp2.setHeader("Content-Length", "128"); resp2.setHeader("ETag", "\"etag\""); resp2.setHeader("Date", DateUtils.formatDate(tenSecondsAgo)); resp1.setHeader("Last-Modified", DateUtils.formatDate(tenSecondsAgo)); @@ -1585,6 +1586,125 @@ public class TestCachingHttpClient { Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } + @Test + public void testNegotiateResponseFromVariantsForwardsCallIfNoVariantETags() throws ProtocolException, IOException { + impl = new CachingHttpClient(mockBackend); + + HttpResponse originResponse = HttpTestUtils.make200Response(); + + Set variants = new HashSet(); + variants.add(HttpTestUtils.makeCacheEntry()); + variants.add(HttpTestUtils.makeCacheEntry()); + variants.add(HttpTestUtils.makeCacheEntry()); + + Capture cap = new Capture(); + + EasyMock.expect(mockBackend.execute(EasyMock.isA(HttpHost.class), + EasyMock.capture(cap), EasyMock.isA(HttpContext.class))) + .andReturn(originResponse); + + replayMocks(); + HttpResponse resp = impl.negotiateResponseFromVariants(host, request, context, variants); + verifyMocks(); + + Assert.assertTrue(cap.hasCaptured()); + + HttpRequest captured = cap.getValue(); + Assert.assertNull(captured.getFirstHeader(HeaderConstants.IF_NONE_MATCH)); + + Assert.assertEquals(resp.getStatusLine().getStatusCode(), HttpStatus.SC_OK); + } + + @Test + public void testNegotiateResponseFromVariantsReturnsVariantAndUpdatesEntryOnBackend304() throws Exception { + HttpCacheEntry variant1 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag1") }); + HttpCacheEntry variant2 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") }); + HttpCacheEntry variant3 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag3") }); + + Set variants = new HashSet(); + variants.add(variant1); + variants.add(variant2); + variants.add(variant3); + + HttpRequest variantConditionalRequest = new BasicHttpRequest("GET", "http://foo.com/bar", HttpVersion.HTTP_1_1); + variantConditionalRequest.addHeader(new BasicHeader(HeaderConstants.IF_NONE_MATCH, "etag1, etag2, etag3")); + + HttpResponse originResponse = new BasicHttpResponse(HttpVersion.HTTP_1_1, + HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + originResponse.addHeader(HeaderConstants.ETAG, "etag2"); + + HttpCacheEntry updatedMatchedEntry = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") }); + + HttpResponse matchedResponse = HttpTestUtils.make200Response(); + HttpResponse response = HttpTestUtils.make200Response(); + + conditionalVariantRequestBuilderReturns(variants, variantConditionalRequest); + + backendCall(variantConditionalRequest, originResponse); + + EasyMock.expect(mockCache.updateCacheEntry(EasyMock.same(host), EasyMock.same(variantConditionalRequest), EasyMock.same(variant2), EasyMock.same(originResponse), EasyMock.isA(Date.class), EasyMock.isA(Date.class))).andReturn(updatedMatchedEntry); + + EasyMock.expect(mockResponseGenerator.generateResponse(updatedMatchedEntry)).andReturn(matchedResponse); + + EasyMock.expect(mockSuitabilityChecker.isConditional(request)).andReturn(false); + + EasyMock.expect(mockCache.cacheAndReturnResponse(EasyMock.same(host), EasyMock.same(request), EasyMock.same(matchedResponse), EasyMock.isA(Date.class), EasyMock.isA(Date.class))).andReturn(response); + + replayMocks(); + HttpResponse resp = impl.negotiateResponseFromVariants(host, request, context, variants); + verifyMocks(); + + Assert.assertEquals(HttpStatus.SC_OK,resp.getStatusLine().getStatusCode()); + } + + @Test + public void testNegotiateResponseFromVariantsReturns200OnBackend200() throws Exception { + HttpCacheEntry variant1 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag1") }); + HttpCacheEntry variant2 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") }); + HttpCacheEntry variant3 = HttpTestUtils + .makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag3") }); + + HttpCacheEntry cacheEntry = null; + + Set variants = new HashSet(); + variants.add(variant1); + variants.add(variant2); + variants.add(variant3); + + HttpRequest variantConditionalRequest = new BasicHttpRequest("GET", "http://foo.com/bar", HttpVersion.HTTP_1_1); + variantConditionalRequest.addHeader(new BasicHeader(HeaderConstants.IF_NONE_MATCH, "etag1, etag2, etag3")); + + HttpResponse originResponse = HttpTestUtils.make200Response(); + originResponse.addHeader(HeaderConstants.ETAG, "etag4"); + + HttpResponse response = HttpTestUtils.make200Response(); + + conditionalVariantRequestBuilderReturns(variants, variantConditionalRequest); + + backendCall(variantConditionalRequest, originResponse); + + responseProtocolValidationIsCalled(); + + responsePolicyAllowsCaching(true); + + EasyMock.expect(mockCache.getCacheEntry(host, variantConditionalRequest)).andReturn(cacheEntry); + + EasyMock.expect(mockCache.cacheAndReturnResponse(EasyMock.same(host), EasyMock.same(variantConditionalRequest), EasyMock.same(originResponse), EasyMock.isA(Date.class), EasyMock.isA(Date.class))).andReturn(response); + + replayMocks(); + HttpResponse resp = impl.negotiateResponseFromVariants(host, request, context, variants); + verifyMocks(); + + Assert.assertEquals(HttpStatus.SC_OK,resp.getStatusLine().getStatusCode()); + + } + @Test public void testReturns200ForIfModifiedSinceFailsIfRequestServedFromOrigin() throws Exception { @@ -1630,6 +1750,154 @@ public class TestCachingHttpClient { Assert.assertEquals(HttpStatus.SC_OK, result.getStatusLine().getStatusCode()); } + @Test + public void testVariantMissServerIfReturns304CacheReturns200() throws Exception { + impl = new CachingHttpClient(mockBackend); + Date now = new Date(); + + HttpRequest req1 = new HttpGet("http://foo.example.com"); + req1.addHeader("Accept-Encoding", "gzip"); + + HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + resp1.setEntity(HttpTestUtils.makeBody(128)); + resp1.setHeader("Content-Length", "128"); + resp1.setHeader("Etag", "\"gzip_etag\""); + resp1.setHeader("Date", DateUtils.formatDate(now)); + resp1.setHeader("Vary", "Accept-Encoding"); + resp1.setHeader("Cache-Control", "public, max-age=3600"); + + HttpRequest req2 = new HttpGet("http://foo.example.com"); + req2.addHeader("Accept-Encoding", "deflate"); + + HttpRequest req2Server = new HttpGet("http://foo.example.com"); + req2Server.addHeader("Accept-Encoding", "deflate"); + req2Server.addHeader("If-None-Match", "\"gzip_etag\""); + + HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + resp2.setEntity(HttpTestUtils.makeBody(128)); + resp2.setHeader("Content-Length", "128"); + resp2.setHeader("Etag", "\"deflate_etag\""); + resp2.setHeader("Date", DateUtils.formatDate(now)); + resp2.setHeader("Vary", "Accept-Encoding"); + resp2.setHeader("Cache-Control", "public, max-age=3600"); + + HttpRequest req3 = new HttpGet("http://foo.example.com"); + req3.addHeader("Accept-Encoding", "gzip,deflate"); + + HttpRequest req3Server = new HttpGet("http://foo.example.com"); + req3Server.addHeader("Accept-Encoding", "gzip,deflate"); + req3Server.addHeader("If-None-Match", "\"gzip_etag\",\"deflate_etag\""); + + HttpResponse resp3 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + resp3.setEntity(HttpTestUtils.makeBody(128)); + resp3.setHeader("Content-Length", "128"); + resp3.setHeader("Etag", "\"gzip_etag\""); + resp3.setHeader("Date", DateUtils.formatDate(now)); + resp3.setHeader("Vary", "Accept-Encoding"); + resp3.setHeader("Cache-Control", "public, max-age=3600"); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp1); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp2); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp3); + + + replayMocks(); + HttpResponse result1 = impl.execute(host, req1); + + HttpResponse result2 = impl.execute(host, req2); + + HttpResponse result3 = impl.execute(host, req3); + + verifyMocks(); + Assert.assertEquals(HttpStatus.SC_OK, result1.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_OK, result2.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_OK, result3.getStatusLine().getStatusCode()); + } + + @Test + public void testVariantsMissServerReturns304CacheReturns304() throws Exception { + impl = new CachingHttpClient(mockBackend); + Date now = new Date(); + + HttpRequest req1 = new HttpGet("http://foo.example.com"); + req1.addHeader("Accept-Encoding", "gzip"); + + HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + resp1.setEntity(HttpTestUtils.makeBody(128)); + resp1.setHeader("Content-Length", "128"); + resp1.setHeader("Etag", "\"gzip_etag\""); + resp1.setHeader("Date", DateUtils.formatDate(now)); + resp1.setHeader("Vary", "Accept-Encoding"); + resp1.setHeader("Cache-Control", "public, max-age=3600"); + + HttpRequest req2 = new HttpGet("http://foo.example.com"); + req2.addHeader("Accept-Encoding", "deflate"); + + HttpRequest req2Server = new HttpGet("http://foo.example.com"); + req2Server.addHeader("Accept-Encoding", "deflate"); + req2Server.addHeader("If-None-Match", "\"gzip_etag\""); + + HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK"); + resp2.setEntity(HttpTestUtils.makeBody(128)); + resp2.setHeader("Content-Length", "128"); + resp2.setHeader("Etag", "\"deflate_etag\""); + resp2.setHeader("Date", DateUtils.formatDate(now)); + resp2.setHeader("Vary", "Accept-Encoding"); + resp2.setHeader("Cache-Control", "public, max-age=3600"); + + HttpRequest req4 = new HttpGet("http://foo.example.com"); + req4.addHeader("Accept-Encoding", "gzip,identity"); + req4.addHeader("If-None-Match", "\"gzip_etag\""); + + HttpRequest req4Server = new HttpGet("http://foo.example.com"); + req4Server.addHeader("Accept-Encoding", "gzip,identity"); + req4Server.addHeader("If-None-Match", "\"gzip_etag\""); + + HttpResponse resp4 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + resp4.setHeader("Etag", "\"gzip_etag\""); + resp4.setHeader("Date", DateUtils.formatDate(now)); + resp4.setHeader("Vary", "Accept-Encoding"); + resp4.setHeader("Cache-Control", "public, max-age=3600"); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp1); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp2); + + EasyMock.expect( + mockBackend.execute(EasyMock.isA(HttpHost.class), EasyMock + .isA(HttpRequest.class), (HttpContext) EasyMock + .isNull())).andReturn(resp4); + + replayMocks(); + HttpResponse result1 = impl.execute(host, req1); + + HttpResponse result2 = impl.execute(host, req2); + + HttpResponse result4 = impl.execute(host, req4); + verifyMocks(); + Assert.assertEquals(HttpStatus.SC_OK, result1.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_OK, result2.getStatusLine().getStatusCode()); + Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result4.getStatusLine().getStatusCode()); + + } + @Test public void testIsSharedCache() { Assert.assertTrue(impl.isSharedCache()); @@ -1647,6 +1915,9 @@ public class TestCachingHttpClient { EasyMock.expect(mockCache.getCacheEntry(EasyMock.same(host), EasyMock.isA(HttpRequest.class))) .andThrow(new IOException()).anyTimes(); + EasyMock.expect(mockCache.getVariantCacheEntries(EasyMock.same(host), + EasyMock.isA(HttpRequest.class))) + .andThrow(new IOException()).anyTimes(); EasyMock.expect(mockCache.cacheAndReturnResponse(EasyMock.same(host), EasyMock.isA(HttpRequest.class), EasyMock.isA(HttpResponse.class), EasyMock.isA(Date.class), EasyMock.isA(Date.class))) @@ -1665,6 +1936,10 @@ public class TestCachingHttpClient { EasyMock.expect(mockCache.getCacheEntry(host, request)).andReturn(result); } + private void getVariantCacheEntriesReturns(Set result) throws IOException { + EasyMock.expect(mockCache.getVariantCacheEntries(host, request)).andReturn(result); + } + private void cacheInvalidatorWasCalled() throws IOException { mockCache.flushInvalidatedCacheEntriesFor( EasyMock.anyObject(), @@ -1693,12 +1968,18 @@ public class TestCachingHttpClient { EasyMock.anyObject())).andReturn(b); } + private void conditionalVariantRequestBuilderReturns(Set variantEntries, HttpRequest validate) + throws Exception { + EasyMock.expect(mockConditionalRequestBuilder.buildConditionalRequestFromVariants(request, variantEntries)) + .andReturn(validate); + } + private void conditionalRequestBuilderReturns(HttpRequest validate) throws Exception { EasyMock.expect(mockConditionalRequestBuilder - .buildConditionalRequest(request, entry)) - .andReturn(validate); - } + .buildConditionalRequest(request, entry)) + .andReturn(validate); +} private void getCurrentDateReturns(Date date) { EasyMock.expect(impl.getCurrentDate()).andReturn(date); diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java index 467ddcba6..8ccd7888b 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestConditionalRequestBuilder.java @@ -27,12 +27,15 @@ package org.apache.http.impl.client.cache; import java.util.Date; +import java.util.HashSet; +import java.util.Set; import org.apache.http.Header; import org.apache.http.HeaderElement; import org.apache.http.HttpRequest; import org.apache.http.HttpVersion; import org.apache.http.ProtocolException; +import org.apache.http.client.cache.HeaderConstants; import org.apache.http.client.cache.HttpCacheEntry; import org.apache.http.impl.cookie.DateUtils; import org.apache.http.message.BasicHeader; @@ -299,4 +302,64 @@ public class TestConditionalRequestBuilder { result.getFirstHeader("User-Agent").getValue()); } + @Test + public void testBuildConditionalRequestFromVariants() throws Exception { + String etag1 = "123"; + String etag2 = "456"; + String etag3 = "789"; + + Set variantEntries = new HashSet(); + variantEntries.add(HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag1) })); + variantEntries.add(HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag2) })); + variantEntries.add(HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag3) })); + + HttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries); + + // seems like a lot of work, but necessary, check for existence and exclusiveness + String ifNoneMatch = conditional.getFirstHeader(HeaderConstants.IF_NONE_MATCH).getValue(); + Assert.assertTrue(ifNoneMatch.contains(etag1)); + Assert.assertTrue(ifNoneMatch.contains(etag2)); + Assert.assertTrue(ifNoneMatch.contains(etag3)); + ifNoneMatch = ifNoneMatch.replace(etag1, ""); + ifNoneMatch = ifNoneMatch.replace(etag2, ""); + ifNoneMatch = ifNoneMatch.replace(etag3, ""); + ifNoneMatch = ifNoneMatch.replace(",",""); + ifNoneMatch = ifNoneMatch.replace(" ", ""); + Assert.assertEquals(ifNoneMatch, ""); + } + + @Test + public void testBuildConditionalRequestFromVariantsWithNoETags() throws Exception { + Set variantEntries = new HashSet(); + variantEntries.add(HttpTestUtils.makeCacheEntry()); + variantEntries.add(HttpTestUtils.makeCacheEntry()); + variantEntries.add(HttpTestUtils.makeCacheEntry()); + + HttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries); + + Assert.assertNull(conditional.getFirstHeader(HeaderConstants.IF_NONE_MATCH)); + } + + @Test + public void testBuildConditionalRequestFromVariantsMixedETagPresence() throws Exception { + String etag1 = "123"; + String etag2 = "456"; + + Set variantEntries = new HashSet(); + variantEntries.add(HttpTestUtils.makeCacheEntry()); + variantEntries.add(HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag1) })); + variantEntries.add(HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag2) })); + + HttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries); + + // seems like a lot of work, but necessary, check for existence and exclusiveness + String ifNoneMatch = conditional.getFirstHeader(HeaderConstants.IF_NONE_MATCH).getValue(); + Assert.assertTrue(ifNoneMatch.contains(etag1)); + Assert.assertTrue(ifNoneMatch.contains(etag2)); + ifNoneMatch = ifNoneMatch.replace(etag1, ""); + ifNoneMatch = ifNoneMatch.replace(etag2, ""); + ifNoneMatch = ifNoneMatch.replace(",",""); + ifNoneMatch = ifNoneMatch.replace(" ", ""); + Assert.assertEquals(ifNoneMatch, ""); + } }