From 5ba07015356816589cb6ae579eb842e811572928 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Mon, 12 Jun 2023 21:55:02 +0200 Subject: [PATCH] HTTPCLIENT-2277: Update Freshness Lifetime Calculation (RFC 9111 4.2.1) This commit enhances the getFreshnessLifetime() method in the CacheValidityPolicy class to better comply with RFC 9111 4.2.1. The method now accounts for a negative Duration between the Date and Expires header fields. --- .../http/impl/cache/CacheValidityPolicy.java | 93 +++++++++++++++---- .../CachedResponseSuitabilityChecker.java | 8 +- .../http/impl/cache/CachingExecBase.java | 2 +- .../impl/cache/TestCacheValidityPolicy.java | 35 +++++-- 4 files changed, 103 insertions(+), 35 deletions(-) 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 1f39716e8..5ac3fbc9f 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 @@ -34,13 +34,38 @@ import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.util.TimeValue; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; class CacheValidityPolicy { + private static final Logger LOG = LoggerFactory.getLogger(CacheValidityPolicy.class); + + public static final TimeValue MAX_AGE = TimeValue.ofSeconds(Integer.MAX_VALUE + 1L); - CacheValidityPolicy() { + + private final float heuristicCoefficient; + private final TimeValue heuristicDefaultLifetime; + + + /** + * Constructs a CacheValidityPolicy with the provided CacheConfig. If the config is null, it will use + * default heuristic coefficient and default heuristic lifetime from CacheConfig.DEFAULT. + * + * @param config The CacheConfig to use for this CacheValidityPolicy. If null, default values are used. + */ + CacheValidityPolicy(final CacheConfig config) { super(); + this.heuristicCoefficient = config != null ? config.getHeuristicCoefficient() : CacheConfig.DEFAULT.getHeuristicCoefficient(); + this.heuristicDefaultLifetime = config != null ? config.getHeuristicDefaultLifetime() : CacheConfig.DEFAULT.getHeuristicDefaultLifetime(); + } + + /** + * Default constructor for CacheValidityPolicy. Initializes the policy with default values. + */ + CacheValidityPolicy() { + this(null); } @@ -48,23 +73,59 @@ class CacheValidityPolicy { return TimeValue.ofSeconds(getCorrectedInitialAge(entry).toSeconds() + getResidentTime(entry, now).toSeconds()); } + /** + * Calculate the freshness lifetime of a response based on the provided cache control and cache entry. + * + * + * @param responseCacheControl the cache control directives associated with the response. + * @param entry the cache entry associated with the response. + * @return the calculated freshness lifetime as a {@link TimeValue}. + */ public TimeValue getFreshnessLifetime(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry) { - final long maxAge = getMaxAge(responseCacheControl); + // If the cache is shared and the s-maxage response directive is present, use its value + final long sharedMaxAge = responseCacheControl.getSharedMaxAge(); + if (sharedMaxAge > -1) { + if (LOG.isDebugEnabled()) { + LOG.debug("Using s-maxage directive for freshness lifetime calculation: {} seconds", sharedMaxAge); + } + return TimeValue.ofSeconds(sharedMaxAge); + } + + // If the max-age response directive is present, use its value + final long maxAge = responseCacheControl.getMaxAge(); if (maxAge > -1) { + if (LOG.isDebugEnabled()) { + LOG.debug("Using max-age directive for freshness lifetime calculation: {} seconds", maxAge); + } return TimeValue.ofSeconds(maxAge); } + // If the Expires response header field is present, use its value minus the value of the Date response header field final Instant dateValue = entry.getInstant(); - if (dateValue == null) { - return TimeValue.ZERO_MILLISECONDS; + if (dateValue != null) { + final Instant expiry = DateUtils.parseStandardDate(entry, HttpHeaders.EXPIRES); + if (expiry != null) { + final Duration diff = Duration.between(dateValue, expiry); + if (diff.isNegative()) { + if (LOG.isDebugEnabled()) { + LOG.debug("Negative freshness lifetime detected. Content is already expired. Returning zero freshness lifetime."); + } + return TimeValue.ZERO_MILLISECONDS; + } + return TimeValue.ofSeconds(diff.getSeconds()); + } } - final Instant expiry = DateUtils.parseStandardDate(entry, HttpHeaders.EXPIRES); - if (expiry == null) { - return TimeValue.ZERO_MILLISECONDS; + // 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."); } - final Duration diff = Duration.between(dateValue, expiry); - return TimeValue.ofSeconds(diff.getSeconds()); + return getHeuristicFreshnessLifetime(entry); } public boolean isResponseFresh(final ResponseCacheControl responseCacheControl, final HttpCacheEntry entry, @@ -82,17 +143,13 @@ class CacheValidityPolicy { * * @param entry the cache entry * @param now what time is it currently (When is right NOW) - * @param coefficient Part of the heuristic for cache entry freshness - * @param defaultLifetime How long can I assume a cache entry is default TTL * @return {@code true} if the response is fresh */ - public boolean isResponseHeuristicallyFresh(final HttpCacheEntry entry, - final Instant now, final float coefficient, final TimeValue defaultLifetime) { - return getCurrentAge(entry, now).compareTo(getHeuristicFreshnessLifetime(entry, coefficient, defaultLifetime)) == -1; + public boolean isResponseHeuristicallyFresh(final HttpCacheEntry entry, final Instant now) { + return getCurrentAge(entry, now).compareTo(getHeuristicFreshnessLifetime(entry)) == -1; } - public TimeValue getHeuristicFreshnessLifetime(final HttpCacheEntry entry, - final float coefficient, final TimeValue defaultLifetime) { + public TimeValue getHeuristicFreshnessLifetime(final HttpCacheEntry entry) { final Instant dateValue = entry.getInstant(); final Instant lastModifiedValue = DateUtils.parseStandardDate(entry, HttpHeaders.LAST_MODIFIED); @@ -102,10 +159,10 @@ class CacheValidityPolicy { if (diff.isNegative()) { return TimeValue.ZERO_MILLISECONDS; } - return TimeValue.ofSeconds((long) (coefficient * diff.getSeconds())); + return TimeValue.ofSeconds((long) (heuristicCoefficient * diff.getSeconds())); } - return defaultLifetime; + return heuristicDefaultLifetime; } public boolean isRevalidatable(final HttpCacheEntry entry) { 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 3c27768f5..60b2f6127 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 @@ -52,8 +52,6 @@ class CachedResponseSuitabilityChecker { private final boolean sharedCache; private final boolean useHeuristicCaching; - private final float heuristicCoefficient; - private final TimeValue heuristicDefaultLifetime; private final CacheValidityPolicy validityStrategy; CachedResponseSuitabilityChecker(final CacheValidityPolicy validityStrategy, @@ -62,12 +60,10 @@ class CachedResponseSuitabilityChecker { this.validityStrategy = validityStrategy; this.sharedCache = config.isSharedCache(); this.useHeuristicCaching = config.isHeuristicCachingEnabled(); - this.heuristicCoefficient = config.getHeuristicCoefficient(); - this.heuristicDefaultLifetime = config.getHeuristicDefaultLifetime(); } CachedResponseSuitabilityChecker(final CacheConfig config) { - this(new CacheValidityPolicy(), config); + this(new CacheValidityPolicy(config), config); } private boolean isFreshEnough(final RequestCacheControl requestCacheControl, @@ -77,7 +73,7 @@ class CachedResponseSuitabilityChecker { return true; } if (useHeuristicCaching && - validityStrategy.isResponseHeuristicallyFresh(entry, now, heuristicCoefficient, heuristicDefaultLifetime)) { + validityStrategy.isResponseHeuristicallyFresh(entry, now)) { return true; } if (originInsistsOnFreshness(responseCacheControl)) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java index eb466d545..e81e0eba5 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java @@ -100,7 +100,7 @@ public class CachingExecBase { CachingExecBase(final CacheConfig config) { super(); this.cacheConfig = config != null ? config : CacheConfig.DEFAULT; - this.validityPolicy = new CacheValidityPolicy(); + this.validityPolicy = new CacheValidityPolicy(config); this.responseGenerator = new CachedHttpResponseGenerator(this.validityPolicy); this.cacheableRequestPolicy = new CacheableRequestPolicy(); this.suitabilityChecker = new CachedResponseSuitabilityChecker(this.validityPolicy, this.cacheConfig); 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 fd1902821..75b264d8e 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 @@ -175,19 +175,24 @@ public class TestCacheValidityPolicy { } @Test - public void testFreshnessLifetimeIsMostRestrictiveOfMaxAgeAndSMaxAge() { + public void testFreshnessLifetimeUsesSharedMaxAgeInSharedCache() { + // assuming impl represents a shared cache final ResponseCacheControl cacheControl = ResponseCacheControl.builder() .setMaxAge(10) .setSharedMaxAge(20) .build(); final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - assertEquals(TimeValue.ofSeconds(10), impl.getFreshnessLifetime(cacheControl, entry)); + assertEquals(TimeValue.ofSeconds(20), impl.getFreshnessLifetime(cacheControl, entry)); + } - final ResponseCacheControl cacheControl2 = ResponseCacheControl.builder() - .setMaxAge(20) - .setSharedMaxAge(10) + @Test + public void testFreshnessLifetimeUsesMaxAgeWhenSharedMaxAgeNotPresent() { + // assuming impl represents a shared cache + final ResponseCacheControl cacheControl = ResponseCacheControl.builder() + .setMaxAge(10) .build(); - assertEquals(TimeValue.ofSeconds(10), impl.getFreshnessLifetime(cacheControl2, entry)); + final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); + assertEquals(TimeValue.ofSeconds(10), impl.getFreshnessLifetime(cacheControl, entry)); } @Test @@ -227,14 +232,14 @@ public class TestCacheValidityPolicy { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(oneSecondAgo)), new BasicHeader("Last-Modified", DateUtils.formatStandardDate(elevenSecondsAgo))); - assertEquals(TimeValue.ofSeconds(1), impl.getHeuristicFreshnessLifetime(entry, 0.1f, TimeValue.ZERO_MILLISECONDS)); + assertEquals(TimeValue.ofSeconds(1), impl.getHeuristicFreshnessLifetime(entry)); } @Test public void testHeuristicFreshnessLifetimeDefaultsProperly() { - final TimeValue defaultFreshness = TimeValue.ofSeconds(10); + final TimeValue defaultFreshness = TimeValue.ofSeconds(0); final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); - assertEquals(defaultFreshness, impl.getHeuristicFreshnessLifetime(entry, 0.1f, defaultFreshness)); + assertEquals(defaultFreshness, impl.getHeuristicFreshnessLifetime(entry)); } @Test @@ -242,7 +247,7 @@ public class TestCacheValidityPolicy { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( new BasicHeader("Date", DateUtils.formatStandardDate(elevenSecondsAgo)), new BasicHeader("Last-Modified", DateUtils.formatStandardDate(oneSecondAgo))); - assertTrue(TimeValue.isNonNegative(impl.getHeuristicFreshnessLifetime(entry, 0.1f, TimeValue.ofSeconds(10)))); + assertTrue(TimeValue.isNonNegative(impl.getHeuristicFreshnessLifetime(entry))); } @Test @@ -265,6 +270,16 @@ public class TestCacheValidityPolicy { assertTrue(impl.isResponseFresh(responseCacheControl, entry, now)); } + @Test + public void testHeuristicFreshnessLifetimeCustomProperly() { + final CacheConfig cacheConfig = CacheConfig.custom().setHeuristicDefaultLifetime(TimeValue.ofSeconds(10)) + .setHeuristicCoefficient(0.5f).build(); + impl = new CacheValidityPolicy(cacheConfig); + final TimeValue defaultFreshness = TimeValue.ofSeconds(10); + final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(); + assertEquals(defaultFreshness, impl.getHeuristicFreshnessLifetime(entry)); + } + @Test public void testResponseIsNotFreshIfFreshnessLifetimeEqualsCurrentAge() { final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry();