HTTPCLIENT-1032: renamed HttpCache#getVariantCacheEntries to

HttpCache#getVariantCacheEntriesWithEtags and had it return a
Map of etags to cache entries. Doing the filtering here makes
it simpler downstream, and returning a map makes it easier to
find the appropriate entry when handling a 304 indicating we
should use an existing variant.

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1045084 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Jonathan Moore 2010-12-13 11:55:36 +00:00
parent 7f95be93c3
commit 618754b6a9
8 changed files with 67 additions and 154 deletions

View File

@ -29,10 +29,7 @@ package org.apache.http.impl.client.cache;
import java.io.IOException;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.Header;
@ -41,6 +38,7 @@ import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.HttpVersion;
import org.apache.http.client.cache.HeaderConstants;
import org.apache.http.client.cache.HttpCacheEntry;
import org.apache.http.client.cache.HttpCacheStorage;
import org.apache.http.client.cache.HttpCacheUpdateCallback;
@ -241,13 +239,17 @@ class BasicHttpCache implements HttpCache {
cacheInvalidator.flushInvalidatedCacheEntries(host, request);
}
public Set<HttpCacheEntry> getVariantCacheEntries(HttpHost host, HttpRequest request)
public Map<String, HttpCacheEntry> getVariantCacheEntriesWithEtags(HttpHost host, HttpRequest request)
throws IOException {
Set<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
Map<String,HttpCacheEntry> variants = new HashMap<String,HttpCacheEntry>();
HttpCacheEntry root = storage.getEntry(uriExtractor.getURI(host, request));
if (root == null || !root.hasVariants()) return variants;
for(String variantCacheKey : root.getVariantMap().values()) {
variants.add(storage.getEntry(variantCacheKey));
HttpCacheEntry entry = storage.getEntry(variantCacheKey);
Header etagHeader = entry.getFirstHeader(HeaderConstants.ETAG);
if (etagHeader != null) {
variants.put(etagHeader.getValue(), entry);
}
}
return variants;
}

View File

@ -30,7 +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.Map;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.commons.logging.Log;
@ -412,9 +412,9 @@ public class CachingHttpClient implements HttpClient {
"Gateway Timeout");
}
Set<HttpCacheEntry> variantEntries = null;
Map<String,HttpCacheEntry> variantEntries = null;
try {
variantEntries = responseCache.getVariantCacheEntries(target, request);
variantEntries = responseCache.getVariantCacheEntriesWithEtags(target, request);
} catch (IOException ioe) {
log.warn("Unable to retrieve variant entries from cache", ioe);
}
@ -575,7 +575,7 @@ public class CachingHttpClient implements HttpClient {
HttpResponse negotiateResponseFromVariants(HttpHost target,
HttpRequest request, HttpContext context,
Set<HttpCacheEntry> variantEntries) throws IOException, ProtocolException {
Map<String, HttpCacheEntry> variantEntries) throws IOException, ProtocolException {
HttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants(request, variantEntries);
Date requestDate = getCurrentDate();
@ -594,19 +594,9 @@ public class CachingHttpClient implements HttpClient {
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;
}
}
}
HttpCacheEntry matchedEntry = variantEntries.get(resultEtag);
if (matchedEntry == null) {
log.debug("304 response did not contain ETag matching one sent in If-None-Match");

View File

@ -26,8 +26,7 @@
*/
package org.apache.http.impl.client.cache;
import java.util.Set;
import java.util.Map;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.HttpRequest;
@ -93,7 +92,8 @@ class ConditionalRequestBuilder {
* @return the wrapped request
* @throws ProtocolException when I am unable to build a new origin request.
*/
public HttpRequest buildConditionalRequestFromVariants(HttpRequest request, Set<HttpCacheEntry> variantEntries)
public HttpRequest buildConditionalRequestFromVariants(HttpRequest request,
Map<String, HttpCacheEntry> variantEntries)
throws ProtocolException {
RequestWrapper wrapperRequest = new RequestWrapper(request);
wrapperRequest.resetHeaders();
@ -101,25 +101,15 @@ class ConditionalRequestBuilder {
// 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());
for(String etag : variantEntries.keySet()) {
if (!first) {
etags.append(",");
}
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;
etags.append(etag);
}
wrapperRequest.setHeader(HeaderConstants.IF_NONE_MATCH, etags.toString());
return wrapperRequest;
}

View File

@ -28,8 +28,7 @@ package org.apache.http.impl.client.cache;
import java.io.IOException;
import java.util.Date;
import java.util.Set;
import java.util.Map;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
@ -69,14 +68,14 @@ interface HttpCache {
throws IOException;
/**
* Retrieve all variants from the cache, if there are no variants than an empty
* {@link Set} is returned
* Retrieve all variants from the cache, if there are no variants then an empty
* {@link Map} is returned
* @param host
* @param request
* @return
* @return a <code>Map</code> mapping Etags to variant cache entries
* @throws IOException
*/
Set<HttpCacheEntry> getVariantCacheEntries(HttpHost host, HttpRequest request)
Map<String,HttpCacheEntry> getVariantCacheEntriesWithEtags(HttpHost host, HttpRequest request)
throws IOException;
/**

View File

@ -26,8 +26,7 @@
*/
package org.apache.http.impl.client.cache;
import java.util.HashSet;
import java.util.HashMap;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
@ -104,8 +103,8 @@ 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<HttpCacheEntry>()).anyTimes();
EasyMock.expect(mockCache.getVariantCacheEntriesWithEtags(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class)))
.andReturn(new HashMap<String,HttpCacheEntry>()).anyTimes();
mockCache.flushCacheEntriesFor(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class));
EasyMock.expectLastCall().anyTimes();

View File

@ -37,8 +37,6 @@ import java.io.InputStream;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
@ -340,7 +338,7 @@ public class TestBasicHttpCache {
HttpHost host = new HttpHost("foo.example.com");
HttpRequest request = new HttpGet("http://foo.example.com/bar");
Set<HttpCacheEntry> variants = impl.getVariantCacheEntries(host, request);
Map<String,HttpCacheEntry> variants = impl.getVariantCacheEntriesWithEtags(host, request);
assertNotNull(variants);
assertEquals(0, variants.size());
@ -366,7 +364,7 @@ public class TestBasicHttpCache {
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("ETag", "\"etag2\"");
resp2.setHeader("Vary", "Accept-Encoding");
resp2.setHeader("Content-Encoding","gzip");
resp2.setHeader("Vary", "Accept-Encoding");
@ -374,7 +372,7 @@ public class TestBasicHttpCache {
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);
Map<String,HttpCacheEntry> variants = impl.getVariantCacheEntriesWithEtags(host, req1);
assertNotNull(variants);
assertEquals(2, variants.size());

View File

@ -30,10 +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.HashMap;
import java.util.List;
import java.util.Set;
import java.util.Map;
import org.apache.http.Header;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
@ -301,7 +300,7 @@ public class TestCachingHttpClient {
cacheInvalidatorWasCalled();
requestPolicyAllowsCaching(true);
getCacheEntryReturns(null);
getVariantCacheEntriesReturns(new HashSet<HttpCacheEntry>());
getVariantCacheEntriesReturns(new HashMap<String,HttpCacheEntry>());
requestProtocolValidationIsCalled();
requestIsFatallyNonCompliant(null);
@ -1585,58 +1584,29 @@ 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<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") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag1\"") });
HttpCacheEntry variant2 = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag2\"") });
HttpCacheEntry variant3 = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag3") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag3\"") });
Set<HttpCacheEntry> variants = new HashSet<HttpCacheEntry>();
variants.add(variant1);
variants.add(variant2);
variants.add(variant3);
Map<String,HttpCacheEntry> variants = new HashMap<String,HttpCacheEntry>();
variants.put("\"etag1\"", variant1);
variants.put("\"etag2\"", variant2);
variants.put("\"etag3\"", 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");
originResponse.addHeader(HeaderConstants.ETAG, "\"etag2\"");
HttpCacheEntry updatedMatchedEntry = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag2\"") });
HttpResponse matchedResponse = HttpTestUtils.make200Response();
HttpResponse response = HttpTestUtils.make200Response();
@ -1663,18 +1633,18 @@ public class TestCachingHttpClient {
@Test
public void testNegotiateResponseFromVariantsReturns200OnBackend200() throws Exception {
HttpCacheEntry variant1 = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag1") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag1\"") });
HttpCacheEntry variant2 = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag2") });
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "\"etag2\"") });
HttpCacheEntry variant3 = HttpTestUtils
.makeCacheEntry(new Header[] {new BasicHeader(HeaderConstants.ETAG, "etag3") });
.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);
Map<String,HttpCacheEntry> variants = new HashMap<String,HttpCacheEntry>();
variants.put("\"etag1\"", variant1);
variants.put("\"etag2\"", variant2);
variants.put("\"etag3\"", 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"));
@ -1914,7 +1884,7 @@ 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.expect(mockCache.getVariantCacheEntriesWithEtags(EasyMock.same(host),
EasyMock.isA(HttpRequest.class)))
.andThrow(new IOException()).anyTimes();
EasyMock.expect(mockCache.cacheAndReturnResponse(EasyMock.same(host),
@ -1988,8 +1958,8 @@ 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 getVariantCacheEntriesReturns(Map<String,HttpCacheEntry> result) throws IOException {
EasyMock.expect(mockCache.getVariantCacheEntriesWithEtags(host, request)).andReturn(result);
}
private void cacheInvalidatorWasCalled() throws IOException {
@ -2020,7 +1990,7 @@ public class TestCachingHttpClient {
EasyMock.<HttpCacheEntry>anyObject())).andReturn(b);
}
private void conditionalVariantRequestBuilderReturns(Set<HttpCacheEntry> variantEntries, HttpRequest validate)
private void conditionalVariantRequestBuilderReturns(Map<String,HttpCacheEntry> variantEntries, HttpRequest validate)
throws Exception {
EasyMock.expect(mockConditionalRequestBuilder.buildConditionalRequestFromVariants(request, variantEntries))
.andReturn(validate);

View File

@ -27,9 +27,8 @@
package org.apache.http.impl.client.cache;
import java.util.Date;
import java.util.HashSet;
import java.util.Set;
import java.util.HashMap;
import java.util.Map;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.HttpRequest;
@ -304,14 +303,14 @@ public class TestConditionalRequestBuilder {
@Test
public void testBuildConditionalRequestFromVariants() throws Exception {
String etag1 = "123";
String etag2 = "456";
String etag3 = "789";
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) }));
Map<String,HttpCacheEntry> variantEntries = new HashMap<String,HttpCacheEntry>();
variantEntries.put(etag1, HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag1) }));
variantEntries.put(etag2, HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag2) }));
variantEntries.put(etag3, HttpTestUtils.makeCacheEntry(new Header[] { new BasicHeader("ETag", etag3) }));
HttpRequest conditional = impl.buildConditionalRequestFromVariants(request, variantEntries);
@ -328,38 +327,4 @@ public class TestConditionalRequestBuilder {
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, "");
}
}