From 16147b1852d03bb1e9b3b039cf5df0292c9a61d6 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 7 Jan 2018 14:51:02 +0100 Subject: [PATCH] Completed rewrite of re-validation code in the classic caching exec interceptor; added re-validation to the async caching exec interceptor --- .../http/impl/cache/AsyncCachingExec.java | 70 +++++++-- .../http/impl/cache/BasicHttpAsyncCache.java | 9 ++ .../http/impl/cache/BasicHttpCache.java | 9 ++ .../client5/http/impl/cache/CacheConfig.java | 140 +++--------------- .../http/impl/cache/CacheRevalidatorBase.java | 9 +- .../client5/http/impl/cache/CachingExec.java | 107 ++++++++----- .../cache/CachingHttp2AsyncClientBuilder.java | 31 +++- .../cache/CachingHttpAsyncClientBuilder.java | 31 +++- .../impl/cache/CachingHttpClientBuilder.java | 44 ++++-- .../cache/DefaultAsyncCacheRevalidator.java | 112 +++++++------- .../impl/cache/DefaultCacheRevalidator.java | 59 +++----- .../http/impl/cache/HttpAsyncCache.java | 2 + .../hc/client5/http/impl/cache/HttpCache.java | 2 + .../schedule/ImmediateSchedulingStrategy.java | 2 +- .../http/impl/cache/AbstractProtocolTest.java | 12 +- .../http/impl/cache/TestCachingExec.java | 14 +- .../http/impl/cache/TestCachingExecChain.java | 30 ++-- .../cache/TestCachingHttpClientBuilder.java | 48 ------ .../cache/TestHttpCacheJiraNumber1147.java | 2 +- .../impl/cache/TestProtocolDeviations.java | 2 +- .../impl/cache/TestProtocolRequirements.java | 10 +- .../impl/cache/TestRFC5861Compliance.java | 67 ++++++--- .../hc/client5/http/impl/ExecSupport.java | 4 + .../hc/client5/http/impl/Operations.java | 37 +++++ .../InternalAbstractHttpAsyncClient.java | 2 +- .../impl/async/MinimalHttp2AsyncClient.java | 2 +- .../impl/async/MinimalHttpAsyncClient.java | 2 +- .../http/impl/classic/InternalHttpClient.java | 2 +- 28 files changed, 487 insertions(+), 374 deletions(-) delete mode 100644 httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingHttpClientBuilder.java diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java index 6989bba6c..36620b7e7 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java @@ -32,6 +32,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -47,8 +48,10 @@ import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.ResourceFactory; import org.apache.hc.client5.http.cache.ResourceIOException; +import org.apache.hc.client5.http.impl.ExecSupport; import org.apache.hc.client5.http.impl.RequestCopier; import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.http.schedule.SchedulingStrategy; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -87,21 +90,16 @@ public class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler { private final HttpAsyncCache responseCache; + private final DefaultAsyncCacheRevalidator cacheRevalidator; private final ConditionalRequestBuilder conditionalRequestBuilder; - public AsyncCachingExec(final HttpAsyncCache cache, final CacheConfig config) { + AsyncCachingExec(final HttpAsyncCache cache, final DefaultAsyncCacheRevalidator cacheRevalidator, final CacheConfig config) { super(config); this.responseCache = Args.notNull(cache, "Response cache"); + this.cacheRevalidator = cacheRevalidator; this.conditionalRequestBuilder = new ConditionalRequestBuilder<>(RequestCopier.INSTANCE); } - public AsyncCachingExec( - final ResourceFactory resourceFactory, - final HttpAsyncCacheStorage storage, - final CacheConfig config) { - this(new BasicHttpAsyncCache(resourceFactory, storage), config); - } - AsyncCachingExec( final HttpAsyncCache responseCache, final CacheValidityPolicy validityPolicy, @@ -109,16 +107,37 @@ public AsyncCachingExec( final CachedHttpResponseGenerator responseGenerator, final CacheableRequestPolicy cacheableRequestPolicy, final CachedResponseSuitabilityChecker suitabilityChecker, - final ConditionalRequestBuilder conditionalRequestBuilder, final ResponseProtocolCompliance responseCompliance, final RequestProtocolCompliance requestCompliance, + final DefaultAsyncCacheRevalidator cacheRevalidator, + final ConditionalRequestBuilder conditionalRequestBuilder, final CacheConfig config) { super(validityPolicy, responseCachingPolicy, responseGenerator, cacheableRequestPolicy, suitabilityChecker, responseCompliance, requestCompliance, config); this.responseCache = responseCache; + this.cacheRevalidator = cacheRevalidator; this.conditionalRequestBuilder = conditionalRequestBuilder; } + public AsyncCachingExec( + final HttpAsyncCache cache, + final ScheduledExecutorService executorService, + final SchedulingStrategy schedulingStrategy, + final CacheConfig config) { + this(cache, + executorService != null ? new DefaultAsyncCacheRevalidator(executorService, schedulingStrategy) : null, + config); + } + + public AsyncCachingExec( + final ResourceFactory resourceFactory, + final HttpAsyncCacheStorage storage, + final ScheduledExecutorService executorService, + final SchedulingStrategy schedulingStrategy, + final CacheConfig config) { + this(new BasicHttpAsyncCache(resourceFactory, storage), executorService, schedulingStrategy, config); + } + private void triggerResponse( final SimpleHttpResponse cacheResponse, final AsyncExecChain.Scope scope, @@ -609,7 +628,38 @@ private void handleCacheHit( triggerResponse(cacheResponse, scope, asyncExecCallback); } else if (!(entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { log.debug("Revalidating cache entry"); - revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry); + if (cacheRevalidator != null + && !staleResponseNotAllowed(request, entry, now) + && validityPolicy.mayReturnStaleWhileRevalidating(entry, now)) { + log.debug("Serving stale with asynchronous revalidation"); + try { + final SimpleHttpResponse cacheResponse = generateCachedResponse(request, context, entry, now); + final String exchangeId = ExecSupport.getNextExchangeId(); + final AsyncExecChain.Scope fork = new AsyncExecChain.Scope( + exchangeId, + scope.route, + scope.originalRequest, + new ComplexFuture<>(null), + HttpClientContext.create(), + scope.execRuntime.fork()); + cacheRevalidator.revalidateCacheEntry( + responseCache.generateKey(target, request, entry), + asyncExecCallback, + new DefaultAsyncCacheRevalidator.RevalidationCall() { + + @Override + public void execute(final AsyncExecCallback asyncExecCallback) { + revalidateCacheEntry(target, request, entityProducer, fork, chain, asyncExecCallback, entry); + } + + }); + triggerResponse(cacheResponse, scope, asyncExecCallback); + } catch (final ResourceIOException ex) { + asyncExecCallback.failed(ex); + } + } else { + revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry); + } } else { log.debug("Cache entry not usable; calling backend"); callBackend(target, request, entityProducer, scope, chain, asyncExecCallback); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java index 945da46cf..7b38fa092 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java @@ -84,6 +84,15 @@ public BasicHttpAsyncCache(final ResourceFactory resourceFactory, final HttpAsyn this( resourceFactory, storage, CacheKeyGenerator.INSTANCE); } + @Override + public String generateKey(final HttpHost host, final HttpRequest request, final HttpCacheEntry cacheEntry) { + if (cacheEntry == null) { + return cacheKeyGenerator.generateKey(host, request); + } else { + return cacheKeyGenerator.generateKey(host, request, cacheEntry); + } + } + @Override public Cancellable flushCacheEntriesFor( final HttpHost host, final HttpRequest request, final FutureCallback callback) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java index 2cfdcb45b..248cbccfc 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java @@ -88,6 +88,15 @@ public BasicHttpCache() { this(CacheConfig.DEFAULT); } + @Override + public String generateKey(final HttpHost host, final HttpRequest request, final HttpCacheEntry cacheEntry) { + if (cacheEntry == null) { + return cacheKeyGenerator.generateKey(host, request); + } else { + return cacheKeyGenerator.generateKey(host, request, cacheEntry); + } + } + @Override public void flushCacheEntriesFor(final HttpHost host, final HttpRequest request) { if (log.isDebugEnabled()) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java index d63e62315..21c92fd42 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java @@ -90,15 +90,10 @@ *

Background validation. The cache module supports the * {@code stale-while-revalidate} directive of * RFC5861, which allows - * certain cache entry revalidations to happen in the background. You may - * want to tweak the settings for the {@link - * CacheConfig#getAsynchronousWorkersCore() minimum} and {@link - * CacheConfig#getAsynchronousWorkersMax() maximum} number of background - * worker threads, as well as the {@link - * CacheConfig#getAsynchronousWorkerIdleLifetimeSecs() maximum time they - * can be idle before being reclaimed}. You can also control the {@link - * CacheConfig#getRevalidationQueueSize() size of the queue} used for - * revalidations when there aren't enough workers to keep up with demand.

+ * certain cache entry revalidations to happen in the background. Asynchronous + * validation is enabled by default but it could be disabled by setting the number + * of re-validation workers to {@code 0} with {@link CacheConfig#getAsynchronousWorkers()} + * parameter

*/ public class CacheConfig implements Cloneable { @@ -142,21 +137,7 @@ public class CacheConfig implements Cloneable { /** Default number of worker threads to allow for background revalidations * resulting from the stale-while-revalidate directive. */ - public static final int DEFAULT_ASYNCHRONOUS_WORKERS_MAX = 1; - - /** Default minimum number of worker threads to allow for background - * revalidations resulting from the stale-while-revalidate directive. - */ - public static final int DEFAULT_ASYNCHRONOUS_WORKERS_CORE = 1; - - /** Default maximum idle lifetime for a background revalidation thread - * before it gets reclaimed. - */ - public static final int DEFAULT_ASYNCHRONOUS_WORKER_IDLE_LIFETIME_SECS = 60; - - /** Default maximum queue length for background revalidation requests. - */ - public static final int DEFAULT_REVALIDATION_QUEUE_SIZE = 100; + public static final int DEFAULT_ASYNCHRONOUS_WORKERS = 1; public static final CacheConfig DEFAULT = new Builder().build(); @@ -170,10 +151,7 @@ public class CacheConfig implements Cloneable { private final long heuristicDefaultLifetime; private final boolean sharedCache; private final boolean freshnessCheckEnabled; - private final int asynchronousWorkersMax; - private final int asynchronousWorkersCore; - private final int asynchronousWorkerIdleLifetimeSecs; - private final int revalidationQueueSize; + private final int asynchronousWorkers; private final boolean neverCacheHTTP10ResponsesWithQuery; CacheConfig( @@ -187,10 +165,7 @@ public class CacheConfig implements Cloneable { final long heuristicDefaultLifetime, final boolean sharedCache, final boolean freshnessCheckEnabled, - final int asynchronousWorkersMax, - final int asynchronousWorkersCore, - final int asynchronousWorkerIdleLifetimeSecs, - final int revalidationQueueSize, + final int asynchronousWorkers, final boolean neverCacheHTTP10ResponsesWithQuery) { super(); this.maxObjectSize = maxObjectSize; @@ -203,10 +178,7 @@ public class CacheConfig implements Cloneable { this.heuristicDefaultLifetime = heuristicDefaultLifetime; this.sharedCache = sharedCache; this.freshnessCheckEnabled = freshnessCheckEnabled; - this.asynchronousWorkersMax = asynchronousWorkersMax; - this.asynchronousWorkersCore = asynchronousWorkersCore; - this.asynchronousWorkerIdleLifetimeSecs = asynchronousWorkerIdleLifetimeSecs; - this.revalidationQueueSize = revalidationQueueSize; + this.asynchronousWorkers = asynchronousWorkers; this.neverCacheHTTP10ResponsesWithQuery = neverCacheHTTP10ResponsesWithQuery; } @@ -306,33 +278,8 @@ public boolean isFreshnessCheckEnabled() { * revalidations due to the {@code stale-while-revalidate} directive. A * value of 0 means background revalidations are disabled. */ - public int getAsynchronousWorkersMax() { - return asynchronousWorkersMax; - } - - /** - * Returns the minimum number of threads to keep alive for background - * revalidations due to the {@code stale-while-revalidate} directive. - */ - public int getAsynchronousWorkersCore() { - return asynchronousWorkersCore; - } - - /** - * Returns the current maximum idle lifetime in seconds for a - * background revalidation worker thread. If a worker thread is idle - * for this long, and there are more than the core number of worker - * threads alive, the worker will be reclaimed. - */ - public int getAsynchronousWorkerIdleLifetimeSecs() { - return asynchronousWorkerIdleLifetimeSecs; - } - - /** - * Returns the current maximum queue size for background revalidations. - */ - public int getRevalidationQueueSize() { - return revalidationQueueSize; + public int getAsynchronousWorkers() { + return asynchronousWorkers; } @Override @@ -354,10 +301,7 @@ public static Builder copy(final CacheConfig config) { .setHeuristicCoefficient(config.getHeuristicCoefficient()) .setHeuristicDefaultLifetime(config.getHeuristicDefaultLifetime()) .setSharedCache(config.isSharedCache()) - .setAsynchronousWorkersMax(config.getAsynchronousWorkersMax()) - .setAsynchronousWorkersCore(config.getAsynchronousWorkersCore()) - .setAsynchronousWorkerIdleLifetimeSecs(config.getAsynchronousWorkerIdleLifetimeSecs()) - .setRevalidationQueueSize(config.getRevalidationQueueSize()) + .setAsynchronousWorkers(config.getAsynchronousWorkers()) .setNeverCacheHTTP10ResponsesWithQueryString(config.isNeverCacheHTTP10ResponsesWithQuery()); } @@ -374,10 +318,7 @@ public static class Builder { private long heuristicDefaultLifetime; private boolean sharedCache; private boolean freshnessCheckEnabled; - private int asynchronousWorkersMax; - private int asynchronousWorkersCore; - private int asynchronousWorkerIdleLifetimeSecs; - private int revalidationQueueSize; + private int asynchronousWorkers; private boolean neverCacheHTTP10ResponsesWithQuery; Builder() { @@ -391,10 +332,7 @@ public static class Builder { this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME; this.sharedCache = true; this.freshnessCheckEnabled = true; - this.asynchronousWorkersMax = DEFAULT_ASYNCHRONOUS_WORKERS_MAX; - this.asynchronousWorkersCore = DEFAULT_ASYNCHRONOUS_WORKERS_CORE; - this.asynchronousWorkerIdleLifetimeSecs = DEFAULT_ASYNCHRONOUS_WORKER_IDLE_LIFETIME_SECS; - this.revalidationQueueSize = DEFAULT_REVALIDATION_QUEUE_SIZE; + this.asynchronousWorkers = DEFAULT_ASYNCHRONOUS_WORKERS; } /** @@ -495,42 +433,11 @@ public Builder setSharedCache(final boolean sharedCache) { /** * Sets the maximum number of threads to allow for background * revalidations due to the {@code stale-while-revalidate} directive. - * @param asynchronousWorkersMax number of threads; a value of 0 disables background + * @param asynchronousWorkers number of threads; a value of 0 disables background * revalidations. */ - public Builder setAsynchronousWorkersMax(final int asynchronousWorkersMax) { - this.asynchronousWorkersMax = asynchronousWorkersMax; - return this; - } - - /** - * Sets the minimum number of threads to keep alive for background - * revalidations due to the {@code stale-while-revalidate} directive. - * @param asynchronousWorkersCore should be greater than zero and less than or equal - * to {@code getAsynchronousWorkersMax()} - */ - public Builder setAsynchronousWorkersCore(final int asynchronousWorkersCore) { - this.asynchronousWorkersCore = asynchronousWorkersCore; - return this; - } - - /** - * Sets the current maximum idle lifetime in seconds for a - * background revalidation worker thread. If a worker thread is idle - * for this long, and there are more than the core number of worker - * threads alive, the worker will be reclaimed. - * @param asynchronousWorkerIdleLifetimeSecs idle lifetime in seconds - */ - public Builder setAsynchronousWorkerIdleLifetimeSecs(final int asynchronousWorkerIdleLifetimeSecs) { - this.asynchronousWorkerIdleLifetimeSecs = asynchronousWorkerIdleLifetimeSecs; - return this; - } - - /** - * Sets the current maximum queue size for background revalidations. - */ - public Builder setRevalidationQueueSize(final int revalidationQueueSize) { - this.revalidationQueueSize = revalidationQueueSize; + public Builder setAsynchronousWorkers(final int asynchronousWorkers) { + this.asynchronousWorkers = asynchronousWorkers; return this; } @@ -547,6 +454,11 @@ public Builder setNeverCacheHTTP10ResponsesWithQueryString( return this; } + public Builder setFreshnessCheckEnabled(final boolean freshnessCheckEnabled) { + this.freshnessCheckEnabled = freshnessCheckEnabled; + return this; + } + public CacheConfig build() { return new CacheConfig( maxObjectSize, @@ -559,10 +471,7 @@ public CacheConfig build() { heuristicDefaultLifetime, sharedCache, freshnessCheckEnabled, - asynchronousWorkersMax, - asynchronousWorkersCore, - asynchronousWorkerIdleLifetimeSecs, - revalidationQueueSize, + asynchronousWorkers, neverCacheHTTP10ResponsesWithQuery); } @@ -581,10 +490,7 @@ public String toString() { .append(", heuristicDefaultLifetime=").append(this.heuristicDefaultLifetime) .append(", sharedCache=").append(this.sharedCache) .append(", freshnessCheckEnabled=").append(this.freshnessCheckEnabled) - .append(", asynchronousWorkersMax=").append(this.asynchronousWorkersMax) - .append(", asynchronousWorkersCore=").append(this.asynchronousWorkersCore) - .append(", asynchronousWorkerIdleLifetimeSecs=").append(this.asynchronousWorkerIdleLifetimeSecs) - .append(", revalidationQueueSize=").append(this.revalidationQueueSize) + .append(", asynchronousWorkers=").append(this.asynchronousWorkers) .append(", neverCacheHTTP10ResponsesWithQuery=").append(this.neverCacheHTTP10ResponsesWithQuery) .append("]"); return builder.toString(); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheRevalidatorBase.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheRevalidatorBase.java index 25c0ecf99..be9d3af9f 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheRevalidatorBase.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheRevalidatorBase.java @@ -33,6 +33,7 @@ import java.util.Set; import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -62,7 +63,7 @@ interface ScheduledExecutor { } - public static ScheduledExecutor wrap(final ScheduledThreadPoolExecutor threadPoolExecutor) { + public static ScheduledExecutor wrap(final ScheduledExecutorService executorService) { return new ScheduledExecutor() { @@ -70,18 +71,18 @@ public static ScheduledExecutor wrap(final ScheduledThreadPoolExecutor threadPoo public ScheduledFuture schedule(final Runnable command, final TimeValue timeValue) throws RejectedExecutionException { Args.notNull(command, "Runnable"); Args.notNull(timeValue, "Time value"); - return threadPoolExecutor.schedule(command, timeValue.getDuration(), timeValue.getTimeUnit()); + return executorService.schedule(command, timeValue.getDuration(), timeValue.getTimeUnit()); } @Override public void shutdown() { - threadPoolExecutor.shutdown(); + executorService.shutdown(); } @Override public void awaitTermination(final Timeout timeout) throws InterruptedException { Args.notNull(timeout, "Timeout"); - threadPoolExecutor.awaitTermination(timeout.getDuration(), timeout.getTimeUnit()); + executorService.awaitTermination(timeout.getDuration(), timeout.getTimeUnit()); } }; diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java index 774c72698..a01e696c9 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java @@ -31,6 +31,7 @@ import java.util.Date; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.async.methods.SimpleBody; @@ -43,8 +44,10 @@ import org.apache.hc.client5.http.cache.ResourceIOException; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChainHandler; +import org.apache.hc.client5.http.impl.ExecSupport; import org.apache.hc.client5.http.impl.classic.ClassicRequestCopier; import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.http.schedule.SchedulingStrategy; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -101,27 +104,18 @@ public class CachingExec extends CachingExecBase implements ExecChainHandler { private final HttpCache responseCache; + private final DefaultCacheRevalidator cacheRevalidator; private final ConditionalRequestBuilder conditionalRequestBuilder; private final Logger log = LogManager.getLogger(getClass()); - public CachingExec(final HttpCache cache, final CacheConfig config) { + CachingExec(final HttpCache cache, final DefaultCacheRevalidator cacheRevalidator, final CacheConfig config) { super(config); this.responseCache = Args.notNull(cache, "Response cache"); + this.cacheRevalidator = cacheRevalidator; this.conditionalRequestBuilder = new ConditionalRequestBuilder<>(ClassicRequestCopier.INSTANCE); } - public CachingExec( - final ResourceFactory resourceFactory, - final HttpCacheStorage storage, - final CacheConfig config) { - this(new BasicHttpCache(resourceFactory, storage), config); - } - - public CachingExec() { - this(new BasicHttpCache(), CacheConfig.DEFAULT); - } - CachingExec( final HttpCache responseCache, final CacheValidityPolicy validityPolicy, @@ -129,16 +123,37 @@ public CachingExec() { final CachedHttpResponseGenerator responseGenerator, final CacheableRequestPolicy cacheableRequestPolicy, final CachedResponseSuitabilityChecker suitabilityChecker, - final ConditionalRequestBuilder conditionalRequestBuilder, final ResponseProtocolCompliance responseCompliance, final RequestProtocolCompliance requestCompliance, + final DefaultCacheRevalidator cacheRevalidator, + final ConditionalRequestBuilder conditionalRequestBuilder, final CacheConfig config) { super(validityPolicy, responseCachingPolicy, responseGenerator, cacheableRequestPolicy, suitabilityChecker, responseCompliance, requestCompliance, config); this.responseCache = responseCache; + this.cacheRevalidator = cacheRevalidator; this.conditionalRequestBuilder = conditionalRequestBuilder; } + public CachingExec( + final HttpCache cache, + final ScheduledExecutorService executorService, + final SchedulingStrategy schedulingStrategy, + final CacheConfig config) { + this(cache, + executorService != null ? new DefaultCacheRevalidator(executorService, schedulingStrategy) : null, + config); + } + + public CachingExec( + final ResourceFactory resourceFactory, + final HttpCacheStorage storage, + final ScheduledExecutorService executorService, + final SchedulingStrategy schedulingStrategy, + final CacheConfig config) { + this(new BasicHttpCache(resourceFactory, storage), executorService, schedulingStrategy, config); + } + @Override public ClassicHttpResponse execute( final ClassicHttpRequest request, @@ -167,7 +182,7 @@ public ClassicHttpResponse execute( final SimpleHttpResponse fatalErrorResponse = getFatallyNoncompliantResponse(request, context); if (fatalErrorResponse != null) { - return convert(fatalErrorResponse); + return convert(fatalErrorResponse, scope); } requestCompliance.makeRequestCompliant(request); @@ -188,7 +203,7 @@ public ClassicHttpResponse execute( } } - private static ClassicHttpResponse convert(final SimpleHttpResponse cacheResponse) { + private static ClassicHttpResponse convert(final SimpleHttpResponse cacheResponse, final ExecChain.Scope scope) { if (cacheResponse == null) { return null; } @@ -205,6 +220,7 @@ private static ClassicHttpResponse convert(final SimpleHttpResponse cacheRespons response.setEntity(new ByteArrayEntity(body.getBodyBytes(), body.getContentType())); } } + scope.clientContext.setAttribute(HttpCoreContext.HTTP_RESPONSE, response); return response; } @@ -240,15 +256,11 @@ private ClassicHttpResponse handleCacheHit( if (suitabilityChecker.canCachedResponseBeUsed(target, request, entry, now)) { log.debug("Cache hit"); try { - final ClassicHttpResponse response = convert(generateCachedResponse(request, context, entry, now)); - context.setAttribute(HttpCoreContext.HTTP_RESPONSE, response); - return response; + return convert(generateCachedResponse(request, context, entry, now), scope); } catch (final ResourceIOException ex) { recordCacheFailure(target, request); if (!mayCallBackend(request)) { - final ClassicHttpResponse response = convert(generateGatewayTimeout(context)); - context.setAttribute(HttpCoreContext.HTTP_RESPONSE, response); - return response; + return convert(generateGatewayTimeout(context), scope); } else { setResponseStatus(scope.clientContext, CacheResponseStatus.FAILURE); return chain.proceed(request, scope); @@ -256,15 +268,38 @@ private ClassicHttpResponse handleCacheHit( } } else if (!mayCallBackend(request)) { log.debug("Cache entry not suitable but only-if-cached requested"); - final ClassicHttpResponse response = convert(generateGatewayTimeout(context)); - context.setAttribute(HttpCoreContext.HTTP_RESPONSE, response); - return response; + return convert(generateGatewayTimeout(context), scope); } else if (!(entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { log.debug("Revalidating cache entry"); try { - return revalidateCacheEntry(target, request, scope, chain, entry); + if (cacheRevalidator != null + && !staleResponseNotAllowed(request, entry, now) + && validityPolicy.mayReturnStaleWhileRevalidating(entry, now)) { + log.debug("Serving stale with asynchronous revalidation"); + final String exchangeId = ExecSupport.getNextExchangeId(); + final ExecChain.Scope fork = new ExecChain.Scope( + exchangeId, + scope.route, + scope.originalRequest, + scope.execRuntime.fork(null), + HttpClientContext.create()); + final SimpleHttpResponse response = generateCachedResponse(request, context, entry, now); + cacheRevalidator.revalidateCacheEntry( + responseCache.generateKey(target, request, entry), + new DefaultCacheRevalidator.RevalidationCall() { + + @Override + public ClassicHttpResponse execute() throws HttpException, IOException { + return revalidateCacheEntry(target, request, fork, chain, entry); + } + + }); + return convert(response, scope); + } else { + return revalidateCacheEntry(target, request, scope, chain, entry); + } } catch (final IOException ioex) { - return convert(handleRevalidationFailure(request, context, entry, now)); + return convert(handleRevalidationFailure(request, context, entry, now), scope); } } else { log.debug("Cache entry not usable; calling backend"); @@ -307,9 +342,9 @@ ClassicHttpResponse revalidateCacheEntry( target, request, cacheEntry, backendResponse, requestDate, responseDate); if (suitabilityChecker.isConditional(request) && suitabilityChecker.allConditionalsMatch(request, updatedEntry, new Date())) { - return convert(responseGenerator.generateNotModifiedResponse(updatedEntry)); + return convert(responseGenerator.generateNotModifiedResponse(updatedEntry), scope); } - return convert(responseGenerator.generateResponse(request, updatedEntry)); + return convert(responseGenerator.generateResponse(request, updatedEntry), scope); } if (staleIfErrorAppliesTo(statusCode) @@ -318,7 +353,7 @@ ClassicHttpResponse revalidateCacheEntry( try { final SimpleHttpResponse cachedResponse = responseGenerator.generateResponse(request, cacheEntry); cachedResponse.addHeader(HeaderConstants.WARNING, "110 localhost \"Response is stale\""); - return convert(cachedResponse); + return convert(cachedResponse, scope); } finally { backendResponse.close(); } @@ -344,7 +379,7 @@ ClassicHttpResponse handleBackendResponse( final boolean cacheable = responseCachingPolicy.isResponseCacheable(request, backendResponse); if (cacheable) { storeRequestIfModifiedSinceFor304Response(request, backendResponse); - return cacheAndReturnResponse(target, request, backendResponse, requestDate, responseDate); + return cacheAndReturnResponse(target, request, backendResponse, scope, requestDate, responseDate); } else { log.debug("Backend response is not cacheable"); responseCache.flushCacheEntriesFor(target, request); @@ -356,6 +391,7 @@ ClassicHttpResponse cacheAndReturnResponse( final HttpHost target, final HttpRequest request, final ClassicHttpResponse backendResponse, + final ExecChain.Scope scope, final Date requestSent, final Date responseReceived) throws IOException { log.debug("Caching backend response"); @@ -395,7 +431,7 @@ ClassicHttpResponse cacheAndReturnResponse( cacheEntry = responseCache.createCacheEntry(target, request, backendResponse, buf, requestSent, responseReceived); log.debug("Backend response successfully cached (freshness check skipped)"); } - return convert(responseGenerator.generateResponse(request, cacheEntry)); + return convert(responseGenerator.generateResponse(request, cacheEntry), scope); } private ClassicHttpResponse handleCacheMiss( @@ -423,8 +459,7 @@ ClassicHttpResponse negotiateResponseFromVariants( final ExecChain.Scope scope, final ExecChain chain, final Map variants) throws IOException, HttpException { - final ClassicHttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants( - request, variants); + final ClassicHttpRequest conditionalRequest = conditionalRequestBuilder.buildConditionalRequestFromVariants(request, variants); final Date requestDate = getCurrentDate(); final ClassicHttpResponse backendResponse = chain.proceed(conditionalRequest, scope); @@ -468,11 +503,11 @@ ClassicHttpResponse negotiateResponseFromVariants( target, conditionalRequest, backendResponse, matchingVariant, requestDate, responseDate); backendResponse.close(); if (shouldSendNotModifiedResponse(request, responseEntry)) { - return convert(responseGenerator.generateNotModifiedResponse(responseEntry)); + return convert(responseGenerator.generateNotModifiedResponse(responseEntry), scope); } - final SimpleHttpResponse resp = responseGenerator.generateResponse(request, responseEntry); + final SimpleHttpResponse response = responseGenerator.generateResponse(request, responseEntry); responseCache.reuseVariantEntryFor(target, request, matchingVariant); - return convert(resp); + return convert(response, scope); } catch (final IOException | RuntimeException ex) { backendResponse.close(); throw ex; diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttp2AsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttp2AsyncClientBuilder.java index 83b69350c..927d79cf2 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttp2AsyncClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttp2AsyncClientBuilder.java @@ -29,6 +29,8 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.apache.hc.client5.http.async.AsyncExecChainHandler; import org.apache.hc.client5.http.cache.HttpAsyncCacheInvalidator; @@ -38,6 +40,8 @@ import org.apache.hc.client5.http.cache.ResourceFactory; import org.apache.hc.client5.http.impl.ChainElements; import org.apache.hc.client5.http.impl.async.Http2AsyncClientBuilder; +import org.apache.hc.client5.http.impl.schedule.ImmediateSchedulingStrategy; +import org.apache.hc.client5.http.schedule.SchedulingStrategy; import org.apache.hc.core5.http.config.NamedElementChain; /** @@ -51,6 +55,7 @@ public class CachingHttp2AsyncClientBuilder extends Http2AsyncClientBuilder { private ResourceFactory resourceFactory; private HttpAsyncCacheStorage storage; private File cacheDir; + private SchedulingStrategy schedulingStrategy; private CacheConfig cacheConfig; private HttpAsyncCacheInvalidator httpCacheInvalidator; private boolean deleteCache; @@ -84,6 +89,11 @@ public final CachingHttp2AsyncClientBuilder setCacheDir(final File cacheDir) { return this; } + public final CachingHttp2AsyncClientBuilder setSchedulingStrategy(final SchedulingStrategy schedulingStrategy) { + this.schedulingStrategy = schedulingStrategy; + return this; + } + public final CachingHttp2AsyncClientBuilder setCacheConfig(final CacheConfig cacheConfig) { this.cacheConfig = cacheConfig; return this; @@ -138,7 +148,26 @@ public void close() throws IOException { CacheKeyGenerator.INSTANCE, this.httpCacheInvalidator != null ? this.httpCacheInvalidator : new DefaultAsyncCacheInvalidator()); - final AsyncCachingExec cachingExec = new AsyncCachingExec(httpCache, config); + DefaultAsyncCacheRevalidator cacheRevalidator = null; + if (config.getAsynchronousWorkers() > 0) { + final ScheduledExecutorService executorService = new ScheduledThreadPoolExecutor(config.getAsynchronousWorkers()); + addCloseable(new Closeable() { + + @Override + public void close() throws IOException { + executorService.shutdownNow(); + } + + }); + cacheRevalidator = new DefaultAsyncCacheRevalidator( + executorService, + this.schedulingStrategy != null ? this.schedulingStrategy : ImmediateSchedulingStrategy.INSTANCE); + } + + final AsyncCachingExec cachingExec = new AsyncCachingExec( + httpCache, + cacheRevalidator, + config); execChainDefinition.addBefore(ChainElements.PROTOCOL.name(), cachingExec, ChainElements.CACHING.name()); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java index f44a04c74..7295347de 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java @@ -29,6 +29,8 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.apache.hc.client5.http.async.AsyncExecChainHandler; import org.apache.hc.client5.http.cache.HttpAsyncCacheInvalidator; @@ -38,6 +40,8 @@ import org.apache.hc.client5.http.cache.ResourceFactory; import org.apache.hc.client5.http.impl.ChainElements; import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder; +import org.apache.hc.client5.http.impl.schedule.ImmediateSchedulingStrategy; +import org.apache.hc.client5.http.schedule.SchedulingStrategy; import org.apache.hc.core5.http.config.NamedElementChain; /** @@ -51,6 +55,7 @@ public class CachingHttpAsyncClientBuilder extends HttpAsyncClientBuilder { private ResourceFactory resourceFactory; private HttpAsyncCacheStorage storage; private File cacheDir; + private SchedulingStrategy schedulingStrategy; private CacheConfig cacheConfig; private HttpAsyncCacheInvalidator httpCacheInvalidator; private boolean deleteCache; @@ -84,6 +89,11 @@ public final CachingHttpAsyncClientBuilder setCacheDir(final File cacheDir) { return this; } + public final CachingHttpAsyncClientBuilder setSchedulingStrategy(final SchedulingStrategy schedulingStrategy) { + this.schedulingStrategy = schedulingStrategy; + return this; + } + public final CachingHttpAsyncClientBuilder setCacheConfig(final CacheConfig cacheConfig) { this.cacheConfig = cacheConfig; return this; @@ -138,7 +148,26 @@ public void close() throws IOException { CacheKeyGenerator.INSTANCE, this.httpCacheInvalidator != null ? this.httpCacheInvalidator : new DefaultAsyncCacheInvalidator()); - final AsyncCachingExec cachingExec = new AsyncCachingExec(httpCache, config); + DefaultAsyncCacheRevalidator cacheRevalidator = null; + if (config.getAsynchronousWorkers() > 0) { + final ScheduledExecutorService executorService = new ScheduledThreadPoolExecutor(config.getAsynchronousWorkers()); + addCloseable(new Closeable() { + + @Override + public void close() throws IOException { + executorService.shutdownNow(); + } + + }); + cacheRevalidator = new DefaultAsyncCacheRevalidator( + executorService, + this.schedulingStrategy != null ? this.schedulingStrategy : ImmediateSchedulingStrategy.INSTANCE); + } + + final AsyncCachingExec cachingExec = new AsyncCachingExec( + httpCache, + cacheRevalidator, + config); execChainDefinition.addBefore(ChainElements.PROTOCOL.name(), cachingExec, ChainElements.CACHING.name()); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java index 98364794c..714b34ca6 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java @@ -29,6 +29,8 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import org.apache.hc.client5.http.cache.HttpCacheInvalidator; import org.apache.hc.client5.http.cache.HttpCacheStorage; @@ -36,6 +38,8 @@ import org.apache.hc.client5.http.classic.ExecChainHandler; import org.apache.hc.client5.http.impl.ChainElements; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.client5.http.impl.schedule.ImmediateSchedulingStrategy; +import org.apache.hc.client5.http.schedule.SchedulingStrategy; import org.apache.hc.core5.http.config.NamedElementChain; /** @@ -49,6 +53,7 @@ public class CachingHttpClientBuilder extends HttpClientBuilder { private ResourceFactory resourceFactory; private HttpCacheStorage storage; private File cacheDir; + private SchedulingStrategy schedulingStrategy; private CacheConfig cacheConfig; private HttpCacheInvalidator httpCacheInvalidator; private boolean deleteCache; @@ -68,31 +73,32 @@ public final CachingHttpClientBuilder setResourceFactory( return this; } - public final CachingHttpClientBuilder setHttpCacheStorage( - final HttpCacheStorage storage) { + public final CachingHttpClientBuilder setHttpCacheStorage(final HttpCacheStorage storage) { this.storage = storage; return this; } - public final CachingHttpClientBuilder setCacheDir( - final File cacheDir) { + public final CachingHttpClientBuilder setCacheDir(final File cacheDir) { this.cacheDir = cacheDir; return this; } - public final CachingHttpClientBuilder setCacheConfig( - final CacheConfig cacheConfig) { + public final CachingHttpClientBuilder setSchedulingStrategy(final SchedulingStrategy schedulingStrategy) { + this.schedulingStrategy = schedulingStrategy; + return this; + } + + public final CachingHttpClientBuilder setCacheConfig(final CacheConfig cacheConfig) { this.cacheConfig = cacheConfig; return this; } - public final CachingHttpClientBuilder setHttpCacheInvalidator( - final HttpCacheInvalidator cacheInvalidator) { + public final CachingHttpClientBuilder setHttpCacheInvalidator(final HttpCacheInvalidator cacheInvalidator) { this.httpCacheInvalidator = cacheInvalidator; return this; } - public CachingHttpClientBuilder setDeleteCache(final boolean deleteCache) { + public final CachingHttpClientBuilder setDeleteCache(final boolean deleteCache) { this.deleteCache = deleteCache; return this; } @@ -136,7 +142,25 @@ public void close() throws IOException { CacheKeyGenerator.INSTANCE, this.httpCacheInvalidator != null ? this.httpCacheInvalidator : new DefaultCacheInvalidator()); - final CachingExec cachingExec = new CachingExec(httpCache, config); + DefaultCacheRevalidator cacheRevalidator = null; + if (config.getAsynchronousWorkers() > 0) { + final ScheduledExecutorService executorService = new ScheduledThreadPoolExecutor(config.getAsynchronousWorkers()); + addCloseable(new Closeable() { + + @Override + public void close() throws IOException { + executorService.shutdownNow(); + } + + }); + cacheRevalidator = new DefaultCacheRevalidator( + executorService, + this.schedulingStrategy != null ? this.schedulingStrategy : ImmediateSchedulingStrategy.INSTANCE); + } + final CachingExec cachingExec = new CachingExec( + httpCache, + cacheRevalidator, + config); execChainDefinition.addBefore(ChainElements.PROTOCOL.name(), cachingExec, ChainElements.CACHING.name()); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultAsyncCacheRevalidator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultAsyncCacheRevalidator.java index 632412402..fd247419d 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultAsyncCacheRevalidator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultAsyncCacheRevalidator.java @@ -26,20 +26,20 @@ */ package org.apache.hc.client5.http.impl.cache; -import java.util.concurrent.ExecutionException; +import java.io.IOException; import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hc.client5.http.async.AsyncExecCallback; -import org.apache.hc.client5.http.async.AsyncExecChain; -import org.apache.hc.client5.http.cache.HttpCacheEntry; +import org.apache.hc.client5.http.impl.Operations; import org.apache.hc.client5.http.schedule.SchedulingStrategy; -import org.apache.hc.core5.http.HttpHost; -import org.apache.hc.core5.http.HttpRequest; -import org.apache.hc.core5.http.nio.AsyncEntityProducer; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.nio.AsyncDataConsumer; import org.apache.hc.core5.util.TimeValue; import org.apache.hc.core5.util.Timeout; @@ -49,33 +49,10 @@ */ class DefaultAsyncCacheRevalidator extends CacheRevalidatorBase { - private static final Future NOOP_FUTURE = new Future() { + interface RevalidationCall { - @Override - public Void get() throws InterruptedException, ExecutionException { - return null; - } - - @Override - public Void get(final long timeout, final TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { - return null; - } - @Override - public boolean cancel(final boolean mayInterruptIfRunning) { - return false; - } - - @Override - public boolean isCancelled() { - return false; - } - - @Override - public boolean isDone() { - return true; - } - - }; + void execute(AsyncExecCallback asyncExecCallback);; + } static class InternalScheduledExecutor implements ScheduledExecutor { @@ -89,7 +66,7 @@ static class InternalScheduledExecutor implements ScheduledExecutor { public Future schedule(final Runnable command, final TimeValue timeValue) throws RejectedExecutionException { if (timeValue.toMillis() <= 0) { command.run(); - return NOOP_FUTURE; + return new Operations.CompletedFuture(null); } else { return executor.schedule(command, timeValue); } @@ -107,7 +84,6 @@ public void awaitTermination(final Timeout timeout) throws InterruptedException } - private final AsyncCachingExec cachingExec; private final CacheKeyGenerator cacheKeyGenerator; /** @@ -116,42 +92,72 @@ public void awaitTermination(final Timeout timeout) throws InterruptedException */ public DefaultAsyncCacheRevalidator( final ScheduledExecutor scheduledExecutor, - final SchedulingStrategy schedulingStrategy, - final AsyncCachingExec cachingExec) { + final SchedulingStrategy schedulingStrategy) { super(new InternalScheduledExecutor(scheduledExecutor), schedulingStrategy); - this.cachingExec = cachingExec; this.cacheKeyGenerator = CacheKeyGenerator.INSTANCE; } /** * Create CacheValidator which will make ache revalidation requests - * using the supplied {@link SchedulingStrategy} and {@link ScheduledThreadPoolExecutor}. + * using the supplied {@link SchedulingStrategy} and {@link ScheduledExecutorService}. */ public DefaultAsyncCacheRevalidator( - final ScheduledThreadPoolExecutor scheduledThreadPoolExecutor, - final SchedulingStrategy schedulingStrategy, - final AsyncCachingExec cachingExec) { - this(wrap(scheduledThreadPoolExecutor), schedulingStrategy, cachingExec); + final ScheduledExecutorService executorService, + final SchedulingStrategy schedulingStrategy) { + this(wrap(executorService), schedulingStrategy); } /** * Schedules an asynchronous re-validation */ public void revalidateCacheEntry( - final HttpHost target, - final HttpRequest request, - final AsyncEntityProducer entityProducer, - final AsyncExecChain.Scope scope, - final AsyncExecChain chain, + final String cacheKey , final AsyncExecCallback asyncExecCallback, - final HttpCacheEntry entry) { - final String cacheKey = cacheKeyGenerator.generateKey(target, request, entry); + final RevalidationCall call) { scheduleRevalidation(cacheKey, new Runnable() { @Override public void run() { - cachingExec.revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry); + call.execute(new AsyncExecCallback() { + + private final AtomicReference responseRef = new AtomicReference<>(null); + + @Override + public AsyncDataConsumer handleResponse( + final HttpResponse response, final EntityDetails entityDetails) throws HttpException, IOException { + responseRef.set(response); + return asyncExecCallback.handleResponse(response, entityDetails); + } + + @Override + public void completed() { + final HttpResponse httpResponse = responseRef.getAndSet(null); + if (httpResponse != null && httpResponse.getCode() < HttpStatus.SC_SERVER_ERROR && !isStale(httpResponse)) { + jobSuccessful(cacheKey); + } else { + jobFailed(cacheKey); + } + asyncExecCallback.completed(); + } + + @Override + public void failed(final Exception cause) { + if (cause instanceof IOException) { + log.debug("Asynchronous revalidation failed due to I/O error", cause); + } else if (cause instanceof HttpException) { + log.error("HTTP protocol exception during asynchronous revalidation", cause); + } else { + log.error("Unexpected runtime exception thrown during asynchronous revalidation", cause); + } + try { + jobFailed(cacheKey); + } finally { + asyncExecCallback.failed(cause); + } + } + + }); } }); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheRevalidator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheRevalidator.java index f033b86aa..8e343b35b 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheRevalidator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheRevalidator.java @@ -27,15 +27,11 @@ package org.apache.hc.client5.http.impl.cache; import java.io.IOException; -import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ScheduledExecutorService; -import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.schedule.SchedulingStrategy; -import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpException; -import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; /** @@ -44,8 +40,10 @@ */ class DefaultCacheRevalidator extends CacheRevalidatorBase { - private final CachingExec cachingExec; - private final CacheKeyGenerator cacheKeyGenerator; + interface RevalidationCall { + + ClassicHttpResponse execute() throws IOException, HttpException; + } /** * Create DefaultCacheRevalidator which will make ache revalidation requests @@ -53,56 +51,45 @@ class DefaultCacheRevalidator extends CacheRevalidatorBase { */ public DefaultCacheRevalidator( final CacheRevalidatorBase.ScheduledExecutor scheduledExecutor, - final SchedulingStrategy schedulingStrategy, - final CachingExec cachingExec) { + final SchedulingStrategy schedulingStrategy) { super(scheduledExecutor, schedulingStrategy); - this.cachingExec = cachingExec; - this.cacheKeyGenerator = CacheKeyGenerator.INSTANCE; - } /** * Create CacheValidator which will make ache revalidation requests - * using the supplied {@link SchedulingStrategy} and {@link ScheduledThreadPoolExecutor}. + * using the supplied {@link SchedulingStrategy} and {@link ScheduledExecutorService}. */ public DefaultCacheRevalidator( - final ScheduledThreadPoolExecutor scheduledThreadPoolExecutor, - final SchedulingStrategy schedulingStrategy, - final CachingExec cachingExec) { - this(wrap(scheduledThreadPoolExecutor), schedulingStrategy, cachingExec); + final ScheduledExecutorService scheduledThreadPoolExecutor, + final SchedulingStrategy schedulingStrategy) { + this(wrap(scheduledThreadPoolExecutor), schedulingStrategy); } /** * Schedules an asynchronous re-validation */ public void revalidateCacheEntry( - final HttpHost target, - final ClassicHttpRequest request, - final ExecChain.Scope scope, - final ExecChain chain, - final HttpCacheEntry entry) { - final String cacheKey = cacheKeyGenerator.generateKey(target, request, entry); + final String cacheKey, + final RevalidationCall call) { scheduleRevalidation(cacheKey, new Runnable() { @Override public void run() { - try { - try (ClassicHttpResponse httpResponse = cachingExec.revalidateCacheEntry(target, request, scope, chain, entry)) { - if (httpResponse.getCode() < HttpStatus.SC_SERVER_ERROR && !isStale(httpResponse)) { - jobSuccessful(cacheKey); - } else { - jobFailed(cacheKey); - } + try (ClassicHttpResponse httpResponse = call.execute()) { + if (httpResponse.getCode() < HttpStatus.SC_SERVER_ERROR && !isStale(httpResponse)) { + jobSuccessful(cacheKey); + } else { + jobFailed(cacheKey); } - } catch (final IOException ioe) { + } catch (final IOException ex) { jobFailed(cacheKey); - log.debug("Asynchronous revalidation failed due to I/O error", ioe); - } catch (final HttpException pe) { + log.debug("Asynchronous revalidation failed due to I/O error", ex); + } catch (final HttpException ex) { jobFailed(cacheKey); - log.error("HTTP protocol exception during asynchronous revalidation", pe); - } catch (final RuntimeException re) { + log.error("HTTP protocol exception during asynchronous revalidation", ex); + } catch (final RuntimeException ex) { jobFailed(cacheKey); - log.error("Unexpected runtime exception thrown during asynchronous revalidation" + re); + log.error("Unexpected runtime exception thrown during asynchronous revalidation", ex); } } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java index 8f1bd0ec6..dc4b2e6f2 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpAsyncCache.java @@ -44,6 +44,8 @@ @Internal interface HttpAsyncCache { + String generateKey (HttpHost host, HttpRequest request, HttpCacheEntry cacheEntry); + /** * Clear all matching {@link HttpCacheEntry}s. */ diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java index 8cb0e07a3..d2c6e6613 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCache.java @@ -40,6 +40,8 @@ */ interface HttpCache { + String generateKey (HttpHost host, HttpRequest request, HttpCacheEntry cacheEntry); + /** * Clear all matching {@link HttpCacheEntry}s. */ diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/schedule/ImmediateSchedulingStrategy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/schedule/ImmediateSchedulingStrategy.java index bb498f659..362b1ad72 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/schedule/ImmediateSchedulingStrategy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/schedule/ImmediateSchedulingStrategy.java @@ -39,7 +39,7 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class ImmediateSchedulingStrategy implements SchedulingStrategy { - private final static ImmediateSchedulingStrategy INSTANCE = new ImmediateSchedulingStrategy(); + public final static ImmediateSchedulingStrategy INSTANCE = new ImmediateSchedulingStrategy(); @Override public TimeValue schedule(final int attemptNumber) { diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/AbstractProtocolTest.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/AbstractProtocolTest.java index 1d433fe46..1adf72e63 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/AbstractProtocolTest.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/AbstractProtocolTest.java @@ -57,7 +57,7 @@ public abstract class AbstractProtocolTest { protected HttpEntity body; protected HttpClientContext context; protected ExecChain mockExecChain; - protected ExecRuntime mockEndpoint; + protected ExecRuntime mockExecRuntime; protected HttpCache mockCache; protected ClassicHttpRequest request; protected ClassicHttpResponse originResponse; @@ -101,18 +101,18 @@ public void setUp() { cache = new BasicHttpCache(config); mockExecChain = EasyMock.createNiceMock(ExecChain.class); - mockEndpoint = EasyMock.createNiceMock(ExecRuntime.class); + mockExecRuntime = EasyMock.createNiceMock(ExecRuntime.class); mockCache = EasyMock.createNiceMock(HttpCache.class); impl = createCachingExecChain(cache, config); } public ClassicHttpResponse execute(final ClassicHttpRequest request) throws IOException, HttpException { return impl.execute(ClassicRequestCopier.INSTANCE.copy(request), new ExecChain.Scope( - "test", route, request, mockEndpoint, context), mockExecChain); + "test", route, request, mockExecRuntime, context), mockExecChain); } protected ExecChainHandler createCachingExecChain(final HttpCache cache, final CacheConfig config) { - return new CachingExec(cache, config); + return new CachingExec(cache, null, config); } protected boolean supportsRangeAndContentRangeHeaders(final ExecChainHandler impl) { @@ -148,7 +148,7 @@ protected void emptyMockCacheExpectsNoPuts() throws Exception { mockExecChain = EasyMock.createNiceMock(ExecChain.class); mockCache = EasyMock.createNiceMock(HttpCache.class); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); EasyMock.expect(mockCache.getCacheEntry(EasyMock.isA(HttpHost.class), EasyMock.isA(HttpRequest.class))) .andReturn(null).anyTimes(); @@ -174,7 +174,7 @@ protected void behaveAsNonSharedCache() { .setMaxObjectSize(MAX_BYTES) .setSharedCache(false) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, null, config); } public AbstractProtocolTest() { diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java index 446512a24..95969a3bc 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExec.java @@ -65,7 +65,6 @@ import org.easymock.IExpectationSetters; import org.junit.Assert; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; @SuppressWarnings("boxing") // test code @@ -107,9 +106,10 @@ public CachingExec createCachingExecChain( final CachedHttpResponseGenerator mockResponseGenerator, final CacheableRequestPolicy mockRequestPolicy, final CachedResponseSuitabilityChecker mockSuitabilityChecker, - final ConditionalRequestBuilder mockConditionalRequestBuilder, final ResponseProtocolCompliance mockResponseProtocolCompliance, final RequestProtocolCompliance mockRequestProtocolCompliance, + final DefaultCacheRevalidator mockCacheRevalidator, + final ConditionalRequestBuilder mockConditionalRequestBuilder, final CacheConfig config) { return impl = new CachingExec( mockCache, @@ -118,15 +118,16 @@ public CachingExec createCachingExecChain( mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, - mockConditionalRequestBuilder, mockResponseProtocolCompliance, mockRequestProtocolCompliance, + mockCacheRevalidator, + mockConditionalRequestBuilder, config); } @Override public CachingExec createCachingExecChain(final HttpCache cache, final CacheConfig config) { - return impl = new CachingExec(cache, config); + return impl = new CachingExec(cache, null, config); } @Override @@ -210,7 +211,7 @@ public void testUnsuitableUnvalidatableCacheEntryCausesBackendRequest() throws E Assert.assertEquals(1, impl.getCacheUpdates()); } - @Test @Ignore + @Test public void testUnsuitableValidatableCacheEntryCausesRevalidation() throws Exception { mockImplMethods(REVALIDATE_CACHE_ENTRY); requestPolicyAllowsCaching(true); @@ -447,9 +448,10 @@ private void mockImplMethods(final String... methods) { mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, - mockConditionalRequestBuilder, mockResponseProtocolCompliance, mockRequestProtocolCompliance, + mockCacheRevalidator, + mockConditionalRequestBuilder, config).addMockedMethods(methods).createNiceMock(); } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java index 32375332b..b84d91ef2 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java @@ -104,10 +104,11 @@ public abstract class TestCachingExecChain { protected CachedHttpResponseGenerator mockResponseGenerator; private HttpClientResponseHandler mockHandler; private ClassicHttpRequest mockUriRequest; - protected ConditionalRequestBuilder mockConditionalRequestBuilder; private HttpRequest mockConditionalRequest; protected ResponseProtocolCompliance mockResponseProtocolCompliance; protected RequestProtocolCompliance mockRequestProtocolCompliance; + protected DefaultCacheRevalidator mockCacheRevalidator; + protected ConditionalRequestBuilder mockConditionalRequestBuilder; protected CacheConfig config; protected HttpRoute route; @@ -130,10 +131,11 @@ public void setUp() { mockUriRequest = createNiceMock(ClassicHttpRequest.class); mockCacheEntry = createNiceMock(HttpCacheEntry.class); mockResponseGenerator = createNiceMock(CachedHttpResponseGenerator.class); - mockConditionalRequestBuilder = createNiceMock(ConditionalRequestBuilder.class); mockConditionalRequest = createNiceMock(HttpRequest.class); mockResponseProtocolCompliance = createNiceMock(ResponseProtocolCompliance.class); mockRequestProtocolCompliance = createNiceMock(RequestProtocolCompliance.class); + mockCacheRevalidator = createNiceMock(DefaultCacheRevalidator.class); + mockConditionalRequestBuilder = createNiceMock(ConditionalRequestBuilder.class); mockStorage = createNiceMock(HttpCacheStorage.class); config = CacheConfig.DEFAULT; @@ -143,9 +145,9 @@ public void setUp() { context = HttpCacheContext.create(); entry = HttpTestUtils.makeCacheEntry(); impl = createCachingExecChain(mockCache, mockValidityPolicy, - mockResponsePolicy, mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, - mockConditionalRequestBuilder, mockResponseProtocolCompliance, - mockRequestProtocolCompliance, config); + mockResponsePolicy, mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, + mockResponseProtocolCompliance,mockRequestProtocolCompliance, + mockCacheRevalidator, mockConditionalRequestBuilder, config); } public abstract CachingExec createCachingExecChain( @@ -153,8 +155,9 @@ public abstract CachingExec createCachingExecChain( ResponseCachingPolicy responseCachingPolicy, CachedHttpResponseGenerator responseGenerator, CacheableRequestPolicy cacheableRequestPolicy, CachedResponseSuitabilityChecker suitabilityChecker, - ConditionalRequestBuilder conditionalRequestBuilder, ResponseProtocolCompliance responseCompliance, RequestProtocolCompliance requestCompliance, + DefaultCacheRevalidator cacheRevalidator, + ConditionalRequestBuilder conditionalRequestBuilder, CacheConfig config); public abstract CachingExec createCachingExecChain(HttpCache cache, CacheConfig config); @@ -1242,11 +1245,11 @@ public void testTooLargeResponsesAreNotCached() throws Exception { mockCache = EasyMock.createStrictMock(HttpCache.class); impl = createCachingExecChain(mockCache, mockValidityPolicy, mockResponsePolicy, mockResponseGenerator, mockRequestPolicy, mockSuitabilityChecker, - mockConditionalRequestBuilder, mockResponseProtocolCompliance, - mockRequestProtocolCompliance, config); + mockResponseProtocolCompliance, mockRequestProtocolCompliance, + mockCacheRevalidator, mockConditionalRequestBuilder, config); final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + final ClassicHttpRequest request = new HttpGet("http://foo.example.com/bar"); final Date now = new Date(); final Date requestSent = new Date(now.getTime() - 3 * 1000L); @@ -1260,8 +1263,8 @@ public void testTooLargeResponsesAreNotCached() throws Exception { originResponse.setHeader("ETag", "\"etag\""); replayMocks(); - - impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, mockEndpoint, context); + impl.cacheAndReturnResponse(host, request, originResponse, scope, requestSent, responseReceived); verifyMocks(); } @@ -1269,7 +1272,7 @@ public void testTooLargeResponsesAreNotCached() throws Exception { @Test public void testSmallEnoughResponsesAreCached() throws Exception { final HttpHost host = new HttpHost("foo.example.com"); - final HttpRequest request = new HttpGet("http://foo.example.com/bar"); + final ClassicHttpRequest request = new HttpGet("http://foo.example.com/bar"); final Date now = new Date(); final Date requestSent = new Date(now.getTime() - 3 * 1000L); @@ -1297,7 +1300,8 @@ public void testSmallEnoughResponsesAreCached() throws Exception { same(httpCacheEntry))).andReturn(response).once(); replayMocks(); - impl.cacheAndReturnResponse(host, request, originResponse, requestSent, responseReceived); + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, mockEndpoint, context); + impl.cacheAndReturnResponse(host, request, originResponse, scope, requestSent, responseReceived); verifyMocks(); } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingHttpClientBuilder.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingHttpClientBuilder.java deleted file mode 100644 index ade397322..000000000 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingHttpClientBuilder.java +++ /dev/null @@ -1,48 +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 - * . - * - */ -package org.apache.hc.client5.http.impl.cache; - -import org.junit.Test; - -public class TestCachingHttpClientBuilder { - - @Test - public void testAsynchronousWorkersMax0() throws Exception { - final CacheConfig cacheConfig = CacheConfig.custom() - .setAsynchronousWorkersMax(0) - .build(); - // Asynchronous validation should be disabled but we should not get an - // Exception - CachingHttpClientBuilder.create().setCacheConfig(cacheConfig).build(); - } - - @Test - public void testNullCacheConfig() throws Exception { - CachingHttpClientBuilder.create().setCacheConfig(null).build(); - } - -} diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java index f66565438..a07b09c48 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestHttpCacheJiraNumber1147.java @@ -142,7 +142,7 @@ public void testIssue1147() throws Exception { } protected ExecChainHandler createCachingExecChain(final BasicHttpCache cache, final CacheConfig config) { - return new CachingExec(cache, config); + return new CachingExec(cache, null, config); } } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolDeviations.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolDeviations.java index 5f56ff163..e42088a41 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolDeviations.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolDeviations.java @@ -127,7 +127,7 @@ private ClassicHttpResponse execute(final ClassicHttpRequest request) throws IOE } protected ExecChainHandler createCachingExecChain(final HttpCache cache, final CacheConfig config) { - return new CachingExec(cache, config); + return new CachingExec(cache, null, config); } private ClassicHttpResponse make200Response() { diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java index 2fe25fa8e..75ecea7f3 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java @@ -2166,7 +2166,7 @@ public void testMustReturnACacheEntryIfItCanRevalidateIt() throws Exception { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); request = new BasicClassicHttpRequest("GET", "/thing"); @@ -2217,7 +2217,7 @@ public void testMustReturnAFreshEnoughCacheEntryIfItHasIt() throws Exception { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); request = new BasicClassicHttpRequest("GET", "/thing"); EasyMock.expect(mockCache.getCacheEntry(EasyMock.eq(host), eqRequest(request))).andReturn(entry); @@ -2263,7 +2263,7 @@ public void testMustServeAppropriateErrorOrWarningIfNoOriginCommunicationPossibl final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); request = new BasicClassicHttpRequest("GET", "/thing"); EasyMock.expect(mockCache.getCacheEntry(EasyMock.eq(host), eqRequest(request))).andReturn(entry); @@ -2471,7 +2471,7 @@ public void testAgeHeaderPopulatedFromCacheEntryCurrentAge() throws Exception { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); request = new BasicClassicHttpRequest("GET", "/thing"); EasyMock.expect(mockCache.getCacheEntry(EasyMock.eq(host), eqRequest(request))).andReturn(entry); @@ -2521,7 +2521,7 @@ public void testHeuristicCacheOlderThan24HoursHasWarningAttached() throws Except final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(requestTime, responseTime, hdrs, bytes); - impl = new CachingExec(mockCache, config); + impl = new CachingExec(mockCache, null, config); request = new BasicClassicHttpRequest("GET", "/thing"); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestRFC5861Compliance.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestRFC5861Compliance.java index 1e11448b6..e46661436 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestRFC5861Compliance.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestRFC5861Compliance.java @@ -32,7 +32,10 @@ import java.io.ByteArrayInputStream; import java.util.Date; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.apache.hc.client5.http.impl.schedule.ImmediateSchedulingStrategy; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -41,7 +44,9 @@ import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.io.entity.InputStreamEntity; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; -import org.junit.Ignore; +import org.easymock.EasyMock; +import org.junit.After; +import org.junit.Before; import org.junit.Test; /** @@ -51,6 +56,31 @@ */ public class TestRFC5861Compliance extends AbstractProtocolTest { + private ScheduledExecutorService executorService; + + @Before + public void setup() { + executorService = new ScheduledThreadPoolExecutor(1); + EasyMock.expect(mockExecRuntime.fork(null)).andReturn(mockExecRuntime).anyTimes(); + } + + @After + public void cleanup() { + executorService.shutdownNow(); + } + + @Override + protected void replayMocks() { + super.replayMocks(); + EasyMock.replay(mockExecRuntime); + } + + @Override + protected void verifyMocks() { + super.verifyMocks(); + EasyMock.verify(mockExecRuntime); + } + /* * "The stale-if-error Cache-Control extension indicates that when an * error is encountered, a cached stale response MAY be used to satisfy @@ -169,7 +199,7 @@ public void testStaleIfErrorInResponseNeedNotYieldToProxyRevalidateForPrivateCac throws Exception{ final CacheConfig configUnshared = CacheConfig.custom() .setSharedCache(false).build(); - impl = new CachingExec(new BasicHttpCache(configUnshared), configUnshared); + impl = new CachingExec(new BasicHttpCache(configUnshared), null, configUnshared); final Date tenSecondsAgo = new Date(new Date().getTime() - 10 * 1000L); final ClassicHttpRequest req1 = HttpTestUtils.makeDefaultRequest(); @@ -323,16 +353,16 @@ public void testStaleIfErrorInRequestIsFalseReturnsError() * * http://tools.ietf.org/html/rfc5861 */ - @Test @Ignore + @Test public void testStaleWhileRevalidateReturnsStaleEntryWithWarning() throws Exception { config = CacheConfig.custom() .setMaxCacheEntries(MAX_ENTRIES) .setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1) + .setAsynchronousWorkers(1) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, executorService, ImmediateSchedulingStrategy.INSTANCE, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -365,17 +395,12 @@ public void testStaleWhileRevalidateReturnsStaleEntryWithWarning() } @Test - public void testHTTPCLIENT1470() { - impl = new CachingExec(cache, null); - } - - @Test @Ignore public void testStaleWhileRevalidateReturnsStaleNonRevalidatableEntryWithWarning() throws Exception { config = CacheConfig.custom().setMaxCacheEntries(MAX_ENTRIES).setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1).build(); + .setAsynchronousWorkers(1).build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, executorService, ImmediateSchedulingStrategy.INSTANCE, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -406,17 +431,17 @@ public void testStaleWhileRevalidateReturnsStaleNonRevalidatableEntryWithWarning assertTrue(warning110Found); } - @Test @Ignore + @Test public void testCanAlsoServeStale304sWhileRevalidating() throws Exception { config = CacheConfig.custom() .setMaxCacheEntries(MAX_ENTRIES) .setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1) + .setAsynchronousWorkers(1) .setSharedCache(false) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, executorService, ImmediateSchedulingStrategy.INSTANCE, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -460,9 +485,9 @@ public void testStaleWhileRevalidateYieldsToMustRevalidate() config = CacheConfig.custom() .setMaxCacheEntries(MAX_ENTRIES) .setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1) + .setAsynchronousWorkers(1) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, null, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -508,10 +533,10 @@ public void testStaleWhileRevalidateYieldsToProxyRevalidateForSharedCache() config = CacheConfig.custom() .setMaxCacheEntries(MAX_ENTRIES) .setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1) + .setAsynchronousWorkers(1) .setSharedCache(true) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, null, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); @@ -557,10 +582,10 @@ public void testStaleWhileRevalidateYieldsToExplicitFreshnessRequest() config = CacheConfig.custom() .setMaxCacheEntries(MAX_ENTRIES) .setMaxObjectSize(MAX_BYTES) - .setAsynchronousWorkersMax(1) + .setAsynchronousWorkers(1) .setSharedCache(true) .build(); - impl = new CachingExec(cache, config); + impl = new CachingExec(cache, null, config); final ClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "/"); final ClassicHttpResponse resp1 = HttpTestUtils.make200Response(); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java index 9be815370..235d61c8c 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java @@ -36,4 +36,8 @@ public static long getNextExecNumber() { return COUNT.incrementAndGet(); } + public static String getNextExchangeId() { + return String.format("ex-%08X", COUNT.incrementAndGet()); + } + } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/Operations.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/Operations.java index f6b6351c7..076e4e185 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/Operations.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/Operations.java @@ -27,7 +27,10 @@ package org.apache.hc.client5.http.impl; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.apache.hc.core5.concurrent.Cancellable; @@ -42,6 +45,40 @@ public boolean cancel() { }; + public static class CompletedFuture implements Future { + + private final T result; + + public CompletedFuture(final T result) { + this.result = result; + } + + @Override + public T get() throws InterruptedException, ExecutionException { + return result; + } + + @Override + public T get(final long timeout, final TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { + return result; + } + @Override + public boolean cancel(final boolean mayInterruptIfRunning) { + return false; + } + + @Override + public boolean isCancelled() { + return false; + } + + @Override + public boolean isDone() { + return true; + } + + }; + public static Cancellable nonCancellable() { return NOOP_CANCELLABLE; } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java index 813494ef3..7b6e279cc 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java @@ -158,7 +158,7 @@ public void sendRequest( clientContext.setRequestConfig(requestConfig); } final HttpRoute route = determineRoute(request, clientContext); - final String exchangeId = String.format("ex-%08X", ExecSupport.getNextExecNumber()); + final String exchangeId = ExecSupport.getNextExchangeId(); final AsyncExecRuntime execRuntime = crerateAsyncExecRuntime(); if (log.isDebugEnabled()) { log.debug(exchangeId + ": preparing request execution"); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttp2AsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttp2AsyncClient.java index 4347e3f45..2ba4cbe75 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttp2AsyncClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttp2AsyncClient.java @@ -210,7 +210,7 @@ public void streamEnd(final List trailers) throws HttpExceptio }; if (log.isDebugEnabled()) { - final String exchangeId = String.format("ex-%08X", ExecSupport.getNextExecNumber()); + final String exchangeId = ExecSupport.getNextExchangeId(); log.debug(ConnPoolSupport.getId(session) + ": executing message exchange " + exchangeId); session.addLast(new ExecutionCommand( new LoggingAsyncClientExchangeHandler(log, exchangeId, internalExchangeHandler), diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttpAsyncClient.java index 35b1fce5c..684b03778 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttpAsyncClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/MinimalHttpAsyncClient.java @@ -396,7 +396,7 @@ public void execute(final AsyncClientExchangeHandler exchangeHandler, final Http Asserts.check(!released.get(), "Endpoint has already been released"); if (log.isDebugEnabled()) { - final String exchangeId = String.format("ex-%08X", ExecSupport.getNextExecNumber()); + final String exchangeId = ExecSupport.getNextExchangeId(); log.debug(ConnPoolSupport.getId(connectionEndpoint) + ": executing message exchange " + exchangeId); connectionEndpoint.execute( new LoggingAsyncClientExchangeHandler(log, exchangeId, exchangeHandler), diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java index ecab4054a..e4fd36ae0 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalHttpClient.java @@ -158,7 +158,7 @@ protected CloseableHttpResponse doExecute( } setupContext(localcontext); final HttpRoute route = determineRoute(target, request, localcontext); - final String exchangeId = String.format("ex-%08X", ExecSupport.getNextExecNumber()); + final String exchangeId = ExecSupport.getNextExchangeId(); final ExecRuntime execRuntime = new InternalExecRuntime(log, connManager, requestExecutor, request instanceof CancellableAware ? (CancellableAware) request : null); final ExecChain.Scope scope = new ExecChain.Scope(exchangeId, route, request, execRuntime, localcontext);