You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2023/08/20 09:42:21 UTC
[httpcomponents-client] 09/38: Complete the implementation of stale-if-error support as per RFC 5861
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch 5.4.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
commit 390a7cbb5b7501eebd64440d7401a1696433feec
Author: Arturo Bernal <ar...@gmail.com>
AuthorDate: Fri Apr 21 22:11:30 2023 +0200
Complete the implementation of stale-if-error support as per RFC 5861
- Updated handling of IOExceptions to serve stale responses when allowed by stale-if-error
---
.../client5/http/impl/cache/AsyncCachingExec.java | 20 ++++-
.../hc/client5/http/impl/cache/CachingExec.java | 9 ++-
.../http/impl/cache/TestCachingExecChain.java | 86 ++++++++++++++++++++++
3 files changed, 112 insertions(+), 3 deletions(-)
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 d22432612..0ca8b2f0b 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
@@ -648,10 +648,11 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
triggerResponse(cacheResponse, scope, asyncExecCallback);
} else if (!(entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) {
LOG.debug("Revalidating cache entry");
+ final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(entry);
if (cacheRevalidator != null
&& !staleResponseNotAllowed(request, entry, now)
&& validityPolicy.mayReturnStaleWhileRevalidating(entry, now)
- || responseCachingPolicy.isStaleIfErrorEnabled(entry)) {
+ || staleIfErrorEnabled) {
LOG.debug("Serving stale with asynchronous revalidation");
try {
final SimpleHttpResponse cacheResponse = generateCachedResponse(request, context, entry, now);
@@ -672,7 +673,22 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
asyncExecCallback1 -> revalidateCacheEntry(target, request, entityProducer, fork, chain, asyncExecCallback1, entry));
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
- asyncExecCallback.failed(ex);
+ if (staleIfErrorEnabled) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Serving stale response due to IOException and stale-if-error enabled");
+ }
+ try {
+ final SimpleHttpResponse cacheResponse = generateCachedResponse(request, context, entry, now);
+ triggerResponse(cacheResponse, scope, asyncExecCallback);
+ } catch (final ResourceIOException ex2) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Failed to generate cached response, falling back to backend", ex2);
+ }
+ callBackend(target, request, entityProducer, scope, chain, asyncExecCallback);
+ }
+ } else {
+ asyncExecCallback.failed(ex);
+ }
}
} else {
revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry);
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 7d7d392aa..fd649fdc4 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
@@ -271,11 +271,12 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
return convert(generateGatewayTimeout(context), scope);
} else if (!(entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) {
LOG.debug("Revalidating cache entry");
+ final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(entry);
try {
if (cacheRevalidator != null
&& !staleResponseNotAllowed(request, entry, now)
&& validityPolicy.mayReturnStaleWhileRevalidating(entry, now)
- || responseCachingPolicy.isStaleIfErrorEnabled(entry)) {
+ || staleIfErrorEnabled) {
LOG.debug("Serving stale with asynchronous revalidation");
final String exchangeId = ExecSupport.getNextExchangeId();
context.setExchangeId(exchangeId);
@@ -293,6 +294,12 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
}
return revalidateCacheEntry(target, request, scope, chain, entry);
} catch (final IOException ioex) {
+ if (staleIfErrorEnabled) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Serving stale response due to IOException and stale-if-error enabled");
+ }
+ return convert(generateCachedResponse(request, context, entry, now), scope);
+ }
return convert(handleRevalidationFailure(request, context, entry, now), scope);
}
} else {
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 f701e29a8..7f1d3801c 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
@@ -37,6 +37,9 @@ import java.net.SocketTimeoutException;
import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
+import java.util.Arrays;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
@@ -45,6 +48,7 @@ import org.apache.hc.client5.http.cache.CacheResponseStatus;
import org.apache.hc.client5.http.cache.HttpCacheContext;
import org.apache.hc.client5.http.cache.HttpCacheEntry;
import org.apache.hc.client5.http.cache.HttpCacheStorage;
+import org.apache.hc.client5.http.cache.ResourceIOException;
import org.apache.hc.client5.http.classic.ExecChain;
import org.apache.hc.client5.http.classic.ExecRuntime;
import org.apache.hc.client5.http.classic.methods.HttpGet;
@@ -53,6 +57,7 @@ import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.client5.http.utils.DateUtils;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
@@ -64,6 +69,7 @@ import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
import org.apache.hc.core5.net.URIAuthority;
+import org.apache.hc.core5.util.TimeValue;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -82,6 +88,7 @@ public class TestCachingExecChain {
@Mock
HttpCacheStorage mockStorage;
private DefaultCacheRevalidator cacheRevalidator;
+ private CachedHttpResponseGenerator responseGenerator;
@Spy
HttpCache cache = new BasicHttpCache();
CacheConfig config;
@@ -91,6 +98,15 @@ public class TestCachingExecChain {
HttpCacheContext context;
HttpCacheEntry entry;
CachingExec impl;
+ CacheValidityPolicy validityPolicy;
+ ResponseCachingPolicy responseCachingPolicy;
+ CacheableRequestPolicy cacheableRequestPolicy;
+ CachedResponseSuitabilityChecker suitabilityChecker;
+ ResponseProtocolCompliance responseCompliance;
+ RequestProtocolCompliance requestCompliance;
+ ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder;
+ CacheConfig customConfig;
+ HttpCache responseCache;
@BeforeEach
public void setUp() {
@@ -102,6 +118,17 @@ public class TestCachingExecChain {
context = HttpCacheContext.create();
entry = HttpTestUtils.makeCacheEntry();
cacheRevalidator = mock(DefaultCacheRevalidator.class);
+ responseGenerator = mock(CachedHttpResponseGenerator.class);
+ customConfig = mock(CacheConfig.class);
+
+ validityPolicy = mock(CacheValidityPolicy.class);
+ responseCachingPolicy = mock(ResponseCachingPolicy.class);
+ cacheableRequestPolicy = mock(CacheableRequestPolicy.class);
+ suitabilityChecker = mock(CachedResponseSuitabilityChecker.class);
+ responseCompliance = mock(ResponseProtocolCompliance.class);
+ requestCompliance = mock(RequestProtocolCompliance.class);
+ conditionalRequestBuilder = mock(ConditionalRequestBuilder.class);
+ responseCache = mock(HttpCache.class);
impl = new CachingExec(cache, null, CacheConfig.DEFAULT);
}
@@ -1359,4 +1386,63 @@ public class TestCachingExecChain {
Mockito.verify(cacheRevalidator, Mockito.times(1)).revalidateCacheEntry(Mockito.any(), Mockito.any());
}
+ @Test
+ public void testStaleIfErrorEnabledWithIOException() throws Exception {
+
+ impl = new CachingExec(responseCache, validityPolicy, responseCachingPolicy,
+ responseGenerator, cacheableRequestPolicy, suitabilityChecker,
+ responseCompliance, requestCompliance, cacheRevalidator,
+ conditionalRequestBuilder, customConfig);
+ // Create the first request and response
+ final BasicClassicHttpRequest req1 = new BasicClassicHttpRequest("GET", "http://foo.example.com/");
+ final BasicClassicHttpRequest req2 = new BasicClassicHttpRequest("GET", "http://foo.example.com/");
+ final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_GATEWAY_TIMEOUT, "OK");
+ resp1.setEntity(HttpTestUtils.makeBody(128));
+ resp1.setHeader("Content-Length", "128");
+ resp1.setHeader("ETag", "\"etag\"");
+ resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now().minus(Duration.ofHours(10))));
+ resp1.setHeader("Cache-Control", "public, max-age=-1, stale-while-revalidate=1");
+ final ClassicHttpResponse resp2 = HttpTestUtils.make200Response();
+
+ // Set up the mock response chain
+ Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
+ Mockito.when(responseCachingPolicy.isStaleIfErrorEnabled(Mockito.any())).thenReturn(true);
+ Mockito.when(cacheableRequestPolicy.isServableFromCache(Mockito.any())).thenReturn(true);
+ Mockito.when(validityPolicy.getStaleness(Mockito.any(), Mockito.any())).thenReturn(TimeValue.MAX_VALUE);
+
+ final SimpleHttpResponse response = SimpleHttpResponse.create(HttpStatus.SC_OK);
+ final AtomicInteger callCount = new AtomicInteger(0);
+
+ Mockito.doAnswer(invocation -> {
+ if (callCount.getAndIncrement() == 0) {
+ throw new ResourceIOException("ResourceIOException");
+ } else {
+ // Replace this with the actual return value for the second call
+ return response;
+ }
+ }).when(responseGenerator).generateResponse(Mockito.any(), Mockito.any());
+
+ // Execute the first request and assert the response
+ final ClassicHttpResponse response1 = execute(req1);
+ final HttpCacheEntry httpCacheEntry = HttpTestUtils.makeCacheEntry();
+ Mockito.when(responseCache.getCacheEntry(Mockito.any(), Mockito.any())).thenReturn(httpCacheEntry);
+ Assertions.assertEquals(HttpStatus.SC_GATEWAY_TIMEOUT, response1.getCode());
+
+ Mockito.when(mockExecRuntime.fork(Mockito.any())).thenReturn(mockExecRuntime);
+ Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp2);
+
+ final ClassicHttpResponse response2 = execute(req2);
+ // Verify that the response has the expected status code (e.g., 200)
+ Assertions.assertEquals(HttpStatus.SC_OK, response2.getCode());
+
+ // Verify that the response has the expected headers or other properties
+ // For example, you can check for the "Warning" header indicating a stale response:
+ final Optional<Header> warningHeader = Arrays.stream(response.getHeaders())
+ .filter(header -> header.getName().equalsIgnoreCase("Warning"))
+ .findFirst();
+
+ Assertions.assertTrue(warningHeader.isPresent());
+ Assertions.assertTrue(warningHeader.get().getValue().contains("110"));
+ }
+
}