HSEARCH-5482 Simple Elasticsearch client based on java.net.http.HttpClient #4896

marko-bekhta · 2025-11-14T17:58:20Z

https://hibernate.atlassian.net/browse/HSEARCH-5482

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

…lient

marko-bekhta

@yrodiere @lucamolteni

if someone wants to have a look 😉 🙂

This one adds a new JDK based http client.

marko-bekhta · 2025-11-20T11:26:27Z

...arch/src/main/java/org/hibernate/search/backend/elasticsearch/client/impl/RestJdkClient.java

+import org.hibernate.search.util.common.impl.Closer;
+
+@Incubating
+public class RestJdkClient implements AutoCloseable {


I put the client and node provider into the SPI package only because we usually say that "you can pass your own instance of the rest client to us" and if the client is in the impl package.. then it somewhat defeats that point

Why wouldn't users/frameworks just pass the HttpClient instance in that case, instead of a Hibernate Search-specific interface?

* An external Elasticsearch client instance that Hibernate Search should use for all requests to Elasticsearch. * <p> * If this is set, Hibernate Search will not attempt to create its own Elasticsearch, * and all other client-related configuration properties * (hosts/uris, authentication, discovery, timeouts, max connections, configurer, ...) * will be ignored.

I wanted to go with that option of HttpClient ... this (HttpClient) client on its own won't know about the hosts/uris and maybe more ? So I was looking at it from this prespective: for other clients we expect the wrapper client around the apache one and not the apache client itself... and I was trying to follow the same pattern here 🫣.

If the user passed a HttpClient, it would be primarily to share the connection pool (if any) with other uses of the client elsewhere, IMO. It'd be fine to still have the uris and more configured through Hibernate Search -- after all, the user passed the client, they can pass more.

But I can understand your view as well. +0 from me I guess :)

marko-bekhta · 2025-11-20T11:34:54Z

...arch/src/main/java/org/hibernate/search/backend/elasticsearch/client/impl/RestJdkClient.java

+		if ( httpClient != null ) {
+			try ( Closer<Exception> closer = new Closer<>() ) {
+				Optional<Executor> executor = httpClient.executor();
+				// May look a bit silly ... but close was only added in JDK 21:
+				if ( ( (Object) httpClient ) instanceof AutoCloseable closeable ) {
+					closer.push( AutoCloseable::close, closeable );
+				}
+				if ( executor.isPresent() && executor.get() instanceof AutoCloseable closeable ) {
+					closer.push( AutoCloseable::close, closeable );
+				}
+			}
+			finally {
+				httpClient = null;
+			}
+		}


Why does the closing look as it does ? well .....
Prior to JDK 21, this client does not expose a way to close it.... and it starts a thread and an executor (unless provided, which we do here), which are supposedly stopped and cleaned up once the client is not referenced from anywhere ...
But TBH .. I'd rather limit the use of this client to JDK 21+ 🙂

But TBH .. I'd rather limit the use of this client to JDK 21+ 🙂

+1 that's a perfectly reasonable limitation.

...earch/src/main/java/org/hibernate/search/backend/elasticsearch/client/impl/NodeProvider.java

marko-bekhta · 2025-11-20T16:19:47Z

...ain/java/org/hibernate/search/backend/elasticsearch/client/impl/ClientJdkGsonHttpEntity.java

+
+	public ClientJdkGsonHttpEntity(Gson gson, List<JsonObject> bodyParts) throws IOException {
+		super( gson, bodyParts );
+		delegate = HttpRequest.BodyPublishers.ofInputStream( this::getContent );


HttpRequest.BodyPublisher didn't fit the abstract gson entity that easily so I thought I'll just delegate to the existing one instead.

marko-bekhta · 2025-11-20T16:20:21Z

.../org/hibernate/search/backend/elasticsearch/client/impl/ClientJdkHttpRequestInterceptor.java

+	// https://docs.oracle.com/en/java/javase/25/docs/api/java.net.http/module-summary.html
+	//
+	// Host header is one of the restricted ^ so we skip it here:
+	private static final Set<String> HEADERS_TO_IGNORE = Set.of( "host" );


some of those JDK client properties can only be set as system properties ... and what "restricted" headers to allow is one of those configs.
I've tested the aws signing with the host header skipped and it seems to work ok, so I'll leave it as this for now.

marko-bekhta · 2025-11-20T16:24:23Z

backend/elasticsearch-client/jdk-rest-client/pom.xml

+        <version>8.2.0-SNAPSHOT</version>
+        <relativePath>../../../build/parents/public</relativePath>
+    </parent>
+    <artifactId>hibernate-search-backend-elasticsearch-client-jdk</artifactId>


just to confirm the new artifact name/package:
(org.hibernate.search.backend.elasticsearch.client.jdk)

Wasn't the plan to include this client directly in the Elasticsearch backend, because it doesn't require any additional dependency?

it was the original idea, yes ... but I hesitated about doing so in the end 🙂
Reason ? well ... we already have a dependency on the rest4 based client and if we add the jdk one, with the current logic we'll end up using the JDK one, and user wouldn't be able to force the use of rest4 one Then adding one of the apache 5 based ones won't work at all without excludes.. all of that because we'll have too many clients on the classpath and we wouldn't know which one to pick...
hence I thought .. make this one as another module, then in the next version replace the rest4 dependency with this one and move the rest4 code out of the backend into the client while move this one back in ...

otherwise we need to rethink how we determine which client factory to select + probably have a config property on the API side to specify it instead of the SPI (

hibernate-search/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/cfg/spi/ElasticsearchBackendSpiSettings.java

Line 30 in 6e8a388

public static final String BACKEND_WORK_EXECUTOR_PROVIDER = PREFIX + Radicals.BACKEND_WORK_EXECUTOR_PROVIDER;

)

otherwise we need to rethink how we determine which client factory to select + probably have a config property on the API side to specify it instead of the SPI (

That's what I expected TBH. Ignore the JDK one by default (because the rest4 one must remain the default for backwards compatibility) and allow passing a config property to switch to the JDK one.

…assing a reference to the HttpClient instead

sonarqubecloud · 2025-11-21T19:40:35Z

Quality Gate failed

Failed conditions
63.7% Coverage on New Code (required ≥ 80%)
4.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

marko-bekhta force-pushed the feat/HSEARCH-5482-Simple-Elasticsearch-client-based-on-javanethttpHttpClient branch 10 times, most recently from a4bd553 to 473746a Compare November 18, 2025 19:33

HSEARCH-5482 Simple Elasticsearch client based on java.net.http.HttpC…

da7373e

…lient

marko-bekhta force-pushed the feat/HSEARCH-5482-Simple-Elasticsearch-client-based-on-javanethttpHttpClient branch 3 times, most recently from c407d26 to 4a3dd9e Compare November 20, 2025 08:48

marko-bekhta marked this pull request as ready for review November 20, 2025 11:23

marko-bekhta force-pushed the feat/HSEARCH-5482-Simple-Elasticsearch-client-based-on-javanethttpHttpClient branch from 4a3dd9e to 90873f6 Compare November 20, 2025 16:22

marko-bekhta commented Nov 20, 2025

View reviewed changes

HSEARCH-5482 Mention the JDK client in the docs

8429125

marko-bekhta force-pushed the feat/HSEARCH-5482-Simple-Elasticsearch-client-based-on-javanethttpHttpClient branch from 90873f6 to 8429125 Compare November 21, 2025 09:05

marko-bekhta added 2 commits November 21, 2025 11:23

HSEARCH-5482 Hide the REST JDK client in the impl package and allow p…

4d62f8d

…assing a reference to the HttpClient instead

HSEARCH-5482 Move JDK client into Elasticsearch backend

b9bee13

marko-bekhta force-pushed the feat/HSEARCH-5482-Simple-Elasticsearch-client-based-on-javanethttpHttpClient branch from b26b28f to b9bee13 Compare November 21, 2025 17:49

Uh oh!

HSEARCH-5482 Simple Elasticsearch client based on java.net.http.HttpClient #4896

Are you sure you want to change the base?

HSEARCH-5482 Simple Elasticsearch client based on java.net.http.HttpClient #4896

Uh oh!

Conversation

marko-bekhta commented Nov 14, 2025

Uh oh!

marko-bekhta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marko-bekhta Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 21, 2025

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marko-bekhta Nov 21, 2025 •

edited

Loading