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 d43a0c4d9..9f48eb570 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 @@ -125,6 +125,27 @@ class BasicHttpCache implements HttpCache { } } + public void reuseVariantEntryFor(HttpHost target, final HttpRequest req, + final Variant variant) throws IOException { + final String parentCacheKey = uriExtractor.getURI(target, req); + final HttpCacheEntry entry = variant.getEntry(); + final String variantKey = uriExtractor.getVariantKey(req, entry); + final String variantCacheKey = variant.getCacheKey(); + + HttpCacheUpdateCallback callback = new HttpCacheUpdateCallback() { + public HttpCacheEntry update(HttpCacheEntry existing) + throws IOException { + return doGetUpdatedParentEntry(req.getRequestLine().getUri(), + existing, entry, variantKey, variantCacheKey); + } + }; + + try { + storage.updateEntry(parentCacheKey, callback); + } catch (HttpCacheUpdateException e) { + log.warn("Could not update key [" + parentCacheKey + "]", e); + } + } boolean isIncompleteResponse(HttpResponse resp, Resource resource) { int status = resp.getStatusLine().getStatusCode(); 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 567a67ee3..f3b31f26b 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 @@ -632,7 +632,6 @@ public class CachingHttpClient implements HttpClient { cacheUpdates.getAndIncrement(); setResponseStatus(context, CacheResponseStatus.VALIDATED); - // SHOULD update cache entry according to rfc HttpCacheEntry responseEntry = matchedEntry; try { responseEntry = responseCache.updateVariantCacheEntry(target, conditionalRequest, @@ -643,9 +642,9 @@ public class CachingHttpClient implements HttpClient { HttpResponse resp = responseGenerator.generateResponse(responseEntry); try { - resp = responseCache.cacheAndReturnResponse(target, request, resp, requestDate, responseDate); + responseCache.reuseVariantEntryFor(target, request, matchingVariant); } catch (IOException ioe) { - log.warn("Could not cache entry", ioe); + log.warn("Could not update cache entry to reuse variant", ioe); } if (suitabilityChecker.isConditional(request) && suitabilityChecker.allConditionalsMatch(request, responseEntry, new Date())) { diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java index 5671045b6..bdeea9a33 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/HttpCache.java @@ -127,4 +127,14 @@ interface HttpCache { Date responseReceived, String cacheKey) throws IOException; + /** + * Specifies cache should reuse the given cached variant to satisfy + * requests whose varying headers match those of the given client request. + * @param target host of the upstream client request + * @param req request sent by upstream client + * @param variant variant cache entry to reuse + * @throws IOException may be thrown during cache update + */ + void reuseVariantEntryFor(HttpHost target, final HttpRequest req, + final Variant variant) throws IOException; } 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 05b515c55..970491e31 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 @@ -1584,52 +1584,6 @@ public class TestCachingHttpClient { Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } - @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\"") }); - - Map variants = new HashMap(); - variants.put("\"etag1\"", new Variant("A","B",variant1)); - variants.put("\"etag2\"", new Variant("C","D",variant2)); - variants.put("\"etag3\"", new Variant("E","F",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.updateVariantCacheEntry(EasyMock.same(host), EasyMock.same(variantConditionalRequest), EasyMock.same(variant2), EasyMock.same(originResponse), EasyMock.isA(Date.class), EasyMock.isA(Date.class), EasyMock.eq("D"))).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 diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java index 04279f0a3..1174b5e1a 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java @@ -927,4 +927,42 @@ public class TestProtocolRecommendations extends AbstractProtocolTest { assertEquals(DateUtils.formatDate(now), result1.getFirstHeader("Date").getValue()); assertEquals(DateUtils.formatDate(now), result2.getFirstHeader("Date").getValue()); } + + @Test + public void testResponseToExistingVariantsIsCachedForFutureResponses() + throws Exception { + + Date now = new Date(); + Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L); + + HttpRequest req1 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + req1.setHeader("User-Agent", "agent1"); + + HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Date", DateUtils.formatDate(tenSecondsAgo)); + resp1.setHeader("Vary", "User-Agent"); + resp1.setHeader("Cache-Control", "max-age=3600"); + resp1.setHeader("ETag", "\"etag1\""); + + backendExpectsAnyRequest().andReturn(resp1); + + HttpRequest req2 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + req2.setHeader("User-Agent", "agent2"); + + HttpResponse resp2 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not Modified"); + resp2.setHeader("Date", DateUtils.formatDate(now)); + resp2.setHeader("ETag", "\"etag1\""); + + backendExpectsAnyRequest().andReturn(resp2); + + HttpRequest req3 = new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1); + req3.setHeader("User-Agent", "agent2"); + + replayMocks(); + impl.execute(host, req1); + impl.execute(host, req2); + impl.execute(host, req3); + verifyMocks(); + } + }