Index: src/main/java/org/apache/http/client/cache/HttpCacheInvalidator.java =================================================================== --- src/main/java/org/apache/http/client/cache/HttpCacheInvalidator.java (revision 1529201) +++ src/main/java/org/apache/http/client/cache/HttpCacheInvalidator.java (working copy) @@ -1,29 +1,3 @@ -/* - * ==================================================================== - * 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 - * . - * - */ package org.apache.http.client.cache; import org.apache.http.HttpHost; @@ -33,26 +7,55 @@ /** * Given a particular HttpRequest, flush any cache entries that this request * would invalidate. - * + * * @since 4.3 */ public interface HttpCacheInvalidator { - /** - * Remove cache entries from the cache that are no longer fresh or have been - * invalidated in some way. - * - * @param host - * The backend host we are talking to - * @param req - * The HttpRequest to that host - */ - void flushInvalidatedCacheEntries(HttpHost host, HttpRequest req); + /** + * Remove cache entries from the cache that are no longer fresh or have been + * invalidated in some way. + *

+ * This method is called at the very beginning of the request execution, + * before any attempt to get content from the cache and/or execute the + * request. + * + * @param host + * The backend host we are talking to + * @param request + * The HttpRequest to that host + */ + public void flushInvalidatedCacheEntries(HttpHost host, HttpRequest request); - /** - * Flushes entries that were invalidated by the given response received for - * the given host/request pair. - */ - void flushInvalidatedCacheEntries(HttpHost host, HttpRequest request, HttpResponse response); + /** + * Clear all matching {@link HttpCacheEntry}s. + *

+ * This method is called after the response was received, in case the + * response is not cacheable. + *

+ * {@link #flushInvalidatedCacheEntries(HttpHost, HttpRequest, HttpResponse)} + * is always called before this method. + * + * @param host + * The backend host we are talking to + * @param request + * The HttpRequest to that host + */ + public void flushCacheEntriesFor(HttpHost host, HttpRequest request); + + /** + * Flushes entries that were invalidated by the given response received for + * the given host/request pair. + *

+ * This method is called after the response was received. + * + * @param host + * The backend host we are talking to + * @param request + * The HttpRequest to that host + * @param request + * The HttpResponse from that host + */ + public void flushInvalidatedCacheEntries(HttpHost host, HttpRequest request, HttpResponse response); } Index: src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java =================================================================== --- src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (revision 1529201) +++ src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java (working copy) @@ -71,22 +71,22 @@ private final Log log = LogFactory.getLog(getClass()); - public BasicHttpCache( + public BasicHttpCache( final ResourceFactory resourceFactory, final HttpCacheStorage storage, final CacheConfig config, final CacheKeyGenerator uriExtractor, final HttpCacheInvalidator cacheInvalidator) { this.resourceFactory = resourceFactory; - this.uriExtractor = uriExtractor; + this.uriExtractor = uriExtractor; this.cacheEntryUpdater = new CacheEntryUpdater(resourceFactory); this.maxObjectSizeBytes = config.getMaxObjectSize(); this.responseGenerator = new CachedHttpResponseGenerator(); this.storage = storage; this.cacheInvalidator = cacheInvalidator; } - - public BasicHttpCache( + + public BasicHttpCache( final ResourceFactory resourceFactory, final HttpCacheStorage storage, final CacheConfig config, @@ -113,8 +113,7 @@ public void flushCacheEntriesFor(final HttpHost host, final HttpRequest request) throws IOException { if (!safeRequestMethods.contains(request.getRequestLine().getMethod())) { - final String uri = uriExtractor.getURI(host, request); - storage.removeEntry(uri); + cacheInvalidator.flushCacheEntriesFor(host, request); } } Index: src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java =================================================================== --- src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java (revision 1529201) +++ src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java (working copy) @@ -285,4 +285,8 @@ } return responseDate.before(entryDate); } + + public void flushCacheEntriesFor(HttpHost host, HttpRequest req) { + flushEntry( cacheKeyGenerator.getURI(host, req) ); + } } Index: src/main/java/org/apache/http/impl/client/cache/CachingHttpClientBuilder.java =================================================================== --- src/main/java/org/apache/http/impl/client/cache/CachingHttpClientBuilder.java (revision 1529201) +++ src/main/java/org/apache/http/impl/client/cache/CachingHttpClientBuilder.java (working copy) @@ -47,6 +47,7 @@ private File cacheDir; private CacheConfig cacheConfig; private SchedulingStrategy schedulingStrategy; + private CacheKeyGenerator cacheKeyGenerator; private HttpCacheInvalidator httpCacheInvalidator; public static CachingHttpClientBuilder create() { @@ -86,12 +87,20 @@ this.schedulingStrategy = schedulingStrategy; return this; } - - public final CachingHttpClientBuilder setHttpCacheInvalidator( - final HttpCacheInvalidator cacheInvalidator) { - this.httpCacheInvalidator = cacheInvalidator; - return this; - } + + public final CachingHttpClientBuilder setHttpCacheInvalidator(HttpCacheInvalidator cacheInvalidator) { + this.httpCacheInvalidator = cacheInvalidator; + return this; + } + + public final CachingHttpClientBuilder setCacheKeyGenerator(CacheKeyGenerator cacheKeyGenerator) { + // This method is required to ensure that the cacheInvalidator uses the same + // cacheKeyGenerator than the HttpCache. + // Without this method, the cacheInvalidator MUST be created with + // org.apache.http.impl.client.cache.CacheKeyGenerator. + this.cacheKeyGenerator = cacheKeyGenerator; + return this; + } @Override protected ClientExecChain decorateMainExec(final ClientExecChain mainExec) { @@ -115,13 +124,17 @@ } } final AsynchronousValidator revalidator = createAsynchronousRevalidator(config); - final CacheKeyGenerator uriExtractor = new CacheKeyGenerator(); + + CacheKeyGenerator uriExtractor = this.cacheKeyGenerator; + if (uriExtractor == null) { + uriExtractor = new CacheKeyGenerator(); + } - HttpCacheInvalidator cacheInvalidator = this.httpCacheInvalidator; - if (cacheInvalidator == null) { - cacheInvalidator = new CacheInvalidator(uriExtractor, storage); - } - + HttpCacheInvalidator cacheInvalidator = this.httpCacheInvalidator; + if (cacheInvalidator == null) { + cacheInvalidator = new CacheInvalidator(uriExtractor, storage); + } + return new CachingExec(mainExec, new BasicHttpCache( resourceFactory, Index: src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java =================================================================== --- src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (revision 1529201) +++ src/main/java/org/apache/http/impl/client/cache/ResponseCachingPolicy.java (working copy) @@ -95,7 +95,8 @@ } /** - * Determines if an HttpResponse can be cached. + * Determines if an HttpResponse can be cached, based on method (only GET is cacheable), + * status code, content-length (cache configuration) and headers (date, expires, age, vary, cache-control) . * * @param httpMethod What type of request was this, a GET, PUT, other? * @param response The origin response Index: src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java =================================================================== --- src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java (revision 1529201) +++ src/test/java/org/apache/http/impl/client/cache/TestProtocolRecommendations.java (working copy) @@ -40,17 +40,20 @@ import org.apache.http.Header; import org.apache.http.HeaderElement; import org.apache.http.HttpEntityEnclosingRequest; +import org.apache.http.HttpHost; 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.ClientProtocolException; +import org.apache.http.client.cache.HttpCacheInvalidator; import org.apache.http.client.methods.HttpExecutionAware; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpRequestWrapper; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.client.utils.DateUtils; +import org.apache.http.impl.execchain.ClientExecChain; import org.apache.http.message.BasicHttpEntityEnclosingRequest; import org.apache.http.message.BasicHttpRequest; import org.apache.http.message.BasicHttpResponse; @@ -1830,5 +1833,74 @@ assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); } + + + /** + * Ensure cache invalidation can be customized to prevent cache invalidation on error. + * + * @throws Exception + */ + @Test + public void testCacheNotInvalidatedOnError() throws Exception { + config = CacheConfig.custom().setMaxCacheEntries(MAX_ENTRIES).setMaxObjectSize(MAX_BYTES).build(); + CacheKeyGenerator uriExtractor = new CacheKeyGenerator() ; + BasicHttpCacheStorage storage = new BasicHttpCacheStorage(config); + HttpCacheInvalidator invalidator = new CacheInvalidator(uriExtractor, storage) { + + @Override + public void flushInvalidatedCacheEntries(HttpHost host, HttpRequest req) { + // Do not flush cache entries before sending request + } + + @Override + public void flushInvalidatedCacheEntries(HttpHost host, HttpRequest request, HttpResponse response) { + if( response.getStatusLine().getStatusCode() < 400){ + super.flushInvalidatedCacheEntries(host, request, response); + } + } + @Override + public void flushCacheEntriesFor(HttpHost host, HttpRequest req) { + // Do not flush if response is not cacheable. + } + + }; + cache = new BasicHttpCache(new HeapResourceFactory(), storage,config, uriExtractor, invalidator); + mockBackend = EasyMock.createNiceMock(ClientExecChain.class); + impl = createCachingExecChain(mockBackend, cache, config); + + final HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1)); + final HttpResponse resp1 = HttpTestUtils.make200Response(); + resp1.setHeader("Cache-Control", "max-age=3600"); + + backendExpectsAnyRequestAndReturn(resp1); + + final HttpRequestWrapper req1_2 = HttpRequestWrapper + .wrap(new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1)); + // No call to backend expected : served from cache + + final HttpRequestWrapper req2 = HttpRequestWrapper + .wrap(new BasicHttpRequest("POST", "/", HttpVersion.HTTP_1_1)); + // Returns an error : should not invalidate cache + final HttpResponse resp2 = HttpTestUtils.make500Response(); + backendExpectsAnyRequestAndReturn(resp2); + + final HttpRequestWrapper req3 = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "/", HttpVersion.HTTP_1_1)); + // No call to backend expected : served from cache + + replayMocks(); + final HttpResponse result1 = impl.execute(route, req1, context, null); + final HttpResponse result1_2 = impl.execute(route, req1_2, context, null); + impl.execute(route, req2, context, null); + + // Will fail with NPE is request is sent to backend. + final HttpResponse result3 = impl.execute(route, req3, context, null); + + verifyMocks(); + + assertTrue(HttpTestUtils.semanticallyTransparent(result1, result1)); + assertTrue(HttpTestUtils.semanticallyTransparent(result1, result1_2)); + assertTrue(HttpTestUtils.semanticallyTransparent(result1, result3)); + } + }