Jon et al
I started looking at the Caching API more closely primarily to find out
how difficult / feasible it should be to put together alternative cache
backends such as echache or file system based. I stumbled upon a few
minor issues I thought I should discuss with you.
(1) HttpCacheUpdateCallback
I think I understand the rationale behind the interface but kind of
dislike the fact that its implementations often manipulate the cache
behind the scene in non-obvious ways. One call to #updateCacheEntry may
result in mutation of multiple entries and multiple invocations of
#putEntry methods. Do we need #updateCacheEntry at all? What we need is
ability to update the cache while ensuring the consistency of its
content. Why not just use an exclusive lock to execute multiple cache
operations as a unit of work?
Please take a look at the patch attached and let me know if you can live
with the proposed changes?
(2) Do we really need HttpCacheEntrySerializer? It does not appear to be
used anywhere but in tests.
Oleg
Index: src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java (working copy)
@@ -46,7 +46,7 @@
private static final ProtocolVersion HTTP_1_1 = new ProtocolVersion("HTTP", 1, 1);
private CacheInvalidator impl;
- private HttpCache<CacheEntry> mockCache;
+ private HttpCache<String, CacheEntry> mockCache;
private HttpHost host;
private URIExtractor extractor;
private CacheEntry mockEntry;
Index: src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/TestProtocolRequirements.java (working copy)
@@ -82,7 +82,7 @@
private HttpEntity body;
private HttpEntity mockEntity;
private HttpClient mockBackend;
- private HttpCache<CacheEntry> mockCache;
+ private HttpCache<String, CacheEntry> mockCache;
private HttpRequest request;
private HttpResponse originResponse;
@@ -99,7 +99,7 @@
originResponse = make200Response();
- HttpCache<CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
+ HttpCache<String, CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
mockBackend = EasyMock.createMock(HttpClient.class);
mockEntity = EasyMock.createMock(HttpEntity.class);
mockCache = EasyMock.createMock(HttpCache.class);
Index: src/test/java/org/apache/http/impl/client/cache/DoNotTestProtocolRequirements.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/DoNotTestProtocolRequirements.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/DoNotTestProtocolRequirements.java (working copy)
@@ -62,7 +62,7 @@
private HttpHost host;
private HttpEntity mockEntity;
private HttpClient mockBackend;
- private HttpCache<CacheEntry> mockCache;
+ private HttpCache<String, CacheEntry> mockCache;
private HttpRequest request;
private HttpResponse originResponse;
@@ -77,7 +77,7 @@
originResponse = make200Response();
- HttpCache<CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
+ HttpCache<String, CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
mockBackend = EasyMock.createMock(HttpClient.class);
mockEntity = EasyMock.createMock(HttpEntity.class);
mockCache = EasyMock.createMock(HttpCache.class);
Index: src/test/java/org/apache/http/impl/client/cache/TestResponseCache.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/TestResponseCache.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/TestResponseCache.java (working copy)
@@ -26,18 +26,11 @@
*/
package org.apache.http.impl.client.cache;
-import org.apache.http.client.cache.HttpCacheOperationException;
-import org.apache.http.client.cache.HttpCacheUpdateCallback;
-import org.apache.http.entity.ByteArrayEntity;
import org.easymock.classextension.EasyMock;
import org.junit.Assert;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-
public class TestResponseCache {
private BasicHttpCache cache;
@@ -135,45 +128,4 @@
Assert.assertNull("This cache should not have anything in it!", gone);
}
- @Test
- @Ignore
- public void testCacheEntryCallbackUpdatesCacheEntry() throws HttpCacheOperationException, IOException {
-
- final byte[] expectedArray = new byte[] { 1, 2, 3, 4, 5 };
-
- CacheEntry entry = EasyMock.createMock(CacheEntry.class);
- CacheEntry entry2 = EasyMock.createMock(CacheEntry.class);
-
- cache.putEntry("foo", entry);
- cache.putEntry("bar", entry2);
-
- cache.updateCacheEntry("foo", new HttpCacheUpdateCallback<CacheEntry>() {
-
- public CacheEntry getUpdatedEntry(CacheEntry existing) {
- CacheEntry updated = new CacheEntry(
- existing.getRequestDate(),
- existing.getRequestDate(),
- existing.getProtocolVersion(),
- existing.getAllHeaders(),
- new ByteArrayEntity(expectedArray),
- existing.getStatusCode(),
- existing.getReasonPhrase());
- cache.removeEntry("bar");
- return updated;
- }
- });
-
- CacheEntry afterUpdate = cache.getEntry("foo");
- CacheEntry bar = cache.getEntry("bar");
-
- Assert.assertNull(bar);
-
- byte[] bytes;
- ByteArrayOutputStream stream = new ByteArrayOutputStream();
- afterUpdate.getBody().writeTo(stream);
- bytes = stream.toByteArray();
-
- Assert.assertArrayEquals(expectedArray,bytes);
- }
-
}
Index: src/test/java/org/apache/http/impl/client/cache/TestProtocolDeviations.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/TestProtocolDeviations.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/TestProtocolDeviations.java (working copy)
@@ -77,7 +77,7 @@
private HttpEntity body;
private HttpEntity mockEntity;
private HttpClient mockBackend;
- private HttpCache<CacheEntry> mockCache;
+ private HttpCache<String, CacheEntry> mockCache;
private HttpRequest request;
private HttpResponse originResponse;
@@ -94,7 +94,7 @@
originResponse = make200Response();
- HttpCache<CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
+ HttpCache<String, CacheEntry> cache = new BasicHttpCache(MAX_ENTRIES);
mockBackend = EasyMock.createMock(HttpClient.class);
mockEntity = EasyMock.createMock(HttpEntity.class);
mockCache = EasyMock.createMock(HttpCache.class);
Index: src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
===================================================================
--- src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java (revision 961637)
+++ src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java (working copy)
@@ -26,9 +26,6 @@
*/
package org.apache.http.impl.client.cache;
-import static junit.framework.Assert.assertTrue;
-
-import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
@@ -50,16 +47,10 @@
import org.apache.http.client.HttpClient;
import org.apache.http.client.ResponseHandler;
import org.apache.http.client.cache.HttpCache;
-import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.conn.ClientConnectionManager;
-import org.apache.http.conn.scheme.PlainSocketFactory;
-import org.apache.http.conn.scheme.Scheme;
-import org.apache.http.conn.scheme.SchemeRegistry;
-import org.apache.http.conn.ssl.SSLSocketFactory;
import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
+import org.apache.http.message.BasicHeader;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.message.BasicStatusLine;
import org.apache.http.params.HttpParams;
@@ -67,7 +58,6 @@
import org.easymock.classextension.EasyMock;
import org.junit.Assert;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
public class TestCachingHttpClient {
@@ -93,7 +83,7 @@
private CacheInvalidator mockInvalidator;
private CacheableRequestPolicy mockRequestPolicy;
private HttpClient mockBackend;
- private HttpCache<CacheEntry> mockCache;
+ private HttpCache<String, CacheEntry> cache;
private CachedResponseSuitabilityChecker mockSuitabilityChecker;
private ResponseCachingPolicy mockResponsePolicy;
private HttpRequest mockRequest;
@@ -134,10 +124,10 @@
@Before
public void setUp() {
+ cache = new BasicHttpCache(10);
mockInvalidator = EasyMock.createMock(CacheInvalidator.class);
mockRequestPolicy = EasyMock.createMock(CacheableRequestPolicy.class);
mockBackend = EasyMock.createMock(HttpClient.class);
- mockCache = EasyMock.createMock(HttpCache.class);
mockSuitabilityChecker = EasyMock.createMock(CachedResponseSuitabilityChecker.class);
mockResponsePolicy = EasyMock.createMock(ResponseCachingPolicy.class);
mockConnectionManager = EasyMock.createMock(ClientConnectionManager.class);
@@ -169,7 +159,7 @@
responseDate = new Date();
host = new HttpHost("foo.example.com");
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance);
@@ -187,7 +177,6 @@
EasyMock.replay(mockResponseGenerator);
EasyMock.replay(mockExtractor);
EasyMock.replay(mockBackend);
- EasyMock.replay(mockCache);
EasyMock.replay(mockConnectionManager);
EasyMock.replay(mockContext);
EasyMock.replay(mockHandler);
@@ -221,7 +210,6 @@
EasyMock.verify(mockResponseGenerator);
EasyMock.verify(mockExtractor);
EasyMock.verify(mockBackend);
- EasyMock.verify(mockCache);
EasyMock.verify(mockConnectionManager);
EasyMock.verify(mockContext);
EasyMock.verify(mockHandler);
@@ -308,7 +296,7 @@
cacheEntryHasVariants(false);
extractTheURI(theURI);
- putInCache(theURI);
+ cache.putEntry(theURI, mockCacheEntry);
replayMocks();
impl.storeInCache(host, mockRequest, mockCacheEntry);
@@ -318,24 +306,29 @@
@Test
public void testCacheUpdateAddsVariantURIToParentEntry() throws Exception {
- final String variantURI = "variantURI";
-
final CacheEntry entry = new CacheEntry(new Date(), new Date(), HTTP_1_1,
- new Header[] {}, new ByteArrayEntity(new byte[] {}), 200, "OK");
+ new Header[] { new BasicHeader("Vary", "Accept-Encoding")},
+ new ByteArrayEntity(new byte[] {}), 200, "OK");
- extractVariantURI(variantURI, entry);
- putInCache(variantURI, entry);
+ Assert.assertTrue(entry.hasVariants());
+
+ extractTheURI("this");
+ extractVariantURI("that", entry);
replayMocks();
- CacheEntry updatedEntry = impl.doGetUpdatedParentEntry(null, host, mockRequest, entry);
+ impl.storeInCache(host, mockRequest, entry);
verifyMocks();
- assertTrue(updatedEntry.getVariantURIs().contains(variantURI));
+ Assert.assertEquals(2, cache.size());
+ CacheEntry parent = cache.getEntry("this");
+ Assert.assertNotNull(parent);
+ CacheEntry child = cache.getEntry("that");
+ Assert.assertNotNull(child);
+ Assert.assertTrue(parent.getVariantURIs().contains("that"));
}
-
@Test
public void testCacheMissCausesBackendRequest() throws Exception {
mockImplMethods(GET_CACHE_ENTRY, CALL_BACKEND);
@@ -418,7 +411,7 @@
cacheEntryUpdaterCalled();
cacheEntryHasVariants(false, mockUpdatedCacheEntry);
extractTheURI("http://foo.example.com";);
- putInCache("http://foo.example.com";, mockUpdatedCacheEntry);
+ cache.putEntry("http://foo.example.com";, mockUpdatedCacheEntry);
responseIsGeneratedFromCache(mockUpdatedCacheEntry);
replayMocks();
@@ -503,7 +496,6 @@
responseProtocolValidationIsCalled();
extractTheURI(theURI);
- removeFromCache(theURI);
replayMocks();
HttpResponse result = impl.handleBackendResponse(host, mockRequest, currentDate,
@@ -518,7 +510,6 @@
final String theURI = "theURI";
extractTheURI(theURI);
- gotCacheMiss(theURI);
replayMocks();
CacheEntry result = impl.getCacheEntry(host, mockRequest);
@@ -530,8 +521,9 @@
public void testGetCacheEntryFetchesFromCacheOnCacheHitIfNoVariants() throws Exception {
final String theURI = "theURI";
+ cache.putEntry(theURI, mockCacheEntry);
+
extractTheURI(theURI);
- gotCacheHit(theURI);
cacheEntryHasVariants(false);
replayMocks();
@@ -545,11 +537,11 @@
final String theURI = "theURI";
final String variantURI = "variantURI";
+ cache.putEntry(theURI, mockCacheEntry);
+
extractTheURI(theURI);
- gotCacheHit(theURI);
cacheEntryHasVariants(true);
extractVariantURI(variantURI);
- gotCacheMiss(variantURI);
replayMocks();
CacheEntry result = impl.getCacheEntry(host, mockRequest);
@@ -562,11 +554,12 @@
final String theURI = "theURI";
final String variantURI = "variantURI";
+ cache.putEntry(theURI, mockCacheEntry);
+ cache.putEntry(variantURI, mockVariantCacheEntry);
+
extractTheURI(theURI);
- gotCacheHit(theURI, mockCacheEntry);
cacheEntryHasVariants(true);
extractVariantURI(variantURI);
- gotCacheHit(variantURI, mockVariantCacheEntry);
replayMocks();
CacheEntry result = impl.getCacheEntry(host, mockRequest);
@@ -628,7 +621,7 @@
final HttpRequest theRequest = mockRequest;
final HttpResponse theResponse = mockBackendResponse;
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -660,7 +653,7 @@
final ResponseHandler<Object> theHandler = mockHandler;
final Object value = new Object();
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -700,7 +693,7 @@
final HttpResponse theResponse = mockBackendResponse;
final HttpContext theContext = mockContext;
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -732,7 +725,7 @@
final HttpUriRequest theRequest = mockUriRequest;
final HttpResponse theResponse = mockBackendResponse;
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -762,7 +755,7 @@
final HttpContext theContext = mockContext;
final HttpResponse theResponse = mockBackendResponse;
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -795,7 +788,7 @@
final HttpResponse theResponse = mockBackendResponse;
final Object theValue = new Object();
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -830,7 +823,7 @@
final HttpResponse theResponse = mockBackendResponse;
final Object theValue = new Object();
impl = new CachingHttpClient(mockBackend, mockResponsePolicy, mockEntryGenerator,
- mockExtractor, mockCache, mockResponseGenerator, mockInvalidator,
+ mockExtractor, cache, mockResponseGenerator, mockInvalidator,
mockRequestPolicy, mockSuitabilityChecker, mockConditionalRequestBuilder,
mockCacheEntryUpdater, mockResponseProtocolCompliance,
mockRequestProtocolCompliance) {
@@ -874,38 +867,11 @@
}
@Test
- @Ignore
- public void testRealResultsMatch() throws IOException {
-
- SchemeRegistry schemeRegistry = new SchemeRegistry();
- schemeRegistry.register(new Scheme("http", 80, PlainSocketFactory.getSocketFactory()));
- schemeRegistry.register(new Scheme("https", 443, SSLSocketFactory.getSocketFactory()));
-
- ClientConnectionManager cm = new ThreadSafeClientConnManager(schemeRegistry);
- HttpClient httpClient = new DefaultHttpClient(cm);
-
- HttpCache<CacheEntry> cacheImpl = new BasicHttpCache(100);
-
- CachingHttpClient cachingClient = new CachingHttpClient(httpClient, cacheImpl, 8192);
-
- HttpUriRequest request = new HttpGet("http://www.fancast.com/static-28262/styles/base.css";);
-
- HttpClient baseClient = new DefaultHttpClient();
-
- HttpResponse cachedResponse = cachingClient.execute(request);
- HttpResponse realResponse = baseClient.execute(request);
-
- byte[] cached = readResponse(cachedResponse);
- byte[] real = readResponse(realResponse);
-
- Assert.assertArrayEquals(cached, real);
- }
-
- @Test
public void testResponseIsGeneratedWhenCacheEntryIsUsable() throws Exception {
final String theURI = "http://foo";;
-
+ cache.putEntry(theURI, mockCacheEntry);
+
requestIsFatallyNonCompliant(null);
requestInspectsRequestLine();
requestProtocolValidationIsCalled();
@@ -913,7 +879,6 @@
requestPolicyAllowsCaching(true);
cacheEntrySuitable(true);
extractTheURI(theURI);
- gotCacheHit(theURI);
responseIsGeneratedFromCache();
cacheEntryHasVariants(false);
@@ -1058,18 +1023,6 @@
Assert.assertTrue(impl.isSharedCache());
}
- private byte[] readResponse(HttpResponse response) {
- try {
- ByteArrayOutputStream s1 = new ByteArrayOutputStream();
- response.getEntity().writeTo(s1);
- return s1.toByteArray();
- } catch (Exception ex) {
- return new byte[]{};
-
- }
-
- }
-
private void cacheInvalidatorWasCalled() {
mockInvalidator.flushInvalidatedCacheEntries(host, mockRequest);
}
@@ -1120,10 +1073,6 @@
mockResponseReader);
}
- private void removeFromCache(String theURI) throws Exception {
- mockCache.removeEntry(theURI);
- }
-
private void requestPolicyAllowsCaching(boolean allow) {
org.easymock.EasyMock.expect(mockRequestPolicy.isServableFromCache(mockRequest)).andReturn(
allow);
@@ -1160,24 +1109,12 @@
.andReturn(allow);
}
- private void gotCacheMiss(String theURI) throws Exception {
- org.easymock.EasyMock.expect(mockCache.getEntry(theURI)).andReturn(null);
- }
-
private void cacheEntrySuitable(boolean suitable) {
org.easymock.EasyMock.expect(
mockSuitabilityChecker.canCachedResponseBeUsed(host, mockRequest, mockCacheEntry))
.andReturn(suitable);
}
- private void gotCacheHit(String theURI) throws Exception {
- org.easymock.EasyMock.expect(mockCache.getEntry(theURI)).andReturn(mockCacheEntry);
- }
-
- private void gotCacheHit(String theURI, CacheEntry entry) throws Exception {
- org.easymock.EasyMock.expect(mockCache.getEntry(theURI)).andReturn(entry);
- }
-
private void cacheEntryHasVariants(boolean b) {
org.easymock.EasyMock.expect(mockCacheEntry.hasVariants()).andReturn(b);
}
@@ -1216,14 +1153,6 @@
variantURI);
}
- private void putInCache(String theURI) throws Exception {
- mockCache.putEntry(theURI, mockCacheEntry);
- }
-
- private void putInCache(String theURI, CacheEntry entry) throws Exception {
- mockCache.putEntry(theURI, entry);
- }
-
private void generateCacheEntry(Date requestDate, Date responseDate, byte[] bytes) {
org.easymock.EasyMock.expect(
mockEntryGenerator.generateEntry(requestDate, responseDate, mockBackendResponse,
@@ -1264,7 +1193,7 @@
private void mockImplMethods(String... methods) {
mockedImpl = true;
impl = EasyMock.createMockBuilder(CachingHttpClient.class).withConstructor(mockBackend,
- mockResponsePolicy, mockEntryGenerator, mockExtractor, mockCache,
+ mockResponsePolicy, mockEntryGenerator, mockExtractor, cache,
mockResponseGenerator, mockInvalidator, mockRequestPolicy, mockSuitabilityChecker,
mockConditionalRequestBuilder, mockCacheEntryUpdater,
mockResponseProtocolCompliance, mockRequestProtocolCompliance).addMockedMethods(
Index: src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java
===================================================================
--- src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java (revision 961637)
+++ src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java (working copy)
@@ -48,7 +48,7 @@
@ThreadSafe // so long as the cache implementation is thread-safe
public class CacheInvalidator {
- private final HttpCache<CacheEntry> cache;
+ private final HttpCache<String, CacheEntry> cache;
private final URIExtractor uriExtractor;
private final Log log = LogFactory.getLog(getClass());
@@ -60,7 +60,7 @@
* @param uriExtractor Provides identifiers for the keys to store cache entries
* @param cache the cache to store items away in
*/
- public CacheInvalidator(URIExtractor uriExtractor, HttpCache<CacheEntry> cache) {
+ public CacheInvalidator(URIExtractor uriExtractor, HttpCache<String, CacheEntry> cache) {
this.uriExtractor = uriExtractor;
this.cache = cache;
}
Index: src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
===================================================================
--- src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (revision 961637)
+++ src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (working copy)
@@ -26,14 +26,13 @@
*/
package org.apache.http.impl.client.cache;
-import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import org.apache.http.annotation.ThreadSafe;
import org.apache.http.client.cache.HttpCache;
-import org.apache.http.client.cache.HttpCacheOperationException;
-import org.apache.http.client.cache.HttpCacheUpdateCallback;
/**
* Implements {...@link HttpCache} using LinkedHashMap for backing store
@@ -41,30 +40,44 @@
* @since 4.1
*/
@ThreadSafe
-public class BasicHttpCache implements HttpCache<CacheEntry> {
+public class BasicHttpCache implements HttpCache<String, CacheEntry> {
- private final LinkedHashMap<String, CacheEntry> baseMap = new LinkedHashMap<String, CacheEntry>(20,
- 0.75f, true) {
+ static class InternalHashMap extends LinkedHashMap<String, CacheEntry> {
+ private final int maxEntries;
+
private static final long serialVersionUID = -7750025207539768511L;
+ InternalHashMap(int maxEntries) {
+ super(20, 0.75f, true);
+ this.maxEntries = maxEntries;
+ }
+
@Override
protected boolean removeEldestEntry(Map.Entry<String, CacheEntry> eldest) {
return size() > maxEntries;
}
+
};
- private final Map<String, CacheEntry> syncMap;
+ private final Lock lock;
+ private final Map<String, CacheEntry> map;
- private final int maxEntries;
-
public BasicHttpCache(int maxEntries) {
- this.maxEntries = maxEntries;
- syncMap = Collections.synchronizedMap(baseMap);
+ super();
+ this.lock = new ReentrantLock();
+ this.map = new InternalHashMap(maxEntries);
}
/**
- * Places a CacheEntry in the cache
+ * Returns
+ */
+ public Lock getLock() {
+ return lock;
+ }
+
+ /**
+ * Places an entry in the cache
*
* @param url
* Url to use as the cache key
@@ -72,7 +85,12 @@
* CacheEntry to place in the cache
*/
public void putEntry(String url, CacheEntry entry) {
- syncMap.put(url, entry);
+ lock.lock();
+ try {
+ map.put(url, entry);
+ } finally {
+ lock.unlock();
+ }
}
/**
@@ -83,24 +101,36 @@
* @return CacheEntry if one exists, or null for cache miss
*/
public CacheEntry getEntry(String url) {
- return syncMap.get(url);
+ lock.lock();
+ try {
+ return map.get(url);
+ } finally {
+ lock.unlock();
+ }
}
/**
- * Removes a CacheEntry from the cache
+ * Removes an Entry from the cache
*
* @param url
* Url that is the cache key
*/
public void removeEntry(String url) {
- syncMap.remove(url);
+ lock.lock();
+ try {
+ map.remove(url);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized void updateCacheEntry(
- String url,
- HttpCacheUpdateCallback<CacheEntry> callback) throws HttpCacheOperationException {
- CacheEntry existingEntry = syncMap.get(url);
- CacheEntry updated = callback.getUpdatedEntry(existingEntry);
- syncMap.put(url, updated);
+ public int size() {
+ lock.lock();
+ try {
+ return map.size();
+ } finally {
+ lock.unlock();
+ }
}
+
}
Index: src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
===================================================================
--- src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (revision 961637)
+++ src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (working copy)
@@ -49,7 +49,6 @@
import org.apache.http.client.ResponseHandler;
import org.apache.http.client.cache.HttpCache;
import org.apache.http.client.cache.HttpCacheOperationException;
-import org.apache.http.client.cache.HttpCacheUpdateCallback;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.conn.ClientConnectionManager;
import org.apache.http.entity.ByteArrayEntity;
@@ -76,7 +75,7 @@
private final ResponseCachingPolicy responseCachingPolicy;
private final CacheEntryGenerator cacheEntryGenerator;
private final URIExtractor uriExtractor;
- private final HttpCache<CacheEntry> responseCache;
+ private final HttpCache<String, CacheEntry> responseCache;
private final CachedHttpResponseGenerator responseGenerator;
private final CacheInvalidator cacheInvalidator;
private final CacheableRequestPolicy cacheableRequestPolicy;
@@ -112,9 +111,8 @@
this.requestCompliance = new RequestProtocolCompliance();
}
- public CachingHttpClient(HttpCache<CacheEntry> cache, int maxObjectSizeBytes) {
+ public CachingHttpClient(HttpCache<String, CacheEntry> cache, int maxObjectSizeBytes) {
this.responseCache = cache;
-
this.backend = new DefaultHttpClient();
this.maxObjectSizeBytes = maxObjectSizeBytes;
this.responseCachingPolicy = new ResponseCachingPolicy(maxObjectSizeBytes);
@@ -130,9 +128,8 @@
this.requestCompliance = new RequestProtocolCompliance();
}
- public CachingHttpClient(HttpClient client, HttpCache<CacheEntry> cache, int maxObjectSizeBytes) {
+ public CachingHttpClient(HttpClient client, HttpCache<String, CacheEntry> cache, int maxObjectSizeBytes) {
this.responseCache = cache;
-
this.backend = client;
this.responseCachingPolicy = new ResponseCachingPolicy(maxObjectSizeBytes);
this.cacheEntryGenerator = new CacheEntryGenerator();
@@ -150,7 +147,7 @@
public CachingHttpClient(HttpClient backend, ResponseCachingPolicy responseCachingPolicy,
CacheEntryGenerator cacheEntryGenerator, URIExtractor uriExtractor,
- HttpCache<CacheEntry> responseCache, CachedHttpResponseGenerator responseGenerator,
+ HttpCache<String, CacheEntry> responseCache, CachedHttpResponseGenerator responseGenerator,
CacheInvalidator cacheInvalidator, CacheableRequestPolicy cacheableRequestPolicy,
CachedResponseSuitabilityChecker suitabilityChecker,
ConditionalRequestBuilder conditionalRequestBuilder, CacheEntryUpdater entryUpdater,
@@ -488,14 +485,24 @@
protected void storeInCache(HttpHost target, HttpRequest request, CacheEntry entry) {
if (entry.hasVariants()) {
+ responseCache.getLock().lock();
try {
String uri = uriExtractor.getURI(target, request);
- HttpCacheUpdateCallback<CacheEntry> callback = storeVariantEntry(
- target, request, entry);
- responseCache.updateCacheEntry(uri, callback);
+ CacheEntry existing = responseCache.getEntry(uri);
+ String variantURI = uriExtractor.getVariantURI(target, request, entry);
+ responseCache.putEntry(variantURI, entry);
+ CacheEntry updated;
+ if (existing != null) {
+ updated = existing.addVariantURI(variantURI);
+ } else {
+ updated = entry.addVariantURI(variantURI);
+ }
+ responseCache.putEntry(uri, updated);
} catch (HttpCacheOperationException ex) {
log.debug("Was unable to PUT/UPDATE an entry into the cache based on the uri provided",
ex);
+ } finally {
+ responseCache.getLock().unlock();
}
} else {
storeNonVariantEntry(target, request, entry);
@@ -511,35 +518,6 @@
}
}
- protected HttpCacheUpdateCallback<CacheEntry> storeVariantEntry(
- final HttpHost target,
- final HttpRequest req,
- final CacheEntry entry) {
-
- return new HttpCacheUpdateCallback<CacheEntry>() {
- public CacheEntry getUpdatedEntry(CacheEntry existing) throws HttpCacheOperationException {
-
- return doGetUpdatedParentEntry(existing, target, req, entry);
- }
- };
- }
-
- protected CacheEntry doGetUpdatedParentEntry(
- CacheEntry existing,
- HttpHost target,
- HttpRequest req,
- CacheEntry entry) throws HttpCacheOperationException {
-
- String variantURI = uriExtractor.getVariantURI(target, req, entry);
- responseCache.putEntry(variantURI, entry);
-
- if (existing != null) {
- return existing.addVariantURI(variantURI);
- } else {
- return entry.addVariantURI(variantURI);
- }
- }
-
protected HttpResponse correctIncompleteResponse(HttpResponse resp,
byte[] bodyBytes) {
int status = resp.getStatusLine().getStatusCode();
Index: src/main/java/org/apache/http/client/cache/HttpCache.java
===================================================================
--- src/main/java/org/apache/http/client/cache/HttpCache.java (revision 961637)
+++ src/main/java/org/apache/http/client/cache/HttpCache.java (working copy)
@@ -26,18 +26,37 @@
*/
package org.apache.http.client.cache;
+import java.util.concurrent.locks.Lock;
+
/**
* @since 4.1
*/
-public interface HttpCache<E> {
+public interface HttpCache<K, E> {
- void putEntry(String url, E entry) throws HttpCacheOperationException;
+ /**
+ * Returns the cache lock, which can be used to gain exclusive access
+ * to the cache when multiple entries must be updated in one transaction.
+ */
+ Lock getLock();
+
+ /**
+ * Places an entry in the cache.
+ */
+ void putEntry(K key, E entry) throws HttpCacheOperationException;
- E getEntry(String url) throws HttpCacheOperationException;
+ /**
+ * Returns an entry from the cache, if it exists.
+ */
+ E getEntry(K key) throws HttpCacheOperationException;
- void removeEntry(String url) throws HttpCacheOperationException;
+ /**
+ * Removes an entry from the cache with the given key, if it exists.
+ */
+ void removeEntry(K key) throws HttpCacheOperationException;
- void updateCacheEntry(
- String url, HttpCacheUpdateCallback<E> callback) throws HttpCacheOperationException;
+ /**
+ * Returns the number of entries in this cache.
+ */
+ int size();
}
Index: src/main/java/org/apache/http/client/cache/HttpCacheUpdateCallback.java
===================================================================
--- src/main/java/org/apache/http/client/cache/HttpCacheUpdateCallback.java (revision 961637)
+++ src/main/java/org/apache/http/client/cache/HttpCacheUpdateCallback.java (working copy)
@@ -1,46 +0,0 @@
-/*
- * ====================================================================
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- * ====================================================================
- *
- * This software consists of voluntary contributions made by many
- * individuals on behalf of the Apache Software Foundation. For more
- * information on the Apache Software Foundation, please see
- * <http://www.apache.org/>.
- *
- */
-package org.apache.http.client.cache;
-
-public interface HttpCacheUpdateCallback<E> {
-
- /**
- * Returns the new cache entry that should replace an existing one.
- *
- * @param existing
- * the cache entry current in-place in the cache, possibly
- * <code>null</code> if nonexistent
- * @return CacheEntry the cache entry that should replace it, again,
- * possible <code>null</code>
- * @throws HttpCacheOperationException
- * exception containing information about a failure in the cache
- *
- * @since 4.1
- */
- E getUpdatedEntry(E existing) throws HttpCacheOperationException;
-
-}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
