Factored out request URI generation and normalization logic fron CacheKeyGenerator into HttpCacheSupport

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

@@ -38,7 +38,6 @@ import java.util.List;
 
 import org.apache.hc.client5.http.cache.HeaderConstants;
 import org.apache.hc.client5.http.cache.HttpCacheEntry;
-import org.apache.hc.client5.http.utils.URIUtils;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.Header;
@@ -46,8 +45,6 @@ import org.apache.hc.core5.http.HeaderElement;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.message.MessageSupport;
-import org.apache.hc.core5.net.URIAuthority;
-import org.apache.hc.core5.net.URIBuilder;
 
 /**
  * @since 4.1
@@ -55,80 +52,41 @@ import org.apache.hc.core5.net.URIBuilder;
 @Contract(threading = ThreadingBehavior.IMMUTABLE)
 class CacheKeyGenerator {
 
-    private static final URI BASE_URI = URI.create("http://example.com/");
+    /**
-
+     * Computes a key for the given request {@link URI} that can be used as
-    private URI normalize(final URI uri) throws URISyntaxException {
+     * a unique identifier for cached resources. The URI is expected to
-        final URIBuilder builder = new URIBuilder(URIUtils.resolve(BASE_URI, uri)) ;
+     * in an absolute form.
-        if (builder.getHost() != null) {
+     *
-            if (builder.getScheme() == null) {
+     * @param requestUri request URI
-                builder.setScheme("http");
+     * @return cache key
-            }
+     */
-            if (builder.getPort() == -1) {
+    public String generateKey(final URI requestUri) {
-                if ("http".equalsIgnoreCase(builder.getScheme())) {
+        try {
-                    builder.setPort(80);
+            final URI normalizeRequestUri = HttpCacheSupport.normalize(requestUri);
-                } else if ("https".equalsIgnoreCase(builder.getScheme())) {
+            return normalizeRequestUri.toASCIIString();
-                    builder.setPort(443);
+        } catch (final URISyntaxException ex) {
-                }
+            return requestUri.toASCIIString();
-            }
         }
-        if (builder.getPath() == null) {
-            builder.setPath("/");
-        }
-        return builder.build();
     }
 
     /**
-     * For a given {@link HttpHost} and {@link HttpRequest} get a URI from the
+     * Computes a key for the given {@link HttpHost} and {@link HttpRequest}
-     * pair that I can use as an identifier KEY into my HttpCache
+     * that can be used as a unique identifier for cached resources.
      *
      * @param host The host for this request
-     * @param req the {@link HttpRequest}
+     * @param request the {@link HttpRequest}
-     * @return String the extracted URI
+     * @return cache key
      */
-    public String generateKey(final HttpHost host, final HttpRequest req) {
+    public String generateKey(final HttpHost host, final HttpRequest request) {
-        final StringBuilder buf = new StringBuilder();
+        final String s = HttpCacheSupport.getRequestUri(request, host);
-        final URIAuthority authority = req.getAuthority();
-        if (authority != null) {
-            final String scheme = req.getScheme();
-            buf.append(scheme != null ? scheme : "http").append("://");
-            buf.append(authority.getHostName());
-            if (authority.getPort() >= 0) {
-                buf.append(":").append(authority.getPort());
-            }
-        }
-        final String path = req.getPath();
-        if (path == null) {
-            buf.append("/");
-        } else {
-            if (buf.length() > 0 && !path.startsWith("/")) {
-                buf.append("/");
-            }
-            buf.append(path);
-        }
-        final String s = buf.toString();
         try {
-            URI uri = new URI(s);
+            return generateKey(new URI(s));
-            if (!uri.isAbsolute()) {
-                uri = URIUtils.rewriteURI(uri, host);
-            }
-            return normalize(uri).toASCIIString();
         } catch (final URISyntaxException ex) {
             return s;
         }
     }
 
-    public String generateKey(final URI uri) {
+    private String getFullHeaderValue(final Header[] headers) {
-        if (uri == null) {
-            return null;
-        }
-        try {
-            return normalize(uri).toASCIIString();
-        } catch (final URISyntaxException ex) {
-            return uri.toString();
-        }
-    }
-
-    protected String getFullHeaderValue(final Header[] headers) {
         if (headers == null) {
             return "";
         }
@@ -144,30 +102,30 @@ class CacheKeyGenerator {
     }
 
     /**
-     * For a given {@link HttpHost} and {@link HttpRequest} if the request has a
+     * Computes a key for the given {@link HttpHost} and {@link HttpRequest}
-     * VARY header - I need to get an additional URI from the pair of host and
+     * that can be used as a unique identifier for cached resources. if the request has a
-     * request so that I can also store the variant into my HttpCache.
+     * {@literal VARY} header the identifier will also include variant key.
      *
      * @param host The host for this request
-     * @param req the {@link HttpRequest}
+     * @param request the {@link HttpRequest}
      * @param entry the parent entry used to track the variants
-     * @return String the extracted variant URI
+     * @return cache key
      */
-    public String generateVariantURI(final HttpHost host, final HttpRequest req, final HttpCacheEntry entry) {
+    public String generateVariantURI(final HttpHost host, final HttpRequest request, final HttpCacheEntry entry) {
         if (!entry.hasVariants()) {
-            return generateKey(host, req);
+            return generateKey(host, request);
         }
-        return generateVariantKey(req, entry) + generateKey(host, req);
+        return generateVariantKey(request, entry) + generateKey(host, request);
     }
 
     /**
-     * Compute a "variant key" from the headers of a given request that are
+     * Computes a "variant key" from the headers of a given request that are
      * covered by the Vary header of a given cache entry. Any request whose
      * varying headers match those of this request should have the same
      * variant key.
      * @param req originating request
      * @param entry cache entry in question that has variants
-     * @return a {@code String} variant key
+     * @return variant key
      */
     public String generateVariantKey(final HttpRequest req, final HttpCacheEntry entry) {
         final List<String> variantHeaderNames = new ArrayList<>();

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

@@ -159,12 +159,13 @@ class DefaultCacheInvalidator implements HttpCacheInvalidator {
     }
 
     protected void flushUriIfSameHost(final URI requestURI, final URI targetURI) {
-        final URI canonicalTarget = parse(cacheKeyGenerator.generateKey(targetURI));
+        try {
-        if (canonicalTarget == null) {
+            final URI canonicalTarget = HttpCacheSupport.normalize(targetURI);
-            return;
+            if (canonicalTarget.isAbsolute()
-        }
+                    && canonicalTarget.getAuthority().equalsIgnoreCase(requestURI.getAuthority())) {
-        if (canonicalTarget.getAuthority().equalsIgnoreCase(requestURI.getAuthority())) {
+                flushEntry(canonicalTarget.toString());
-            flushEntry(canonicalTarget.toString());
+            }
+        } catch (final URISyntaxException ignore) {
         }
     }
 

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

@@ -0,0 +1,123 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.cache;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hc.client5.http.utils.URIUtils;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.net.URIAuthority;
+import org.apache.hc.core5.net.URIBuilder;
+import org.apache.hc.core5.util.Args;
+
+/**
+ * HTTP cache support utilities.
+ *
+ * @since 5.0
+ */
+public final class HttpCacheSupport {
+
+    private static final URI BASE_URI = URI.create("http://example.com/");
+
+    /**
+     * 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 request the {@link HttpRequest}
+     * @param target target host
+     *
+     * @return String the request URI
+     */
+    public static String getRequestUri(final HttpRequest request, final HttpHost target) {
+        Args.notNull(request, "HTTP request");
+        Args.notNull(target, "Target");
+        final StringBuilder buf = new StringBuilder();
+        final URIAuthority authority = request.getAuthority();
+        if (authority != null) {
+            final String scheme = request.getScheme();
+            buf.append(scheme != null ? scheme : "http").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.
+     *
+     * @param requestUri original request URI
+     * @return normalized URI.
+     * @throws URISyntaxException
+     */
+    public static URI normalize(final URI requestUri) throws URISyntaxException {
+        Args.notNull(requestUri, "URI");
+        final URIBuilder builder = new URIBuilder(requestUri.isAbsolute() ? URIUtils.resolve(BASE_URI, requestUri) : requestUri) ;
+        if (builder.getHost() != null) {
+            if (builder.getScheme() == null) {
+                builder.setScheme("http");
+            }
+            if (builder.getPort() <= -1) {
+                if ("http".equalsIgnoreCase(builder.getScheme())) {
+                    builder.setPort(80);
+                } else if ("https".equalsIgnoreCase(builder.getScheme())) {
+                    builder.setPort(443);
+                }
+            }
+        }
+        builder.setFragment(null);
+        if (builder.getPath() == null) {
+            builder.setPath("/");
+        }
+        return builder.build();
+    }
+
+}

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

@@ -120,9 +120,9 @@ public class TestCacheKeyGenerator {
         when(mockEntry.hasVariants()).thenReturn(false);
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };
@@ -140,9 +140,9 @@ public class TestCacheKeyGenerator {
 
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };
@@ -165,9 +165,9 @@ public class TestCacheKeyGenerator {
         final Header[] varyHeaders = { new BasicHeader("Vary", "Accept-Encoding") };
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };
@@ -192,9 +192,9 @@ public class TestCacheKeyGenerator {
         final Header[] uaHeaders = { new BasicHeader("User-Agent", "browser") };
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };
@@ -221,9 +221,9 @@ public class TestCacheKeyGenerator {
         final Header[] uaHeaders = { new BasicHeader("User-Agent", "browser") };
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };
@@ -250,9 +250,9 @@ public class TestCacheKeyGenerator {
         final Header[] uaHeaders = { new BasicHeader("User-Agent", "browser") };
         extractor = new CacheKeyGenerator() {
             @Override
-            public String generateKey(final HttpHost h, final HttpRequest req) {
+            public String generateKey(final HttpHost h, final HttpRequest request) {
                 Assert.assertSame(defaultHost, h);
-                Assert.assertSame(mockRequest, req);
+                Assert.assertSame(mockRequest, request);
                 return theURI;
             }
         };