Moved methods used by the public API classes from CacheSupport to Cac…

…heKeyGenerator; added test cases
apache · Oct 24, 2023 · 1435869 · 1435869
1 parent 819ea82
commit 1435869
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 109 deletions.
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntryFactory.java
@@ -36,7 +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.CacheKeyGenerator;
 import org.apache.hc.client5.http.utils.DateUtils;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.Internal;
@@ -163,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).
      *
@@ -215,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,

diff --git a/...lient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java b/...lient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java
@@ -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)) {

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java
@@ -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)) {

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheKeyGenerator.java b/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) {

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java
@@ -27,7 +27,6 @@
 package org.apache.hc.client5.http.impl.cache;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.BitSet;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -36,13 +35,8 @@
 import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.http.FormattedHeader;
 import org.apache.hc.core5.http.Header;
-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.http.message.ParserCursor;
-import org.apache.hc.core5.net.URIAuthority;
-import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.CharArrayBuffer;
 import org.apache.hc.core5.util.TextUtils;
@@ -57,90 +51,6 @@
 @Internal
 public final class CacheSupport {
 
-    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 : 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.
-     */
-    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(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.
-     */
-    public static URI normalize(final String requestUri) {
-        if (requestUri == null) {
-            return null;
-        }
-        try {
-            return normalize(new URI(requestUri));
-        } catch (final URISyntaxException ex) {
-            return null;
-        }
-    }
-
     private static final BitSet COMMA = Tokenizer.INIT_BITSET(',');
 
     // This method should be provided by MessageSupport from core
@@ -179,7 +89,7 @@ public static URI getLocationURI(final URI requestUri, final MessageHeaders resp
         if (h == null) {
             return null;
         }
-        final URI locationUri = CacheSupport.normalize(h.getValue());
+        final URI locationUri = CacheKeyGenerator.normalize(h.getValue());
         if (locationUri == null) {
             return requestUri;
         }

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java
@@ -42,14 +42,12 @@
 @Deprecated
 public final class HttpCacheSupport {
 
-    private static final URI BASE_URI = URI.create("http://example.com/");
-
     public static String getRequestUri(final HttpRequest request, final HttpHost target) {
-        return CacheSupport.getRequestUri(request, target);
+        return CacheKeyGenerator.getRequestUri(target, request);
     }
 
     public static URI normalize(final URI requestUri) throws URISyntaxException {
-        return CacheSupport.normalize(requestUri);
+        return CacheKeyGenerator.normalize(requestUri);
     }
 
     /**
@@ -73,7 +71,7 @@ public static URI normalizeQuetly(final String requestUri) {
      * @since 5.2
      */
     public static URI normalizeQuietly(final String requestUri) {
-        return CacheSupport.normalize(requestUri);
+        return CacheKeyGenerator.normalize(requestUri);
     }
 
 }
diff --git a/...ent5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java b/...ent5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java
@@ -26,6 +26,8 @@
  */
 package org.apache.hc.client5.http.impl.cache;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.Collections;
 
@@ -51,6 +53,54 @@ public void setUp() throws Exception {
         extractor = CacheKeyGenerator.INSTANCE;
     }
 
+    @Test
+    public void testGetRequestUri() {
+        Assertions.assertEquals("http://foo.example.com/stuff?huh",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("bar.example.com"),
+                        new HttpGet("http://foo.example.com/stuff?huh")));
+
+        Assertions.assertEquals("http://bar.example.com/stuff?huh",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("bar.example.com"),
+                        new HttpGet("/stuff?huh")));
+
+        Assertions.assertEquals("http://foo.example.com:8888/stuff?huh",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("bar.example.com", 8080),
+                        new HttpGet("http://foo.example.com:8888/stuff?huh")));
+
+        Assertions.assertEquals("https://bar.example.com:8443/stuff?huh",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("https", "bar.example.com", 8443),
+                        new HttpGet("/stuff?huh")));
+
+        Assertions.assertEquals("http://foo.example.com/",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("bar.example.com"),
+                        new HttpGet("http://foo.example.com")));
+
+        Assertions.assertEquals("http://bar.example.com/stuff?huh",
+                CacheKeyGenerator.getRequestUri(
+                        new HttpHost("bar.example.com"),
+                        new HttpGet("stuff?huh")));
+    }
+
+    @Test
+    public void testNormalizeRequestUri() throws URISyntaxException {
+        Assertions.assertEquals(URI.create("http://bar.example.com:80/stuff?huh"),
+                CacheKeyGenerator.normalize(URI.create("//bar.example.com/stuff?huh")));
+
+        Assertions.assertEquals(URI.create("http://bar.example.com:80/stuff?huh"),
+                CacheKeyGenerator.normalize(URI.create("http://bar.example.com/stuff?huh")));
+
+        Assertions.assertEquals(URI.create("http://bar.example.com:80/stuff?huh"),
+                CacheKeyGenerator.normalize(URI.create("http://bar.example.com/stuff?huh#there")));
+
+        Assertions.assertEquals(URI.create("http://bar.example.com:80/stuff?huh"),
+                CacheKeyGenerator.normalize(URI.create("HTTP://BAR.example.com/p1/p2/../../stuff?huh")));
+    }
+
     @Test
     public void testExtractsUriFromAbsoluteUriInRequest() {
         final HttpHost host = new HttpHost("bar.example.com");