Randomize client implementation, remove cloud-id on Rest5, add tests for overloaded methods

Author: swallez

java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java

@@ -77,15 +77,9 @@ public abstract class ElasticsearchTransportBase implements ElasticsearchTranspo
         }
     }
 
-    private final TransportHttpClient httpClient;
-    private final Instrumentation instrumentation;
-
-    @Override
-    public void close() throws IOException {
-        httpClient.close();
-    }
-
-    private final JsonpMapper mapper;
+    protected final TransportHttpClient httpClient;
+    protected final Instrumentation instrumentation;
+    protected final JsonpMapper mapper;
     protected final TransportOptions transportOptions;
 
     public ElasticsearchTransportBase(TransportHttpClient httpClient, TransportOptions options,
@@ -113,6 +107,20 @@ public ElasticsearchTransportBase(
         this.instrumentation = instrumentation;
     }
 
+    /** INTERNAL, used only for tests. */
+    protected ElasticsearchTransportBase cloneWith(
+        @Nullable TransportOptions options,
+        @Nullable JsonpMapper mapper,
+        @Nullable Instrumentation instrumentation
+    ) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void close() throws IOException {
+        httpClient.close();
+    }
+
     @Override
     public final JsonpMapper jsonpMapper() {
         return mapper;
@@ -123,6 +131,10 @@ public final TransportOptions options() {
         return transportOptions;
     }
 
+    public TransportHttpClient httpClient() {
+        return httpClient;
+    }
+
     @Override
     public final <RequestT, ResponseT, ErrorT> ResponseT performRequest(
         RequestT request,

java-client/src/main/java/co/elastic/clients/transport/Transport.java

@@ -26,6 +26,7 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
 
 /**
  * The transport layer that allows {@link ApiClient}s to send requests.
@@ -51,4 +52,24 @@ <RequestT, ResponseT, ErrorT> CompletableFuture<ResponseT> performRequestAsync(
      * {@link #performRequestAsync(Object, Endpoint, TransportOptions)};
      */
     TransportOptions options();
+
+    /**
+     * Clone this transport with new options.
+     *
+     * @throws UnsupportedOperationException
+     */
+    default Transport withOptions(@Nullable TransportOptions options) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * Clone this transport with additional options. The lambda expression is provided an options builder
+     * initialized with the transport's current options.
+     *
+     * @param fn a lambda expression that takes the current options as input
+     * @throws UnsupportedOperationException
+     */
+    default Transport withOptions(Function<TransportOptions.Builder, TransportOptions.Builder> fn) {
+        return withOptions(fn.apply(options().toBuilder()).build());
+    }
 }

java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientTransport.java

@@ -21,9 +21,13 @@
 
 import co.elastic.clients.json.JsonpMapper;
 import co.elastic.clients.transport.ElasticsearchTransportBase;
+import co.elastic.clients.transport.Transport;
+import co.elastic.clients.transport.TransportOptions;
 import co.elastic.clients.transport.instrumentation.Instrumentation;
 import co.elastic.clients.transport.rest5_client.low_level.Rest5Client;
 
+import javax.annotation.Nullable;
+
 public class Rest5ClientTransport extends ElasticsearchTransportBase {
 
     private final Rest5Client restClient;
@@ -45,4 +49,20 @@ public Rest5ClientTransport(Rest5Client restClient, JsonpMapper jsonpMapper, Res
     public Rest5Client restClient() {
         return this.restClient;
     }
+
+    @Override
+    public Transport withOptions(@Nullable TransportOptions options) {
+        return new Rest5ClientTransport(restClient, mapper, Rest5ClientOptions.of(options), instrumentation);
+    }
+
+    /** INTERNAL, used only for tests. */
+    @Override
+    protected ElasticsearchTransportBase cloneWith(TransportOptions options, JsonpMapper mapper, Instrumentation instrumentation) {
+        return new Rest5ClientTransport(
+            restClient,
+            mapper != null ? mapper : this.mapper,
+            Rest5ClientOptions.of(options != null ? options : this.transportOptions),
+            instrumentation != null ? instrumentation : this.instrumentation
+        );
+    }
 }

java-client/src/main/java/co/elastic/clients/transport/rest5_client/low_level/Rest5Client.java

@@ -154,50 +154,14 @@ public class Rest5Client implements Closeable {
 
     /**
      * Returns a new {@link Rest5ClientBuilder} to help with {@link Rest5Client} creation.
-     * Creates a new builder instance and sets the nodes that the client will send requests to.
-     *
-     * @param cloudId a valid elastic cloud cloudId that will route to a cluster. The cloudId is located in
-     *                the user console https://cloud.elastic.co and will resemble a string like the following
-     *                optionalHumanReadableName:dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRlbGFzdGljc2VhcmNoJGtpYmFuYQ==
+     * Creates a new builder instance and sets the hosts that the client will send requests to.
      */
-    public static Rest5ClientBuilder builder(String cloudId) {
-        // there is an optional first portion of the cloudId that is a human readable string, but it is not
-        // used.
-        if (cloudId.contains(":")) {
-            if (cloudId.indexOf(':') == cloudId.length() - 1) {
-                throw new IllegalStateException("cloudId " + cloudId + " must begin with a human readable " +
-                    "identifier followed by a colon");
-            }
-            cloudId = cloudId.substring(cloudId.indexOf(':') + 1);
-        }
-
-        String decoded = new String(Base64.getDecoder().decode(cloudId), UTF_8);
-        // once decoded the parts are separated by a $ character.
-        // they are respectively domain name and optional port, elasticsearch id, kibana id
-        String[] decodedParts = decoded.split("\\$");
-        if (decodedParts.length != 3) {
-            throw new IllegalStateException("cloudId " + cloudId + " did not decode to a cluster identifier" +
-                " correctly");
-        }
-
-        // domain name and optional port
-        String[] domainAndMaybePort = decodedParts[0].split(":", 2);
-        String domain = domainAndMaybePort[0];
-        int port;
-
-        if (domainAndMaybePort.length == 2) {
-            try {
-                port = Integer.parseInt(domainAndMaybePort[1]);
-            } catch (NumberFormatException nfe) {
-                throw new IllegalStateException("cloudId " + cloudId + " does not contain a valid port " +
-                    "number");
-            }
-        } else {
-            port = 443;
+    public static Rest5ClientBuilder builder(URI... uris) {
+        if (uris == null || uris.length == 0) {
+            throw new IllegalArgumentException("uris must not be null nor empty");
         }
-
-        String url = decodedParts[1] + "." + domain;
-        return builder(new HttpHost("https", url, port));
+        List<Node> nodes = Arrays.stream(uris).map(u -> new Node(HttpHost.create(u))).toList();
+        return new Rest5ClientBuilder(nodes);
     }
 
     /**

java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java

@@ -227,7 +227,7 @@ public int size() {
         @Override
         public BinaryData body() throws IOException {
             HttpEntity entity = restResponse.getEntity();
-            return entity == null ? null : new HttpEntityBinaryData(restResponse.getEntity());
+            return entity == null ? null : new HttpEntityBinaryData(entity);
         }
 
         @Nullable

java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientTransport.java

@@ -21,9 +21,13 @@
 
 import co.elastic.clients.json.JsonpMapper;
 import co.elastic.clients.transport.ElasticsearchTransportBase;
+import co.elastic.clients.transport.Transport;
+import co.elastic.clients.transport.TransportOptions;
 import co.elastic.clients.transport.instrumentation.Instrumentation;
 import org.elasticsearch.client.RestClient;
 
+import javax.annotation.Nullable;
+
 public class RestClientTransport extends ElasticsearchTransportBase {
 
     private final RestClient restClient;
@@ -45,4 +49,21 @@ public RestClientTransport(RestClient restClient, JsonpMapper jsonpMapper, RestC
     public RestClient restClient() {
         return this.restClient;
     }
+
+
+    @Override
+    public Transport withOptions(@Nullable TransportOptions options) {
+        return new RestClientTransport(restClient, mapper, RestClientOptions.of(options), instrumentation);
+    }
+
+    /** INTERNAL, used only for tests. */
+    @Override
+    protected ElasticsearchTransportBase cloneWith(TransportOptions options, JsonpMapper mapper, Instrumentation instrumentation) {
+        return new RestClientTransport(
+            restClient,
+            mapper != null ? mapper : this.mapper,
+            RestClientOptions.of(options != null ? options : this.transportOptions),
+            instrumentation != null ? instrumentation : this.instrumentation
+        );
+    }
 }

java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java

@@ -37,7 +37,7 @@
 import java.util.function.Function;
 
 /**
- * A transport implementation that always returns the same result. Used for doc snippets where we can to check
+ * A transport implementation that always returns the same result. Used for doc snippets where we can check
  * compilation and do very simple tests.
  */
 public class DocTestsTransport implements ElasticsearchTransport {

java-client/src/test/java/co/elastic/clients/elasticsearch/ElasticsearchTestClient.java

@@ -24,6 +24,7 @@
 import co.elastic.clients.transport.rest5_client.Rest5ClientTransport;
 import co.elastic.clients.transport.rest5_client.low_level.Rest5Client;
 import co.elastic.clients.transport.rest_client.RestClientTransport;
+import com.sun.net.httpserver.HttpServer;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.http.HttpHost;
@@ -35,26 +36,35 @@
 import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import java.net.URI;
+import java.security.SecureRandom;
 import java.util.Base64;
-import java.util.Random;
 
 public class ElasticsearchTestClient {
 
+    protected enum ClientImpl { Rest4, Rest5 }
+
     // Same value for all tests in a test run
-    private static final int RAND = new Random().nextInt(100);
+    private static final ClientImpl flavor;
+    static {
+        var flavors = ClientImpl.values();
+        flavor = flavors[new SecureRandom().nextInt(flavors.length)];
+    }
 
     private static JsonpMapper mapper(JsonpMapper mapper) {
         return mapper != null ? mapper : new JsonbJsonpMapper();
     }
 
     public static ElasticsearchClient createClient(String url, @Nullable JsonpMapper mapper, @Nullable SSLContext sslContext) {
-        if(RAND % 2 == 0) {
-            System.out.println("Using a Rest4 client");
-            return createRest4Client(url, mapper, sslContext);
-        } else {
-            System.out.println("Using a Rest5 client");
-            return createRest5Client(url, mapper, sslContext);
-        }
+        System.out.println("Using a " + flavor + " client");
+        return switch (flavor) {
+            case Rest4 -> createRest4Client(url, mapper, sslContext);
+            case Rest5 -> createRest5Client(url, mapper, sslContext);
+        };
+    }
+
+    public static ElasticsearchClient createClient(HttpServer server, @Nullable JsonpMapper mapper) {
+        var address = server.getAddress();
+        return createClient("http://" + address.getHostString() + ":" + address.getPort(), mapper, null);
     }
 
     public static ElasticsearchClient createRest4Client(String url, @Nullable JsonpMapper mapper, @Nullable SSLContext sslContext) {

java-client/src/test/java/co/elastic/clients/elasticsearch/ElasticsearchTestServer.java

@@ -48,6 +48,8 @@ public class ElasticsearchTestServer implements AutoCloseable {
 
     private final String[] plugins;
     private volatile ElasticsearchContainer container;
+    private String url;
+    private SSLContext sslContext;
     private ElasticsearchClient client;
 
     private static ElasticsearchTestServer global;
@@ -95,6 +97,8 @@ public ElasticsearchTestServer(String... plugins) {
     private static final int RAND = new Random().nextInt(100);
 
     protected void setup(String url, SSLContext sslContext) {
+        this.url = url;
+        this.sslContext = sslContext;
         this.client = ElasticsearchTestClient.createClient(url, null, sslContext);
     }
 
@@ -244,6 +248,14 @@ public ElasticsearchContainer container() {
         return this.container;
     }
 
+    public String url() {
+        return url;
+    }
+
+    public SSLContext sslContext() {
+        return sslContext;
+    }
+
     public ElasticsearchClient client() {
         return client;
     }