Skip to content

Commit

Permalink
HTTPCLIENT-2277: deprecation of obsolete config parameters and remova…
Browse files Browse the repository at this point in the history 
…l of oudated or meaningless tests (#484)

* HTTPCLIENT-2277: Deprecated 303 response caching switch as no longer required by RFC 9111

* Javadoc improvements (no functional changes)

* HTTPCLIENT-2277: Revision of HTTP cache protocol requirement and recommendation test cases:
* Removed links to RFC 2616
* Removed verbatim quotes from RFC 2616
* Removed obsolete test cases and test cases without result verification / assertions
* Removed test cases unrelated to HTTP caching
* Removed test cases without test result assertions
  • Loading branch information
@ok2c
ok2c committed Dec 13, 2023
1 parent c7d7919 commit 1b0e9cf
Show file tree
Hide file tree
Showing 10 changed files with 313 additions and 2,860 deletions.
91 changes: 30 additions & 61 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,18 @@
import org.apache.hc.core5.util.TimeValue;

/**
* <p>Java Beans-style configuration for caching {@link org.apache.hc.client5.http.classic.HttpClient}.
* Any class in the caching module that has configuration options should take a
* {@link CacheConfig} argument in one of its constructors. A
* {@code CacheConfig} instance has sane and conservative defaults, so the
* easiest way to specify options is to get an instance and then set just
* the options you want to modify from their defaults.</p>
*
* <p><b>N.B.</b> This class is only for caching-specific configuration; to
* configure the behavior of the rest of the client, configure the
* {@link org.apache.hc.client5.http.classic.HttpClient} used as the &quot;backend&quot;
* for the {@code CachingHttpClient}.</p>
* <p>Configuration for HTTP caches</p>
*
* <p>Cache configuration can be grouped into the following categories:</p>
*
* <p><b>Cache size.</b> If the backend storage supports these limits, you
* <p><b>Protocol options.</b> I some cases the HTTP protocol allows for
* conditional behaviors or optional protocol extensions. Such conditional
* protocol behaviors or extensions can be turned on or off here.
* See {@link CacheConfig#isNeverCacheHTTP10ResponsesWithQuery()},
* {@link CacheConfig#isNeverCacheHTTP11ResponsesWithQuery()},
* {@link CacheConfig#isStaleIfErrorEnabled()}</p>
*
* <p><b>Cache size.</b> If the backend storage supports these limits, one
* can specify the {@link CacheConfig#getMaxCacheEntries maximum number of
* cache entries} as well as the {@link CacheConfig#getMaxObjectSize()}
* maximum cacheable response body size}.</p>
Expand All @@ -54,46 +51,25 @@
* responses to requests with {@code Authorization} headers or responses
* marked with {@code Cache-Control: private}. If, however, the cache
* is only going to be used by one logical "user" (behaving similarly to a
* browser cache), then you will want to {@link
* CacheConfig#isSharedCache()} turn off the shared cache setting}.</p>
*
* <p><b>303 caching</b>. RFC2616 explicitly disallows caching 303 responses;
* however, the HTTPbis working group says they can be cached
* if explicitly indicated in the response headers and permitted by the request method.
* (They also indicate that disallowing 303 caching is actually an unintended
* spec error in RFC2616).
* This behavior is off by default, to err on the side of a conservative
* adherence to the existing standard, but you may want to
* {@link Builder#setAllow303Caching(boolean) enable it}.
* browser cache), then one may want to {@link CacheConfig#isSharedCache()}
* turn off the shared cache setting}.</p>
*
* <p><b>Weak ETags on PUT/DELETE If-Match requests</b>. RFC2616 explicitly
* prohibits the use of weak validators in non-GET requests, however, the
* HTTPbis working group says while the limitation for weak validators on ranged
* requests makes sense, weak ETag validation is useful on full non-GET
* requests; e.g., PUT with If-Match. This behavior is off by default, to err on
* the side of a conservative adherence to the existing standard, but you may
* want to {@link Builder#setWeakETagOnPutDeleteAllowed(boolean) enable it}.
*
* <p><b>Heuristic caching</b>. Per RFC2616, a cache may cache certain cache
* entries even if no explicit cache control headers are set by the origin.
* This behavior is off by default, but you may want to turn this on if you
* are working with an origin that doesn't set proper headers but where you
* still want to cache the responses. You will want to {@link
* CacheConfig#isHeuristicCachingEnabled()} enable heuristic caching},
* <p><b>Heuristic caching</b>. Per HTTP caching specification, a cache may
* cache certain cache entries even if no explicit cache control headers are
* set by the origin. This behavior is off by default, but you may want to
* turn this on if you are working with an origin that doesn't set proper
* headers but where one may still want to cache the responses. Use {@link
* CacheConfig#isHeuristicCachingEnabled()} to enable heuristic caching},
* then specify either a {@link CacheConfig#getHeuristicDefaultLifetime()
* default freshness lifetime} and/or a {@link
* CacheConfig#getHeuristicCoefficient() fraction of the time since
* the resource was last modified}. See Sections
* <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.2.2">
* 13.2.2</a> and <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.2.4">
* 13.2.4</a> of the HTTP/1.1 RFC for more details on heuristic caching.</p>
* the resource was last modified}.
*
* <p><b>Background validation</b>. The cache module supports the
* {@code stale-while-revalidate} directive of
* <a href="http://tools.ietf.org/html/rfc5861">RFC5861</a>, which allows
* certain cache entry revalidations to happen in the background. Asynchronous
* validation is enabled by default but it could be disabled by setting the number
* of re-validation workers to {@code 0} with {@link CacheConfig#getAsynchronousWorkers()}
* {@code stale-while-revalidate} directive, which allows certain cache entry
* revalidations to happen in the background. Asynchronous validation is enabled
* by default but it could be disabled by setting the number of re-validation
* workers to {@code 0} with {@link CacheConfig#getAsynchronousWorkers()}
* parameter</p>
*/
public class CacheConfig implements Cloneable {
Expand All @@ -113,8 +89,10 @@ public class CacheConfig implements Cloneable {
*/
public final static int DEFAULT_MAX_UPDATE_RETRIES = 1;

/** Default setting for 303 caching
/**
* @deprecated No longer applicable. Do not use.
*/
@Deprecated
public final static boolean DEFAULT_303_CACHING_ENABLED = false;

/**
Expand Down Expand Up @@ -147,7 +125,6 @@ public class CacheConfig implements Cloneable {
private final long maxObjectSize;
private final int maxCacheEntries;
private final int maxUpdateRetries;
private final boolean allow303Caching;
private final boolean heuristicCachingEnabled;
private final float heuristicCoefficient;
private final TimeValue heuristicDefaultLifetime;
Expand All @@ -168,7 +145,6 @@ public class CacheConfig implements Cloneable {
final long maxObjectSize,
final int maxCacheEntries,
final int maxUpdateRetries,
final boolean allow303Caching,
final boolean heuristicCachingEnabled,
final float heuristicCoefficient,
final TimeValue heuristicDefaultLifetime,
Expand All @@ -182,7 +158,6 @@ public class CacheConfig implements Cloneable {
this.maxObjectSize = maxObjectSize;
this.maxCacheEntries = maxCacheEntries;
this.maxUpdateRetries = maxUpdateRetries;
this.allow303Caching = allow303Caching;
this.heuristicCachingEnabled = heuristicCachingEnabled;
this.heuristicCoefficient = heuristicCoefficient;
this.heuristicDefaultLifetime = heuristicDefaultLifetime;
Expand Down Expand Up @@ -257,11 +232,11 @@ public int getMaxUpdateRetries(){
}

/**
* Returns whether 303 caching is enabled.
* @return {@code true} if it is enabled.
* @deprecated No longer applicable. Do not use.
*/
@Deprecated
public boolean is303CachingEnabled() {
return allow303Caching;
return true;
}

/**
Expand Down Expand Up @@ -356,7 +331,6 @@ public static class Builder {
private long maxObjectSize;
private int maxCacheEntries;
private int maxUpdateRetries;
private boolean allow303Caching;
private boolean heuristicCachingEnabled;
private float heuristicCoefficient;
private TimeValue heuristicDefaultLifetime;
Expand All @@ -371,7 +345,6 @@ public static class Builder {
this.maxObjectSize = DEFAULT_MAX_OBJECT_SIZE_BYTES;
this.maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES;
this.maxUpdateRetries = DEFAULT_MAX_UPDATE_RETRIES;
this.allow303Caching = DEFAULT_303_CACHING_ENABLED;
this.heuristicCachingEnabled = DEFAULT_HEURISTIC_CACHING_ENABLED;
this.heuristicCoefficient = DEFAULT_HEURISTIC_COEFFICIENT;
this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME;
Expand Down Expand Up @@ -407,12 +380,10 @@ public Builder setMaxUpdateRetries(final int maxUpdateRetries) {
}

/**
* Enables or disables 303 caching.
* @param allow303Caching should be {@code true} to
* permit 303 caching, {@code false} to disable it.
* @deprecated Has no effect. Do not use.
*/
@Deprecated
public Builder setAllow303Caching(final boolean allow303Caching) {
this.allow303Caching = allow303Caching;
return this;
}

Expand Down Expand Up @@ -537,7 +508,6 @@ public CacheConfig build() {
maxObjectSize,
maxCacheEntries,
maxUpdateRetries,
allow303Caching,
heuristicCachingEnabled,
heuristicCoefficient,
heuristicDefaultLifetime,
Expand All @@ -557,7 +527,6 @@ public String toString() {
builder.append("[maxObjectSize=").append(this.maxObjectSize)
.append(", maxCacheEntries=").append(this.maxCacheEntries)
.append(", maxUpdateRetries=").append(this.maxUpdateRetries)
.append(", 303CachingEnabled=").append(this.allow303Caching)
.append(", heuristicCachingEnabled=").append(this.heuristicCachingEnabled)
.append(", heuristicCoefficient=").append(this.heuristicCoefficient)
.append(", heuristicDefaultLifetime=").append(this.heuristicDefaultLifetime)
Expand Down
3 changes: 1 addition & 2 deletions ...ache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ SimpleHttpResponse generateNotModifiedResponse(final HttpCacheEntry entry) {
final SimpleHttpResponse response = new SimpleHttpResponse(HttpStatus.SC_NOT_MODIFIED, "Not Modified");

// The response MUST include the following headers
// (http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html)

// - Date, unless its omission is required by section 14.8.1
// - Date
Header dateHeader = entry.getFirstHeader(HttpHeaders.DATE);
if (dateHeader == null) {
dateHeader = new BasicHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(Instant.now()));
Expand Down
15 changes: 1 addition & 14 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@

public class CachingExecBase {

final static boolean SUPPORTS_RANGE_AND_CONTENT_RANGE_HEADERS = false;

final AtomicLong cacheHits = new AtomicLong();
final AtomicLong cacheMisses = new AtomicLong();
final AtomicLong cacheUpdates = new AtomicLong();
Expand Down Expand Up @@ -98,7 +96,6 @@ public class CachingExecBase {
this.cacheConfig.getMaxObjectSize(),
this.cacheConfig.isSharedCache(),
this.cacheConfig.isNeverCacheHTTP10ResponsesWithQuery(),
this.cacheConfig.is303CachingEnabled(),
this.cacheConfig.isNeverCacheHTTP11ResponsesWithQuery(),
this.cacheConfig.isStaleIfErrorEnabled());
}
Expand Down Expand Up @@ -268,16 +265,6 @@ void setResponseStatus(final HttpContext context, final CacheResponseStatus valu
}
}

/**
* Reports whether this {@code CachingHttpClient} implementation
* supports byte-range requests as specified by the {@code Range}
* and {@code Content-Range} headers.
* @return {@code true} if byte-range requests are supported
*/
boolean supportsRangeAndContentRangeHeaders() {
return SUPPORTS_RANGE_AND_CONTENT_RANGE_HEADERS;
}

Instant getCurrentDate() {
return Instant.now();
}
Expand All @@ -299,7 +286,7 @@ boolean revalidationResponseIsTooOld(final HttpResponse backendResponse, final H
// either backend response or cached entry did not have a valid
// Date header, so we can't tell if they are out of order
// according to the origin clock; thus we can skip the
// unconditional retry recommended in 13.2.6 of RFC 2616.
// unconditional retry.
return DateSupport.isBefore(backendResponse, cacheEntry, HttpHeaders.DATE);
}

Expand Down
66 changes: 16 additions & 50 deletions ...ent5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@
import java.time.Duration;
import java.time.Instant;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;

Expand Down Expand Up @@ -71,20 +68,12 @@ class ResponseCachingPolicy {
*/
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.RFC_1123_DATE_TIME;

private final static Set<Integer> CACHEABLE_STATUS_CODES =
new HashSet<>(Arrays.asList(HttpStatus.SC_OK,
HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION,
HttpStatus.SC_MULTIPLE_CHOICES,
HttpStatus.SC_MOVED_PERMANENTLY,
HttpStatus.SC_GONE));

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

private final long maxObjectSizeBytes;
private final boolean sharedCache;
private final boolean neverCache1_0ResponsesWithQueryString;
private final boolean neverCache1_1ResponsesWithQueryString;
private final Set<Integer> uncacheableStatusCodes;

/**
* A flag indicating whether serving stale cache entries is allowed when an error occurs
Expand All @@ -94,32 +83,6 @@ class ResponseCachingPolicy {
*/
private final boolean staleIfErrorEnabled;

/**
* Define a cache policy that limits the size of things that should be stored
* in the cache to a maximum of {@link HttpResponse} bytes in size.
*
* @param maxObjectSizeBytes the size to limit items into the cache
* @param sharedCache whether to behave as a shared cache (true) or a
* non-shared/private cache (false)
* @param neverCache1_0ResponsesWithQueryString true to never cache HTTP 1.0 responses with a query string, false
* to cache if explicit cache headers are found.
* @param allow303Caching if this policy is permitted to cache 303 response
* @param neverCache1_1ResponsesWithQueryString {@code true} to never cache HTTP 1.1 responses with a query string,
* {@code false} to cache if explicit cache headers are found.
*/
public ResponseCachingPolicy(final long maxObjectSizeBytes,
final boolean sharedCache,
final boolean neverCache1_0ResponsesWithQueryString,
final boolean allow303Caching,
final boolean neverCache1_1ResponsesWithQueryString) {
this(maxObjectSizeBytes,
sharedCache,
neverCache1_0ResponsesWithQueryString,
allow303Caching,
neverCache1_1ResponsesWithQueryString,
false);
}

/**
* Constructs a new ResponseCachingPolicy with the specified cache policy settings and stale-if-error support.
*
Expand All @@ -129,8 +92,6 @@ public ResponseCachingPolicy(final long maxObjectSizeBytes,
* non-shared/private cache (false)
* @param neverCache1_0ResponsesWithQueryString {@code true} to never cache HTTP 1.0 responses with a query string,
* {@code false} to cache if explicit cache headers are found.
* @param allow303Caching {@code true} if this policy is permitted to cache 303 responses,
* {@code false} otherwise
* @param neverCache1_1ResponsesWithQueryString {@code true} to never cache HTTP 1.1 responses with a query string,
* {@code false} to cache if explicit cache headers are found.
* @param staleIfErrorEnabled {@code true} to enable the stale-if-error cache directive, which
Expand All @@ -141,18 +102,12 @@ public ResponseCachingPolicy(final long maxObjectSizeBytes,
public ResponseCachingPolicy(final long maxObjectSizeBytes,
final boolean sharedCache,
final boolean neverCache1_0ResponsesWithQueryString,
final boolean allow303Caching,
final boolean neverCache1_1ResponsesWithQueryString,
final boolean staleIfErrorEnabled) {
this.maxObjectSizeBytes = maxObjectSizeBytes;
this.sharedCache = sharedCache;
this.neverCache1_0ResponsesWithQueryString = neverCache1_0ResponsesWithQueryString;
this.neverCache1_1ResponsesWithQueryString = neverCache1_1ResponsesWithQueryString;
if (allow303Caching) {
uncacheableStatusCodes = new HashSet<>(Collections.singletonList(HttpStatus.SC_PARTIAL_CONTENT));
} else {
uncacheableStatusCodes = new HashSet<>(Arrays.asList(HttpStatus.SC_PARTIAL_CONTENT, HttpStatus.SC_SEE_OTHER));
}
this.staleIfErrorEnabled = staleIfErrorEnabled;
}

Expand All @@ -172,15 +127,15 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
}

final int status = response.getCode();
if (CACHEABLE_STATUS_CODES.contains(status)) {
if (isKnownCacheableStatusCode(status)) {
// these response codes MAY be cached
cacheable = true;
} else if (uncacheableStatusCodes.contains(status)) {
} else if (isKnownNonCacheableStatusCode(status)) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} response is not cacheable", status);
}
return false;
} else if (unknownStatusCode(status)) {
} else if (isUnknownStatusCode(status)) {
// a response with an unknown status code MUST NOT be
// cached
if (LOG.isDebugEnabled()) {
Expand Down Expand Up @@ -247,7 +202,19 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
return cacheable || isExplicitlyCacheable(cacheControl, response);
}

private boolean unknownStatusCode(final int status) {
private static boolean isKnownCacheableStatusCode(final int status) {
return status == HttpStatus.SC_OK ||
status == HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION ||
status == HttpStatus.SC_MULTIPLE_CHOICES ||
status == HttpStatus.SC_MOVED_PERMANENTLY ||
status == HttpStatus.SC_GONE;
}

private static boolean isKnownNonCacheableStatusCode(final int status) {
return status == HttpStatus.SC_PARTIAL_CONTENT;
}

private static boolean isUnknownStatusCode(final int status) {
if (status >= 100 && status <= 101) {
return false;
}
Expand Down Expand Up @@ -507,7 +474,6 @@ boolean responseContainsNoCacheDirective(final ResponseCacheControl responseCach
}

/**
* This method checks if a given HTTP status code is understood according to RFC 7231.
* Understood status codes include:
* - All 2xx (Successful) status codes (200-299)
* - All 3xx (Redirection) status codes (300-399)
Expand Down
Loading

0 comments on commit 1b0e9cf

Please sign in to comment.