You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2010/10/15 21:43:19 UTC

svn commit: r1023085 - in /httpcomponents/httpclient/trunk/httpclient-cache/src: main/java/org/apache/http/client/cache/ main/java/org/apache/http/impl/client/cache/ test/java/org/apache/http/impl/client/cache/

Author: olegk
Date: Fri Oct 15 19:43:18 2010
New Revision: 1023085

URL: http://svn.apache.org/viewvc?rev=1023085&view=rev
Log:
HTTPCLIENT-1008: send all variants' ETags on "variant miss"
Contributed by Michajlo Matijkiw <michajlo_matijkiw at comcast.com> and Mohammed Azeem Uddin <mohammedazeem_uddin at comcast.com>

Modified:
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.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/TestBasicHttpCache.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

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java?rev=1023085&r1=1023084&r2=1023085&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/client/cache/HttpCache.java Fri Oct 15 19:43:18 2010
@@ -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;
@@ -67,6 +68,17 @@ public interface HttpCache {
         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<HttpCacheEntry> getVariantCacheEntries(HttpHost host, HttpRequest request)
+        throws IOException;
+
+    /**
      * Store a {@link HttpResponse} in the cache if possible, and return
      * @param host
      * @param request

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java?rev=1023085&r1=1023084&r2=1023085&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java Fri Oct 15 19:43:18 2010
@@ -211,4 +211,20 @@ public class BasicHttpCache implements H
 
     }
 
+    public Set<HttpCacheEntry> getVariantCacheEntries(HttpHost host, HttpRequest request)
+            throws IOException {
+        Set<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
+
+        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

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=1023085&r1=1023084&r2=1023085&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 Fri Oct 15 19:43:18 2010
@@ -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 implement
             if (log.isDebugEnabled()) {
                 RequestLine rl = request.getRequestLine();
                 log.debug("Cache miss [host: " + target + "; uri: " + rl.getUri() + "]");
+            }
 
+            Set<HttpCacheEntry> 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 implement
 
     }
 
+    HttpResponse negotiateResponseFromVariants(HttpHost target,
+            HttpRequest request, HttpContext context,
+            Set<HttpCacheEntry> 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 implement
         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) {

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=1023085&r1=1023084&r2=1023085&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 Fri Oct 15 19:43:18 2010
@@ -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;
@@ -81,6 +83,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<HttpCacheEntry> 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)
      * our cache may actually receive a response to a normal conditional

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=1023085&r1=1023084&r2=1023085&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 Fri Oct 15 19:43:18 2010
@@ -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 AbstractProtocolTe
 
         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<HttpCacheEntry>()).anyTimes();
+
         mockCache.flushCacheEntriesFor(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class));
         EasyMock.expectLastCall().anyTimes();
 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java?rev=1023085&r1=1023084&r2=1023085&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java (original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java Fri Oct 15 19:43:18 2010
@@ -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<HttpCacheEntry> 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<HttpCacheEntry> variants = impl.getVariantCacheEntries(host, req1);
+
+        assertNotNull(variants);
+        assertEquals(2, variants.size());
+
+    }
 
 }

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=1023085&r1=1023084&r2=1023085&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 Fri Oct 15 19:43:18 2010
@@ -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.ClientProt
 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<HttpCacheEntry>());
 
         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));
@@ -1586,6 +1587,125 @@ public class TestCachingHttpClient {
     }
 
     @Test
+    public void testNegotiateResponseFromVariantsForwardsCallIfNoVariantETags() throws ProtocolException, IOException {
+        impl = new CachingHttpClient(mockBackend);
+
+        HttpResponse originResponse = HttpTestUtils.make200Response();
+
+        Set<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
+        variants.add(HttpTestUtils.makeCacheEntry());
+        variants.add(HttpTestUtils.makeCacheEntry());
+        variants.add(HttpTestUtils.makeCacheEntry());
+
+        Capture<HttpRequest> cap = new Capture<HttpRequest>();
+
+        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<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
+        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<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
+        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 {
         impl = new CachingHttpClient(mockBackend);
@@ -1631,6 +1751,154 @@ public class TestCachingHttpClient {
     }
 
     @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<HttpCacheEntry> result) throws IOException {
+        EasyMock.expect(mockCache.getVariantCacheEntries(host, request)).andReturn(result);
+    }
+
     private void cacheInvalidatorWasCalled()  throws IOException {
         mockCache.flushInvalidatedCacheEntriesFor(
                 EasyMock.<HttpHost>anyObject(),
@@ -1693,13 +1968,19 @@ public class TestCachingHttpClient {
                 EasyMock.<HttpCacheEntry>anyObject())).andReturn(b);
     }
 
-    private void conditionalRequestBuilderReturns(HttpRequest validate)
+    private void conditionalVariantRequestBuilderReturns(Set<HttpCacheEntry> variantEntries, HttpRequest validate)
             throws Exception {
-        EasyMock.expect(mockConditionalRequestBuilder
-                .buildConditionalRequest(request, entry))
+        EasyMock.expect(mockConditionalRequestBuilder.buildConditionalRequestFromVariants(request, variantEntries))
             .andReturn(validate);
     }
 
+    private void conditionalRequestBuilderReturns(HttpRequest validate)
+            throws Exception {
+        EasyMock.expect(mockConditionalRequestBuilder
+        .buildConditionalRequest(request, entry))
+    .andReturn(validate);
+}
+
     private void getCurrentDateReturns(Date date) {
         EasyMock.expect(impl.getCurrentDate()).andReturn(date);
     }

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=1023085&r1=1023084&r2=1023085&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 Fri Oct 15 19:43:18 2010
@@ -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 TestConditionalRequestBuild
                 result.getFirstHeader("User-Agent").getValue());
     }
 
+    @Test
+    public void testBuildConditionalRequestFromVariants() throws Exception {
+        String etag1 = "123";
+        String etag2 = "456";
+        String etag3 = "789";
+
+        Set<HttpCacheEntry> variantEntries = new HashSet<HttpCacheEntry>();
+        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<HttpCacheEntry> variantEntries = new HashSet<HttpCacheEntry>();
+        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<HttpCacheEntry> variantEntries = new HashSet<HttpCacheEntry>();
+        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, "");
+    }
 }