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 2021/02/06 17:12:12 UTC

[httpcomponents-client] 02/02: Deprecated some URIUtils methods in favor of URIBuilder

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

commit 4b295dff15d1eac091e6129b3a13f343885e7d0e
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Feb 6 18:05:01 2021 +0100

    Deprecated some URIUtils methods in favor of URIBuilder
---
 .../hc/client5/testing/sync/TestRedirects.java     | 46 ++++++++++------
 .../client5/http/impl/async/AsyncProtocolExec.java | 12 +++--
 .../hc/client5/http/impl/classic/ProtocolExec.java |  8 +--
 .../org/apache/hc/client5/http/utils/URIUtils.java | 15 ++++++
 .../apache/hc/client5/http/utils/TestURIUtils.java | 61 ----------------------
 5 files changed, 56 insertions(+), 86 deletions(-)

diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
index a2fc26d..6cc7fd8 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
@@ -44,7 +44,6 @@ import org.apache.hc.client5.http.cookie.CookieStore;
 import org.apache.hc.client5.http.impl.cookie.BasicClientCookie;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.protocol.RedirectLocations;
-import org.apache.hc.client5.http.utils.URIUtils;
 import org.apache.hc.client5.testing.OldPathRedirectResolver;
 import org.apache.hc.client5.testing.classic.RedirectingDecorator;
 import org.apache.hc.client5.testing.redirect.Redirect;
@@ -65,6 +64,7 @@ import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.hc.core5.net.URIBuilder;
 import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Test;
@@ -93,7 +93,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_MULTIPLE_CHOICES, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/oldlocation/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/oldlocation/100").build(),
+                    reqWrapper.getUri());
 
             final RedirectLocations redirects = context.getRedirectLocations();
             Assert.assertNotNull(redirects);
@@ -123,7 +124,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_MULTIPLE_CHOICES, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/oldlocation/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/oldlocation/100").build(),
+                    reqWrapper.getUri());
 
             final RedirectLocations redirects = context.getRedirectLocations();
             Assert.assertNotNull(redirects);
@@ -154,13 +156,14 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             final RedirectLocations redirects = context.getRedirectLocations();
             Assert.assertNotNull(redirects);
             Assert.assertEquals(1, redirects.size());
 
-            final URI redirect = URIUtils.rewriteURI(new URI("/random/100"), target);
+            final URI redirect = new URIBuilder().setHttpHost(target).setPath("/random/100").build();
             Assert.assertTrue(redirects.contains(redirect));
 
             EntityUtils.consume(response.getEntity());
@@ -188,7 +191,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/50"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/50").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
@@ -253,7 +257,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/123"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/123").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
@@ -283,7 +288,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/oldlocation/stuff"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/oldlocation/stuff").build(),
+                    reqWrapper.getUri());
 
             final RedirectLocations redirects = context.getRedirectLocations();
             Assert.assertNotNull(redirects);
@@ -317,7 +323,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_USE_PROXY, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/oldlocation/stuff"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/oldlocation/stuff").build(),
+                    reqWrapper.getUri());
 
             final RedirectLocations redirects = context.getRedirectLocations();
             Assert.assertNotNull(redirects);
@@ -348,7 +355,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/123"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/123").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
@@ -433,7 +441,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/echo/stuff"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/echo/stuff").build(),
+                    reqWrapper.getUri());
             Assert.assertEquals("GET", reqWrapper.getMethod());
 
             EntityUtils.consume(response.getEntity());
@@ -473,7 +482,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
@@ -512,7 +522,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
@@ -619,7 +630,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             final Header[] headers = reqWrapper.getHeaders("Cookie");
             Assert.assertEquals("There can only be one (cookie)", 1, headers.length);
@@ -651,7 +663,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             final Header header = reqWrapper.getFirstHeader(HttpHeaders.USER_AGENT);
             Assert.assertEquals("my-test-client", header.getValue());
@@ -695,7 +708,8 @@ public class TestRedirects extends LocalServerTestBase {
             final HttpRequest reqWrapper = context.getRequest();
 
             Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+            Assert.assertEquals(new URIBuilder().setHttpHost(target).setPath("/random/100").build(),
+                    reqWrapper.getUri());
 
             EntityUtils.consume(response.getEntity());
         }
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
index 406c26f..7317a0d 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
@@ -46,7 +46,6 @@ import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.AuthSupport;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
-import org.apache.hc.client5.http.utils.URIUtils;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
@@ -64,6 +63,7 @@ import org.apache.hc.core5.http.nio.AsyncEntityProducer;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
 import org.apache.hc.core5.net.URIAuthority;
+import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.Args;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -113,16 +113,18 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         }
 
         final HttpRoute route = scope.route;
+        final HttpHost target = route.getTargetHost();
+        final HttpHost proxy = route.getProxyHost();
         final HttpClientContext clientContext = scope.clientContext;
 
         final HttpRequest request;
-        if (route.getProxyHost() != null && !route.isTunnelled()) {
+        if (proxy != null && !route.isTunnelled()) {
             try {
                 URI uri = userRequest.getUri();
                 if (!uri.isAbsolute()) {
-                    uri = URIUtils.rewriteURI(uri, route.getTargetHost(), true);
-                } else {
-                    uri = URIUtils.rewriteURI(uri);
+                    uri = new URIBuilder(uri)
+                            .setHttpHost(target)
+                            .build();
                 }
                 request = HttpProxyRequest.rewrite(userRequest, uri);
             } catch (final URISyntaxException ex) {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
index 4fb7a90..af79242 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
@@ -45,7 +45,6 @@ import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.AuthSupport;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
-import org.apache.hc.client5.http.utils.URIUtils;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
@@ -63,6 +62,7 @@ import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
 import org.apache.hc.core5.net.URIAuthority;
+import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.Args;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -127,9 +127,9 @@ public final class ProtocolExec implements ExecChainHandler {
                 try {
                     URI uri = userRequest.getUri();
                     if (!uri.isAbsolute()) {
-                        uri = URIUtils.rewriteURI(uri, target, true);
-                    } else {
-                        uri = URIUtils.rewriteURI(uri);
+                        uri = new URIBuilder(uri)
+                                .setHttpHost(target)
+                                .build();
                     }
                     request = ClassicHttpProxyRequest.rewrite(userRequest, uri);
                 } catch (final URISyntaxException ex) {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java b/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
index cfe8a35..f3e55ca 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
@@ -63,7 +63,10 @@ public class URIUtils {
      *
      * @throws URISyntaxException
      *             If the resulting URI is invalid.
+     *
+     * @deprecated Use {@link URIBuilder}.
      */
+    @Deprecated
     public static URI rewriteURI(
             final URI uri,
             final HttpHost target,
@@ -106,7 +109,10 @@ public class URIUtils {
      * A convenience method for
      * {@link URIUtils#rewriteURI(URI, HttpHost, boolean)} that always keeps the
      * fragment.
+     *
+     * @deprecated Use {@link URIBuilder}.
      */
+    @Deprecated
     public static URI rewriteURI(
             final URI uri,
             final HttpHost target) throws URISyntaxException {
@@ -123,7 +129,10 @@ public class URIUtils {
      *            original URI.
      * @throws URISyntaxException
      *             If the resulting URI is invalid.
+     *
+     * @deprecated Use {@link URIBuilder}.
      */
+    @Deprecated
     public static URI rewriteURI(final URI uri) throws URISyntaxException {
         Args.notNull(uri, "URI");
         if (uri.isOpaque()) {
@@ -287,7 +296,10 @@ public class URIUtils {
      * Convenience factory method for {@link URI} instances.
      *
      * @since 5.0
+     *
+     * @deprecated Use {@link URIBuilder}.
      */
+    @Deprecated
     public static URI create(final HttpHost host, final String path) throws URISyntaxException {
         final URIBuilder builder = new URIBuilder(path);
         if (host != null) {
@@ -300,7 +312,10 @@ public class URIUtils {
      * Convenience factory method for {@link URI} instances.
      *
      * @since 5.0
+     *
+     * @deprecated Use {@link URIBuilder}.
      */
+    @Deprecated
     public static URI create(final String scheme, final URIAuthority host, final String path) throws URISyntaxException {
         final URIBuilder builder = new URIBuilder(path);
         if (scheme != null) {
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java b/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
index 2d8e09a..1210f00 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
@@ -44,67 +44,6 @@ public class TestURIUtils {
     private final URI baseURI = URI.create("http://a/b/c/d;p?q");
 
     @Test
-    public void testRewrite() throws Exception {
-        final HttpHost target = new HttpHost("thathost", -1);
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost/stuff"), target).toString());
-        Assert.assertEquals("/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost/stuff"), null).toString());
-        Assert.assertEquals("/", URIUtils.rewriteURI(
-                URI.create("http://thishost//"), null).toString());
-        Assert.assertEquals("/stuff/morestuff", URIUtils.rewriteURI(
-                URI.create("http://thishost//stuff/morestuff"), null).toString());
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost/stuff#crap"), target, true).toString());
-        Assert.assertEquals("http://thathost/stuff#crap", URIUtils.rewriteURI(
-                URI.create("http://thishost/stuff#crap"), target, false).toString());
-        Assert.assertEquals("http://thathost/", URIUtils.rewriteURI(
-                URI.create("http://thishost#crap"), target, true).toString());
-        Assert.assertEquals("http://thathost/#crap", URIUtils.rewriteURI(
-                URI.create("http://thishost#crap"), target, false).toString());
-        Assert.assertEquals("/stuff/", URIUtils.rewriteURI(
-                URI.create("http://thishost//////////////stuff/"), null).toString());
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thathost/stuff")).toString());
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thathost/stuff#fragment")).toString());
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://userinfo@thathost/stuff#fragment")).toString());
-        Assert.assertEquals("http://thathost/", URIUtils.rewriteURI(
-                URI.create("http://thathost")).toString());
-        Assert.assertEquals("http://thathost/", URIUtils.rewriteURI(
-                URI.create("http://ThatHost")).toString());
-        Assert.assertEquals("http://that_host/", URIUtils.rewriteURI(
-                URI.create("http://That_Host")).toString());
-        Assert.assertEquals("http://thishost/Fragment_identifier%23Examples",
-                URIUtils.rewriteURI(
-                        URI.create("http://thishost/Fragment_identifier%23Examples")).toString());
-        Assert.assertEquals("http://thathost/foo%3Abar", URIUtils.rewriteURI(
-                URI.create("http://thishost/foo%3Abar"), target).toString());
-    }
-
-    @Test
-    public void testRewritePort() throws Exception {
-        HttpHost target = new HttpHost("thathost", 8080); // port should be copied
-        Assert.assertEquals("http://thathost:8080/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost:80/stuff#crap"), target, true).toString());
-        Assert.assertEquals("http://thathost:8080/stuff#crap", URIUtils.rewriteURI(
-                URI.create("http://thishost:80/stuff#crap"), target, false).toString());
-        target = new HttpHost("thathost", -1); // input port should be dropped
-        Assert.assertEquals("http://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost:80/stuff#crap"), target, true).toString());
-        Assert.assertEquals("http://thathost/stuff#crap", URIUtils.rewriteURI(
-                URI.create("http://thishost:80/stuff#crap"), target, false).toString());
-    }
-
-    @Test
-    public void testRewriteScheme() throws Exception {
-        final HttpHost target = new HttpHost("file", "thathost", -1); // scheme should be copied
-        Assert.assertEquals("file://thathost/stuff", URIUtils.rewriteURI(
-                URI.create("http://thishost:80/stuff#crap"), target, true).toString());
-    }
-
-    @Test
     public void testNormalization() {
         Assert.assertEquals("example://a/b/c/%7Bfoo%7D", URIUtils.resolve(this.baseURI, "eXAMPLE://a/./b/../b/%63/%7bfoo%7d").toString());
         Assert.assertEquals("http://www.example.com/%3C", URIUtils.resolve(this.baseURI, "http://www.example.com/%3c").toString());