Skip to content

Commit

Permalink
Made caching of 403 responses by the classic and async protocol handl…
Browse files Browse the repository at this point in the history 
…ers more consistent
  • Loading branch information
@ok2c
ok2c committed Jan 3, 2024
1 parent c5de696 commit f24c801
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 93 deletions.
169 changes: 86 additions & 83 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 @@ -515,21 +515,20 @@ public void cancelled() {
final ResponseCacheControl responseCacheControl = CacheControlHeaderParser.INSTANCE.parse(backendResponse);
final boolean cacheable = responseCachingPolicy.isResponseCacheable(responseCacheControl, request, backendResponse);
if (cacheable) {
cachingConsumerRef.set(new CachingAsyncDataConsumer(exchangeId, asyncExecCallback, backendResponse, entityDetails));
storeRequestIfModifiedSinceFor304Response(request, backendResponse);
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("{} backend response is not cacheable", exchangeId);
}
}
final CachingAsyncDataConsumer cachingDataConsumer = cachingConsumerRef.get();
if (cachingDataConsumer != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} caching backend response", exchangeId);
}
final CachingAsyncDataConsumer cachingDataConsumer = new CachingAsyncDataConsumer(
exchangeId, asyncExecCallback, backendResponse, entityDetails);
cachingConsumerRef.set(cachingDataConsumer);
return cachingDataConsumer;
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("{} backend response is not cacheable", exchangeId);
}
return asyncExecCallback.handleResponse(backendResponse, entityDetails);
}
return asyncExecCallback.handleResponse(backendResponse, entityDetails);
}

@Override
Expand Down Expand Up @@ -576,13 +575,87 @@ public void cancelled() {

}

void triggerCachedResponse(final HttpCacheEntry entry) {
try {
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
asyncExecCallback.failed(ex);
}
}

@Override
public void completed() {
final String exchangeId = scope.exchangeId;
final CachingAsyncDataConsumer cachingDataConsumer = cachingConsumerRef.getAndSet(null);
if (cachingDataConsumer != null && !cachingDataConsumer.writtenThrough.get()) {
final ByteArrayBuffer buffer = cachingDataConsumer.bufferRef.getAndSet(null);
final HttpResponse backendResponse = cachingDataConsumer.backendResponse;
if (cachingDataConsumer == null || cachingDataConsumer.writtenThrough.get()) {
asyncExecCallback.completed();
return;
}
final HttpResponse backendResponse = cachingDataConsumer.backendResponse;
final ByteArrayBuffer buffer = cachingDataConsumer.bufferRef.getAndSet(null);

// Handle 304 Not Modified responses
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
responseCache.match(target, request, new FutureCallback<CacheMatch>() {

@Override
public void completed(final CacheMatch result) {
final CacheHit hit = result != null ? result.hit : null;
if (hit != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} existing cache entry found, updating cache entry", exchangeId);
}
responseCache.update(
hit,
target,
request,
backendResponse,
requestDate,
responseDate,
new FutureCallback<CacheHit>() {

@Override
public void completed(final CacheHit updated) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} cache entry updated, generating response from updated entry", exchangeId);
}
triggerCachedResponse(updated.entry);
}
@Override
public void failed(final Exception cause) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} request failed: {}", exchangeId, cause.getMessage());
}
asyncExecCallback.failed(cause);
}

@Override
public void cancelled() {
if (LOG.isDebugEnabled()) {
LOG.debug("{} cache entry updated aborted", exchangeId);
}
asyncExecCallback.failed(new InterruptedIOException());
}

});
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
}
}

@Override
public void failed(final Exception cause) {
asyncExecCallback.failed(cause);
}

@Override
public void cancelled() {
asyncExecCallback.failed(new InterruptedIOException());
}

});
} else {
if (cacheConfig.isFreshnessCheckEnabled()) {
final CancellableDependency operation = scope.cancellableDependency;
operation.setDependency(responseCache.match(target, request, new FutureCallback<CacheMatch>() {
Expand All @@ -594,12 +667,7 @@ public void completed(final CacheMatch result) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} backend already contains fresher cache entry", exchangeId);
}
try {
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, hit.entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
asyncExecCallback.failed(ex);
}
triggerCachedResponse(hit.entry);
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
}
Expand All @@ -619,8 +687,6 @@ public void cancelled() {
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
}
} else {
asyncExecCallback.completed();
}
}

Expand Down Expand Up @@ -1183,69 +1249,6 @@ public AsyncDataConsumer handleResponse(
final EntityDetails entityDetails) throws HttpException, IOException {
final Instant responseDate = getCurrentDate();
final AsyncExecCallback callback;
// Handle 304 Not Modified responses
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
responseCache.match(target, request, new FutureCallback<CacheMatch>() {
@Override
public void completed(final CacheMatch result) {
final CacheHit hit = result != null ? result.hit : null;
if (hit != null) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} existing cache entry found, updating cache entry", exchangeId);
}
responseCache.update(
hit,
target,
request,
backendResponse,
requestDate,
responseDate,
new FutureCallback<CacheHit>() {

@Override
public void completed(final CacheHit updated) {
try {
if (LOG.isDebugEnabled()) {
LOG.debug("{} cache entry updated, generating response from updated entry", exchangeId);
}
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, updated.entry);
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: {}", exchangeId, cause.getMessage());
}
asyncExecCallback.failed(cause);
}

@Override
public void cancelled() {
if (LOG.isDebugEnabled()) {
LOG.debug("{} cache entry updated aborted", exchangeId);
}
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);
} else {
Expand Down
21 changes: 11 additions & 10 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 @@ -454,12 +454,16 @@ ClassicHttpResponse handleBackendResponse(
final boolean cacheable = responseCachingPolicy.isResponseCacheable(responseCacheControl, request, backendResponse);
if (cacheable) {
storeRequestIfModifiedSinceFor304Response(request, backendResponse);
if (LOG.isDebugEnabled()) {
LOG.debug("{} caching backend response", exchangeId);
}
return cacheAndReturnResponse(exchangeId, target, request, backendResponse, requestDate, responseDate);
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("{} backend response is not cacheable", exchangeId);
}
return backendResponse;
}
if (LOG.isDebugEnabled()) {
LOG.debug("{} backend response is not cacheable", exchangeId);
}
return backendResponse;
}

ClassicHttpResponse cacheAndReturnResponse(
Expand All @@ -469,12 +473,9 @@ ClassicHttpResponse cacheAndReturnResponse(
final ClassicHttpResponse backendResponse,
final Instant requestSent,
final Instant responseReceived) throws IOException {
if (LOG.isDebugEnabled()) {
LOG.debug("{} caching backend response", exchangeId);
}

final int statusCode = backendResponse.getCode();
// handle 304 Not Modified responses
if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) {
if (statusCode == HttpStatus.SC_NOT_MODIFIED) {
final CacheMatch result = responseCache.match(target ,request);
final CacheHit hit = result != null ? result.hit : null;
if (hit != null) {
Expand Down Expand Up @@ -514,7 +515,7 @@ ClassicHttpResponse cacheAndReturnResponse(
backendResponse.close();

CacheHit hit;
if (cacheConfig.isFreshnessCheckEnabled()) {
if (cacheConfig.isFreshnessCheckEnabled() && statusCode != HttpStatus.SC_NOT_MODIFIED) {
final CacheMatch result = responseCache.match(target ,request);
hit = result != null ? result.hit : null;
if (HttpCacheEntry.isNewer(hit != null ? hit.entry : null, backendResponse)) {
Expand Down

0 comments on commit f24c801

Please sign in to comment.