Skip to content

Commit

Permalink
HTTPCLIENT-2277: Revised cache validation logic for conformance with …
Browse files Browse the repository at this point in the history
…the specification requirements per RFC 9111 section 4
  • Loading branch information
ok2c committed Nov 11, 2023
1 parent 27c0957 commit 8c1de25
Show file tree
Hide file tree
Showing 12 changed files with 395 additions and 440 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ enum CacheSuitability {
FRESH, // the cache entry is fresh and can be used to satisfy the request
FRESH_ENOUGH, // the cache entry is deemed fresh enough and can be used to satisfy the request
STALE, // the cache entry is stale and may be unsuitable to satisfy the request
STALE_WHILE_REVALIDATED, // the cache entry is stale but may be unsuitable to satisfy the request
// while being re-validated at the same time
REVALIDATION_REQUIRED
// the cache entry is stale and must not be used to satisfy the request
// without revalidation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,34 +146,6 @@ TimeValue getHeuristicFreshnessLifetime(final HttpCacheEntry entry) {
return heuristicDefaultLifetime;
}

public boolean isRevalidatable(final HttpCacheEntry entry) {
return entry.containsHeader(HttpHeaders.ETAG) || entry.containsHeader(HttpHeaders.LAST_MODIFIED);
}

public boolean mayReturnStaleWhileRevalidating(final ResponseCacheControl responseCacheControl,
final HttpCacheEntry entry, final Instant now) {
if (responseCacheControl.getStaleWhileRevalidate() >= 0) {
final TimeValue staleness = getStaleness(responseCacheControl, entry, now);
if (staleness.compareTo(TimeValue.ofSeconds(responseCacheControl.getStaleWhileRevalidate())) <= 0) {
return true;
}
}
return false;
}

public boolean mayReturnStaleIfError(final RequestCacheControl requestCacheControl,
final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry,
final Instant now) {
final TimeValue staleness = getStaleness(responseCacheControl, entry, now);
return mayReturnStaleIfError(requestCacheControl, staleness) ||
mayReturnStaleIfError(responseCacheControl, staleness);
}

private boolean mayReturnStaleIfError(final CacheControl responseCacheControl, final TimeValue staleness) {
return responseCacheControl.getStaleIfError() >= 0 &&
staleness.compareTo(TimeValue.ofSeconds(responseCacheControl.getStaleIfError())) <= 0;
}

TimeValue getApparentAge(final HttpCacheEntry entry) {
final Instant dateValue = entry.getInstant();
if (dateValue == null) {
Expand Down Expand Up @@ -238,13 +210,4 @@ TimeValue getResidentTime(final HttpCacheEntry entry, final Instant now) {
return TimeValue.ofSeconds(diff.getSeconds());
}

TimeValue getStaleness(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry, final Instant now) {
final TimeValue age = getCurrentAge(entry, now);
final TimeValue freshness = getFreshnessLifetime(responseCacheControl, entry);
if (age.compareTo(freshness) <= 0) {
return TimeValue.ZERO_MILLISECONDS;
}
return TimeValue.ofSeconds(age.toSeconds() - freshness.toSeconds());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,16 @@ class CachedResponseSuitabilityChecker {

private static final Logger LOG = LoggerFactory.getLogger(CachedResponseSuitabilityChecker.class);

private final boolean sharedCache;
private final CacheValidityPolicy validityStrategy;
private final boolean sharedCache;
private final boolean staleifError;

CachedResponseSuitabilityChecker(final CacheValidityPolicy validityStrategy,
final CacheConfig config) {
super();
this.validityStrategy = validityStrategy;
this.sharedCache = config.isSharedCache();
this.staleifError = config.isStaleIfErrorEnabled();
}

CachedResponseSuitabilityChecker(final CacheConfig config) {
Expand Down Expand Up @@ -173,6 +175,13 @@ public CacheSuitability assessSuitability(final RequestCacheControl requestCache
LOG.debug("The cache entry is fresh");
return CacheSuitability.FRESH;
} else {
if (responseCacheControl.getStaleWhileRevalidate() > 0) {
final long stale = currentAge.compareTo(freshnessLifetime) > 0 ? currentAge.toSeconds() - freshnessLifetime.toSeconds() : 0;
if (stale < responseCacheControl.getStaleWhileRevalidate()) {
LOG.debug("The cache entry is stale but suitable while being revalidated");
return CacheSuitability.STALE_WHILE_REVALIDATED;
}
}
LOG.debug("The cache entry is stale");
return CacheSuitability.STALE;
}
Expand Down Expand Up @@ -349,4 +358,30 @@ boolean lastModifiedValidatorMatches(final HttpRequest request, final HttpCacheE
return true;
}

public boolean isSuitableIfError(final RequestCacheControl requestCacheControl,
final ResponseCacheControl responseCacheControl,
final HttpCacheEntry entry,
final Instant now) {
// Explicit cache control
if (requestCacheControl.getStaleIfError() > 0 || responseCacheControl.getStaleIfError() > 0) {
final TimeValue currentAge = validityStrategy.getCurrentAge(entry, now);
final TimeValue freshnessLifetime = validityStrategy.getFreshnessLifetime(responseCacheControl, entry);
if (requestCacheControl.getMinFresh() > 0 && requestCacheControl.getMinFresh() < freshnessLifetime.toSeconds()) {
return false;
}
final long stale = currentAge.compareTo(freshnessLifetime) > 0 ? currentAge.toSeconds() - freshnessLifetime.toSeconds() : 0;
if (requestCacheControl.getStaleIfError() > 0 && stale < requestCacheControl.getStaleIfError()) {
return true;
}
if (responseCacheControl.getStaleIfError() > 0 && stale < responseCacheControl.getStaleIfError()) {
return true;
}
}
// Global override
if (staleifError && requestCacheControl.getStaleIfError() == -1 && responseCacheControl.getStaleIfError() == -1) {
return true;
}
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
Expand Down Expand Up @@ -251,7 +252,7 @@ private ClassicHttpResponse handleCacheHit(
if (cacheSuitability == CacheSuitability.FRESH || cacheSuitability == CacheSuitability.FRESH_ENOUGH) {
LOG.debug("Cache hit");
try {
return convert(generateCachedResponse(hit.entry, request, context), scope);
return convert(generateCachedResponse(request, context, hit.entry), scope);
} catch (final ResourceIOException ex) {
recordCacheFailure(target, request);
if (!mayCallBackend(requestCacheControl)) {
Expand All @@ -273,38 +274,37 @@ private ClassicHttpResponse handleCacheHit(
} else if (hit.entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request)) {
LOG.debug("Cache entry with NOT_MODIFIED does not match the non-conditional request; calling backend");
return callBackend(target, request, scope, chain);
} else if (cacheSuitability == CacheSuitability.REVALIDATION_REQUIRED || cacheSuitability == CacheSuitability.STALE) {
LOG.debug("Revalidating cache entry");
final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(responseCacheControl, hit.entry);
} else if (cacheSuitability == CacheSuitability.REVALIDATION_REQUIRED) {
LOG.debug("Revalidation required; revalidating cache entry");
try {
if (cacheRevalidator != null
&& !staleResponseNotAllowed(requestCacheControl, responseCacheControl, hit.entry, now)
&& (validityPolicy.mayReturnStaleWhileRevalidating(responseCacheControl, hit.entry, now) || staleIfErrorEnabled)) {
LOG.debug("Serving stale with asynchronous revalidation");
final String exchangeId = ExecSupport.getNextExchangeId();
context.setExchangeId(exchangeId);
final ExecChain.Scope fork = new ExecChain.Scope(
exchangeId,
scope.route,
scope.originalRequest,
scope.execRuntime.fork(null),
HttpClientContext.create());
final SimpleHttpResponse response = generateCachedResponse(hit.entry, request, context);
cacheRevalidator.revalidateCacheEntry(
hit.getEntryKey(),
() -> revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, fork, chain));
return convert(response, scope);
}
return revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, scope, chain);
} 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(hit.entry, request, context), scope);
}
return convert(handleRevalidationFailure(requestCacheControl, responseCacheControl, hit.entry, request, context, now), scope);
return revalidateCacheEntry(responseCacheControl, hit, target, request, scope, chain);
} catch (final IOException ex) {
LOG.debug(ex.getMessage(), ex);
return convert(generateGatewayTimeout(scope.clientContext), scope);
}
} else if (cacheSuitability == CacheSuitability.STALE_WHILE_REVALIDATED) {
if (cacheRevalidator != null) {
LOG.debug("Serving stale with asynchronous revalidation");
final String exchangeId = ExecSupport.getNextExchangeId();
context.setExchangeId(exchangeId);
final ExecChain.Scope fork = new ExecChain.Scope(
exchangeId,
scope.route,
scope.originalRequest,
scope.execRuntime.fork(null),
HttpClientContext.create());
cacheRevalidator.revalidateCacheEntry(
hit.getEntryKey(),
() -> revalidateCacheEntry(responseCacheControl, hit, target, request, fork, chain));
final SimpleHttpResponse response = unvalidatedCacheHit(request, context, hit.entry);
return convert(response, scope);
} else {
LOG.debug("Revalidating stale cache entry (asynchronous revalidation disabled)");
return revalidateCacheEntryWithFallback(requestCacheControl, responseCacheControl, hit, target, request, scope, chain);
}
} else if (cacheSuitability == CacheSuitability.STALE) {
LOG.debug("Revalidating stale cache entry");
return revalidateCacheEntryWithFallback(requestCacheControl, responseCacheControl, hit, target, request, scope, chain);
} else {
LOG.debug("Cache entry not usable; calling backend");
return callBackend(target, request, scope, chain);
Expand All @@ -313,7 +313,6 @@ private ClassicHttpResponse handleCacheHit(
}

ClassicHttpResponse revalidateCacheEntry(
final RequestCacheControl requestCacheControl,
final ResponseCacheControl responseCacheControl,
final CacheHit hit,
final HttpHost target,
Expand All @@ -328,7 +327,7 @@ ClassicHttpResponse revalidateCacheEntry(
try {
Instant responseDate = getCurrentDate();

if (revalidationResponseIsTooOld(backendResponse, hit.entry)) {
if (HttpCacheEntry.isNewer(hit.entry, backendResponse)) {
backendResponse.close();
final ClassicHttpRequest unconditional = conditionalRequestBuilder.buildUnconditionalRequest(
scope.originalRequest);
Expand All @@ -341,25 +340,9 @@ ClassicHttpResponse revalidateCacheEntry(
if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) {
recordCacheUpdate(scope.clientContext);
}

if (statusCode == HttpStatus.SC_NOT_MODIFIED) {
final CacheHit updated = responseCache.update(hit, target, request, backendResponse, requestDate, responseDate);
if (suitabilityChecker.isConditional(request)
&& suitabilityChecker.allConditionalsMatch(request, updated.entry, Instant.now())) {
return convert(responseGenerator.generateNotModifiedResponse(updated.entry), scope);
}
return convert(responseGenerator.generateResponse(request, updated.entry), scope);
}

if (staleIfErrorAppliesTo(statusCode)
&& !staleResponseNotAllowed(requestCacheControl, responseCacheControl, hit.entry, getCurrentDate())
&& validityPolicy.mayReturnStaleIfError(requestCacheControl, responseCacheControl, hit.entry, responseDate)) {
try {
final SimpleHttpResponse cachedResponse = responseGenerator.generateResponse(request, hit.entry);
return convert(cachedResponse, scope);
} finally {
backendResponse.close();
}
return convert(generateCachedResponse(request, scope.clientContext, updated.entry), scope);
}
return handleBackendResponse(target, conditionalRequest, scope, requestDate, responseDate, backendResponse);
} catch (final IOException | RuntimeException ex) {
Expand All @@ -368,6 +351,41 @@ ClassicHttpResponse revalidateCacheEntry(
}
}

ClassicHttpResponse revalidateCacheEntryWithFallback(
final RequestCacheControl requestCacheControl,
final ResponseCacheControl responseCacheControl,
final CacheHit hit,
final HttpHost target,
final ClassicHttpRequest request,
final ExecChain.Scope scope,
final ExecChain chain) throws HttpException, IOException {
final HttpClientContext context = scope.clientContext;
final ClassicHttpResponse response;
try {
response = revalidateCacheEntry(responseCacheControl, hit, target, request, scope, chain);
} catch (final IOException ex) {
if (suitabilityChecker.isSuitableIfError(requestCacheControl, responseCacheControl, hit.entry, getCurrentDate())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Serving stale response due to IOException and stale-if-error enabled");
}
return convert(unvalidatedCacheHit(request, context, hit.entry), scope);
} else {
LOG.debug(ex.getMessage(), ex);
return convert(generateGatewayTimeout(context), scope);
}
}
final int status = response.getCode();
if (staleIfErrorAppliesTo(status) &&
suitabilityChecker.isSuitableIfError(requestCacheControl, responseCacheControl, hit.entry, getCurrentDate())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Serving stale response due to {} status and stale-if-error enabled", status);
}
EntityUtils.consume(response.getEntity());
return convert(unvalidatedCacheHit(request, context, hit.entry), scope);
}
return response;
}

ClassicHttpResponse handleBackendResponse(
final HttpHost target,
final ClassicHttpRequest request,
Expand Down Expand Up @@ -518,7 +536,7 @@ ClassicHttpResponse negotiateResponseFromVariants(
return callBackend(target, request, scope, chain);
}

if (revalidationResponseIsTooOld(backendResponse, match.entry)) {
if (HttpCacheEntry.isNewer(match.entry, backendResponse)) {
final ClassicHttpRequest unconditional = conditionalRequestBuilder.buildUnconditionalRequest(request);
return callBackend(target, unconditional, scope, chain);
}
Expand Down
Loading

0 comments on commit 8c1de25

Please sign in to comment.