HTTPCLIENT-1032: during variant negotiation, when the origin specifies

reuse of an existing variant, we no longer cache another copy of the
variant, but rather update the variantMap in the parent entry to
reflect the variant's reuse for requests whose varying headers match
those of the current request.

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

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();

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())) {

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;
 }

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<String,Variant> variants = new HashMap<String,Variant>();
-        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

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();
+    }
+
 }