Deprecate the never-cache-responses-with-query options and make them opt-in

Assigning heuristic freshness to a response with a query component is no longer prohibited by the HTTP caching specification. ResponseCachingPolicy now consults the never-cache-query options only when they are explicitly enabled, so in the default configuration a response with a query component is cached like any other. isNeverCacheHTTP10ResponsesWithQuery, isNeverCacheHTTP11ResponsesWithQuery and their builder setters are deprecated, as is HTTP/1.0 response caching. An origin that does not want a response cached should send an explicit directive such as Cache-Control: no-cache.

Author: arturobernalg

httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheConfig.java

@@ -41,6 +41,12 @@
  * {@link CacheConfig#isNeverCacheHTTP11ResponsesWithQuery()},
  * {@link CacheConfig#isStaleIfErrorEnabled()}</p>
  *
+ * <p><b>Query string and HTTP/1.0 caching.</b> The options that suppress caching of responses with a query
+ * component are deprecated, since assigning heuristic freshness to such a response is no longer prohibited; an
+ * origin that does not want a response cached should send an explicit directive such as
+ * {@code Cache-Control: no-cache}. HTTP/1.0 response caching is deprecated as a whole and will be removed in a
+ * future release.</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()}
@@ -189,7 +195,13 @@ public long getMaxObjectSize() {
      * Returns whether the cache will never cache HTTP 1.0 responses with a query string or not.
      * @return {@code true} to not cache query string responses, {@code false} to cache if explicit cache headers are
      * found
+     *
+     * @deprecated Assigning heuristic freshness to a response with a query component is no longer prohibited, so this
+     * option no longer serves a purpose. An origin that does not want such a response cached should send an explicit
+     * directive such as {@code Cache-Control: no-cache}. HTTP/1.0 response caching is deprecated and will be removed
+     * in a future release.
      */
+    @Deprecated
     public boolean isNeverCacheHTTP10ResponsesWithQuery() {
         return neverCacheHTTP10ResponsesWithQuery;
     }
@@ -205,7 +217,12 @@ public boolean isNeverCacheHTTP10ResponsesWithQuery() {
      * @return {@code true} if HTTP/1.1 responses with query strings should never be cached;
      * {@code false} otherwise.
      * @since 5.4
+     *
+     * @deprecated Assigning heuristic freshness to a response with a query component is no longer prohibited, so this
+     * option no longer serves a purpose. An origin that does not want such a response cached should send an explicit
+     * directive such as {@code Cache-Control: no-cache}.
      */
+    @Deprecated
     public boolean isNeverCacheHTTP11ResponsesWithQuery() {
         return neverCacheHTTP11ResponsesWithQuery;
     }
@@ -499,7 +516,13 @@ public Builder setAsynchronousWorkers(final int asynchronousWorkers) {
          * to better emulate IE, which also never caches responses, regardless of what caching
          * headers may be present.
          * @return this instance.
+         *
+         * @deprecated Assigning heuristic freshness to a response with a query component is no longer prohibited, so
+         * this option no longer serves a purpose. An origin that does not want such a response cached should send an
+         * explicit directive such as {@code Cache-Control: no-cache}. HTTP/1.0 response caching is deprecated and will
+         * be removed in a future release.
          */
+        @Deprecated
         public Builder setNeverCacheHTTP10ResponsesWithQueryString(
                 final boolean neverCacheHTTP10ResponsesWithQuery) {
             this.neverCacheHTTP10ResponsesWithQuery = neverCacheHTTP10ResponsesWithQuery;
@@ -535,7 +558,12 @@ public Builder setFreshnessCheckEnabled(final boolean freshnessCheckEnabled) {
          *
          * @param neverCacheHTTP11ResponsesWithQuery whether to never cache HTTP/1.1 responses with a query string
          * @return this instance.
+         *
+         * @deprecated Assigning heuristic freshness to a response with a query component is no longer prohibited, so
+         * this option no longer serves a purpose. An origin that does not want such a response cached should send an
+         * explicit directive such as {@code Cache-Control: no-cache}.
          */
+        @Deprecated
         public Builder setNeverCacheHTTP11ResponsesWithQueryString(
                 final boolean neverCacheHTTP11ResponsesWithQuery) {
             this.neverCacheHTTP11ResponsesWithQuery = neverCacheHTTP11ResponsesWithQuery;

httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java

@@ -75,6 +75,8 @@ public class CachingExecBase {
         this.cacheConfig = config != null ? config : CacheConfig.DEFAULT;
     }
 
+    // The query-string caching options are deprecated but still honoured while they remain on the API.
+    @SuppressWarnings("deprecation")
     CachingExecBase(final CacheConfig config) {
         super();
         this.cacheConfig = config != null ? config : CacheConfig.DEFAULT;

httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java

@@ -123,10 +123,11 @@ public boolean isResponseCacheable(final RequestCacheControl requestCacheControl
         }
 
         if (request.getPath().contains("?")) {
-            if (neverCache1_0ResponsesWithQueryString && from1_0Origin(response)) {
+            if (neverCache1_0ResponsesWithQueryString && from1_0Origin(response) || neverCache1_1ResponsesWithQueryString) {
                 LOG.debug("Response is not cacheable as it had a query string");
                 return false;
-            } else if (!neverCache1_1ResponsesWithQueryString && !isExplicitlyCacheable(cacheControl, response)) {
+            }
+            if (!isExplicitlyCacheable(cacheControl, response)) {
                 LOG.debug("Response is not cacheable as it is missing explicit caching headers");
                 return false;
             }

httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java

@@ -861,9 +861,9 @@ void testIsResponseCacheable() {
         request = new BasicHttpRequest("GET","/foo?s=bar");
         // HTTPbis working group says ok if explicitly indicated by
         // response headers
-        policy = new ResponseCachingPolicy(true, false, true);
         response.setCode(HttpStatus.SC_OK);
         response.setHeader("Date", DateUtils.formatStandardDate(now));
+        responseCacheControl = ResponseCacheControl.builder().setMaxAge(3600).build();
         assertTrue(policy.isResponseCacheable(requestCacheControl, responseCacheControl, request, response));
     }