From 9581cbc7a04f3ce090baa69fe8adecfc15e9a52b Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 8 Dec 2017 13:27:03 +0100 Subject: [PATCH] Factored out request URI generation and normalization logic fron CacheKeyGenerator into HttpCacheSupport --- .../http/impl/cache/CacheKeyGenerator.java | 106 +++++---------- .../impl/cache/DefaultCacheInvalidator.java | 13 +- .../http/impl/cache/HttpCacheSupport.java | 123 ++++++++++++++++++ .../impl/cache/TestCacheKeyGenerator.java | 24 ++-- 4 files changed, 174 insertions(+), 92 deletions(-) create mode 100644 httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java 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 index 770634024..3975e8d97 100644 --- 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,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/"); - - private URI normalize(final URI uri) throws URISyntaxException { - final URIBuilder builder = new URIBuilder(URIUtils.resolve(BASE_URI, uri)) ; - 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); - } - } + /** + * 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 + * in an absolute form. + * + * @param requestUri request URI + * @return cache key + */ + public String generateKey(final URI requestUri) { + try { + final URI normalizeRequestUri = HttpCacheSupport.normalize(requestUri); + return normalizeRequestUri.toASCIIString(); + } 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 - * pair that I can use as an identifier KEY into my HttpCache + * Computes a key for the given {@link HttpHost} and {@link HttpRequest} + * that can be used as a unique identifier for cached resources. * * @param host The host for this request - * @param req the {@link HttpRequest} - * @return String the extracted URI + * @param request the {@link HttpRequest} + * @return cache key */ - public String generateKey(final HttpHost host, final HttpRequest req) { - final StringBuilder buf = new StringBuilder(); - 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(); + public String generateKey(final HttpHost host, final HttpRequest request) { + final String s = HttpCacheSupport.getRequestUri(request, host); try { - URI uri = new URI(s); - if (!uri.isAbsolute()) { - uri = URIUtils.rewriteURI(uri, host); - } - return normalize(uri).toASCIIString(); + return generateKey(new URI(s)); } catch (final URISyntaxException ex) { return s; } } - public String generateKey(final URI uri) { - if (uri == null) { - return null; - } - try { - return normalize(uri).toASCIIString(); - } catch (final URISyntaxException ex) { - return uri.toString(); - } - } - - protected String getFullHeaderValue(final Header[] headers) { + private 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 - * VARY header - I need to get an additional URI from the pair of host and - * request so that I can also store the variant into my HttpCache. + * Computes a key for the given {@link HttpHost} and {@link HttpRequest} + * that can be used as a unique identifier for cached resources. if the request has a + * {@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 variantHeaderNames = new ArrayList<>(); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheInvalidator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheInvalidator.java index 553bd425e..e98894754 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/DefaultCacheInvalidator.java +++ b/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)); - if (canonicalTarget == null) { - return; - } - if (canonicalTarget.getAuthority().equalsIgnoreCase(requestURI.getAuthority())) { - flushEntry(canonicalTarget.toString()); + try { + final URI canonicalTarget = HttpCacheSupport.normalize(targetURI); + if (canonicalTarget.isAbsolute() + && canonicalTarget.getAuthority().equalsIgnoreCase(requestURI.getAuthority())) { + flushEntry(canonicalTarget.toString()); + } + } catch (final URISyntaxException ignore) { } } 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 new file mode 100644 index 000000000..34f4ec4d3 --- /dev/null +++ b/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 + * . + * + */ +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. + *

+ * 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(); + } + +} diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java index d14028dda..f9f928ad4 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java +++ b/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; } };