You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2023/12/23 12:24:24 UTC

(httpcomponents-client) branch master updated: Replace internal methods with those from MessageSupport

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git


The following commit(s) were added to refs/heads/master by this push:
     new 926b2f588 Replace internal methods with those from MessageSupport
926b2f588 is described below

commit 926b2f5884aec607b3304487f894c9d34f1897a2
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Dec 22 12:14:39 2023 +0100

    Replace internal methods with those from MessageSupport
---
 .../client5/http/cache/HttpCacheEntryFactory.java  | 58 +---------------------
 .../client5/http/impl/cache/CacheKeyGenerator.java |  3 +-
 .../hc/client5/http/impl/cache/CacheSupport.java   | 40 ---------------
 .../http/impl/cache/CacheValidityPolicy.java       |  3 +-
 .../cache/CachedResponseSuitabilityChecker.java    |  2 +-
 .../http/impl/cache/ResponseCachingPolicy.java     |  6 +--
 .../http/cache/TestHttpCacheEntryFactory.java      | 27 ----------
 .../hc/client5/http/impl/cache/HttpTestUtils.java  |  4 +-
 8 files changed, 12 insertions(+), 131 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
index 46a8349e6..ee12c52cf 100644
--- 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
@@ -29,27 +29,22 @@ package org.apache.hc.client5.http.cache;
 import java.net.URI;
 import java.time.Instant;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.Set;
-import java.util.TreeSet;
 
 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;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.Header;
-import org.apache.hc.core5.http.HeaderElements;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpMessage;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.HttpStatus;
-import org.apache.hc.core5.http.MessageHeaders;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.message.HeaderGroup;
 import org.apache.hc.core5.http.message.MessageSupport;
@@ -65,39 +60,6 @@ public class HttpCacheEntryFactory {
 
     public static final HttpCacheEntryFactory INSTANCE = new HttpCacheEntryFactory();
 
-    private final static Set<String> HOP_BY_HOP;
-
-    static {
-        final TreeSet<String> set = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
-        set.add(HttpHeaders.CONNECTION);
-        set.add(HttpHeaders.CONTENT_LENGTH);
-        set.add(HttpHeaders.TRANSFER_ENCODING);
-        set.add(HttpHeaders.HOST);
-        set.add(HttpHeaders.KEEP_ALIVE);
-        set.add(HttpHeaders.TE);
-        set.add(HttpHeaders.UPGRADE);
-        set.add(HttpHeaders.PROXY_AUTHORIZATION);
-        set.add("Proxy-Authentication-Info");
-        set.add(HttpHeaders.PROXY_AUTHENTICATE);
-        HOP_BY_HOP = Collections.unmodifiableSet(set);
-    }
-
-    @Internal
-    public static boolean isHopByHop(final String headerName) {
-        if (headerName == null) {
-            return false;
-        }
-        return HOP_BY_HOP.contains(headerName);
-    }
-
-    @Internal
-    public static boolean isHopByHop(final Header header) {
-        if (header == null) {
-            return false;
-        }
-        return isHopByHop(header.getName());
-    }
-
     private static HeaderGroup headers(final Iterator<Header> it) {
         final HeaderGroup headerGroup = new HeaderGroup();
         while (it.hasNext()) {
@@ -115,7 +77,7 @@ public class HttpCacheEntryFactory {
                 headerGroup.addHeader(entryHeader);
             }
         }
-        final Set<String> responseHopByHop = HttpCacheEntryFactory.hopByHopConnectionSpecific(response);
+        final Set<String> responseHopByHop = MessageSupport.hopByHopConnectionSpecific(response);
         for (final Iterator<Header> it = response.headerIterator(); it.hasNext(); ) {
             final Header responseHeader = it.next();
             final String headerName = responseHeader.getName();
@@ -129,24 +91,8 @@ public class HttpCacheEntryFactory {
     /**
      * This method should be provided by the core
      */
-    static Set<String> hopByHopConnectionSpecific(final MessageHeaders headers) {
-        final Header connectionHeader = headers.getFirstHeader(HttpHeaders.CONNECTION);
-        final String connDirective = connectionHeader != null ? connectionHeader.getValue() : null;
-        // Disregard most common 'Close' and 'Keep-Alive' tokens
-        if (connDirective != null &&
-                !connDirective.equalsIgnoreCase(HeaderElements.CLOSE) &&
-                !connDirective.equalsIgnoreCase(HeaderElements.KEEP_ALIVE)) {
-            final TreeSet<String> result = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
-            result.addAll(HOP_BY_HOP);
-            result.addAll(MessageSupport.parseTokens(connectionHeader));
-            return result;
-        } else {
-            return HOP_BY_HOP;
-        }
-    }
-
     static HeaderGroup filterHopByHopHeaders(final HttpMessage message) {
-        final Set<String> hopByHop = hopByHopConnectionSpecific(message);
+        final Set<String> hopByHop = MessageSupport.hopByHopConnectionSpecific(message);
         final HeaderGroup headerGroup = new HeaderGroup();
         for (final Iterator<Header> it = message.headerIterator(); it.hasNext(); ) {
             final Header header = it.next();
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 be346370e..e02626758 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
@@ -56,6 +56,7 @@ import org.apache.hc.core5.http.URIScheme;
 import org.apache.hc.core5.http.message.BasicHeaderElementIterator;
 import org.apache.hc.core5.http.message.BasicHeaderValueFormatter;
 import org.apache.hc.core5.http.message.BasicNameValuePair;
+import org.apache.hc.core5.http.message.MessageSupport;
 import org.apache.hc.core5.net.PercentCodec;
 import org.apache.hc.core5.net.URIAuthority;
 import org.apache.hc.core5.net.URIBuilder;
@@ -207,7 +208,7 @@ public class CacheKeyGenerator implements Resolver<URI, String> {
         final List<String> names = new ArrayList<>();
         for (final Iterator<Header> it = message.headerIterator(HttpHeaders.VARY); it.hasNext(); ) {
             final Header header = it.next();
-            CacheSupport.parseTokens(header, names::add);
+            MessageSupport.parseTokens(header, names::add);
         }
         return names;
     }
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
index 380dbae96..319941220 100644
--- 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,21 +27,14 @@
 package org.apache.hc.client5.http.impl.cache;
 
 import java.net.URI;
-import java.util.BitSet;
 import java.util.Objects;
-import java.util.function.Consumer;
 
 import org.apache.hc.client5.http.utils.URIUtils;
 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.MessageHeaders;
-import org.apache.hc.core5.http.message.ParserCursor;
-import org.apache.hc.core5.util.Args;
-import org.apache.hc.core5.util.CharArrayBuffer;
 import org.apache.hc.core5.util.TextUtils;
 import org.apache.hc.core5.util.TimeValue;
-import org.apache.hc.core5.util.Tokenizer;
 
 /**
  * HTTP cache support utilities.
@@ -51,39 +44,6 @@ import org.apache.hc.core5.util.Tokenizer;
 @Internal
 public final class CacheSupport {
 
-    private static final BitSet COMMA = Tokenizer.INIT_BITSET(',');
-
-    // This method should be provided by MessageSupport from core
-    public static void parseTokens(final CharSequence src, final ParserCursor cursor, final Consumer<String> consumer) {
-        Args.notNull(src, "Source");
-        Args.notNull(cursor, "Cursor");
-        while (!cursor.atEnd()) {
-            final int pos = cursor.getPos();
-            if (src.charAt(pos) == ',') {
-                cursor.updatePos(pos + 1);
-            }
-            final String token = Tokenizer.INSTANCE.parseToken(src, cursor, COMMA);
-            if (consumer != null) {
-                consumer.accept(token);
-            }
-        }
-    }
-
-    // This method should be provided by MessageSupport from core
-    public static void parseTokens(final Header header, final Consumer<String> consumer) {
-        Args.notNull(header, "Header");
-        if (header instanceof FormattedHeader) {
-            final CharArrayBuffer buf = ((FormattedHeader) header).getBuffer();
-            final ParserCursor cursor = new ParserCursor(0, buf.length());
-            cursor.updatePos(((FormattedHeader) header).getValuePos());
-            parseTokens(buf, cursor, consumer);
-        } else {
-            final String value = header.getValue();
-            final ParserCursor cursor = new ParserCursor(0, value.length());
-            parseTokens(value, cursor, consumer);
-        }
-    }
-
     public static URI getLocationURI(final URI requestUri, final MessageHeaders response, final String headerName) {
         final Header h = response.getFirstHeader(headerName);
         if (h == null) {
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
index 2b2a2d52b..63d941763 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java
@@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import org.apache.hc.client5.http.cache.HttpCacheEntry;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.message.MessageSupport;
 import org.apache.hc.core5.util.TimeValue;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -177,7 +178,7 @@ class CacheValidityPolicy {
         final Header age = entry.getFirstHeader(HttpHeaders.AGE);
         if (age != null) {
             final AtomicReference<String> firstToken = new AtomicReference<>();
-            CacheSupport.parseTokens(age, token -> firstToken.compareAndSet(null, token));
+            MessageSupport.parseTokens(age, token -> firstToken.compareAndSet(null, token));
             final long delta = CacheSupport.deltaSeconds(firstToken.get());
             if (delta == -1 && LOG.isDebugEnabled()) {
                 LOG.debug("Malformed Age value: {}", age);
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java
index 4e354cafc..2a655e747 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java
@@ -221,7 +221,7 @@ class CachedResponseSuitabilityChecker {
             final Set<String> headerNames = new HashSet<>();
             while (it.hasNext()) {
                 final Header header = it.next();
-                CacheSupport.parseTokens(header, e -> {
+                MessageSupport.parseTokens(header, e -> {
                     headerNames.add(e.toLowerCase(Locale.ROOT));
                 });
             }
diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
index 31dd0f3dd..93d7dc27b 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
@@ -38,7 +38,7 @@ import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.HttpVersion;
 import org.apache.hc.core5.http.Method;
 import org.apache.hc.core5.http.ProtocolVersion;
-import org.apache.hc.core5.http.message.BasicTokenIterator;
+import org.apache.hc.core5.http.message.MessageSupport;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -170,7 +170,7 @@ class ResponseCachingPolicy {
         }
 
         // Treat responses with `Vary: *` as essentially non-cacheable.
-        final Iterator<String> it = new BasicTokenIterator(response.headerIterator(HttpHeaders.VARY));
+        final Iterator<String> it = MessageSupport.iterateTokens(response, HttpHeaders.VARY);
         while (it.hasNext()) {
             final String token = it.next();
             if ("*".equals(token)) {
@@ -303,7 +303,7 @@ class ResponseCachingPolicy {
     }
 
     private boolean from1_0Origin(final HttpResponse response) {
-        final Iterator<String> it = new BasicTokenIterator(response.headerIterator(HttpHeaders.VIA));
+        final Iterator<String> it = MessageSupport.iterateTokens(response, HttpHeaders.VIA);
         if (it.hasNext()) {
             final String token = it.next();
             return token.startsWith("1.0 ") || token.startsWith("HTTP/1.0 ");
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java
index 7b2bd7ed5..2f1bdcb71 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestHttpCacheEntryFactory.java
@@ -47,7 +47,6 @@ import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.message.BasicHttpRequest;
 import org.apache.hc.core5.http.message.BasicHttpResponse;
 import org.apache.hc.core5.http.message.HeaderGroup;
-import org.apache.hc.core5.http.support.BasicResponseBuilder;
 import org.hamcrest.MatcherAssert;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -87,32 +86,6 @@ public class TestHttpCacheEntryFactory {
         impl = new HttpCacheEntryFactory();
     }
 
-    @Test
-    public void testHopByHopHeaders() {
-        Assertions.assertTrue(HttpCacheEntryFactory.isHopByHop("Connection"));
-        Assertions.assertTrue(HttpCacheEntryFactory.isHopByHop("connection"));
-        Assertions.assertTrue(HttpCacheEntryFactory.isHopByHop("coNNection"));
-        Assertions.assertFalse(HttpCacheEntryFactory.isHopByHop("Content-Type"));
-        Assertions.assertFalse(HttpCacheEntryFactory.isHopByHop("huh"));
-    }
-
-    @Test
-    public void testHopByHopHeadersConnectionSpecific() {
-        final HttpResponse response = BasicResponseBuilder.create(HttpStatus.SC_OK)
-                .addHeader(HttpHeaders.CONNECTION, "blah, blah, this, that")
-                .addHeader(HttpHeaders.CONTENT_TYPE, ContentType.TEXT_PLAIN.toString())
-                .build();
-        final Set<String> hopByHopConnectionSpecific = HttpCacheEntryFactory.hopByHopConnectionSpecific(response);
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("Connection"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("connection"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("coNNection"));
-        Assertions.assertFalse(hopByHopConnectionSpecific.contains("Content-Type"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("blah"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("Blah"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("This"));
-        Assertions.assertTrue(hopByHopConnectionSpecific.contains("That"));
-    }
-
     @Test
     public void testFilterHopByHopAndConnectionSpecificHeaders() {
         response.setHeaders(
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java
index 7f5a8d056..67e43d70a 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java
@@ -37,7 +37,6 @@ import java.util.concurrent.CountDownLatch;
 import java.util.function.Consumer;
 
 import org.apache.hc.client5.http.cache.HttpCacheEntry;
-import org.apache.hc.client5.http.cache.HttpCacheEntryFactory;
 import org.apache.hc.client5.http.cache.Resource;
 import org.apache.hc.client5.http.utils.DateUtils;
 import org.apache.hc.core5.concurrent.FutureCallback;
@@ -57,6 +56,7 @@ import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
 import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.message.HeaderGroup;
+import org.apache.hc.core5.http.message.MessageSupport;
 import org.apache.hc.core5.util.ByteArrayBuffer;
 import org.junit.jupiter.api.Assertions;
 
@@ -116,7 +116,7 @@ public class HttpTestUtils {
      */
     public static boolean isEndToEndHeaderSubset(final HttpMessage r1, final HttpMessage r2) {
         for (final Header h : r1.getHeaders()) {
-            if (!HttpCacheEntryFactory.isHopByHop(h)) {
+            if (!MessageSupport.isHopByHop(h.getName())) {
                 final String r1val = getCanonicalHeaderValue(r1, h.getName());
                 final String r2val = getCanonicalHeaderValue(r2, h.getName());
                 if (!Objects.equals(r1val, r2val)) {