Performance and code optimization in HTTP cache

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

@@ -36,6 +36,7 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 import org.apache.hc.client5.http.utils.DateUtils;
@@ -74,6 +75,9 @@ public class HttpCacheEntry implements MessageHeaders, Serializable {
     private final HeaderGroup responseHeaders;
     private final Resource resource;
     private final Set<String> variants;
+    private final AtomicReference<Instant> dateRef;
+    private final AtomicReference<Instant> expiresRef;
+    private final AtomicReference<Instant> lastModifiedRef;
 
     /**
      * Internal constructor that makes no validation of the input parameters and makes
@@ -100,6 +104,9 @@ public HttpCacheEntry(
         this.responseHeaders = responseHeaders;
         this.resource = resource;
         this.variants = variants != null ? Collections.unmodifiableSet(new HashSet<>(variants)) : null;
+        this.dateRef = new AtomicReference<>();
+        this.expiresRef = new AtomicReference<>();
+        this.lastModifiedRef = new AtomicReference<>();
     }
 
     /**
@@ -169,6 +176,9 @@ public HttpCacheEntry(
         this.responseHeaders.setHeaders(responseHeaders);
         this.resource = resource;
         this.variants = variantMap != null ? Collections.unmodifiableSet(new HashSet<>(variantMap.keySet())) : null;
+        this.dateRef = new AtomicReference<>();
+        this.expiresRef = new AtomicReference<>();
+        this.lastModifiedRef = new AtomicReference<>();
     }
 
     /**
@@ -342,8 +352,41 @@ public Date getDate() {
         return DateUtils.toDate(getInstant());
     }
 
+    private static final Instant NON_VALUE = Instant.ofEpochSecond(Instant.MIN.getEpochSecond(), 0);
+
+    private Instant getInstant(final AtomicReference<Instant> ref, final String headerName) {
+        Instant instant = ref.get();
+        if (instant == null) {
+            instant = DateUtils.parseStandardDate(this, headerName);
+            if (instant == null) {
+                instant = NON_VALUE;
+            }
+            if (!ref.compareAndSet(null, instant)) {
+                instant = ref.get();
+            }
+        }
+        return instant != null && instant != NON_VALUE ? instant : null;
+    }
+
+    /**
+     * @since 5.2
+     */
     public Instant getInstant() {
-        return DateUtils.parseStandardDate(this, HttpHeaders.DATE);
+        return getInstant(dateRef, HttpHeaders.DATE);
+    }
+
+    /**
+     * @since 5.4
+     */
+    public Instant getExpires() {
+        return getInstant(expiresRef, HttpHeaders.EXPIRES);
+    }
+
+    /**
+     * @since 5.4
+     */
+    public Instant getLastModified() {
+        return getInstant(lastModifiedRef, HttpHeaders.LAST_MODIFIED);
     }
 
     /**
@@ -404,6 +447,28 @@ public Iterator<Header> requestHeaderIterator() {
         return requestHeaders.headerIterator();
     }
 
+    /**
+     * Tests if the given {@link HttpCacheEntry} is newer than the given {@link MessageHeaders}
+     * by comparing values of their {@literal DATE} header. In case the given entry, or the message,
+     * or their {@literal DATE} headers are null, this method returns {@code false}.
+     *
+     * @since 5.3
+     */
+    public static boolean isNewer(final HttpCacheEntry entry, final MessageHeaders message) {
+        if (entry == null || message == null) {
+            return false;
+        }
+        final Instant cacheDate = entry.getInstant();
+        if (cacheDate == null) {
+            return false;
+        }
+        final Instant messageDate = DateUtils.parseStandardDate(message, HttpHeaders.DATE);
+        if (messageDate == null) {
+            return false;
+        }
+        return cacheDate.compareTo(messageDate) > 0;
+    }
+
     /**
      * Provides a string representation of this instance suitable for
      * human consumption.

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

@@ -36,8 +36,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 
-import org.apache.hc.client5.http.impl.cache.CacheSupport;
-import org.apache.hc.client5.http.impl.cache.DateSupport;
+import org.apache.hc.client5.http.impl.cache.CacheKeyGenerator;
 import org.apache.hc.client5.http.utils.DateUtils;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.Internal;
@@ -164,12 +163,6 @@ static void ensureDate(final HeaderGroup headers, final Instant instant) {
         }
     }
 
-    static String normalizeRequestUri(final HttpHost host, final HttpRequest request) {
-        final String s = CacheSupport.getRequestUri(request, host);
-        final URI normalizeRequestUri = CacheSupport.normalize(s);
-        return normalizeRequestUri.toASCIIString();
-    }
-
     /**
      * Creates a new root {@link HttpCacheEntry} (parent of multiple variants).
      *
@@ -216,15 +209,16 @@ public HttpCacheEntry create(final Instant requestInstant,
         Args.notNull(host, "Host");
         Args.notNull(request, "Request");
         Args.notNull(response, "Origin response");
-        final String requestUri = normalizeRequestUri(host, request);
+        final String s = CacheKeyGenerator.getRequestUri(host, request);
+        final URI uri = CacheKeyGenerator.normalize(s);
         final HeaderGroup requestHeaders = filterHopByHopHeaders(request);
         final HeaderGroup responseHeaders = filterHopByHopHeaders(response);
         ensureDate(responseHeaders, responseInstant);
         return new HttpCacheEntry(
                 requestInstant,
                 responseInstant,
                 request.getMethod(),
-                requestUri,
+                uri.toASCIIString(),
                 requestHeaders,
                 response.getCode(),
                 responseHeaders,
@@ -252,7 +246,7 @@ public HttpCacheEntry createUpdated(
         Args.check(response.getCode() == HttpStatus.SC_NOT_MODIFIED,
                 "Response must have 304 status code");
         Args.notNull(entry, "Cache entry");
-        if (DateSupport.isAfter(entry, response, HttpHeaders.DATE)) {
+        if (HttpCacheEntry.isNewer(entry, response)) {
             return entry;
         }
         final HeaderGroup mergedHeaders = mergeHeaders(entry, response);

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

@@ -46,6 +46,7 @@
 import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
 import org.apache.hc.client5.http.cache.CacheResponseStatus;
 import org.apache.hc.client5.http.cache.HttpAsyncCacheStorage;
+import org.apache.hc.client5.http.cache.HttpCacheEntry;
 import org.apache.hc.client5.http.cache.ResourceFactory;
 import org.apache.hc.client5.http.cache.ResourceIOException;
 import org.apache.hc.client5.http.impl.ExecSupport;
@@ -531,7 +532,7 @@ public void completed() {
                         @Override
                         public void completed(final CacheMatch result) {
                             final CacheHit hit = result != null ? result.hit : null;
-                            if (DateSupport.isAfter(hit != null ? hit.entry : null, backendResponse, HttpHeaders.DATE)) {
+                            if (HttpCacheEntry.isNewer(hit != null ? hit.entry : null, backendResponse)) {
                                 LOG.debug("Backend already contains fresher cache entry");
                                 try {
                                     final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, hit.entry);

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

@@ -520,7 +520,7 @@ public void completed(final HttpCacheEntry root) {
                     final Header newETag = response.getFirstHeader(HttpHeaders.ETAG);
                     if (existingETag != null && newETag != null &&
                             !Objects.equals(existingETag.getValue(), newETag.getValue()) &&
-                            !DateSupport.isBefore(response, root, HttpHeaders.DATE)) {
+                            !HttpCacheEntry.isNewer(root, response)) {
                         evictAll(root, rootKey);
                     }
                 }
@@ -548,7 +548,7 @@ public Cancellable evictInvalidatedEntries(
                 !Method.isSafe(request.getMethod())) {
             final String rootKey = cacheKeyGenerator.generateKey(host, request);
             evict(rootKey);
-            final URI requestUri = CacheSupport.normalize(CacheSupport.getRequestUri(request, host));
+            final URI requestUri = CacheKeyGenerator.normalize(CacheKeyGenerator.getRequestUri(host, request));
             if (requestUri != null) {
                 final URI contentLocation = CacheSupport.getLocationURI(requestUri, response, HttpHeaders.CONTENT_LOCATION);
                 if (contentLocation != null && CacheSupport.isSameOrigin(requestUri, contentLocation)) {

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

@@ -329,7 +329,7 @@ private void evict(final String rootKey, final HttpResponse response) {
         final Header newETag = response.getFirstHeader(HttpHeaders.ETAG);
         if (existingETag != null && newETag != null &&
                 !Objects.equals(existingETag.getValue(), newETag.getValue()) &&
-                !DateSupport.isBefore(response, root, HttpHeaders.DATE)) {
+                !HttpCacheEntry.isNewer(root, response)) {
             evictAll(root, rootKey);
         }
     }
@@ -344,7 +344,7 @@ public void evictInvalidatedEntries(final HttpHost host, final HttpRequest reque
                 !Method.isSafe(request.getMethod())) {
             final String rootKey = cacheKeyGenerator.generateKey(host, request);
             evict(rootKey);
-            final URI requestUri = CacheSupport.normalize(CacheSupport.getRequestUri(request, host));
+            final URI requestUri = CacheKeyGenerator.normalize(CacheKeyGenerator.getRequestUri(host, request));
             if (requestUri != null) {
                 final URI contentLocation = CacheSupport.getLocationURI(requestUri, response, HttpHeaders.CONTENT_LOCATION);
                 if (contentLocation != null && CacheSupport.isSameOrigin(requestUri, contentLocation)) {

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

@@ -118,7 +118,7 @@ public void parse(final Iterator<Header> headerIterator, final BiConsumer<String
             } else {
                 final String s = header.getValue();
                 if (s == null) {
-                    return;
+                    continue;
                 }
                 buffer = new CharArrayBuffer(s.length());
                 buffer.append(s);
@@ -250,18 +250,11 @@ public final RequestCacheControl parse(final HttpRequest request) {
     }
 
     private static long parseSeconds(final String name, final String value) {
-        if (TextUtils.isEmpty(value)) {
-            return -1;
+        final long delta = CacheSupport.deltaSeconds(value);
+        if (delta == -1 && LOG.isDebugEnabled()) {
+            LOG.debug("Directive {} was malformed: {}", name, value);
         }
-        try {
-            final long n = Long.parseLong(value);
-            return n < 0 ? -1 : n;
-        } catch (final NumberFormatException e) {
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Directive {} was malformed: {}", name, value);
-            }
-        }
-        return 0;
+        return delta;
     }
 
 }

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

@@ -38,14 +38,18 @@
 
 import org.apache.hc.client5.http.cache.HttpCacheEntry;
 import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.function.Resolver;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.MessageHeaders;
+import org.apache.hc.core5.http.URIScheme;
 import org.apache.hc.core5.net.PercentCodec;
+import org.apache.hc.core5.net.URIAuthority;
+import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.Args;
 
 /**
@@ -61,6 +65,91 @@ public String resolve(final URI uri) {
         return generateKey(uri);
     }
 
+    /**
+     * Returns text representation of the request URI of the given {@link HttpRequest}.
+     * This method will use {@link HttpRequest#getPath()}, {@link HttpRequest#getScheme()} and
+     * {@link HttpRequest#getAuthority()} values when available or attributes of target
+     * {@link HttpHost } in order to construct an absolute URI.
+     * <p>
+     * This method will not attempt to ensure validity of the resultant text representation.
+     *
+     * @param target target host
+     * @param request the {@link HttpRequest}
+     *
+     * @return String the request URI
+     */
+    @Internal
+    public static String getRequestUri(final HttpHost target, final HttpRequest request) {
+        Args.notNull(target, "Target");
+        Args.notNull(request, "HTTP request");
+        final StringBuilder buf = new StringBuilder();
+        final URIAuthority authority = request.getAuthority();
+        if (authority != null) {
+            final String scheme = request.getScheme();
+            buf.append(scheme != null ? scheme : URIScheme.HTTP.id).append("://");
+            buf.append(authority.getHostName());
+            if (authority.getPort() >= 0) {
+                buf.append(":").append(authority.getPort());
+            }
+        } else {
+            buf.append(target.getSchemeName()).append("://");
+            buf.append(target.getHostName());
+            if (target.getPort() >= 0) {
+                buf.append(":").append(target.getPort());
+            }
+        }
+        final String path = request.getPath();
+        if (path == null) {
+            buf.append("/");
+        } else {
+            if (buf.length() > 0 && !path.startsWith("/")) {
+                buf.append("/");
+            }
+            buf.append(path);
+        }
+        return buf.toString();
+    }
+
+    /**
+     * Returns normalized representation of the request URI optimized for use as a cache key.
+     * This method ensures the resultant URI has an explicit port in the authority component,
+     * and explicit path component and no fragment.
+     */
+    @Internal
+    public static URI normalize(final URI requestUri) throws URISyntaxException {
+        Args.notNull(requestUri, "URI");
+        final URIBuilder builder = new URIBuilder(requestUri);
+        if (builder.getHost() != null) {
+            if (builder.getScheme() == null) {
+                builder.setScheme(URIScheme.HTTP.id);
+            }
+            if (builder.getPort() <= -1) {
+                if (URIScheme.HTTP.same(builder.getScheme())) {
+                    builder.setPort(80);
+                } else if (URIScheme.HTTPS.same(builder.getScheme())) {
+                    builder.setPort(443);
+                }
+            }
+        }
+        builder.setFragment(null);
+        return builder.normalizeSyntax().build();
+    }
+
+    /**
+     * Lenient URI parser that normalizes valid {@link URI}s and returns {@code null} for malformed URIs.
+     */
+    @Internal
+    public static URI normalize(final String requestUri) {
+        if (requestUri == null) {
+            return null;
+        }
+        try {
+            return CacheKeyGenerator.normalize(new URI(requestUri));
+        } catch (final URISyntaxException ex) {
+            return null;
+        }
+    }
+
     /**
      * Computes a key for the given request {@link URI} that can be used as
      * a unique identifier for cached resources. The URI is expected to
@@ -71,7 +160,7 @@ public String resolve(final URI uri) {
      */
     public String generateKey(final URI requestUri) {
         try {
-            final URI normalizeRequestUri = CacheSupport.normalize(requestUri);
+            final URI normalizeRequestUri = normalize(requestUri);
             return normalizeRequestUri.toASCIIString();
         } catch (final URISyntaxException ex) {
             return requestUri.toASCIIString();
@@ -87,7 +176,7 @@ public String generateKey(final URI requestUri) {
      * @return cache key
      */
     public String generateKey(final HttpHost host, final HttpRequest request) {
-        final String s = CacheSupport.getRequestUri(request, host);
+        final String s = getRequestUri(host, request);
         try {
             return generateKey(new URI(s));
         } catch (final URISyntaxException ex) {