ok2c commented on code in PR #420:
URL:
https://github.com/apache/httpcomponents-client/pull/420#discussion_r1127588323
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java:
##########
@@ -50,6 +51,29 @@
class ResponseCachingPolicy {
+ /**
+ * The default freshness duration for a cached object, in seconds.
+ *
+ * <p>This constant is used to set the default value for the freshness
lifetime of a cached object.
+ * When a new object is added to the cache, it will be assigned this
duration if no other duration
+ * is specified.</p>
+ *
+ * <p>By default, this value is set to 300 seconds (5 minutes).
Applications can customize this
+ * value as needed.</p>
+ */
+ private static final long DEFAULT_FRESHNESS_DURATION = 300L;
Review Comment:
@arturobernalg This is actually the case where the use of
`java.time.Duration` instead of plain `long` might be beneficial.
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java:
##########
@@ -320,4 +354,87 @@ private boolean from1_0Origin(final HttpResponse response)
{
return HttpVersion.HTTP_1_0.equals(version);
}
+ /**
+ * Calculates the freshness lifetime of a response, based on the headers
in the response.
+ * <p>
+ * This method follows the algorithm for calculating the freshness
lifetime described in RFC 7234, section 4.2.1.
+ * The freshness lifetime represents the time interval in seconds during
which the response can be served without
+ * being considered stale. The freshness lifetime calculation takes into
account the s-maxage, max-age, Expires, and
+ * Date headers as follows:
+ * <ul>
+ * <li>If the s-maxage directive is present in the Cache-Control header of
the response, its value is used as the
+ * freshness lifetime for shared caches, which typically serve multiple
users or clients.</li>
+ * <li>If the max-age directive is present in the Cache-Control header of
the response, its value is used as the
+ * freshness lifetime for private caches, which serve a single user or
client.</li>
+ * <li>If the Expires header is present in the response, its value is used
as the expiration time of the response.
+ * The freshness lifetime is calculated as the difference between the
expiration time and the time specified in the
+ * Date header of the response.</li>
+ * <li>If none of the above headers are present or if the calculated
freshness lifetime is invalid, a default value of
+ * 5 minutes is returned.</li>
+ * </ul>
+ *
+ * <p>
+ * Note that caching is a complex topic and cache control directives may
interact with each other in non-trivial ways.
+ * This method provides a basic implementation of the freshness lifetime
calculation algorithm and may not be suitable
+ * for all use cases. Developers should consult the HTTP caching
specifications for more information and consider
+ * implementing additional caching mechanisms as needed.
+ * </p>
+ *
+ * @param response the HTTP response for which to calculate the freshness
lifetime
Review Comment:
@arturobernalg We stopped referencing specific RFCs in javadocs. Those URLs
tend to change and also need to be updated every time the code base gets
revised against a newer set of RFCs.
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java:
##########
@@ -320,4 +354,87 @@ private boolean from1_0Origin(final HttpResponse response)
{
return HttpVersion.HTTP_1_0.equals(version);
}
+ /**
+ * Calculates the freshness lifetime of a response, based on the headers
in the response.
+ * <p>
+ * This method follows the algorithm for calculating the freshness
lifetime described in RFC 7234, section 4.2.1.
+ * The freshness lifetime represents the time interval in seconds during
which the response can be served without
+ * being considered stale. The freshness lifetime calculation takes into
account the s-maxage, max-age, Expires, and
+ * Date headers as follows:
+ * <ul>
+ * <li>If the s-maxage directive is present in the Cache-Control header of
the response, its value is used as the
+ * freshness lifetime for shared caches, which typically serve multiple
users or clients.</li>
+ * <li>If the max-age directive is present in the Cache-Control header of
the response, its value is used as the
+ * freshness lifetime for private caches, which serve a single user or
client.</li>
+ * <li>If the Expires header is present in the response, its value is used
as the expiration time of the response.
+ * The freshness lifetime is calculated as the difference between the
expiration time and the time specified in the
+ * Date header of the response.</li>
+ * <li>If none of the above headers are present or if the calculated
freshness lifetime is invalid, a default value of
+ * 5 minutes is returned.</li>
+ * </ul>
+ *
+ * <p>
+ * Note that caching is a complex topic and cache control directives may
interact with each other in non-trivial ways.
+ * This method provides a basic implementation of the freshness lifetime
calculation algorithm and may not be suitable
+ * for all use cases. Developers should consult the HTTP caching
specifications for more information and consider
+ * implementing additional caching mechanisms as needed.
+ * </p>
+ *
+ * @param response the HTTP response for which to calculate the freshness
lifetime
+ * @return the freshness lifetime of the response, in seconds
+ * @see <a href="https://tools.ietf.org/html/rfc7234#section-4.2.1";>RFC
7234 Section 4.2.1</a> for the freshness
+ * lifetime calculation algorithm.
+ * @see <a href="https://tools.ietf.org/html/rfc7234#section-5.2";>RFC 7234
Section 5.2</a> for a discussion of shared
+ * and private caches.
+ * @see <a href="https://tools.ietf.org/html/rfc2616#section-14.9.3";>RFC
2616 Section 14.9.3</a> for the Expires header
+ * specification.
+ * @see <a href="https://tools.ietf.org/html/rfc822#section-5";>RFC 822
Section 5</a> for the Date header specification.
+ */
+ private long calculateFreshnessLifetime(final HttpResponse response) {
+ // Check if s-maxage is present and use its value if it is
+ final Header cacheControl =
response.getFirstHeader(HttpHeaders.CACHE_CONTROL);
+ if (cacheControl == null) {
+ // If no cache-control header is present, assume no caching
directives and return a default value
+ return DEFAULT_FRESHNESS_DURATION; // 5 minutes
+ }
+
+ final String cacheControlValue = cacheControl.getValue();
+ final String[] headerValues = cacheControlValue.split(",");
+
+ long maxAge = -1;
+ long sharedMaxAge = -1;
+
+ for (final String headerValue : headerValues) {
Review Comment:
@arturobernalg Please use `org.apache.hc.core5.util.Tokenizer` based parsers
from HttpCore to parse header elements, see `AuthChallengeParser` for an
example.
Please also consider what is going to happen if your code has to parse
something like `max-age=boom`. Malformed header values must not cause a runtime
exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
