diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java index 314cf10af..e5d43b606 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java @@ -447,6 +447,13 @@ public Iterator
requestHeaderIterator() { return requestHeaders.headerIterator(); } + /** + * @since 5.3 + */ + public Iterator
requestHeaderIterator(final String headerName) { + return requestHeaders.headerIterator(headerName); + } + /** * Tests if the given {@link HttpCacheEntry} is newer than the given {@link MessageHeaders} * by comparing values of their {@literal DATE} header. In case the given entry, or the message, 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 f30eeff1f..93352ce92 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 @@ -229,6 +229,9 @@ public void execute( } final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request); + if (LOG.isDebugEnabled()) { + LOG.debug("Request cache control: {}", requestCacheControl); + } if (cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) { operation.setDependency(responseCache.match(target, request, new FutureCallback() { @@ -242,6 +245,9 @@ public void completed(final CacheMatch result) { handleCacheMiss(requestCacheControl, root, target, request, entityProducer, scope, chain, asyncExecCallback); } else { final ResponseCacheControl responseCacheControl = CacheControlHeaderParser.INSTANCE.parse(hit.entry); + if (LOG.isDebugEnabled()) { + LOG.debug("Response cache control: {}", responseCacheControl); + } handleCacheHit(requestCacheControl, responseCacheControl, hit, target, request, entityProducer, scope, chain, asyncExecCallback); } } @@ -590,21 +596,11 @@ private void handleCacheHit( recordCacheHit(target, request); final Instant now = getCurrentDate(); - if (requestCacheControl.isNoCache()) { - // Revalidate with the server due to no-cache directive in response - if (LOG.isDebugEnabled()) { - LOG.debug("Revalidating with server due to no-cache directive in response."); - } - revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, entityProducer, scope, chain, asyncExecCallback); - return; + final CacheSuitability cacheSuitability = suitabilityChecker.assessSuitability(requestCacheControl, responseCacheControl, request, hit.entry, now); + if (LOG.isDebugEnabled()) { + LOG.debug("Request {} {}: {}", request.getMethod(), request.getRequestUri(), cacheSuitability); } - - if (suitabilityChecker.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, hit.entry, now)) { - if (responseCachingPolicy.responseContainsNoCacheDirective(responseCacheControl, hit.entry)) { - // Revalidate with the server due to no-cache directive in response - revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, entityProducer, scope, chain, asyncExecCallback); - return; - } + if (cacheSuitability == CacheSuitability.FRESH || cacheSuitability == CacheSuitability.FRESH_ENOUGH) { LOG.debug("Cache hit"); try { final SimpleHttpResponse cacheResponse = generateCachedResponse(hit.entry, request, context); @@ -623,60 +619,66 @@ private void handleCacheHit( } } } - } else if (!mayCallBackend(requestCacheControl)) { - LOG.debug("Cache entry not suitable but only-if-cached requested"); - final SimpleHttpResponse cacheResponse = generateGatewayTimeout(context); - triggerResponse(cacheResponse, scope, asyncExecCallback); - } else if (!(hit.entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { - LOG.debug("Revalidating cache entry"); - final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(responseCacheControl, hit.entry); - if (cacheRevalidator != null - && !staleResponseNotAllowed(requestCacheControl, responseCacheControl, hit.entry, now) - && (validityPolicy.mayReturnStaleWhileRevalidating(responseCacheControl, hit.entry, now) || staleIfErrorEnabled)) { - LOG.debug("Serving stale with asynchronous revalidation"); - try { - final SimpleHttpResponse cacheResponse = generateCachedResponse(hit.entry, request, context); - final String exchangeId = ExecSupport.getNextExchangeId(); - context.setExchangeId(exchangeId); - final AsyncExecChain.Scope fork = new AsyncExecChain.Scope( - exchangeId, - scope.route, - scope.originalRequest, - new ComplexFuture<>(null), - HttpClientContext.create(), - scope.execRuntime.fork(), - scope.scheduler, - scope.execCount); - cacheRevalidator.revalidateCacheEntry( - hit.getEntryKey(), - asyncExecCallback, - asyncExecCallback1 -> revalidateCacheEntry(requestCacheControl, responseCacheControl, - hit, target, request, entityProducer, fork, chain, asyncExecCallback1)); - triggerResponse(cacheResponse, scope, asyncExecCallback); - } catch (final ResourceIOException ex) { - if (staleIfErrorEnabled) { - if (LOG.isDebugEnabled()) { - LOG.debug("Serving stale response due to IOException and stale-if-error enabled"); - } - try { - final SimpleHttpResponse cacheResponse = generateCachedResponse(hit.entry, request, context); - triggerResponse(cacheResponse, scope, asyncExecCallback); - } catch (final ResourceIOException ex2) { + } else { + if (!mayCallBackend(requestCacheControl)) { + final SimpleHttpResponse cacheResponse = generateGatewayTimeout(context); + triggerResponse(cacheResponse, scope, asyncExecCallback); + return; + } + if (cacheSuitability == CacheSuitability.MISMATCH) { + LOG.debug("Cache entry does not match the request; calling backend"); + callBackend(target, request, entityProducer, scope, chain, asyncExecCallback); + } else if (!(hit.entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { + LOG.debug("Revalidating cache entry"); + final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(responseCacheControl, hit.entry); + if (cacheRevalidator != null + && !staleResponseNotAllowed(requestCacheControl, responseCacheControl, hit.entry, now) + && (validityPolicy.mayReturnStaleWhileRevalidating(responseCacheControl, hit.entry, now) || staleIfErrorEnabled)) { + LOG.debug("Serving stale with asynchronous revalidation"); + try { + final SimpleHttpResponse cacheResponse = generateCachedResponse(hit.entry, request, context); + final String exchangeId = ExecSupport.getNextExchangeId(); + context.setExchangeId(exchangeId); + final AsyncExecChain.Scope fork = new AsyncExecChain.Scope( + exchangeId, + scope.route, + scope.originalRequest, + new ComplexFuture<>(null), + HttpClientContext.create(), + scope.execRuntime.fork(), + scope.scheduler, + scope.execCount); + cacheRevalidator.revalidateCacheEntry( + hit.getEntryKey(), + asyncExecCallback, + asyncExecCallback1 -> revalidateCacheEntry(requestCacheControl, responseCacheControl, + hit, target, request, entityProducer, fork, chain, asyncExecCallback1)); + triggerResponse(cacheResponse, scope, asyncExecCallback); + } catch (final ResourceIOException ex) { + if (staleIfErrorEnabled) { if (LOG.isDebugEnabled()) { - LOG.debug("Failed to generate cached response, falling back to backend", ex2); + LOG.debug("Serving stale response due to IOException and stale-if-error enabled"); } - callBackend(target, request, entityProducer, scope, chain, asyncExecCallback); + try { + final SimpleHttpResponse cacheResponse = generateCachedResponse(hit.entry, request, context); + 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 { - asyncExecCallback.failed(ex); } + } else { + revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, entityProducer, scope, chain, asyncExecCallback); } } else { - revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, entityProducer, scope, chain, asyncExecCallback); + LOG.debug("Cache entry not usable; calling backend"); + callBackend(target, request, entityProducer, scope, chain, asyncExecCallback); } - } else { - LOG.debug("Cache entry not usable; calling backend"); - callBackend(target, request, entityProducer, scope, chain, asyncExecCallback); } } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java index d8b6d622f..50dafa9b9 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Locale; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.core5.annotation.Contract; @@ -201,6 +202,21 @@ public static List variantNames(final MessageHeaders message) { return names; } + @Internal + public static void normalizeTokens(final Iterator
iterator, final Consumer consumer) { + final List tokens = new ArrayList<>(); + while (iterator.hasNext()) { + final Header header = iterator.next(); + CacheSupport.parseTokens(header, tokens::add); + } + tokens.stream() + .filter(t -> !t.isEmpty()) + .map(t -> t.toLowerCase(Locale.ROOT)) + .sorted() + .distinct() + .forEach(consumer); + } + /** * Computes a "variant key" for the given request and the given variants. * @param request originating request @@ -222,24 +238,13 @@ public String generateVariantKey(final HttpRequest request, final Collection { + if (!firstToken.compareAndSet(false, true)) { + buf.append(","); + } + buf.append(PercentCodec.encode(t, StandardCharsets.UTF_8)); + }); }); buf.append("}"); return buf.toString(); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSuitability.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSuitability.java new file mode 100644 index 000000000..fbe92ab9e --- /dev/null +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSuitability.java @@ -0,0 +1,46 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.hc.client5.http.impl.cache; + +/** + * @since 5.3 + */ +enum CacheSuitability { + + MISMATCH, // the cache entry does not match the request properties and cannot be used + // to satisfy the request + 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 + REVALIDATION_REQUIRED, + // the cache entry is stale and must not be used to satisfy the request + // without revalidation + NO_STORE // The cache entry must not be stored or must be immediately removed + // from the persistent storage. + +} \ No newline at end of file diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java index a79214d51..ca0dbb463 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java @@ -41,6 +41,7 @@ class CacheValidityPolicy { private static final Logger LOG = LoggerFactory.getLogger(CacheValidityPolicy.class); + private final boolean useHeuristicCaching; private final float heuristicCoefficient; private final TimeValue heuristicDefaultLifetime; @@ -53,6 +54,7 @@ class CacheValidityPolicy { */ CacheValidityPolicy(final CacheConfig config) { super(); + this.useHeuristicCaching = config != null ? config.isHeuristicCachingEnabled() : CacheConfig.DEFAULT.isHeuristicCachingEnabled(); this.heuristicCoefficient = config != null ? config.getHeuristicCoefficient() : CacheConfig.DEFAULT.getHeuristicCoefficient(); this.heuristicDefaultLifetime = config != null ? config.getHeuristicDefaultLifetime() : CacheConfig.DEFAULT.getHeuristicDefaultLifetime(); } @@ -117,35 +119,18 @@ public TimeValue getFreshnessLifetime(final ResponseCacheControl responseCacheCo } } - // No explicit expiration time is present in the response. A heuristic freshness lifetime might be applicable - if (LOG.isDebugEnabled()) { - LOG.debug("No explicit expiration time present in the response. Using heuristic freshness lifetime calculation."); + if (useHeuristicCaching) { + // No explicit expiration time is present in the response. A heuristic freshness lifetime might be applicable + if (LOG.isDebugEnabled()) { + LOG.debug("No explicit expiration time present in the response. Using heuristic freshness lifetime calculation."); + } + return getHeuristicFreshnessLifetime(entry); + } else { + return TimeValue.ZERO_MILLISECONDS; } - return getHeuristicFreshnessLifetime(entry); - } - - public boolean isResponseFresh(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry, - final Instant now) { - return getCurrentAge(entry, now).compareTo(getFreshnessLifetime(responseCacheControl, entry)) == -1; - } - - /** - * Decides if this response is fresh enough based Last-Modified and Date, if available. - * This entry is meant to be used when isResponseFresh returns false. - * - * The algorithm is as follows: - * if last-modified and date are defined, freshness lifetime is coefficient*(date-lastModified), - * else freshness lifetime is defaultLifetime - * - * @param entry the cache entry - * @param now what time is it currently (When is right NOW) - * @return {@code true} if the response is fresh - */ - public boolean isResponseHeuristicallyFresh(final HttpCacheEntry entry, final Instant now) { - return getCurrentAge(entry, now).compareTo(getHeuristicFreshnessLifetime(entry)) == -1; } - public TimeValue getHeuristicFreshnessLifetime(final HttpCacheEntry entry) { + TimeValue getHeuristicFreshnessLifetime(final HttpCacheEntry entry) { final Instant dateValue = entry.getInstant(); final Instant lastModifiedValue = entry.getLastModified(); @@ -189,7 +174,7 @@ private boolean mayReturnStaleIfError(final CacheControl responseCacheControl, f staleness.compareTo(TimeValue.ofSeconds(responseCacheControl.getStaleIfError())) <= 0; } - protected TimeValue getApparentAge(final HttpCacheEntry entry) { + TimeValue getApparentAge(final HttpCacheEntry entry) { final Instant dateValue = entry.getInstant(); if (dateValue == null) { return CacheSupport.MAX_AGE; @@ -216,7 +201,7 @@ protected TimeValue getApparentAge(final HttpCacheEntry entry) { * is negative. If the Age value is invalid (cannot be parsed into a number or contains non-numeric characters), * this method returns 0. */ - protected long getAgeValue(final HttpCacheEntry entry) { + long getAgeValue(final HttpCacheEntry entry) { final Header age = entry.getFirstHeader(HttpHeaders.AGE); if (age != null) { final AtomicReference firstToken = new AtomicReference<>(); @@ -231,44 +216,29 @@ protected long getAgeValue(final HttpCacheEntry entry) { return 0; } - - - protected TimeValue getCorrectedAgeValue(final HttpCacheEntry entry) { + TimeValue getCorrectedAgeValue(final HttpCacheEntry entry) { final long ageValue = getAgeValue(entry); final long responseDelay = getResponseDelay(entry).toSeconds(); return TimeValue.ofSeconds(ageValue + responseDelay); } - protected TimeValue getResponseDelay(final HttpCacheEntry entry) { + TimeValue getResponseDelay(final HttpCacheEntry entry) { final Duration diff = Duration.between(entry.getRequestInstant(), entry.getResponseInstant()); return TimeValue.ofSeconds(diff.getSeconds()); } - protected TimeValue getCorrectedInitialAge(final HttpCacheEntry entry) { + TimeValue getCorrectedInitialAge(final HttpCacheEntry entry) { final long apparentAge = getApparentAge(entry).toSeconds(); final long correctedReceivedAge = getCorrectedAgeValue(entry).toSeconds(); return TimeValue.ofSeconds(Math.max(apparentAge, correctedReceivedAge)); } - protected TimeValue getResidentTime(final HttpCacheEntry entry, final Instant now) { + TimeValue getResidentTime(final HttpCacheEntry entry, final Instant now) { final Duration diff = Duration.between(entry.getResponseInstant(), now); return TimeValue.ofSeconds(diff.getSeconds()); } - - protected long getMaxAge(final ResponseCacheControl responseCacheControl) { - final long maxAge = responseCacheControl.getMaxAge(); - final long sharedMaxAge = responseCacheControl.getSharedMaxAge(); - if (sharedMaxAge == -1) { - return maxAge; - } else if (maxAge == -1) { - return sharedMaxAge; - } else { - return Math.min(maxAge, sharedMaxAge); - } - } - - public TimeValue getStaleness(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry, final Instant now) { + 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) { @@ -277,5 +247,4 @@ public TimeValue getStaleness(final ResponseCacheControl responseCacheControl, f return TimeValue.ofSeconds(age.toSeconds() - freshness.toSeconds()); } - } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java index 0e1161b03..8bd8e8486 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java @@ -26,8 +26,16 @@ */ package org.apache.hc.client5.http.impl.cache; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Instant; +import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Set; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.utils.DateUtils; @@ -51,7 +59,6 @@ class CachedResponseSuitabilityChecker { private static final Logger LOG = LoggerFactory.getLogger(CachedResponseSuitabilityChecker.class); private final boolean sharedCache; - private final boolean useHeuristicCaching; private final CacheValidityPolicy validityStrategy; CachedResponseSuitabilityChecker(final CacheValidityPolicy validityStrategy, @@ -59,140 +66,192 @@ class CachedResponseSuitabilityChecker { super(); this.validityStrategy = validityStrategy; this.sharedCache = config.isSharedCache(); - this.useHeuristicCaching = config.isHeuristicCachingEnabled(); } CachedResponseSuitabilityChecker(final CacheConfig config) { this(new CacheValidityPolicy(config), config); } - private boolean isFreshEnough(final RequestCacheControl requestCacheControl, - final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry, - final Instant now) { - if (validityStrategy.isResponseFresh(responseCacheControl, entry, now)) { - return true; - } - if (useHeuristicCaching && - validityStrategy.isResponseHeuristicallyFresh(entry, now)) { - return true; - } - if (originInsistsOnFreshness(responseCacheControl)) { - return false; - } - if (requestCacheControl.getMaxStale() == -1) { - return false; + /** + * Determine if I can utilize the given {@link HttpCacheEntry} to respond to the given + * {@link HttpRequest}. + * + * @since 5.3 + */ + public CacheSuitability assessSuitability(final RequestCacheControl requestCacheControl, + final ResponseCacheControl responseCacheControl, + final HttpRequest request, + final HttpCacheEntry entry, + final Instant now) { + if (!requestMethodMatch(request, entry)) { + LOG.debug("Request method and the cache entry method do not match"); + return CacheSuitability.MISMATCH; } - return (requestCacheControl.getMaxStale() > validityStrategy.getStaleness(responseCacheControl, entry, now).toSeconds()); - } - private boolean originInsistsOnFreshness(final ResponseCacheControl responseCacheControl) { - if (responseCacheControl.isMustRevalidate()) { - return true; + if (!requestUriMatch(request, entry)) { + LOG.debug("Target request URI and the cache entry request URI do not match"); + return CacheSuitability.MISMATCH; } - if (!sharedCache) { - return false; + + if (!requestHeadersMatch(request, entry)) { + LOG.debug("Request headers nominated by the cached response do not match those of the request associated with the cache entry"); + return CacheSuitability.MISMATCH; } - return responseCacheControl.isProxyRevalidate() || responseCacheControl.getSharedMaxAge() >= 0; - } - /** - * Determine if I can utilize a {@link HttpCacheEntry} to respond to the given - * {@link HttpRequest} - * @since 5.3 - */ - public boolean canCachedResponseBeUsed(final RequestCacheControl requestCacheControl, - final ResponseCacheControl responseCacheControl, final HttpRequest request, - final HttpCacheEntry entry, final Instant now) { + if (!requestHeadersMatch(request, entry)) { + LOG.debug("Request headers nominated by the cached response do not match those of the request associated with the cache entry"); + return CacheSuitability.MISMATCH; + } - if (isGetRequestWithHeadCacheEntry(request, entry)) { - LOG.debug("Cache entry created by HEAD request cannot be used to serve GET request"); - return false; + if (requestCacheControl.isNoCache()) { + LOG.debug("Request contained no-cache directive; the cache entry must be re-validated"); + return CacheSuitability.REVALIDATION_REQUIRED; } - if (!isFreshEnough(requestCacheControl, responseCacheControl, entry, now)) { - LOG.debug("Cache entry is not fresh enough"); - return false; + if (isResponseNoCache(responseCacheControl, entry)) { + LOG.debug("Response contained no-cache directive; the cache entry must be re-validated"); + return CacheSuitability.REVALIDATION_REQUIRED; } if (hasUnsupportedConditionalHeaders(request)) { - LOG.debug("Request contains unsupported conditional headers"); - return false; + LOG.debug("Response from cache is not suitable due to the request containing unsupported conditional headers"); + return CacheSuitability.REVALIDATION_REQUIRED; } if (!isConditional(request) && entry.getStatus() == HttpStatus.SC_NOT_MODIFIED) { LOG.debug("Unconditional request and non-modified cached response"); - return false; + return CacheSuitability.REVALIDATION_REQUIRED; } - if (isConditional(request) && !allConditionalsMatch(request, entry, now)) { - LOG.debug("Conditional request and with mismatched conditions"); - return false; + if (!allConditionalsMatch(request, entry, now)) { + LOG.debug("Response from cache is not suitable due to the conditional request and with mismatched conditions"); + return CacheSuitability.REVALIDATION_REQUIRED; } - if (hasUnsupportedCacheEntryForGet(request, entry)) { - LOG.debug("HEAD response caching enabled but the cache entry does not contain a " + - "request method, entity or a 204 response"); - return false; - } - if (requestCacheControl.isNoCache()) { - LOG.debug("Response contained NO CACHE directive, cache was not suitable"); - return false; + final TimeValue currentAge = validityStrategy.getCurrentAge(entry, now); + final TimeValue freshnessLifetime = validityStrategy.getFreshnessLifetime(responseCacheControl, entry); + + final boolean fresh = currentAge.compareTo(freshnessLifetime) < 0; + + if (!fresh && responseCacheControl.isMustRevalidate()) { + LOG.debug("Response from cache is not suitable due to the response must-revalidate requirement"); + return CacheSuitability.REVALIDATION_REQUIRED; } - if (requestCacheControl.isNoStore()) { - LOG.debug("Response contained NO STORE directive, cache was not suitable"); - return false; + if (!fresh && sharedCache && responseCacheControl.isProxyRevalidate()) { + LOG.debug("Response from cache is not suitable due to the response proxy-revalidate requirement"); + return CacheSuitability.REVALIDATION_REQUIRED; } - if (requestCacheControl.getMaxAge() >= 0) { - if (validityStrategy.getCurrentAge(entry, now).toSeconds() > requestCacheControl.getMaxAge()) { - LOG.debug("Response from cache was not suitable due to max age"); - return false; + if (fresh && requestCacheControl.getMaxAge() >= 0) { + if (currentAge.toSeconds() > requestCacheControl.getMaxAge() && requestCacheControl.getMaxStale() == -1) { + LOG.debug("Response from cache is not suitable due to the request max-age requirement"); + return CacheSuitability.REVALIDATION_REQUIRED; } } - if (requestCacheControl.getMaxStale() >= 0) { - if (validityStrategy.getFreshnessLifetime(responseCacheControl, entry).toSeconds() > requestCacheControl.getMaxStale()) { - LOG.debug("Response from cache was not suitable due to max stale freshness"); - return false; + if (fresh && requestCacheControl.getMinFresh() >= 0) { + if (requestCacheControl.getMinFresh() == 0 || + freshnessLifetime.toSeconds() - currentAge.toSeconds() < requestCacheControl.getMinFresh()) { + LOG.debug("Response from cache is not suitable due to the request min-fresh requirement"); + return CacheSuitability.REVALIDATION_REQUIRED; } } - if (requestCacheControl.getMinFresh() >= 0) { - if (requestCacheControl.getMinFresh() == 0) { - return false; - } - final TimeValue age = validityStrategy.getCurrentAge(entry, now); - final TimeValue freshness = validityStrategy.getFreshnessLifetime(responseCacheControl, entry); - if (freshness.toSeconds() - age.toSeconds() < requestCacheControl.getMinFresh()) { - LOG.debug("Response from cache was not suitable due to min fresh " + - "freshness requirement"); - return false; + if (requestCacheControl.getMaxStale() >= 0) { + final long stale = currentAge.compareTo(freshnessLifetime) > 0 ? currentAge.toSeconds() - freshnessLifetime.toSeconds() : 0; + if (stale >= requestCacheControl.getMaxStale()) { + LOG.debug("Response from cache is not suitable due to the request max-stale requirement"); + return CacheSuitability.REVALIDATION_REQUIRED; + } else { + LOG.debug("The cache entry is fresh enough"); + return CacheSuitability.FRESH_ENOUGH; } } - LOG.debug("Response from cache was suitable"); - return true; - } - - private boolean isGet(final HttpRequest request) { - return Method.GET.isSame(request.getMethod()); + if (fresh) { + LOG.debug("The cache entry is fresh"); + return CacheSuitability.FRESH; + } else { + LOG.debug("The cache entry is stale"); + return CacheSuitability.STALE; + } } - private boolean isHead(final HttpRequest request) { - return Method.HEAD.isSame(request.getMethod()); + boolean requestMethodMatch(final HttpRequest request, final HttpCacheEntry entry) { + return request.getMethod().equalsIgnoreCase(entry.getRequestMethod()) || + (Method.HEAD.isSame(request.getMethod()) && Method.GET.isSame(entry.getRequestMethod())); } - private boolean entryIsNotA204Response(final HttpCacheEntry entry) { - return entry.getStatus() != HttpStatus.SC_NO_CONTENT; + boolean requestUriMatch(final HttpRequest request, final HttpCacheEntry entry) { + try { + final URI requestURI = CacheKeyGenerator.normalize(request.getUri()); + final URI cacheURI = new URI(entry.getRequestURI()); + if (requestURI.isAbsolute()) { + return Objects.equals(requestURI, cacheURI); + } else { + return Objects.equals(requestURI.getPath(), cacheURI.getPath()) && Objects.equals(requestURI.getQuery(), cacheURI.getQuery()); + } + } catch (final URISyntaxException ex) { + return false; + } } - private boolean cacheEntryDoesNotContainMethodAndEntity(final HttpCacheEntry entry) { - return entry.getRequestMethod() == null && entry.getResource() == null; + boolean requestHeadersMatch(final HttpRequest request, final HttpCacheEntry entry) { + final Iterator
it = entry.headerIterator(HttpHeaders.VARY); + if (it.hasNext()) { + final Set headerNames = new HashSet<>(); + while (it.hasNext()) { + final Header header = it.next(); + CacheSupport.parseTokens(header, e -> { + headerNames.add(e.toLowerCase(Locale.ROOT)); + }); + } + final List tokensInRequest = new ArrayList<>(); + final List tokensInCache = new ArrayList<>(); + for (final String headerName: headerNames) { + if (headerName.equalsIgnoreCase("*")) { + return false; + } + CacheKeyGenerator.normalizeTokens(request.headerIterator(headerName), tokensInRequest::add); + CacheKeyGenerator.normalizeTokens(entry.requestHeaderIterator(headerName), tokensInCache::add); + if (!Objects.equals(tokensInRequest, tokensInCache)) { + return false; + } + } + } + return true; } - private boolean hasUnsupportedCacheEntryForGet(final HttpRequest request, final HttpCacheEntry entry) { - return isGet(request) && cacheEntryDoesNotContainMethodAndEntity(entry) && entryIsNotA204Response(entry); + /** + * Determines if the given {@link HttpCacheEntry} requires revalidation based on the presence of the {@code no-cache} directive + * in the Cache-Control header. + *

+ * The method returns true in the following cases: + * - If the {@code no-cache} directive is present without any field names (unqualified). + * - If the {@code no-cache} directive is present with field names, and at least one of these field names is present + * in the headers of the {@link HttpCacheEntry}. + *

+ * If the {@code no-cache} directive is not present in the Cache-Control header, the method returns {@code false}. + */ + boolean isResponseNoCache(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry) { + // If no-cache directive is present and has no field names + if (responseCacheControl.isNoCache()) { + final Set noCacheFields = responseCacheControl.getNoCacheFields(); + if (noCacheFields.isEmpty()) { + LOG.debug("Revalidation required due to unqualified no-cache directive"); + return true; + } + for (final String field : noCacheFields) { + if (entry.containsHeader(field)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Revalidation required due to no-cache directive with field {}", field); + } + return true; + } + } + } + return false; } /** @@ -204,22 +263,6 @@ public boolean isConditional(final HttpRequest request) { return hasSupportedEtagValidator(request) || hasSupportedLastModifiedValidator(request); } - /** - * Determines whether the given request is a {@link org.apache.hc.core5.http.Method#GET} request and the - * associated cache entry was created by a {@link org.apache.hc.core5.http.Method#HEAD} request. - * - * @param request The {@link HttpRequest} to check if it is a {@link org.apache.hc.core5.http.Method#GET} request. - * @param entry The {@link HttpCacheEntry} to check if it was created by - * a {@link org.apache.hc.core5.http.Method#HEAD} request. - * @return true if the request is a {@link org.apache.hc.core5.http.Method#GET} request and the cache entry was - * created by a {@link org.apache.hc.core5.http.Method#HEAD} request, otherwise {@code false}. - * @since 5.3 - */ - public boolean isGetRequestWithHeadCacheEntry(final HttpRequest request, final HttpCacheEntry entry) { - return isGet(request) && Method.HEAD.isSame(entry.getRequestMethod()); - } - - /** * Check that conditionals that are part of this request match * @param request The current httpRequest being made @@ -231,6 +274,10 @@ public boolean allConditionalsMatch(final HttpRequest request, final HttpCacheEn final boolean hasEtagValidator = hasSupportedEtagValidator(request); final boolean hasLastModifiedValidator = hasSupportedLastModifiedValidator(request); + if (!hasEtagValidator && !hasLastModifiedValidator) { + return true; + } + final boolean etagValidatorMatches = (hasEtagValidator) && etagValidatorMatches(request, entry); final boolean lastModifiedValidatorMatches = (hasLastModifiedValidator) && lastModifiedValidatorMatches(request, entry, now); @@ -244,18 +291,18 @@ public boolean allConditionalsMatch(final HttpRequest request, final HttpCacheEn return !hasLastModifiedValidator || lastModifiedValidatorMatches; } - private boolean hasUnsupportedConditionalHeaders(final HttpRequest request) { - return (request.getFirstHeader(HttpHeaders.IF_RANGE) != null - || request.getFirstHeader(HttpHeaders.IF_MATCH) != null - || hasValidDateField(request, HttpHeaders.IF_UNMODIFIED_SINCE)); + boolean hasUnsupportedConditionalHeaders(final HttpRequest request) { + return (request.containsHeader(HttpHeaders.IF_RANGE) + || request.containsHeader(HttpHeaders.IF_MATCH) + || request.containsHeader(HttpHeaders.IF_UNMODIFIED_SINCE)); } - private boolean hasSupportedEtagValidator(final HttpRequest request) { + boolean hasSupportedEtagValidator(final HttpRequest request) { return request.containsHeader(HttpHeaders.IF_NONE_MATCH); } - private boolean hasSupportedLastModifiedValidator(final HttpRequest request) { - return hasValidDateField(request, HttpHeaders.IF_MODIFIED_SINCE); + boolean hasSupportedLastModifiedValidator(final HttpRequest request) { + return request.containsHeader(HttpHeaders.IF_MODIFIED_SINCE); } /** @@ -264,7 +311,7 @@ private boolean hasSupportedLastModifiedValidator(final HttpRequest request) { * @param entry the cache entry * @return boolean does the etag validator match */ - private boolean etagValidatorMatches(final HttpRequest request, final HttpCacheEntry entry) { + boolean etagValidatorMatches(final HttpRequest request, final HttpCacheEntry entry) { final Header etagHeader = entry.getFirstHeader(HttpHeaders.ETAG); final String etag = (etagHeader != null) ? etagHeader.getValue() : null; final Iterator it = MessageSupport.iterate(request, HttpHeaders.IF_NONE_MATCH); @@ -285,7 +332,7 @@ private boolean etagValidatorMatches(final HttpRequest request, final HttpCacheE * @param now right NOW in time * @return boolean Does the last modified header match */ - private boolean lastModifiedValidatorMatches(final HttpRequest request, final HttpCacheEntry entry, final Instant now) { + boolean lastModifiedValidatorMatches(final HttpRequest request, final HttpCacheEntry entry, final Instant now) { final Instant lastModified = entry.getLastModified(); if (lastModified == null) { return false; @@ -302,11 +349,4 @@ private boolean lastModifiedValidatorMatches(final HttpRequest request, final Ht return true; } - private boolean hasValidDateField(final HttpRequest request, final String headerName) { - for(final Header h : request.getHeaders(headerName)) { - final Instant instant = DateUtils.parseStandardDate(h.getValue()); - return instant != null; - } - return false; - } } \ No newline at end of file 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 6311d8865..62e592c14 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 @@ -168,6 +168,9 @@ public ClassicHttpResponse execute( final CacheHit root = result != null ? result.root : null; final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request); + if (LOG.isDebugEnabled()) { + LOG.debug("Request cache control: {}", requestCacheControl); + } if (!cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) { LOG.debug("Request is not servable from cache"); return callBackend(target, request, scope, chain); @@ -179,6 +182,9 @@ public ClassicHttpResponse execute( return handleCacheMiss(requestCacheControl, root, target, request, scope, chain); } else { final ResponseCacheControl responseCacheControl = CacheControlHeaderParser.INSTANCE.parse(hit.entry); + if (LOG.isDebugEnabled()) { + LOG.debug("Response cache control: {}", responseCacheControl); + } return handleCacheHit(requestCacheControl, responseCacheControl, hit, target, request, scope, chain); } } @@ -238,19 +244,11 @@ private ClassicHttpResponse handleCacheHit( recordCacheHit(target, request); final Instant now = getCurrentDate(); - if (requestCacheControl.isNoCache()) { - // Revalidate with the server - return revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, scope, chain); + final CacheSuitability cacheSuitability = suitabilityChecker.assessSuitability(requestCacheControl, responseCacheControl, request, hit.entry, now); + if (LOG.isDebugEnabled()) { + LOG.debug("Request {} {}: {}", request.getMethod(), request.getRequestUri(), cacheSuitability); } - - if (suitabilityChecker.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, hit.entry, now)) { - if (responseCachingPolicy.responseContainsNoCacheDirective(responseCacheControl, hit.entry)) { - // Revalidate with the server due to no-cache directive in response - if (LOG.isDebugEnabled()) { - LOG.debug("Revalidating with server due to no-cache directive in response."); - } - return revalidateCacheEntry(requestCacheControl, responseCacheControl, hit, target, request, scope, chain); - } + if (cacheSuitability == CacheSuitability.FRESH || cacheSuitability == CacheSuitability.FRESH_ENOUGH) { LOG.debug("Cache hit"); try { return convert(generateCachedResponse(hit.entry, request, context), scope); @@ -262,44 +260,50 @@ private ClassicHttpResponse handleCacheHit( setResponseStatus(scope.clientContext, CacheResponseStatus.FAILURE); return chain.proceed(request, scope); } - } else if (!mayCallBackend(requestCacheControl)) { - LOG.debug("Cache entry not suitable but only-if-cached requested"); - return convert(generateGatewayTimeout(context), scope); - } else if (!(hit.entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { - LOG.debug("Revalidating cache entry"); - final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(responseCacheControl, hit.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"); + } else { + if (!mayCallBackend(requestCacheControl)) { + LOG.debug("Cache entry not suitable but only-if-cached requested"); + return convert(generateGatewayTimeout(context), scope); + } + if (cacheSuitability == CacheSuitability.MISMATCH) { + LOG.debug("Cache entry does not match the request; calling backend"); + return callBackend(target, request, scope, chain); + } else if (!(hit.entry.getStatus() == HttpStatus.SC_NOT_MODIFIED && !suitabilityChecker.isConditional(request))) { + LOG.debug("Revalidating cache entry"); + final boolean staleIfErrorEnabled = responseCachingPolicy.isStaleIfErrorEnabled(responseCacheControl, hit.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(generateCachedResponse(hit.entry, request, context), scope); + return convert(handleRevalidationFailure(requestCacheControl, responseCacheControl, hit.entry, request, context, now), scope); } - return convert(handleRevalidationFailure(requestCacheControl, responseCacheControl, hit.entry, request, context, now), scope); + } else { + LOG.debug("Cache entry not usable; calling backend"); + return callBackend(target, request, scope, chain); } - } else { - LOG.debug("Cache entry not usable; calling backend"); - return callBackend(target, request, scope, chain); } } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCacheControl.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCacheControl.java index 33469b533..4e548bd4d 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCacheControl.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCacheControl.java @@ -27,7 +27,9 @@ package org.apache.hc.client5.http.impl.cache; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Set; import org.apache.hc.core5.annotation.Contract; @@ -226,7 +228,6 @@ public boolean isMustRevalidate() { return mustRevalidate; } - /** * Returns whether the proxy-revalidate value is set in the Cache-Control header. * @@ -433,6 +434,12 @@ public Builder setNoCacheFields(final Set noCacheFields) { return this; } + public Builder setNoCacheFields(final String... noCacheFields) { + this.noCacheFields = new HashSet<>(); + this.noCacheFields.addAll(Arrays.asList(noCacheFields)); + return this; + } + public boolean isMustUnderstand() { return mustUnderstand; } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java index 47472f06d..67abff2ff 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java @@ -29,7 +29,6 @@ import java.time.Duration; import java.time.Instant; import java.util.Iterator; -import java.util.Set; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.utils.DateUtils; @@ -397,43 +396,6 @@ boolean isStaleIfErrorEnabled(final ResponseCacheControl cacheControl, final Htt return false; } - /** - * Determines if the given {@link HttpCacheEntry} requires revalidation based on the presence of the {@code no-cache} directive - * in the Cache-Control header. - *

- * The method returns true in the following cases: - * - If the {@code no-cache} directive is present without any field names. - * - If the {@code no-cache} directive is present with field names, and at least one of these field names is present - * in the headers of the {@link HttpCacheEntry}. - *

- * If the {@code no-cache} directive is not present in the Cache-Control header, the method returns {@code false}. - * - * @param entry the {@link HttpCacheEntry} containing the headers to check for the {@code no-cache} directive. - * @return true if revalidation is required based on the {@code no-cache} directive, {@code false} otherwise. - */ - boolean responseContainsNoCacheDirective(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry) { - final Set noCacheFields = responseCacheControl.getNoCacheFields(); - - // If no-cache directive is present and has no field names - if (responseCacheControl.isNoCache() && noCacheFields.isEmpty()) { - LOG.debug("No-cache directive present without field names. Revalidation required."); - return true; - } - - // If no-cache directive is present with field names - if (responseCacheControl.isNoCache()) { - for (final String field : noCacheFields) { - if (entry.getFirstHeader(field) != null) { - if (LOG.isDebugEnabled()) { - LOG.debug("No-cache directive field '{}' found in response headers. Revalidation required.", field); - } - return true; - } - } - } - return false; - } - /** * Understood status codes include: * - All 2xx (Successful) status codes (200-299) diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java index 8a41bbb1b..d5c6942ed 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java @@ -28,15 +28,20 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; +import java.util.List; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.message.BasicHeaderIterator; import org.apache.hc.core5.http.message.BasicHttpRequest; import org.apache.hc.core5.http.support.BasicRequestBuilder; import org.junit.jupiter.api.Assertions; @@ -295,6 +300,33 @@ public void testGetURIWithQueryParameters() { "/full_episodes?foo=bar"))); } + private static Iterator

headers(final Header... headers) { + return new BasicHeaderIterator(headers, null); + } + + @Test + public void testNormalizeHeaderTokens() { + final List tokens = new ArrayList<>(); + CacheKeyGenerator.normalizeTokens(headers( + new BasicHeader("Accept-Encoding", "gzip,zip,deflate") + ), tokens::add); + Assertions.assertEquals(Arrays.asList("deflate", "gzip", "zip"), tokens); + + tokens.clear(); + CacheKeyGenerator.normalizeTokens(headers( + new BasicHeader("Accept-Encoding", " gZip , Zip, , , deflate ") + ), tokens::add); + Assertions.assertEquals(Arrays.asList("deflate", "gzip", "zip"), tokens); + + tokens.clear(); + CacheKeyGenerator.normalizeTokens(headers( + new BasicHeader("Accept-Encoding", "gZip,Zip,,"), + new BasicHeader("Accept-Encoding", " gZip,Zip,,,"), + new BasicHeader("Accept-Encoding", "gZip, ,,,deflate") + ), tokens::add); + Assertions.assertEquals(Arrays.asList("deflate", "gzip", "zip"), tokens); + } + @Test public void testGetVariantKey() { final HttpRequest request = BasicRequestBuilder.get("/blah") diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java index a8e9f407f..11672d099 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java @@ -29,7 +29,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.Instant; @@ -244,26 +243,6 @@ public void testHeuristicFreshnessLifetimeIsNonNegative() { assertTrue(TimeValue.isNonNegative(impl.getHeuristicFreshnessLifetime(entry))); } - @Test - public void testResponseIsFreshIfFreshnessLifetimeExceedsCurrentAge() { - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - final ResponseCacheControl responseCacheControl = ResponseCacheControl.builder().build(); - impl = new CacheValidityPolicy() { - @Override - public TimeValue getCurrentAge(final HttpCacheEntry e, final Instant d) { - assertSame(entry, e); - assertEquals(now, d); - return TimeValue.ofSeconds(6); - } - @Override - public TimeValue getFreshnessLifetime(final ResponseCacheControl cacheControl, final HttpCacheEntry e) { - assertSame(entry, e); - return TimeValue.ofSeconds(10); - } - }; - assertTrue(impl.isResponseFresh(responseCacheControl, entry, now)); - } - @Test public void testHeuristicFreshnessLifetimeCustomProperly() { final CacheConfig cacheConfig = CacheConfig.custom().setHeuristicDefaultLifetime(TimeValue.ofSeconds(10)) @@ -274,46 +253,6 @@ public void testHeuristicFreshnessLifetimeCustomProperly() { assertEquals(defaultFreshness, impl.getHeuristicFreshnessLifetime(entry)); } - @Test - public void testResponseIsNotFreshIfFreshnessLifetimeEqualsCurrentAge() { - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - final ResponseCacheControl responseCacheControl = ResponseCacheControl.builder().build(); - impl = new CacheValidityPolicy() { - @Override - public TimeValue getCurrentAge(final HttpCacheEntry e, final Instant d) { - assertEquals(now, d); - assertSame(entry, e); - return TimeValue.ofSeconds(6); - } - @Override - public TimeValue getFreshnessLifetime(final ResponseCacheControl cacheControl, final HttpCacheEntry e) { - assertSame(entry, e); - return TimeValue.ofSeconds(6); - } - }; - assertFalse(impl.isResponseFresh(responseCacheControl, entry, now)); - } - - @Test - public void testResponseIsNotFreshIfCurrentAgeExceedsFreshnessLifetime() { - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - final ResponseCacheControl responseCacheControl = ResponseCacheControl.builder().build(); - impl = new CacheValidityPolicy() { - @Override - public TimeValue getCurrentAge(final HttpCacheEntry e, final Instant d) { - assertEquals(now, d); - assertSame(entry, e); - return TimeValue.ofSeconds(10); - } - @Override - public TimeValue getFreshnessLifetime(final ResponseCacheControl cacheControl, final HttpCacheEntry e) { - assertSame(entry, e); - return TimeValue.ofSeconds(6); - } - }; - assertFalse(impl.isResponseFresh(responseCacheControl, entry, now)); - } - @Test public void testCacheEntryIsRevalidatableIfHeadersIncludeETag() { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java index bd23f14b5..051a9d597 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java @@ -31,11 +31,12 @@ import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; -import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http.message.BasicHttpRequest; +import org.apache.hc.core5.http.support.BasicRequestBuilder; import org.apache.hc.core5.util.TimeValue; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -69,32 +70,204 @@ public void setUp() { impl = new CachedResponseSuitabilityChecker(CacheConfig.DEFAULT); } - private HttpCacheEntry getEntry(final Header[] headers) { - return HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, headers); + private HttpCacheEntry makeEntry(final Instant requestDate, + final Instant responseDate, + final Method method, + final String requestUri, + final Header[] requestHeaders, + final int status, + final Header[] responseHeaders) { + return HttpTestUtils.makeCacheEntry(requestDate, responseDate, method, requestUri, requestHeaders, + status, responseHeaders, HttpTestUtils.makeNullResource()); + } + + private HttpCacheEntry makeEntry(final Header... headers) { + return makeEntry(elevenSecondsAgo, nineSecondsAgo, Method.GET, "/foo", null, 200, headers); + } + + private HttpCacheEntry makeEntry(final Instant requestDate, + final Instant responseDate, + final Header... headers) { + return makeEntry(requestDate, responseDate, Method.GET, "/foo", null, 200, headers); + } + + private HttpCacheEntry makeEntry(final Method method, final String requestUri, final Header... headers) { + return makeEntry(elevenSecondsAgo, nineSecondsAgo, method, requestUri, null, 200, headers); + } + + private HttpCacheEntry makeEntry(final Method method, final String requestUri, final Header[] requestHeaders, + final int status, final Header[] responseHeaders) { + return makeEntry(elevenSecondsAgo, nineSecondsAgo, method, requestUri, requestHeaders, + status, responseHeaders); + } + + @Test + public void testRequestMethodMatch() { + request = new BasicHttpRequest("GET", "/foo"); + entry = makeEntry(Method.GET, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertTrue(impl.requestMethodMatch(request, entry)); + + request = new BasicHttpRequest("HEAD", "/foo"); + Assertions.assertTrue(impl.requestMethodMatch(request, entry)); + + request = new BasicHttpRequest("POST", "/foo"); + Assertions.assertFalse(impl.requestMethodMatch(request, entry)); + + request = new BasicHttpRequest("HEAD", "/foo"); + entry = makeEntry(Method.HEAD, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertTrue(impl.requestMethodMatch(request, entry)); + + request = new BasicHttpRequest("GET", "/foo"); + entry = makeEntry(Method.HEAD, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertFalse(impl.requestMethodMatch(request, entry)); + } + + @Test + public void testRequestUriMatch() { + request = new BasicHttpRequest("GET", "/foo"); + entry = makeEntry(Method.GET, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertTrue(impl.requestUriMatch(request, entry)); + + request = new BasicHttpRequest("GET", new HttpHost("some-host"), "/foo"); + entry = makeEntry(Method.GET, "http://some-host:80/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertTrue(impl.requestUriMatch(request, entry)); + + request = new BasicHttpRequest("GET", new HttpHost("Some-Host"), "/foo?bar"); + entry = makeEntry(Method.GET, "http://some-host:80/foo?bar", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertTrue(impl.requestUriMatch(request, entry)); + + request = new BasicHttpRequest("GET", new HttpHost("some-other-host"), "/foo"); + entry = makeEntry(Method.GET, "http://some-host:80/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertFalse(impl.requestUriMatch(request, entry)); + + request = new BasicHttpRequest("GET", new HttpHost("some-host"), "/foo?huh"); + entry = makeEntry(Method.GET, "http://some-host:80/foo?bar", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertFalse(impl.requestUriMatch(request, entry)); + } + + @Test + public void testRequestHeadersMatch() { + request = BasicRequestBuilder.get("/foo").build(); + entry = makeEntry( + Method.GET, "/foo", + new Header[]{}, + 200, + new Header[]{ + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) + }); + Assertions.assertTrue(impl.requestHeadersMatch(request, entry)); + + request = BasicRequestBuilder.get("/foo").build(); + entry = makeEntry( + Method.GET, "/foo", + new Header[]{}, + 200, + new Header[]{ + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), + new BasicHeader("Vary", "") + }); + Assertions.assertTrue(impl.requestHeadersMatch(request, entry)); + + request = BasicRequestBuilder.get("/foo") + .addHeader("Accept-Encoding", "blah") + .build(); + entry = makeEntry( + Method.GET, "/foo", + new Header[]{ + new BasicHeader("Accept-Encoding", "blah") + }, + 200, + new Header[]{ + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), + new BasicHeader("Vary", "Accept-Encoding") + }); + Assertions.assertTrue(impl.requestHeadersMatch(request, entry)); + + request = BasicRequestBuilder.get("/foo") + .addHeader("Accept-Encoding", "gzip, deflate, deflate , zip, ") + .build(); + entry = makeEntry( + Method.GET, "/foo", + new Header[]{ + new BasicHeader("Accept-Encoding", " gzip, zip, deflate") + }, + 200, + new Header[]{ + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), + new BasicHeader("Vary", "Accept-Encoding") + }); + Assertions.assertTrue(impl.requestHeadersMatch(request, entry)); + + request = BasicRequestBuilder.get("/foo") + .addHeader("Accept-Encoding", "gzip, deflate, zip") + .build(); + entry = makeEntry( + Method.GET, "/foo", + new Header[]{ + new BasicHeader("Accept-Encoding", " gzip, deflate") + }, + 200, + new Header[]{ + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), + new BasicHeader("Vary", "Accept-Encoding") + }); + Assertions.assertFalse(impl.requestHeadersMatch(request, entry)); + } + + @Test + public void testResponseNoCache() { + entry = makeEntry(new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + responseCacheControl = ResponseCacheControl.builder() + .setNoCache(false) + .build(); + + Assertions.assertFalse(impl.isResponseNoCache(responseCacheControl, entry)); + + responseCacheControl = ResponseCacheControl.builder() + .setNoCache(true) + .build(); + + Assertions.assertTrue(impl.isResponseNoCache(responseCacheControl, entry)); + + responseCacheControl = ResponseCacheControl.builder() + .setNoCache(true) + .setNoCacheFields("stuff", "more-stuff") + .build(); + + Assertions.assertFalse(impl.isResponseNoCache(responseCacheControl, entry)); + + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), + new BasicHeader("stuff", "booh")); + + Assertions.assertTrue(impl.isResponseNoCache(responseCacheControl, entry)); } @Test public void testSuitableIfCacheEntryIsFresh() { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry(new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test public void testNotSuitableIfCacheEntryIsNotFresh() { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(5) .build(); - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.STALE, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -102,14 +275,12 @@ public void testNotSuitableIfRequestHasNoCache() { requestCacheControl = RequestCacheControl.builder() .setNoCache(true) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.REVALIDATION_REQUIRED, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -117,14 +288,12 @@ public void testNotSuitableIfAgeExceedsRequestMaxAge() { requestCacheControl = RequestCacheControl.builder() .setMaxAge(10) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - entry = getEntry(headers); - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); + Assertions.assertEquals(CacheSuitability.REVALIDATION_REQUIRED, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -132,14 +301,12 @@ public void testSuitableIfFreshAndAgeIsUnderRequestMaxAge() { requestCacheControl = RequestCacheControl.builder() .setMaxAge(15) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -147,14 +314,12 @@ public void testSuitableIfFreshAndFreshnessLifetimeGreaterThanRequestMinFresh() requestCacheControl = RequestCacheControl.builder() .setMinFresh(10) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -162,14 +327,12 @@ public void testNotSuitableIfFreshnessLifetimeLessThanRequestMinFresh() { requestCacheControl = RequestCacheControl.builder() .setMinFresh(10) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(15) .build(); - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.REVALIDATION_REQUIRED, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -180,11 +343,11 @@ public void testSuitableEvenIfStaleButPermittedByRequestMaxStale() { final Header[] headers = { new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; - entry = getEntry(headers); + entry = makeEntry(headers); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(5) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH_ENOUGH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -192,14 +355,12 @@ public void testNotSuitableIfStaleButTooStaleForRequestMaxStale() { requestCacheControl = RequestCacheControl.builder() .setMaxStale(2) .build(); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(5) .build(); - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.REVALIDATION_REQUIRED, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -207,28 +368,22 @@ public void testSuitableIfCacheEntryIsHeuristicallyFreshEnough() { final Instant oneSecondAgo = now.minusSeconds(1); final Instant twentyOneSecondsAgo = now.minusSeconds(21); - final Header[] headers = { + entry = makeEntry(oneSecondAgo, oneSecondAgo, new BasicHeader("Date", DateUtils.formatStandardDate(oneSecondAgo)), - new BasicHeader("Last-Modified", DateUtils.formatStandardDate(twentyOneSecondsAgo)) - }; - - entry = HttpTestUtils.makeCacheEntry(oneSecondAgo, oneSecondAgo, headers); + new BasicHeader("Last-Modified", DateUtils.formatStandardDate(twentyOneSecondsAgo))); final CacheConfig config = CacheConfig.custom() .setHeuristicCachingEnabled(true) .setHeuristicCoefficient(0.1f).build(); impl = new CachedResponseSuitabilityChecker(config); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test public void testSuitableIfCacheEntryIsHeuristicallyFreshEnoughByDefault() { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); final CacheConfig config = CacheConfig.custom() .setHeuristicCachingEnabled(true) @@ -236,71 +391,43 @@ public void testSuitableIfCacheEntryIsHeuristicallyFreshEnoughByDefault() { .build(); impl = new CachedResponseSuitabilityChecker(config); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test public void testSuitableIfRequestMethodisHEAD() { final HttpRequest headRequest = new BasicHttpRequest("HEAD", "/foo"); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = getEntry(headers); + entry = makeEntry( + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, headRequest, entry, now)); - } - - @Test - public void testNotSuitableIfRequestMethodIsGETAndEntryResourceIsNull() { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, - Method.HEAD, "/", null, - HttpStatus.SC_OK, headers, - HttpTestUtils.makeNullResource()); - responseCacheControl = ResponseCacheControl.builder() - .setMaxAge(3600) - .build(); - - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, headRequest, entry, now)); } @Test public void testSuitableForGETIfEntryDoesNotSpecifyARequestMethodButContainsEntity() { impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) - }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, - Method.GET, "/", null, - HttpStatus.SC_OK, headers, - HttpTestUtils.makeRandomResource(128)); + entry = makeEntry(Method.GET, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test public void testSuitableForGETIfHeadResponseCachingEnabledAndEntryDoesNotSpecifyARequestMethodButContains204Response() { impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, - Method.GET, "/", null, - HttpStatus.SC_OK, headers, - HttpTestUtils.makeNullResource()); + entry = makeEntry(Method.GET, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } @Test @@ -308,33 +435,26 @@ public void testSuitableForHEADIfHeadResponseCachingEnabledAndEntryDoesNotSpecif final HttpRequest headRequest = new BasicHttpRequest("HEAD", "/foo"); impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) + }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, - Method.GET, "/", null, - HttpStatus.SC_OK, headers, - HttpTestUtils.makeNullResource()); + entry = makeEntry(Method.GET, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); - Assertions.assertTrue(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, headRequest, entry, now)); + Assertions.assertEquals(CacheSuitability.FRESH, impl.assessSuitability(requestCacheControl, responseCacheControl, headRequest, entry, now)); } @Test public void testNotSuitableIfGetRequestWithHeadCacheEntry() { // Prepare a cache entry with HEAD method - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, - Method.HEAD, "/", null, - HttpStatus.SC_OK, headers, - HttpTestUtils.makeNullResource()); + entry = makeEntry(Method.HEAD, "/foo", + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo))); responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) .build(); // Validate that the cache entry is not suitable for the GET request - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); + Assertions.assertEquals(CacheSuitability.MISMATCH, impl.assessSuitability(requestCacheControl, responseCacheControl, request, entry, now)); } } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java index 404a99b72..53a420fec 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java @@ -53,6 +53,7 @@ import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.Method; 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; @@ -102,7 +103,7 @@ public void setUp() throws Exception { body = HttpTestUtils.makeBody(ENTITY_LENGTH); - request = new BasicClassicHttpRequest("GET", "/foo"); + request = new BasicClassicHttpRequest("GET", "/"); context = HttpClientContext.create(); @@ -537,37 +538,6 @@ public void testCacheEntryIsUpdatedWithNewFieldValuesIn304Response() throws Exce Assertions.assertEquals("junk", result.getFirstHeader("X-Extra").getValue()); } - @Test - public void testMustNotUseMultipartByteRangeContentTypeOnCacheGenerated416Responses() throws Exception { - - originResponse.setEntity(HttpTestUtils.makeBody(ENTITY_LENGTH)); - originResponse.setHeader("Content-Length", "128"); - originResponse.setHeader("Cache-Control", "max-age=3600"); - - final ClassicHttpRequest rangeReq = new BasicClassicHttpRequest("GET", "/"); - rangeReq.setHeader("Range", "bytes=1000-1200"); - - final ClassicHttpResponse orig416 = new BasicClassicHttpResponse(416, - "Requested Range Not Satisfiable"); - - // cache may 416 me right away if it understands byte ranges, - // ok to delegate to origin though - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - Mockito.when(mockExecChain.proceed(RequestEquivalent.eq(rangeReq), Mockito.any())).thenReturn(orig416); - - execute(request); - final ClassicHttpResponse result = execute(rangeReq); - - // might have gotten a 416 from the origin or the cache - Assertions.assertEquals(416, result.getCode()); - final Iterator it = MessageSupport.iterate(result, HttpHeaders.CONTENT_TYPE); - while (it.hasNext()) { - final HeaderElement elt = it.next(); - Assertions.assertFalse("multipart/byteranges".equalsIgnoreCase(elt.getName())); - } - Mockito.verify(mockExecChain, Mockito.times(2)).proceed(Mockito.any(), Mockito.any()); - } - @Test public void testMustReturnACacheEntryIfItCanRevalidateIt() throws Exception { @@ -576,17 +546,12 @@ public void testMustReturnACacheEntryIfItCanRevalidateIt() throws Exception { final Instant nineSecondsAgo = now.minusSeconds(9); final Instant eightSecondsAgo = now.minusSeconds(8); - final Header[] hdrs = new Header[] { - new BasicHeader("Date", DateUtils.formatStandardDate(nineSecondsAgo)), - new BasicHeader("Cache-Control", "max-age=0"), - new BasicHeader("ETag", "\"etag\""), - new BasicHeader("Content-Length", "128") - }; - - final byte[] bytes = new byte[128]; - new Random().nextBytes(bytes); - - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); + final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, + Method.GET, "/thing", null, + 200, new Header[] { + new BasicHeader("Date", DateUtils.formatStandardDate(nineSecondsAgo)), + new BasicHeader("ETag", "\"etag\"") + }, HttpTestUtils.makeNullResource()); impl = new CachingExec(mockCache, null, config); @@ -602,6 +567,12 @@ public void testMustReturnACacheEntryIfItCanRevalidateIt() throws Exception { Mockito.when(mockCache.match(Mockito.eq(host), RequestEquivalent.eq(request))).thenReturn( new CacheMatch(new CacheHit("key", entry), null)); Mockito.when(mockExecChain.proceed(RequestEquivalent.eq(validate), Mockito.any())).thenReturn(notModified); + final HttpCacheEntry updated = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, + Method.GET, "/thing", null, + 200, new Header[] { + new BasicHeader("Date", DateUtils.formatStandardDate(now)), + new BasicHeader("ETag", "\"etag\"") + }, HttpTestUtils.makeNullResource()); Mockito.when(mockCache.update( Mockito.any(), Mockito.any(), @@ -609,7 +580,7 @@ public void testMustReturnACacheEntryIfItCanRevalidateIt() throws Exception { Mockito.any(), Mockito.any(), Mockito.any())) - .thenReturn(new CacheHit("key", HttpTestUtils.makeCacheEntry())); + .thenReturn(new CacheHit("key", updated)); execute(request); @@ -672,7 +643,7 @@ public void testAgeHeaderPopulatedFromCacheEntryCurrentAge() throws Exception { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(tenSecondsAgo, eightSecondsAgo, hdrs, bytes); impl = new CachingExec(mockCache, null, config); - request = new BasicClassicHttpRequest("GET", "/thing"); + request = new BasicClassicHttpRequest("GET", "/"); Mockito.when(mockCache.match(Mockito.eq(host), RequestEquivalent.eq(request))).thenReturn( new CacheMatch(new CacheHit("key", entry), null));