Ok, *this* time I've remembered to actually attach the patch. Sigh. :)
-----Original Message-----
From: Moore, Jonathan
Sent: Tuesday, July 13, 2010 10:16 AM
To: 'HttpComponents Project'
Subject: RE: [HttpCache][PATCH] Caching API review (take 2)
Reattaching my patch as a .txt file -- the copy of my message I got back
had the patch stripped from it, so I'm not sure it went through to you
all.
Jon
-----Original Message-----
From: Moore, Jonathan [mailto:jonathan_mo...@comcast.com]
Sent: Tuesday, July 13, 2010 10:03 AM
To: HttpComponents Project
Subject: RE: [HttpCache][PATCH] Caching API review (take 2)
Ok, I've attached a much simpler patch that only uses the callback in a
non-surprising way.
Jon
-----Original Message-----
From: Moore, Jonathan [mailto:jonathan_mo...@comcast.com]
Sent: Tuesday, July 13, 2010 9:32 AM
To: HttpComponents Project
Subject: RE: [HttpCache][PATCH] Caching API review (take 2)
I agree that the lack of explicit locking is good; however, I wonder if
the interface is now offering something that implementations can't
always provide, because not all backing stores will be able to offer
this level of transaction.
The primary example would be several JVMs in a pool of app servers all
sharing the same memcached cache. Memcached can offer an atomic
compare-and-swap, but can't offer a generic "perform all these mutations
synchronously" as if there was a global lock.
As another take, the current implementation (I believe) doesn't actually
depend on everything in the current callback to be atomic, and I believe
we can rewrite what we have using the existing callback but without
doing extra side mutations. Let me take a shot at that and post a patch.
Jon
-----Original Message-----
From: Oleg Kalnichevski [mailto:ol...@apache.org]
Sent: Tuesday, July 13, 2010 9:07 AM
To: HttpComponents Project
Subject: [HttpCache][PATCH] Caching API review (take 2)
Jon et al
How about a slightly different take? No explicit locking is needed. The
callback stays but is made more generic allowing for all kinds of cache
mutations, not just variant updates.
Can you live with this change?
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
### Eclipse Workspace Patch 1.0
#P httpcomponents-client
Index:
httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
===================================================================
---
httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
(revision 963355)
+++
httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java
(working copy)
@@ -488,15 +488,7 @@
protected void storeInCache(HttpHost target, HttpRequest request,
CacheEntry entry) {
if (entry.hasVariants()) {
- try {
- String uri = uriExtractor.getURI(target, request);
- HttpCacheUpdateCallback<CacheEntry> callback =
storeVariantEntry(
- target, request, entry);
- responseCache.updateCacheEntry(uri, callback);
- } catch (HttpCacheOperationException ex) {
- log.debug("Was unable to PUT/UPDATE an entry into the cache
based on the uri provided",
- ex);
- }
+ storeVariantEntry(target, request, entry);
} else {
storeNonVariantEntry(target, request, entry);
}
@@ -511,28 +503,38 @@
}
}
- protected HttpCacheUpdateCallback<CacheEntry> storeVariantEntry(
+ protected void storeVariantEntry(
final HttpHost target,
final HttpRequest req,
final CacheEntry entry) {
-
- return new HttpCacheUpdateCallback<CacheEntry>() {
+
+ final String variantURI = uriExtractor.getVariantURI(target, req,
entry);
+ try {
+ responseCache.putEntry(variantURI, entry);
+ } catch (HttpCacheOperationException e) {
+ log.debug("Was unable to PUT a variant entry into the
cache based on the uri provided", e);
+ }
+
+ HttpCacheUpdateCallback<CacheEntry> callback =
+ new HttpCacheUpdateCallback<CacheEntry>() {
public CacheEntry getUpdatedEntry(CacheEntry existing) throws
HttpCacheOperationException {
- return doGetUpdatedParentEntry(existing, target, req, entry);
+ return doGetUpdatedParentEntry(existing, entry, variantURI);
}
};
+ String parentURI = uriExtractor.getURI(target, req);
+ try {
+ responseCache.updateCacheEntry(parentURI, callback);
+ } catch (HttpCacheOperationException e) {
+ log.debug("Was unable to UPDATE a parent entry for a
variant", e);
+ }
+
}
protected CacheEntry doGetUpdatedParentEntry(
CacheEntry existing,
- HttpHost target,
- HttpRequest req,
- CacheEntry entry) throws HttpCacheOperationException {
-
- String variantURI = uriExtractor.getVariantURI(target, req, entry);
- responseCache.putEntry(variantURI, entry);
-
+ CacheEntry entry, String variantURI) throws
HttpCacheOperationException {
+
if (existing != null) {
return existing.addVariantURI(variantURI);
} else {
Index:
httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
===================================================================
---
httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
(revision 963355)
+++
httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingHttpClient.java
(working copy)
@@ -323,12 +323,9 @@
final CacheEntry entry = new CacheEntry(new Date(), new Date(),
HTTP_1_1,
new Header[] {}, new ByteArrayEntity(new byte[] {}), 200,
"OK");
- extractVariantURI(variantURI, entry);
- putInCache(variantURI, entry);
-
replayMocks();
- CacheEntry updatedEntry = impl.doGetUpdatedParentEntry(null, host,
mockRequest, entry);
+ CacheEntry updatedEntry = impl.doGetUpdatedParentEntry(null, entry,
variantURI);
verifyMocks();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
