You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "ok2c (via GitHub)" <gi...@apache.org> on 2023/03/07 09:45:20 UTC
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #420: Fix calculation of freshness lifetime in HttpResponse
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org