diff --git a/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java b/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java index cac3ce418d859..f3ba3e70cb698 100644 --- a/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java +++ b/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java @@ -42,54 +42,83 @@ public class HttpCommandExecutor implements CommandExecutor { private final URL remoteServer; - public final HttpClient client; - protected final HttpClient.Factory httpClientFactory; protected final Map additionalCommands; + protected final HttpClient client; protected @Nullable CommandCodec commandCodec; protected @Nullable ResponseCodec responseCodec; - private static class DefaultClientFactoryHolder { - static HttpClient.Factory defaultClientFactory = HttpClient.Factory.createDefault(); - } - + @Deprecated(forRemoval = true, since = "4.50.0") public static HttpClient.Factory getDefaultClientFactory() { - return DefaultClientFactoryHolder.defaultClientFactory; + return RemoteWebDriver.DEFAULT_CLIENT_FACTORY; } + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor(URL addressOfRemoteServer) { this(emptyMap(), Require.nonNull("Server URL", addressOfRemoteServer)); } + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor(ClientConfig config) { this( emptyMap(), Require.nonNull("HTTP client configuration", config), - getDefaultClientFactory()); + HttpClient.Factory.createDefault()); + } + + /** + * Creates an {@link HttpCommandExecutor} that supports only standard commands. + * + * @param httpClient the HttpClient to execute commands with + * @param addressOfRemoteServer URL of remote end Selenium server + */ + public HttpCommandExecutor(HttpClient httpClient, URL addressOfRemoteServer) { + this(httpClient, Map.of(), addressOfRemoteServer); } /** * Creates an {@link HttpCommandExecutor} that supports non-standard {@code additionalCommands} in * addition to the standard. * + * @param httpClient the HttpClient to execute commands with * @param additionalCommands additional commands to allow the command executor to process * @param addressOfRemoteServer URL of remote end Selenium server */ + public HttpCommandExecutor( + HttpClient httpClient, + Map additionalCommands, + URL addressOfRemoteServer) { + this.client = httpClient; + this.additionalCommands = + new HashMap<>(Require.nonNull("Additional commands", additionalCommands)); + this.remoteServer = addressOfRemoteServer; + } + + /** + * Creates an {@link HttpCommandExecutor} that supports non-standard {@code additionalCommands} in + * addition to the standard. + * + * @param additionalCommands additional commands to allow the command executor to process + * @param addressOfRemoteServer URL of remote end Selenium server + */ + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor( Map additionalCommands, URL addressOfRemoteServer) { this( Require.nonNull("Additional commands", additionalCommands), Require.nonNull("Server URL", addressOfRemoteServer), - getDefaultClientFactory()); + HttpClient.Factory.createDefault()); } + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor( Map additionalCommands, URL addressOfRemoteServer, ClientConfig config) { this( additionalCommands, config.baseUrl(Require.nonNull("Server URL", addressOfRemoteServer)), - getDefaultClientFactory()); + HttpClient.Factory.createDefault()); } + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor( Map additionalCommands, URL addressOfRemoteServer, @@ -100,15 +129,12 @@ public HttpCommandExecutor( httpClientFactory); } + @Deprecated(forRemoval = true, since = "4.50.0") public HttpCommandExecutor( Map additionalCommands, ClientConfig config, HttpClient.Factory httpClientFactory) { - remoteServer = Require.nonNull("HTTP client configuration", config).baseUrl(); - this.additionalCommands = - new HashMap<>(Require.nonNull("Additional commands", additionalCommands)); - this.httpClientFactory = Require.nonNull("HTTP client factory", httpClientFactory); - this.client = this.httpClientFactory.createClient(config); + this(httpClientFactory.createClient(config), additionalCommands, config.baseUrl()); } /** @@ -147,6 +173,7 @@ protected void defineCommand(String commandName, CommandInfo info) { commandCodec.defineCommand(commandName, info.getMethod(), info.getUrl()); } + @Deprecated(forRemoval = true, since = "4.50.0") public URL getAddressOfRemoteServer() { return remoteServer; } @@ -204,7 +231,6 @@ public Response execute(Command command) throws IOException { } if (QUIT.equals(command.getName())) { client.close(); - httpClientFactory.cleanupIdleClients(); } return response; } catch (UnsupportedCommandException e) { diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index 08ac310f63447..20bd09a9cb273 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -19,7 +19,6 @@ import static java.util.Collections.singleton; import static java.util.Objects.requireNonNull; -import static java.util.Objects.requireNonNullElseGet; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.logging.Level.SEVERE; import static org.openqa.selenium.remote.CapabilityType.PLATFORM_NAME; @@ -38,7 +37,6 @@ import java.util.ArrayList; import java.util.Base64; import java.util.Collection; -import java.util.Collections; import java.util.Date; import java.util.HashSet; import java.util.LinkedHashSet; @@ -102,7 +100,6 @@ import org.openqa.selenium.remote.http.jdk.ConnectionException; import org.openqa.selenium.remote.service.DriverCommandExecutor; import org.openqa.selenium.remote.tracing.TracedHttpClient; -import org.openqa.selenium.remote.tracing.Tracer; import org.openqa.selenium.remote.tracing.opentelemetry.OpenTelemetryTracer; import org.openqa.selenium.virtualauthenticator.Credential; import org.openqa.selenium.virtualauthenticator.HasVirtualAuthenticator; @@ -127,6 +124,7 @@ public class RemoteWebDriver } private static final Logger LOG = Logger.getLogger(RemoteWebDriver.class.getName()); + static final HttpClient.Factory DEFAULT_CLIENT_FACTORY = HttpClient.Factory.createDefault(); /** Boolean system property that defines whether the tracing is enabled or not. */ private static final String WEBDRIVER_REMOTE_ENABLE_TRACING = "webdriver.remote.enableTracing"; @@ -134,6 +132,7 @@ public class RemoteWebDriver private final ElementLocation elementLocation = new ElementLocation(); private Level level = Level.FINE; private ErrorHandler errorHandler = new ErrorHandler(); + private final HttpClient.Factory clientFactory; private final ClientConfig clientConfig; private CommandExecutor executor; protected Capabilities capabilities; @@ -155,19 +154,17 @@ public class RemoteWebDriver @SuppressWarnings("DataFlowIssue") protected RemoteWebDriver() { this.capabilities = new ImmutableCapabilities(); + this.clientFactory = DEFAULT_CLIENT_FACTORY; this.clientConfig = ClientConfig.defaultConfig(); this.executor = null; } public RemoteWebDriver(Capabilities capabilities) { - this( - getDefaultServerURL(), - Require.nonNull("Capabilities", capabilities), - Boolean.parseBoolean(System.getProperty(WEBDRIVER_REMOTE_ENABLE_TRACING, "true"))); + this(getDefaultServerURL(), capabilities); } public RemoteWebDriver(Capabilities capabilities, boolean enableTracing) { - this(getDefaultServerURL(), Require.nonNull("Capabilities", capabilities), enableTracing); + this(getDefaultServerURL(), capabilities, enableTracing); } public RemoteWebDriver(URL remoteAddress, Capabilities capabilities) { @@ -176,12 +173,30 @@ public RemoteWebDriver(URL remoteAddress, Capabilities capabilities) { public RemoteWebDriver(URL remoteAddress, Capabilities capabilities, ClientConfig clientConfig) { this( - createExecutor( - Require.nonNull("Server URL", remoteAddress), - Boolean.parseBoolean(System.getProperty(WEBDRIVER_REMOTE_ENABLE_TRACING, "true")), - clientConfig), - Require.nonNull("Capabilities", capabilities), - clientConfig.baseUrl(remoteAddress)); + remoteAddress, + capabilities, + clientConfig, + Boolean.parseBoolean(System.getProperty(WEBDRIVER_REMOTE_ENABLE_TRACING, "true"))); + } + + // private to ensure the clientFactory is already wrapped into a TracedHttpClient.Factory in case + // the enableTracing is true + private RemoteWebDriver( + Capabilities capabilities, + HttpClient.Factory clientFactory, + ClientConfig clientConfig, + boolean enableTracing) { + this( + enableTracing + ? new TracedCommandExecutor( + new HttpCommandExecutor( + clientFactory.createClient(clientConfig), clientConfig.baseUrl()), + OpenTelemetryTracer.getInstance()) + : new HttpCommandExecutor( + clientFactory.createClient(clientConfig), clientConfig.baseUrl()), + capabilities, + clientFactory, + clientConfig); } public RemoteWebDriver(URL remoteAddress, Capabilities capabilities, boolean enableTracing) { @@ -194,20 +209,36 @@ public RemoteWebDriver( ClientConfig clientConfig, boolean enableTracing) { this( - createExecutor(Require.nonNull("Server URL", remoteAddress), enableTracing, clientConfig), - Require.nonNull("Capabilities", capabilities), - clientConfig.baseUrl(remoteAddress)); + capabilities, + enableTracing + ? new TracedHttpClient.Factory( + OpenTelemetryTracer.getInstance(), DEFAULT_CLIENT_FACTORY) + : DEFAULT_CLIENT_FACTORY, + clientConfig.baseUrl(remoteAddress), + enableTracing); } + // ignore WEBDRIVER_REMOTE_ENABLE_TRACING here, to allow extended external control public RemoteWebDriver(CommandExecutor executor, Capabilities capabilities) { - this(executor, capabilities, ClientConfig.defaultConfig()); + this(executor, capabilities, DEFAULT_CLIENT_FACTORY, ClientConfig.defaultConfig()); } + // ignore WEBDRIVER_REMOTE_ENABLE_TRACING here, to allow extended external control public RemoteWebDriver( CommandExecutor executor, Capabilities capabilities, ClientConfig clientConfig) { + this(executor, capabilities, DEFAULT_CLIENT_FACTORY, clientConfig); + } + + // ignore WEBDRIVER_REMOTE_ENABLE_TRACING here, to allow extended external control + public RemoteWebDriver( + CommandExecutor executor, + Capabilities capabilities, + HttpClient.Factory clientFactory, + ClientConfig clientConfig) { + this.clientFactory = Require.nonNull("Client factory", clientFactory); this.clientConfig = Require.nonNull("Client config", clientConfig); this.executor = Require.nonNull("Command executor", executor); - this.capabilities = requireNonNullElseGet(capabilities, () -> new ImmutableCapabilities()); + this.capabilities = Require.nonNull("Capabilities", capabilities); try { startSession(capabilities); @@ -230,22 +261,6 @@ private static URL getDefaultServerURL() { } } - private static CommandExecutor createExecutor( - URL remoteAddress, boolean enableTracing, ClientConfig clientConfig) { - ClientConfig config = clientConfig.baseUrl(remoteAddress); - if (enableTracing) { - Tracer tracer = OpenTelemetryTracer.getInstance(); - CommandExecutor executor = - new HttpCommandExecutor( - Collections.emptyMap(), - config, - new TracedHttpClient.Factory(tracer, HttpClient.Factory.createDefault())); - return new TracedCommandExecutor(executor, tracer); - } else { - return new HttpCommandExecutor(config); - } - } - @Beta public static RemoteWebDriverBuilder builder() { return new RemoteWebDriverBuilder(); @@ -464,9 +479,8 @@ private Optional createBiDi() { LOG.warning("BiDi was requested but the remote end did not return a valid webSocketUrl."); return Optional.empty(); } - HttpClient.Factory clientFactory = HttpClient.Factory.createDefault(); ClientConfig wsConfig = this.clientConfig.baseUri(wsUri); - HttpClient wsClient = clientFactory.createClient(wsConfig); + HttpClient wsClient = this.clientFactory.createClient(wsConfig); try { Connection biDiConnection = new Connection(wsClient, wsUri.toString()); return Optional.of(new BiDi(biDiConnection, wsConfig.wsTimeout())); diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java b/java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java index ab76196d6e44e..52963499f55e6 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java @@ -397,7 +397,7 @@ private WebDriver getRemoteDriver() { new AddWebDriverSpecHeaders() .andThen(new ErrorFilter()) .andThen(new DumpHttpExchangeFilter()) - .andThen(new CloseHttpClientFilter(clientFactory, client))); + .andThen(new CloseHttpClientFilter(client))); Either result; try { @@ -540,11 +540,9 @@ private NewSessionPayload getPayload() { private static class CloseHttpClientFilter implements Filter { - private final HttpClient.Factory factory; private final HttpClient client; - CloseHttpClientFilter(HttpClient.Factory factory, HttpClient client) { - this.factory = Require.nonNull("Http client factory", factory); + CloseHttpClientFilter(HttpClient client) { this.client = Require.nonNull("Http client", client); } @@ -564,7 +562,6 @@ public HttpHandler apply(HttpHandler next) { } catch (Exception e) { LOG.log(WARNING, "Exception swallowed while closing http client", e); } - factory.cleanupIdleClients(); } }); } diff --git a/java/src/org/openqa/selenium/remote/http/HttpClient.java b/java/src/org/openqa/selenium/remote/http/HttpClient.java index 6cc86a6418a2f..40c81b04f0317 100644 --- a/java/src/org/openqa/selenium/remote/http/HttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/HttpClient.java @@ -91,7 +91,12 @@ default HttpClient createClient(URL url) { HttpClient createClient(ClientConfig config); - /** Closes idle clients. */ + /** + * Closes idle clients. + * + * @deprecated the client knows the Factory and could call a cleanup method after close + */ + @Deprecated(forRemoval = true, since = "4.50.0") default void cleanupIdleClients() { // do nothing by default. } diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java index ffcb287b611bf..c46ad39cbd6a0 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -78,13 +78,14 @@ public class JdkHttpClient implements HttpClient { private static final AtomicInteger POOL_COUNTER = new AtomicInteger(0); private final JdkHttpMessages messages; private final HttpHandler handler; + private final HttpClient.Factory factory; private java.net.http.HttpClient client; private final List websockets; private final ExecutorService executorService; private final Duration readTimeout; private final Duration connectTimeout; - JdkHttpClient(ClientConfig config) { + JdkHttpClient(HttpClient.Factory factory, ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); this.messages = new JdkHttpMessages(config); @@ -146,6 +147,7 @@ protected PasswordAuthentication getPasswordAuthentication() { builder.version(Version.valueOf(version)); } + this.factory = factory; this.client = builder.build(); } @@ -599,6 +601,7 @@ public void close() { } this.client = null; executorService.shutdown(); + this.factory.cleanupIdleClients(); } @AutoService(HttpClient.Factory.class) @@ -608,7 +611,7 @@ public static class Factory implements HttpClient.Factory { @Override public HttpClient createClient(ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); - return new JdkHttpClient(config); + return new JdkHttpClient(this, config); } }