ok2c commented on code in PR #446:
URL:
https://github.com/apache/httpcomponents-client/pull/446#discussion_r1193014635
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControl.java:
##########
@@ -129,6 +140,7 @@ public CacheControl(final long maxAge, final long
sharedMaxAge, final boolean mu
this.proxyRevalidate = proxyRevalidate;
this.cachePublic = cachePublic;
this.stale_while_revalidate = stale_while_revalidate;
+ this.noCacheFields = noCacheFields;
Review Comment:
@arturobernalg Please make this set unmodifiable
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java:
##########
@@ -162,6 +169,19 @@ public final CacheControl parse(final Header header) {
mustRevalidate = true;
} else if
(name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_CACHE)) {
noCache = true;
+ if (value != null) {
+ final Tokenizer.Cursor valueCursor = new
Tokenizer.Cursor(cursor.getPos() - value.length() -1, buffer.length());
Review Comment:
@ There is a way to parse the value in a single pass without creating an
extra `Tokenizer.Cursor`. It is nice to have, though.
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java:
##########
@@ -525,4 +544,51 @@ private CacheControl parseCacheControlHeader(final
MessageHeaders messageHeaders
}
}
+ /**
+ * Determines if the given {@link HttpCacheEntry} requires revalidation
based on the presence of the {@code no-cache} directive
+ * in the Cache-Control header.
+ * <p>
+ * The method returns true in the following cases:
+ * - If the {@code no-cache} directive is present without any field names.
+ * - If the {@code no-cache} directive is present with field names, and at
least one of these field names is present
+ * in the headers of the {@link HttpCacheEntry}.
+ * <p>
+ * If the {@code no-cache} directive is not present in the Cache-Control
header, the method returns {@code false}.
+ *
+ * @param entry the {@link HttpCacheEntry} containing the headers to
check for the {@code no-cache} directive.
+ * @return true if revalidation is required based on the {@code no-cache}
directive, {@code false} otherwise.
+ */
+ boolean responseContainsNoCacheDirective(final HttpCacheEntry entry) {
+ final CacheControl responseCacheControl =
parseCacheControlHeader(entry);
+
+ if (responseCacheControl != null) {
+ final Set<String> noCacheFields =
responseCacheControl.getNoCacheFields();
+
+ // If no-cache directive is present and has no field names
+ if (responseCacheControl.isNoCache() && noCacheFields.isEmpty()) {
+ LOG.debug("No-cache directive present without field names.
Revalidation required.");
+ return true;
+ }
+
+ // If no-cache directive is present with field names
+ if (responseCacheControl.isNoCache()) {
+ final HeaderGroup headerGroup = new HeaderGroup();
+ headerGroup.setHeaders(entry.getHeaders());
Review Comment:
@arturobernalg This looks completely unnecessary. `HttpCacheEntry`has
`#getFirstHeader` method already.
##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControl.java:
##########
@@ -94,14 +97,20 @@ final class CacheControl {
*/
private final long stale_while_revalidate;
Review Comment:
@arturobernalg Minor nitpick: could you please make this variable use the
naming consistent with other variables?
--
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]
