Skip to content

Migrate jetty-client to vertx-web-client #387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
google-labs-jules wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Migrate jetty-client to vertx-web-client #387

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5584bb6
Migrate jetty-client to vertx-web-client
google-labs-jules[bot] Nov 21, 2025
f7562b1
Migrate jetty-client to vertx-web-client
google-labs-jules[bot] Nov 22, 2025
547e7b6
Remove unused disableWWWAuthenticationValidation parameter
google-labs-jules[bot] Nov 22, 2025
db69e2c
Migrate from Jetty to Vert.x WebClient with proper error handling and…
google-labs-jules[bot] Nov 22, 2025
fe097eb
Address review feedback for Vert.x WebClient migration
google-labs-jules[bot] Nov 22, 2025
253edb0
Address final review feedback for Vert.x WebClient migration
google-labs-jules[bot] Nov 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@
<version>3.3.4</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-client</artifactId>
<version>${jetty.version}</version>
<groupId>io.vertx</groupId>
<artifactId>vertx-web-client</artifactId>
<version>5.0.5</version>
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vert.x 5.0.5 is compatible with Quarkus 3.23.2, but since Quarkus already manages Vert.x versions through its BOM, you should let Quarkus manage the Vert.x version to avoid potential conflicts. Remove the explicit version:

<dependency>
    <groupId>io.vertx</groupId>
    <artifactId>vertx-web-client</artifactId>
</dependency>

This ensures compatibility with the Vert.x version that Quarkus expects.

Suggested change
<version>5.0.5</version>

Copilot uses AI. Check for mistakes.
</dependency>
<dependency>
<groupId>jakarta.transaction</groupId>
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/ai/labs/eddi/engine/httpclient/IHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package ai.labs.eddi.engine.httpclient;

import org.eclipse.jetty.http.HttpCookieStore;

import java.net.URI;

public interface IHttpClient {
Expand All @@ -14,8 +12,6 @@ enum Method {
PATCH
}

HttpCookieStore getCookieStore();

IRequest newRequest(URI uri);

IRequest newRequest(URI uri, Method method);
Expand Down
75 changes: 32 additions & 43 deletions src/main/java/ai/labs/eddi/engine/httpclient/bootstrap/HttpClientModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
package ai.labs.eddi.engine.httpclient.bootstrap;

import ai.labs.eddi.engine.httpclient.impl.JettyHttpClient;
import ai.labs.eddi.engine.httpclient.impl.VertxHttpClient;
import io.vertx.core.Vertx;
import io.vertx.ext.web.client.WebClient;
import io.vertx.ext.web.client.WebClientOptions;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.Produces;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.WWWAuthenticationProtocolHandler;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.eclipse.microprofile.context.ManagedExecutor;

import java.util.Arrays;

@ApplicationScoped
public class HttpClientModule {

@Inject
ManagedExecutor executorService;
Vertx vertx;
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vertx instance is injected but Quarkus provides Vert.x through the quarkus-vertx extension. You need to add the Quarkus Vert.x dependency to pom.xml for this injection to work:

<dependency>
    <groupId>io.quarkus</groupId>
    <artifactId>quarkus-vertx</artifactId>
</dependency>

Without this dependency, the Vertx injection will fail at runtime.

Copilot uses AI. Check for mistakes.

@Produces
@ApplicationScoped
public JettyHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxConnectionsQueued") Integer maxConnectionsQueued,
public VertxHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxConnectionsQueued") Integer maxConnectionsQueued,
@ConfigProperty(name = "httpClient.maxConnectionPerRoute") Integer maxConnectionPerRoute,
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maxConnectionsQueued configuration parameter is read but never used. The comment on line 34 notes that there's no direct equivalent in Vert.x WebClientOptions, but the parameter should either be removed from the method signature or a comment should be added explaining why it's not applicable.

This could confuse users who configure this parameter expecting it to have an effect.

Suggested change
public VertxHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxConnectionsQueued") Integer maxConnectionsQueued,
@ConfigProperty(name = "httpClient.maxConnectionPerRoute") Integer maxConnectionPerRoute,
public VertxHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxConnectionPerRoute") Integer maxConnectionPerRoute,

Copilot uses AI. Check for mistakes.
@ConfigProperty(name = "httpClient.requestBufferSize") Integer requestBufferSize,
@ConfigProperty(name = "httpClient.responseBufferSize") Integer responseBufferSize,
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, requestBufferSize and responseBufferSize parameters are read but never used. While the comments on lines 37-40 explain that Vert.x handles buffers dynamically, these unused parameters should either be removed from the method signature or explicitly documented as no longer applicable.

Users configuring these parameters will expect them to have an effect on buffer sizes.

Suggested change
@ConfigProperty(name = "httpClient.requestBufferSize") Integer requestBufferSize,
@ConfigProperty(name = "httpClient.responseBufferSize") Integer responseBufferSize,

Copilot uses AI. Check for mistakes.
Expand All @@ -29,45 +27,36 @@ public JettyHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxC
@ConfigProperty(name = "httpClient.disableWWWAuthenticationValidation")
Boolean disableWWWAuthenticationValidation) {
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disableWWWAuthenticationValidation parameter is read but never used. The comment on lines 53-54 mentions that Vert.x handles auth differently, but this parameter should either be removed or its lack of effect should be clearly documented.

This could affect authentication behavior if users were relying on this configuration.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disableWWWAuthenticationValidation parameter is accepted but never used. In the Jetty implementation, this was used to remove the WWWAuthenticationProtocolHandler. This parameter should either be removed or the functionality should be implemented if it's needed.

Copilot uses AI. Check for mistakes.

try {
HttpClient httpClient = new HttpClient();
httpClient.setExecutor(executorService);
httpClient.setMaxConnectionsPerDestination(maxConnectionsQueued);
httpClient.setMaxRequestsQueuedPerDestination(maxConnectionPerRoute);
httpClient.setRequestBufferSize(requestBufferSize);
httpClient.setResponseBufferSize(responseBufferSize);
httpClient.setMaxRedirects(maxRedirects);
httpClient.setIdleTimeout(idleTimeout);
httpClient.setConnectTimeout(connectTimeout);
httpClient.start();
WebClientOptions options = new WebClientOptions();

if (disableWWWAuthenticationValidation) {
httpClient.getProtocolHandlers().remove(WWWAuthenticationProtocolHandler.NAME);
}
// Mapping configuration
options.setMaxPoolSize(maxConnectionPerRoute); // Closest mapping to MaxConnectionsPerDestination/MaxRequestsQueuedPerDestination
// maxConnectionsQueued (Jetty) -> MaxWaitQueueSize (Vertx)? Vertx doesn't seem to have exact equivalent exposed in options simply.
// setExecutor is handled by Vertx instance.

registerHttpClientShutdownHook(httpClient);
// request/response buffer sizes are not directly configurable in WebClientOptions in the same way,
// but we can control some limits. For now, we might skip exact buffer size mapping unless critical.
// Jetty's setRequestBufferSize/setResponseBufferSize -> Vertx ?
// Vertx handles buffers dynamically mostly.

return new JettyHttpClient(httpClient);
} catch (Exception e) {
System.out.println(Arrays.toString(e.getStackTrace()));
throw new RuntimeException(e.getLocalizedMessage(), e);
options.setMaxRedirects(maxRedirects);
// Vertx: setIdleTimeout(int idleTimeout) - "The amount of time a connection can be idle before it is closed. 0 means no timeout."
// usually seconds in Vertx.
int idleTimeoutSeconds = idleTimeout / 1000;
if (idleTimeout > 0 && idleTimeoutSeconds == 0) {
idleTimeoutSeconds = 1;
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idle timeout conversion from milliseconds to seconds has potential issues. When idleTimeout is between 1 and 999 milliseconds, the result will be 0, which is then changed to 1 second on line 35. However, when idleTimeout is exactly 0, it bypasses the check and sets the timeout to 0 seconds. This inconsistency could cause unexpected behavior. Consider handling the conversion more consistently: if the input is > 0 but < 1000, round up to 1 second; if it's 0, keep it as 0 (which typically means no timeout).

Suggested change
int idleTimeoutSeconds = idleTimeout / 1000;
if (idleTimeout > 0 && idleTimeoutSeconds == 0) {
idleTimeoutSeconds = 1;
int idleTimeoutSeconds;
if (idleTimeout == 0) {
idleTimeoutSeconds = 0; // No timeout
} else if (idleTimeout > 0 && idleTimeout < 1000) {
idleTimeoutSeconds = 1; // Round up to 1 second
} else {
idleTimeoutSeconds = (int) Math.ceil(idleTimeout / 1000.0); // Round up to nearest second

Copilot uses AI. Check for mistakes.
}
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout conversion from milliseconds to seconds using integer division can result in a timeout of 0 seconds (no timeout) for values less than 1000ms. For example, if idleTimeout is 500ms, idleTimeoutSeconds will be 0, which in Vert.x means "no timeout" rather than 500ms.

Consider using TimeUnit conversions or rounding up:

int idleTimeoutSeconds = (int) Math.ceil(idleTimeout / 1000.0);

to ensure timeouts under 1 second are still applied as 1 second rather than being disabled.

Suggested change
int idleTimeoutSeconds = idleTimeout / 1000;
if (idleTimeout > 0 && idleTimeoutSeconds == 0) {
idleTimeoutSeconds = 1;
}
int idleTimeoutSeconds = (int) Math.ceil(idleTimeout / 1000.0);

Copilot uses AI. Check for mistakes.
}
options.setIdleTimeout(idleTimeoutSeconds);

options.setConnectTimeout(connectTimeout); // Vertx: setConnectTimeout(int connectTimeout) in ms.

// disableWWWAuthenticationValidation -> Vertx handles auth differently.
// If this means disable automatic handling of 401, Vertx WebClient doesn't do it automatically unless configured.

// We also need to consider KeepAlive which is usually true by default.

WebClient webClient = WebClient.create(vertx, options);

private void registerHttpClientShutdownHook(final HttpClient httpClient) {
Runtime.getRuntime().addShutdownHook(new Thread("ShutdownHook_HttpClient") {
@Override
public void run() {
try {
if (!httpClient.isStopped()) {
httpClient.stop();
}
} catch (Throwable e) {
String message = "HttpClient did not stop as expected.";
System.out.println(message);
System.out.println(Arrays.toString(e.getStackTrace()));
}
}
});
return new VertxHttpClient(vertx, webClient);
}
}
Loading
Loading