From a942d67e49991f17cdc37bddd14e89e5c70949b6 Mon Sep 17 00:00:00 2001 From: Alex Panchenko <440271+panchenko@users.noreply.github.com> Date: Sat, 9 Aug 2025 00:10:44 +0200 Subject: [PATCH 1/4] fix possible NPE in initBiDi() --- .../remote/AppiumCommandExecutor.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java b/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java index bf02271d3..cd415785d 100644 --- a/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java +++ b/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java @@ -20,7 +20,7 @@ import io.appium.java_client.AppiumClientConfig; import io.appium.java_client.internal.ReflectionHelpers; import lombok.Getter; -import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; import org.openqa.selenium.SessionNotCreatedException; import org.openqa.selenium.WebDriverException; @@ -53,11 +53,12 @@ import static java.util.Optional.ofNullable; import static org.openqa.selenium.remote.DriverCommand.NEW_SESSION; +@NullMarked public class AppiumCommandExecutor extends HttpCommandExecutor { private final Optional serviceOptional; @Getter - private final HttpClient.Factory httpClientFactory; + private final Factory httpClientFactory; @Getter private final AppiumClientConfig appiumClientConfig; @@ -66,32 +67,32 @@ public class AppiumCommandExecutor extends HttpCommandExecutor { * * @param additionalCommands is the map of Appium commands * @param service take a look at {@link DriverService} - * @param httpClientFactory take a look at {@link HttpClient.Factory} + * @param httpClientFactory take a look at {@link Factory} * @param appiumClientConfig take a look at {@link AppiumClientConfig} */ public AppiumCommandExecutor( - @NonNull Map additionalCommands, + Map additionalCommands, @Nullable DriverService service, @Nullable Factory httpClientFactory, - @NonNull AppiumClientConfig appiumClientConfig) { + AppiumClientConfig appiumClientConfig) { super(additionalCommands, appiumClientConfig, - ofNullable(httpClientFactory).orElseGet(AppiumCommandExecutor::getDefaultClientFactory) + ofNullable(httpClientFactory).orElseGet(HttpCommandExecutor::getDefaultClientFactory) ); serviceOptional = ofNullable(service); - this.httpClientFactory = httpClientFactory; + this.httpClientFactory = ofNullable(httpClientFactory).orElseGet(HttpCommandExecutor::getDefaultClientFactory); this.appiumClientConfig = appiumClientConfig; } public AppiumCommandExecutor(Map additionalCommands, DriverService service, - HttpClient.Factory httpClientFactory) { + @Nullable Factory httpClientFactory) { this(additionalCommands, requireNonNull(service), httpClientFactory, AppiumClientConfig.defaultConfig().baseUrl(requireNonNull(service).getUrl())); } public AppiumCommandExecutor(Map additionalCommands, URL addressOfRemoteServer, - HttpClient.Factory httpClientFactory) { + @Nullable Factory httpClientFactory) { this(additionalCommands, null, httpClientFactory, AppiumClientConfig.defaultConfig().baseUrl(requireNonNull(addressOfRemoteServer))); } @@ -140,6 +141,7 @@ public Map getAdditionalCommands() { return getPrivateFieldValue(HttpCommandExecutor.class, "additionalCommands", Map.class); } + @Nullable protected CommandCodec getCommandCodec() { return this.commandCodec; } @@ -163,12 +165,8 @@ protected HttpClient getClient() { * @param serverUrl A url to override. */ protected void overrideServerUrl(URL serverUrl) { - if (this.appiumClientConfig == null) { - return; - } setPrivateFieldValue(HttpCommandExecutor.class, "client", - ofNullable(this.httpClientFactory).orElseGet(AppiumCommandExecutor::getDefaultClientFactory) - .createClient(this.appiumClientConfig.baseUrl(serverUrl))); + this.httpClientFactory.createClient(this.appiumClientConfig.baseUrl(serverUrl))); } private Response createSession(Command command) throws IOException { @@ -186,7 +184,7 @@ private Response createSession(Command command) throws IOException { refreshAdditionalCommands(); setResponseCodec(dialect.getResponseCodec()); Response response = result.createResponse(); - if (this.appiumClientConfig != null && this.appiumClientConfig.isDirectConnectEnabled()) { + if (appiumClientConfig.isDirectConnectEnabled()) { setDirectConnect(response); } From 8feb4254feecc08b8a9e5be3d67d3c39553c0c5a Mon Sep 17 00:00:00 2001 From: Alex Panchenko <440271+panchenko@users.noreply.github.com> Date: Sat, 9 Aug 2025 09:28:14 +0200 Subject: [PATCH 2/4] CR --- .../remote/AppiumCommandExecutor.java | 11 ++--- .../remote/AppiumCommandExecutorTest.java | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java diff --git a/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java b/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java index cd415785d..fbbd10445 100644 --- a/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java +++ b/src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java @@ -58,8 +58,6 @@ public class AppiumCommandExecutor extends HttpCommandExecutor { private final Optional serviceOptional; @Getter - private final Factory httpClientFactory; - @Getter private final AppiumClientConfig appiumClientConfig; /** @@ -81,7 +79,6 @@ public AppiumCommandExecutor( ); serviceOptional = ofNullable(service); - this.httpClientFactory = ofNullable(httpClientFactory).orElseGet(HttpCommandExecutor::getDefaultClientFactory); this.appiumClientConfig = appiumClientConfig; } @@ -141,6 +138,10 @@ public Map getAdditionalCommands() { return getPrivateFieldValue(HttpCommandExecutor.class, "additionalCommands", Map.class); } + public Factory getHttpClientFactory() { + return getPrivateFieldValue(HttpCommandExecutor.class, "httpClientFactory", Factory.class); + } + @Nullable protected CommandCodec getCommandCodec() { return this.commandCodec; @@ -165,8 +166,8 @@ protected HttpClient getClient() { * @param serverUrl A url to override. */ protected void overrideServerUrl(URL serverUrl) { - setPrivateFieldValue(HttpCommandExecutor.class, "client", - this.httpClientFactory.createClient(this.appiumClientConfig.baseUrl(serverUrl))); + HttpClient newClient = getHttpClientFactory().createClient(appiumClientConfig.baseUrl(serverUrl)); + setPrivateFieldValue(HttpCommandExecutor.class, "client", newClient); } private Response createSession(Command command) throws IOException { diff --git a/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java new file mode 100644 index 000000000..d30d37df4 --- /dev/null +++ b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java @@ -0,0 +1,40 @@ +package io.appium.java_client.remote; + +import io.appium.java_client.AppiumClientConfig; +import io.appium.java_client.MobileCommand; +import java.net.MalformedURLException; +import java.net.URL; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +class AppiumCommandExecutorTest { + private static final String APPIUM_URL = "https://appium.exaple.com"; + + private AppiumCommandExecutor createExecutor() { + URL baseUrl; + try { + baseUrl = new URL(APPIUM_URL); + } catch (MalformedURLException e) { + throw new AssertionError(e); + } + AppiumClientConfig clientConfig = AppiumClientConfig.defaultConfig().baseUrl(baseUrl); + return new AppiumCommandExecutor(MobileCommand.commandRepository, clientConfig); + } + + @Test + void getAdditionalCommands() { + assertNotNull(createExecutor().getAdditionalCommands()); + } + + @Test + void getHttpClientFactory() { + assertNotNull(createExecutor().getHttpClientFactory()); + } + + @Test + void overrideServerUrl() { + assertDoesNotThrow(() -> createExecutor().overrideServerUrl(new URL("https://direct.example.com"))); + } +} From ece0c2e8e3a1dc3adece1792d57881e19a04759a Mon Sep 17 00:00:00 2001 From: Alex Panchenko <440271+panchenko@users.noreply.github.com> Date: Sat, 9 Aug 2025 09:30:01 +0200 Subject: [PATCH 3/4] CR --- .../io/appium/java_client/remote/AppiumCommandExecutorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java index d30d37df4..e72b1ab44 100644 --- a/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java +++ b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java @@ -10,7 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; class AppiumCommandExecutorTest { - private static final String APPIUM_URL = "https://appium.exaple.com"; + private static final String APPIUM_URL = "https://appium.example.com"; private AppiumCommandExecutor createExecutor() { URL baseUrl; From 3ff2053eb8c7db1ea70a2aec307a3ddd22801079 Mon Sep 17 00:00:00 2001 From: Alex Panchenko <440271+panchenko@users.noreply.github.com> Date: Sat, 9 Aug 2025 16:16:28 +0200 Subject: [PATCH 4/4] fix checkstyle --- .../appium/java_client/remote/AppiumCommandExecutorTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java index e72b1ab44..38b1b6459 100644 --- a/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java +++ b/src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java @@ -2,9 +2,10 @@ import io.appium.java_client.AppiumClientConfig; import io.appium.java_client.MobileCommand; +import org.junit.jupiter.api.Test; + import java.net.MalformedURLException; import java.net.URL; -import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertNotNull;