Modify to not use URLEncodedUtils (opensearch-project#999)

* modify to not use URLEncodedUtils Signed-off-by: AntCode97 <[email protected]> * add a license header Signed-off-by: AntCode97 <[email protected]> * add pathEncoder test Signed-off-by: AntCode97 <[email protected]> * modify to use ByteBuffer in encodeBytes method Signed-off-by: AntCode97 <[email protected]> * modify to use URLEncoder Signed-off-by: AntCode97 <[email protected]> * delete PathEncoder Signed-off-by: AntCode97 <[email protected]> * modify to use URLEncodedUtils per version per Classpath Signed-off-by: AntCode97 <[email protected]> * fix to use MethodHandle instead of reflection Signed-off-by: AntCode97 <[email protected]> * add CHANGELOG.md Signed-off-by: AntCode97 <[email protected]> --------- Signed-off-by: AntCode97 <[email protected]> Signed-off-by: Andriy Redko <[email protected]>
reta · Jun 5, 2024 · 31fcd87 · 31fcd87
1 parent 2c69025
commit 31fcd87
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Fixed
 - ApacheHttpClient5Transport requires Apache Commons Logging dependency ([#1003](https://github.com/opensearch-project/opensearch-java/pull/1003))
 - Preserve caller information in stack traces when synchronous callers use asynchronous transports ([#656](https://github.com/opensearch-project/opensearch-java/pull/656))
+- Fix java.lang.NoSuchMethodError: org.apache.http.client.utils.URLEncodedUtils.formatSegments w/o httpclient ([#999](https://github.com/opensearch-project/opensearch-java/pull/999))
 
 ### Security
 

diff --git a/java-client/src/main/java/org/opensearch/client/transport/endpoints/SimpleEndpoint.java b/java-client/src/main/java/org/opensearch/client/transport/endpoints/SimpleEndpoint.java
@@ -35,10 +35,10 @@
 import java.util.Collections;
 import java.util.Map;
 import java.util.function.Function;
-import org.apache.http.client.utils.URLEncodedUtils;
 import org.opensearch.client.json.JsonpDeserializer;
 import org.opensearch.client.opensearch._types.ErrorResponse;
 import org.opensearch.client.transport.JsonEndpoint;
+import org.opensearch.client.util.PathEncoder;
 
 public class SimpleEndpoint<RequestT, ResponseT> implements JsonEndpoint<RequestT, ResponseT, ErrorResponse> {
 
@@ -133,7 +133,6 @@ public static RuntimeException noPathTemplateFound(String what) {
     }
 
     public static void pathEncode(String src, StringBuilder dest) {
-        // TODO: avoid dependency on HttpClient here (and use something more efficient)
-        dest.append(URLEncodedUtils.formatSegments(src).substring(1));
+        dest.append(PathEncoder.encode(src));
     }
 }
diff --git a/java-client/src/main/java/org/opensearch/client/util/PathEncoder.java b/java-client/src/main/java/org/opensearch/client/util/PathEncoder.java
@@ -0,0 +1,58 @@
+package org.opensearch.client.util;
+
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+
+public class PathEncoder {
+    private final static String HTTP_CLIENT4_UTILS_CLASS = "org.apache.http.client.utils.URLEncodedUtils";
+    private final static String HTTP_CLIENT5_UTILS_CLASS = "org.apache.hc.core5.net.URLEncodedUtils";
+    private final static MethodHandle FORMAT_SEGMENTS_MH;
+
+    static {
+        Class<?> clazz = null;
+        try {
+            // Try Apache HttpClient4 first since this is a default one
+            clazz = Class.forName(HTTP_CLIENT4_UTILS_CLASS);
+        } catch (final ClassNotFoundException ex) {
+            try {
+                // Fallback to Apache HttpClient4
+                clazz = Class.forName(HTTP_CLIENT5_UTILS_CLASS);
+            } catch (final ClassNotFoundException ex1) {
+                clazz = null;
+            }
+        }
+
+        if (clazz == null) {
+            throw new IllegalStateException(
+                "Either '" + HTTP_CLIENT5_UTILS_CLASS + "' or '" + HTTP_CLIENT4_UTILS_CLASS + "' is required by not found on classpath"
+            );
+        }
+
+        try {
+            FORMAT_SEGMENTS_MH = MethodHandles.lookup()
+                .findStatic(clazz, "formatSegments", MethodType.methodType(String.class, Iterable.class, Charset.class));
+        } catch (final NoSuchMethodException | IllegalAccessException ex) {
+            throw new IllegalStateException("Unable to find 'formatSegments' method in " + clazz + " class");
+        }
+    }
+
+    public static String encode(String uri) {
+        try {
+            return ((String) FORMAT_SEGMENTS_MH.invoke(Collections.singletonList(uri), StandardCharsets.UTF_8)).substring(1);
+        } catch (final Throwable ex) {
+            throw new RuntimeException("Unable to encode URI: " + uri, ex);
+        }
+    }
+}
diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/model/EndpointTest.java b/java-client/src/test/java/org/opensearch/client/opensearch/model/EndpointTest.java
@@ -51,10 +51,19 @@ public void testArrayPathParameter() {
         assertEquals("/a/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
 
         req = RefreshRequest.of(b -> b.index("a", "b"));
-        assertEquals("/a,b/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        if (isHttpClient5Present()) {
+            assertEquals("/a%2Cb/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+
+        } else {
+            assertEquals("/a,b/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        }
 
         req = RefreshRequest.of(b -> b.index("a", "b", "c"));
-        assertEquals("/a,b,c/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        if (isHttpClient5Present()) {
+            assertEquals("/a%2Cb%2Cc/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        } else {
+            assertEquals("/a,b,c/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        }
     }
 
     @Test
@@ -65,7 +74,11 @@ public void testPathEncoding() {
         assertEquals("/a%2Fb/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
 
         req = RefreshRequest.of(b -> b.index("a/b", "c/d"));
-        assertEquals("/a%2Fb,c%2Fd/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        if (isHttpClient5Present()) {
+            assertEquals("/a%2Fb%2Cc%2Fd/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        } else {
+            assertEquals("/a%2Fb,c%2Fd/_refresh", RefreshRequest._ENDPOINT.requestUrl(req));
+        }
 
     }
 
@@ -84,4 +97,13 @@ public void testArrayQueryParameter() {
         req = RefreshRequest.of(b -> b.expandWildcards(ExpandWildcard.All, ExpandWildcard.Closed));
         assertEquals("all,closed", RefreshRequest._ENDPOINT.queryParameters(req).get("expand_wildcards"));
     }
+
+    private static boolean isHttpClient5Present() {
+        try {
+            Class.forName("org.apache.hc.core5.net.URLEncodedUtils");
+            return true;
+        } catch (ClassNotFoundException e) {
+            return false;
+        }
+    }
 }
diff --git a/java-client/src/test/java/org/opensearch/client/util/PathEncoderTest.java b/java-client/src/test/java/org/opensearch/client/util/PathEncoderTest.java
@@ -0,0 +1,31 @@
+package org.opensearch.client.util;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public class PathEncoderTest {
+
+    @Test
+    public void testEncode() {
+        // Test with a simple string
+        String simpleString = "test";
+        String encodedSimpleString = PathEncoder.encode(simpleString);
+        assertEquals(simpleString, encodedSimpleString);
+
+        // Test with a string that contains special characters
+        String specialString = "a/b";
+        String encodedSpecialString = PathEncoder.encode(specialString);
+        assertEquals("a%2Fb", encodedSpecialString);
+
+        // Test with a string that contains alphanumeric characters
+        String alphanumericString = "abc123";
+        String encodedAlphanumericString = PathEncoder.encode(alphanumericString);
+        assertEquals("abc123", encodedAlphanumericString);
+
+        // Test with a string that contains multiple segments
+        String multiSegmentString = "a/b/c/_refresh";
+        String encodedMultiSegmentString = PathEncoder.encode(multiSegmentString);
+        assertEquals("a%2Fb%2Fc%2F_refresh", encodedMultiSegmentString);
+    }
+}