Skip to content

Commit

Permalink
Handling for 304 Not Modified responses in CachingHttpAsyncClient and…
Browse files Browse the repository at this point in the history 
… c. When a 304 response is received, the cache entry is updated and the updated entry is used to generate the response.
  • Loading branch information
@arturobernalg @ok2c
arturobernalg authored and ok2c committed Apr 27, 2023
1 parent 5505e84 commit ee175e0
Show file tree
Hide file tree
Showing 3 changed files with 177 additions and 5 deletions.
61 changes: 61 additions & 0 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
56 changes: 52 additions & 4 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,24 @@ 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,
Expand All @@ -125,12 +132,37 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
final RequestProtocolCompliance requestCompliance,
final DefaultCacheRevalidator cacheRevalidator,
final ConditionalRequestBuilder<ClassicHttpRequest> conditionalRequestBuilder,
final CacheConfig config) {
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,
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) {
this(responseCache,validityPolicy,responseCachingPolicy,responseGenerator,cacheableRequestPolicy,
suitabilityChecker,
responseCompliance,
requestCompliance,
cacheRevalidator,
conditionalRequestBuilder,
config,
null);
}

CachingExec(
Expand All @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down
65 changes: 64 additions & 1 deletion ...ient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -91,6 +95,9 @@ public class TestCachingExecChain {
private CachedHttpResponseGenerator responseGenerator;
@Spy
HttpCache cache = new BasicHttpCache();
@Mock
CacheUpdateHandler cacheUpdateHandler;

CacheConfig config;
HttpRoute route;
HttpHost host;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {

Expand Down

0 comments on commit ee175e0

Please sign in to comment.