mirror of
https://github.com/apache/httpcomponents-client.git
synced 2025-02-28 05:39:07 +00:00
Handling for 304 Not Modified responses in CachingHttpAsyncClient and c. When a 304 response is received, the cache entry is updated and the updated entry is used to generate the response.
This commit is contained in:
parent
5f6d370ccd
commit
0db4f4fa9e
@ -1010,6 +1010,67 @@ public AsyncDataConsumer handleResponse(
|
||||
backendResponse.addHeader("Via", generateViaHeader(backendResponse));
|
||||
|
||||
final AsyncExecCallback callback;
|
||||
// Handle 304 Not Modified responses
|
||||
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
|
||||
responseCache.getCacheEntry(target, request, new FutureCallback<HttpCacheEntry>() {
|
||||
@Override
|
||||
public void completed(final HttpCacheEntry existingEntry) {
|
||||
if (existingEntry != null) {
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("Existing cache entry found, updating cache entry");
|
||||
}
|
||||
responseCache.updateCacheEntry(
|
||||
target,
|
||||
request,
|
||||
existingEntry,
|
||||
backendResponse,
|
||||
requestDate,
|
||||
responseDate,
|
||||
new FutureCallback<HttpCacheEntry>() {
|
||||
|
||||
@Override
|
||||
public void completed(final HttpCacheEntry updatedEntry) {
|
||||
try {
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("Cache entry updated, generating response from updated entry");
|
||||
}
|
||||
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, updatedEntry);
|
||||
triggerResponse(cacheResponse, scope, asyncExecCallback);
|
||||
} catch (final ResourceIOException ex) {
|
||||
asyncExecCallback.failed(ex);
|
||||
}
|
||||
}
|
||||
@Override
|
||||
public void failed(final Exception cause) {
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("Request failed: {}", cause.getMessage());
|
||||
}
|
||||
asyncExecCallback.failed(cause);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void cancelled() {
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("Cache entry updated aborted");
|
||||
}
|
||||
asyncExecCallback.failed(new InterruptedIOException());
|
||||
}
|
||||
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void failed(final Exception cause) {
|
||||
asyncExecCallback.failed(cause);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void cancelled() {
|
||||
asyncExecCallback.failed(new InterruptedIOException());
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
if (backendResponse.getCode() != HttpStatus.SC_NOT_MODIFIED) {
|
||||
callback = new BackendResponseHandler(target, request, requestDate, responseDate, scope, asyncExecCallback);
|
||||
|
@ -103,17 +103,46 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
|
||||
private final HttpCache responseCache;
|
||||
private final DefaultCacheRevalidator cacheRevalidator;
|
||||
private final ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder;
|
||||
private final CacheUpdateHandler cacheUpdateHandler;
|
||||
|
||||
private static final Logger LOG = LoggerFactory.getLogger(CachingExec.class);
|
||||
|
||||
CachingExec(final HttpCache cache, final DefaultCacheRevalidator cacheRevalidator, final CacheConfig config) {
|
||||
CachingExec(final HttpCache cache, final DefaultCacheRevalidator cacheRevalidator, final CacheConfig config, final CacheUpdateHandler cacheUpdateHandler) {
|
||||
super(config);
|
||||
this.responseCache = Args.notNull(cache, "Response cache");
|
||||
this.cacheRevalidator = cacheRevalidator;
|
||||
this.conditionalRequestBuilder = new ConditionalRequestBuilder<>(classicHttpRequest ->
|
||||
ClassicRequestBuilder.copy(classicHttpRequest).build());
|
||||
ClassicRequestBuilder.copy(classicHttpRequest).build());
|
||||
this.cacheUpdateHandler = cacheUpdateHandler != null ? cacheUpdateHandler: new CacheUpdateHandler();
|
||||
}
|
||||
|
||||
CachingExec(final HttpCache cache, final DefaultCacheRevalidator cacheRevalidator, final CacheConfig config) {
|
||||
this(cache, cacheRevalidator, config, null);
|
||||
}
|
||||
|
||||
|
||||
CachingExec(
|
||||
final HttpCache responseCache,
|
||||
final CacheValidityPolicy validityPolicy,
|
||||
final ResponseCachingPolicy responseCachingPolicy,
|
||||
final CachedHttpResponseGenerator responseGenerator,
|
||||
final CacheableRequestPolicy cacheableRequestPolicy,
|
||||
final CachedResponseSuitabilityChecker suitabilityChecker,
|
||||
final ResponseProtocolCompliance responseCompliance,
|
||||
final RequestProtocolCompliance requestCompliance,
|
||||
final DefaultCacheRevalidator cacheRevalidator,
|
||||
final ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder,
|
||||
final CacheConfig config,
|
||||
final CacheUpdateHandler cacheUpdateHandler) {
|
||||
super(validityPolicy, responseCachingPolicy, responseGenerator, cacheableRequestPolicy,
|
||||
suitabilityChecker, responseCompliance, requestCompliance, config);
|
||||
this.responseCache = responseCache;
|
||||
this.cacheRevalidator = cacheRevalidator;
|
||||
this.conditionalRequestBuilder = conditionalRequestBuilder;
|
||||
this.cacheUpdateHandler = cacheUpdateHandler;
|
||||
}
|
||||
|
||||
|
||||
CachingExec(
|
||||
final HttpCache responseCache,
|
||||
final CacheValidityPolicy validityPolicy,
|
||||
@ -126,11 +155,14 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
|
||||
final DefaultCacheRevalidator cacheRevalidator,
|
||||
final ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder,
|
||||
final CacheConfig config) {
|
||||
super(validityPolicy, responseCachingPolicy, responseGenerator, cacheableRequestPolicy,
|
||||
suitabilityChecker, responseCompliance, requestCompliance, config);
|
||||
this.responseCache = responseCache;
|
||||
this.cacheRevalidator = cacheRevalidator;
|
||||
this.conditionalRequestBuilder = conditionalRequestBuilder;
|
||||
this(responseCache,validityPolicy,responseCachingPolicy,responseGenerator,cacheableRequestPolicy,
|
||||
suitabilityChecker,
|
||||
responseCompliance,
|
||||
requestCompliance,
|
||||
cacheRevalidator,
|
||||
conditionalRequestBuilder,
|
||||
config,
|
||||
null);
|
||||
}
|
||||
|
||||
CachingExec(
|
||||
@ -140,7 +172,7 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
|
||||
final CacheConfig config) {
|
||||
this(cache,
|
||||
executorService != null ? new DefaultCacheRevalidator(executorService, schedulingStrategy) : null,
|
||||
config);
|
||||
config, null);
|
||||
}
|
||||
|
||||
CachingExec(
|
||||
@ -395,6 +427,22 @@ ClassicHttpResponse cacheAndReturnResponse(
|
||||
final Instant requestSent,
|
||||
final Instant responseReceived) throws IOException {
|
||||
LOG.debug("Caching backend response");
|
||||
|
||||
// handle 304 Not Modified responses
|
||||
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
|
||||
final HttpCacheEntry existingEntry = responseCache.getCacheEntry(target, request);
|
||||
if (existingEntry != null) {
|
||||
final HttpCacheEntry updatedEntry = cacheUpdateHandler.updateCacheEntry(
|
||||
request.getMethod(),
|
||||
existingEntry,
|
||||
requestSent,
|
||||
responseReceived,
|
||||
backendResponse);
|
||||
|
||||
return convert(responseGenerator.generateResponse(request, updatedEntry), scope);
|
||||
}
|
||||
}
|
||||
|
||||
final ByteArrayBuffer buf;
|
||||
final HttpEntity entity = backendResponse.getEntity();
|
||||
if (entity != null) {
|
||||
|
@ -34,12 +34,15 @@
|
||||
import java.io.InputStream;
|
||||
import java.net.SocketException;
|
||||
import java.net.SocketTimeoutException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
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 java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import org.apache.hc.client5.http.HttpRoute;
|
||||
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
|
||||
@ -68,6 +71,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.http.message.BasicHeader;
|
||||
import org.apache.hc.core5.net.URIAuthority;
|
||||
import org.apache.hc.core5.util.TimeValue;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
@ -91,6 +95,9 @@ public class TestCachingExecChain {
|
||||
private CachedHttpResponseGenerator responseGenerator;
|
||||
@Spy
|
||||
HttpCache cache = new BasicHttpCache();
|
||||
@Mock
|
||||
CacheUpdateHandler cacheUpdateHandler;
|
||||
|
||||
CacheConfig config;
|
||||
HttpRoute route;
|
||||
HttpHost host;
|
||||
@ -130,7 +137,7 @@ public void setUp() {
|
||||
conditionalRequestBuilder = mock(ConditionalRequestBuilder.class);
|
||||
responseCache = mock(HttpCache.class);
|
||||
|
||||
impl = new CachingExec(cache, null, CacheConfig.DEFAULT);
|
||||
impl = new CachingExec(cache, null, CacheConfig.DEFAULT, cacheUpdateHandler);
|
||||
}
|
||||
|
||||
public ClassicHttpResponse execute(final ClassicHttpRequest request) throws IOException, HttpException {
|
||||
@ -1413,6 +1420,62 @@ public void testReturnssetStaleIfErrorEnabled() throws Exception {
|
||||
Mockito.verify(cacheRevalidator, Mockito.times(1)).revalidateCacheEntry(Mockito.any(), Mockito.any());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNotModifiedResponseUpdatesCacheEntry() throws Exception {
|
||||
// Prepare request and host
|
||||
final HttpHost host = new HttpHost("foo.example.com");
|
||||
final ClassicHttpRequest request = new HttpGet("http://foo.example.com/bar");
|
||||
|
||||
// Prepare original cache entry
|
||||
final HttpCacheEntry originalEntry = HttpTestUtils.makeCacheEntry();
|
||||
Mockito.when(cache.getCacheEntry(host, request)).thenReturn(originalEntry);
|
||||
|
||||
// Prepare 304 Not Modified response
|
||||
final Instant now = Instant.now();
|
||||
final Instant requestSent = now.plusSeconds(3);
|
||||
final Instant responseReceived = now.plusSeconds(1);
|
||||
|
||||
final ClassicHttpResponse backendResponse = new BasicClassicHttpResponse(HttpStatus.SC_NOT_MODIFIED, "Not Modified");
|
||||
backendResponse.setHeader("Cache-Control", "public, max-age=3600");
|
||||
backendResponse.setHeader("ETag", "\"etag\"");
|
||||
|
||||
final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, mockExecRuntime, context);
|
||||
|
||||
final Header[] headers = new Header[5];
|
||||
for (int i = 0; i < headers.length; i++) {
|
||||
headers[i] = new BasicHeader("header" + i, "value" + i);
|
||||
}
|
||||
final String body = "Lorem ipsum dolor sit amet";
|
||||
|
||||
final Map<String, String> variantMap = new HashMap<>();
|
||||
variantMap.put("test variant 1", "true");
|
||||
variantMap.put("test variant 2", "true");
|
||||
final HttpCacheEntry cacheEntry = new HttpCacheEntry(
|
||||
Instant.now(),
|
||||
Instant.now(),
|
||||
HttpStatus.SC_NOT_MODIFIED,
|
||||
headers,
|
||||
new HeapResource(body.getBytes(StandardCharsets.UTF_8)), variantMap);
|
||||
|
||||
Mockito.when(cacheUpdateHandler.updateCacheEntry(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(cacheEntry);
|
||||
|
||||
// Call cacheAndReturnResponse with 304 Not Modified response
|
||||
final ClassicHttpResponse cachedResponse = impl.cacheAndReturnResponse(host, request, backendResponse, scope, requestSent, responseReceived);
|
||||
|
||||
|
||||
// Verify cache entry is updated
|
||||
Mockito.verify(cacheUpdateHandler).updateCacheEntry(
|
||||
request.getMethod(),
|
||||
originalEntry,
|
||||
requestSent,
|
||||
responseReceived,
|
||||
backendResponse
|
||||
);
|
||||
|
||||
// Verify response is generated from the updated cache entry
|
||||
Assertions.assertEquals(HttpStatus.SC_NOT_MODIFIED, cachedResponse.getCode());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testStaleIfErrorEnabledWithIOException() throws Exception {
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user