[java] minor performance improvements and code cleanup (SeleniumHQ#14054

) * replaced empty string comparison with isEmpty() invoking * replaced manual array copy with System.arraycopy() * replaced redundant String.format invoking with printf() * replaced deprecated setScriptTimeout with scriptTimeout according to instruction "Use scriptTimeout(Duration)" * replaced iterators with bulk methods invoking * replaced list creations with List.of() * there is no need to create mutable lists in tests to only get elements --------- Co-authored-by: Puja Jagani <[email protected]>
sandeepsuryaprasad · Oct 29, 2024 · 9ccecc5 · 9ccecc5
1 parent 84088eb
commit 9ccecc5
Show file tree

Hide file tree

Showing 15 changed files with 41 additions and 59 deletions.
diff --git a/java/src/org/openqa/selenium/Cookie.java b/java/src/org/openqa/selenium/Cookie.java
@@ -129,7 +129,7 @@ public Cookie(
       String sameSite) {
     this.name = name;
     this.value = value;
-    this.path = path == null || "".equals(path) ? "/" : path;
+    this.path = path == null || path.isEmpty() ? "/" : path;
 
     this.domain = stripPort(domain);
     this.isSecure = isSecure;
@@ -203,7 +203,7 @@ private static String stripPort(String domain) {
   }
 
   public void validate() {
-    if (name == null || "".equals(name) || value == null || path == null) {
+    if (name == null || name.isEmpty() || value == null || path == null) {
       throw new IllegalArgumentException(
           "Required attributes are not set or " + "any non-null attribute set to null");
     }

diff --git a/java/src/org/openqa/selenium/chromium/ChromiumOptions.java b/java/src/org/openqa/selenium/chromium/ChromiumOptions.java
@@ -252,8 +252,7 @@ protected Object getExtraCapability(String capabilityName) {
       return null;
     }
 
-    Map<String, Object> options = new TreeMap<>();
-    experimentalOptions.forEach(options::put);
+    Map<String, Object> options = new TreeMap<>(experimentalOptions);
 
     if (binary != null) {
       options.put("binary", binary);

diff --git a/java/src/org/openqa/selenium/firefox/FileExtension.java b/java/src/org/openqa/selenium/firefox/FileExtension.java
@@ -209,7 +209,7 @@ public Iterator<String> getPrefixes(String uri) {
         id = idNode.getTextContent();
       }
 
-      if (id == null || "".equals(id.trim())) {
+      if (id == null || id.trim().isEmpty()) {
         throw new FileNotFoundException("Cannot install extension with ID: " + id);
       }
       return id;

diff --git a/java/src/org/openqa/selenium/grid/commands/CompletionCommand.java b/java/src/org/openqa/selenium/grid/commands/CompletionCommand.java
@@ -126,10 +126,8 @@ private void outputZshCompletions(PrintStream out) {
         .sorted(Comparator.comparing(CliCommand::getName))
         .forEach(
             cmd -> {
-              out.println(
-                  String.format(
-                      "        '%s:%s'",
-                      cmd.getName(), cmd.getDescription().replace("'", "'\\''")));
+              out.printf(
+                  "        '%s:%s'%n", cmd.getName(), cmd.getDescription().replace("'", "'\\''"));
             });
 
     out.println("      )");
@@ -143,8 +141,8 @@ private void outputZshCompletions(PrintStream out) {
         .forEach(
             cmd -> {
               String shellName = cmd.getName().replace('-', '_');
-              out.println(String.format("        (%s)", cmd.getName()));
-              out.println(String.format("          _selenium_%s", shellName));
+              out.printf("        (%s)%n", cmd.getName());
+              out.printf("          _selenium_%s%n", shellName);
               out.println("          ;;");
             });
 
@@ -155,7 +153,7 @@ private void outputZshCompletions(PrintStream out) {
 
     allCommands.forEach(
         (cmd, options) -> {
-          out.println(String.format("_selenium_%s() {", cmd.getName().replace('-', '_')));
+          out.printf("_selenium_%s() {%n", cmd.getName().replace('-', '_'));
           out.println("  args=(");
 
           options.stream()
@@ -170,17 +168,16 @@ private void outputZshCompletions(PrintStream out) {
                     }
 
                     if (opt.flags().size() == 1) {
-                      out.println(
-                          String.format(
-                              "    '%s[%s]%s'",
-                              opt.flags().iterator().next(), quotedDesc, getZshType(opt)));
+                      out.printf(
+                          "    '%s[%s]%s'%n",
+                          opt.flags().iterator().next(), quotedDesc, getZshType(opt));
                     } else {
                       out.print("    '");
                       out.print(opt.flags.stream().collect(joining(" ", "(", ")")));
                       out.print("'");
                       out.print(opt.flags.stream().collect(joining(",", "{", "}")));
                       out.print("'");
-                      out.print(String.format("[%s]", quotedDesc));
+                      out.printf("[%s]", quotedDesc);
                       out.print(getZshType(opt));
                       out.print("'\n");
                     }

diff --git a/java/src/org/openqa/selenium/grid/config/ConcatenatingConfig.java b/java/src/org/openqa/selenium/grid/config/ConcatenatingConfig.java
@@ -37,7 +37,7 @@ public class ConcatenatingConfig implements Config {
   private final Map<String, String> values;
 
   public ConcatenatingConfig(String prefix, char separator, Map<?, ?> values) {
-    this.prefix = prefix == null || "".equals(prefix) ? "" : (prefix + separator);
+    this.prefix = prefix == null || prefix.isEmpty() ? "" : (prefix + separator);
     this.separator = separator;
 
     this.values =

diff --git a/java/src/org/openqa/selenium/logging/LogLevelMapping.java b/java/src/org/openqa/selenium/logging/LogLevelMapping.java
@@ -71,7 +71,7 @@ public static String getName(Level level) {
   }
 
   public static Level toLevel(String logLevelName) {
-    if (logLevelName == null || "".equals(logLevelName)) {
+    if (logLevelName == null || logLevelName.isEmpty()) {
       // Default the log level to info.
       return Level.INFO;
     }

diff --git a/java/src/org/openqa/selenium/print/PrintOptions.java b/java/src/org/openqa/selenium/print/PrintOptions.java
@@ -67,9 +67,7 @@ public void setPageRanges(String firstRange, String... ranges) {
 
     this.pageRanges[0] = firstRange;
 
-    for (int i = 1; i < ranges.length; i++) {
-      this.pageRanges[i] = ranges[i - 1];
-    }
+    if (ranges.length > 0) System.arraycopy(ranges, 0, this.pageRanges, 1, ranges.length - 1);
   }
 
   public void setPageRanges(List<String> ranges) {

diff --git a/java/src/org/openqa/selenium/remote/RemoteLogs.java b/java/src/org/openqa/selenium/remote/RemoteLogs.java
@@ -106,9 +106,7 @@ public Set<String> getAvailableLogTypes() {
     @SuppressWarnings("unchecked")
     List<String> rawList = (List<String>) raw;
     Set<String> builder = new LinkedHashSet<>();
-    for (String logType : rawList) {
-      builder.add(logType);
-    }
+    builder.addAll(rawList);
     builder.addAll(getAvailableLocalLogs());
     return Set.copyOf(builder);
   }

diff --git a/java/src/org/openqa/selenium/support/events/EventFiringDecorator.java b/java/src/org/openqa/selenium/support/events/EventFiringDecorator.java
@@ -21,6 +21,7 @@
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.openqa.selenium.Alert;
@@ -245,14 +246,10 @@ private void fireBeforeEvents(
     int argsLength = args != null ? args.length : 0;
     Object[] args2 = new Object[argsLength + 1];
     args2[0] = target.getOriginal();
-    for (int i = 0; i < argsLength; i++) {
-      args2[i + 1] = args[i];
-    }
+    if (args != null) System.arraycopy(args, 0, args2, 1, argsLength);
 
     Method m = findMatchingMethod(listener, methodName, args2);
-    if (m != null) {
-      callListenerMethod(m, listener, args2);
-    }
+    if (m != null) callListenerMethod(m, listener, args2);
   }
 
   private void fireAfterEvents(
@@ -261,20 +258,15 @@ private void fireAfterEvents(
 
     boolean isVoid =
         method.getReturnType() == Void.TYPE || method.getReturnType() == WebDriver.Timeouts.class;
+
     int argsLength = args != null ? args.length : 0;
     Object[] args2 = new Object[argsLength + 1 + (isVoid ? 0 : 1)];
     args2[0] = target.getOriginal();
-    for (int i = 0; i < argsLength; i++) {
-      args2[i + 1] = args[i];
-    }
-    if (!isVoid) {
-      args2[args2.length - 1] = res;
-    }
+    if (args != null) System.arraycopy(Objects.requireNonNull(args), 0, args2, 1, argsLength);
+    if (!isVoid) args2[args2.length - 1] = res;
 
     Method m = findMatchingMethod(listener, methodName, args2);
-    if (m != null) {
-      callListenerMethod(m, listener, args2);
-    }
+    if (m != null) callListenerMethod(m, listener, args2);
 
     try {
       if (target.getOriginal() instanceof WebDriver) {

diff --git a/java/test/org/openqa/selenium/ExecutingAsyncJavascriptTest.java b/java/test/org/openqa/selenium/ExecutingAsyncJavascriptTest.java
@@ -28,7 +28,6 @@
 import static org.openqa.selenium.testing.drivers.Browser.SAFARI;
 
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
@@ -45,7 +44,7 @@ class ExecutingAsyncJavascriptTest extends JupiterTestBase {
   public void setUp() {
     assumeTrue(driver instanceof JavascriptExecutor);
     executor = (JavascriptExecutor) driver;
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
   }
 
   @Test
@@ -56,7 +55,7 @@ public void setUp() {
   public void shouldSetAndGetScriptTimeout() {
     Duration timeout = driver.manage().timeouts().getScriptTimeout();
     assertThat(timeout).hasMillis(30000);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(3000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(3000));
     Duration timeout2 = driver.manage().timeouts().getScriptTimeout();
     assertThat(timeout2).hasMillis(3000);
   }
@@ -185,7 +184,7 @@ public void shouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout() {
   @Test
   @NotYetImplemented(SAFARI)
   public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(500));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(500));
     driver.get(pages.ajaxyPage);
     assertThatExceptionOfType(ScriptTimeoutException.class)
         .isThrownBy(
@@ -199,7 +198,7 @@ public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
   @Ignore(IE)
   public void shouldDetectPageLoadsWhileWaitingOnAnAsyncScriptAndReturnAnError() {
     driver.get(pages.ajaxyPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(100));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(100));
     assertThatExceptionOfType(WebDriverException.class)
         .isThrownBy(
             () -> executor.executeAsyncScript("window.location = '" + pages.dynamicPage + "';"));
@@ -244,7 +243,7 @@ public void shouldCatchErrorsWithMessageAndStacktraceWhenExecutingInitialScript(
             t -> {
               Throwable rootCause = getRootCause(t);
               assertThat(rootCause).hasMessageContaining("errormessage");
-              assertThat(Arrays.asList(rootCause.getStackTrace()))
+              assertThat(List.of(rootCause.getStackTrace()))
                   .extracting(StackTraceElement::getMethodName)
                   .contains("functionB");
             });
@@ -266,7 +265,7 @@ void shouldBeAbleToExecuteAsynchronousScripts() {
             "There should only be 1 DIV at this point, which is used for the butter message")
         .isEqualTo(1);
 
-    driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(15));
+    driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(15));
     String text =
         (String)
             executor.executeAsyncScript(
@@ -317,7 +316,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
             + "xhr.send('');"; // empty string to stop firefox 3 from choking
 
     driver.get(pages.ajaxyPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(3));
+    driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(3));
     String response = (String) executor.executeAsyncScript(script, pages.sleepingPage + "?time=2");
     assertThat(response.trim())
         .isEqualTo("<html><head><title>Done</title></head><body>Slept for 2s</body></html>");
@@ -331,7 +330,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
   @Ignore(value = SAFARI, reason = "Does not support alerts yet")
   public void throwsIfScriptTriggersAlert() {
     driver.get(pages.simpleTestPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
     assertThatExceptionOfType(UnhandledAlertException.class)
         .isThrownBy(
             () ->
@@ -350,7 +349,7 @@ public void throwsIfScriptTriggersAlert() {
   @Ignore(value = SAFARI, reason = "Does not support alerts yet")
   public void throwsIfAlertHappensDuringScript() {
     driver.get(pages.slowLoadingAlertPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
     assertThatExceptionOfType(UnhandledAlertException.class)
         .isThrownBy(() -> executor.executeAsyncScript("setTimeout(arguments[0], 1000);"));
     // Shouldn't throw
@@ -365,7 +364,7 @@ public void throwsIfAlertHappensDuringScript() {
   @Ignore(value = SAFARI, reason = "Does not support alerts yet")
   public void throwsIfScriptTriggersAlertWhichTimesOut() {
     driver.get(pages.simpleTestPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
     assertThatExceptionOfType(UnhandledAlertException.class)
         .isThrownBy(
             () ->
@@ -383,7 +382,7 @@ public void throwsIfScriptTriggersAlertWhichTimesOut() {
   @Ignore(value = SAFARI, reason = "Does not support alerts yet")
   public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
     driver.get(pages.slowLoadingAlertPage);
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
     assertThatExceptionOfType(UnhandledAlertException.class)
         .isThrownBy(() -> executor.executeAsyncScript(""));
     // Shouldn't throw
@@ -397,7 +396,7 @@ public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
   @Ignore(FIREFOX)
   @Ignore(value = SAFARI, reason = "Does not support alerts yet")
   public void includesAlertTextInUnhandledAlertException() {
-    driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
+    driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
     String alertText = "Look! An alert!";
     assertThatExceptionOfType(UnhandledAlertException.class)
         .isThrownBy(

diff --git a/java/test/org/openqa/selenium/ExecutingJavascriptTest.java b/java/test/org/openqa/selenium/ExecutingJavascriptTest.java
@@ -264,7 +264,7 @@ public void testShouldThrowAnExceptionWithMessageAndStacktraceWhenTheJavascriptI
             t -> {
               Throwable rootCause = getRootCause(t);
               assertThat(rootCause).hasMessageContaining("errormessage");
-              assertThat(Arrays.asList(rootCause.getStackTrace()))
+              assertThat(List.of(rootCause.getStackTrace()))
                   .extracting(StackTraceElement::getMethodName)
                   .contains("functionB");
             });

diff --git a/java/test/org/openqa/selenium/build/BazelBuild.java b/java/test/org/openqa/selenium/build/BazelBuild.java
@@ -54,7 +54,7 @@ public void build(String target) {
       return;
     }
 
-    if (target == null || "".equals(target)) {
+    if (target == null || target.isEmpty()) {
       throw new IllegalStateException("No targets specified");
     }
     LOG.info("\nBuilding " + target + " ...");

diff --git a/java/test/org/openqa/selenium/grid/router/SessionQueueGridWithTimeoutTest.java b/java/test/org/openqa/selenium/grid/router/SessionQueueGridWithTimeoutTest.java
@@ -30,7 +30,6 @@
 import java.net.URISyntaxException;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -163,7 +162,7 @@ void shouldBeAbleToDeleteTimedoutSessions() {
     try {
       Callable<HttpResponse> sessionCreationTask = () -> createSession(caps);
       List<Future<HttpResponse>> futureList =
-          fixedThreadPoolService.invokeAll(Arrays.asList(sessionCreationTask));
+          fixedThreadPoolService.invokeAll(List.of(sessionCreationTask));
 
       for (Future<HttpResponse> future : futureList) {
         HttpResponse httpResponse = future.get(10, SECONDS);

diff --git a/java/test/org/openqa/selenium/ie/InternetExplorerDriverTest.java b/java/test/org/openqa/selenium/ie/InternetExplorerDriverTest.java
@@ -138,7 +138,7 @@ void testPersistentHoverCanBeTurnedOff() throws Exception {
     // Intentionally wait to make sure hover DOES NOT persist.
     Thread.sleep(1000);
 
-    wait.until(d -> item.getText().equals(""));
+    wait.until(d -> item.getText().isEmpty());
 
     assertThat(item.getText()).isEmpty();
   }

diff --git a/java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java b/java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java
@@ -623,7 +623,7 @@ void canHandleGetTimeoutsCommand() {
   void canHandleSetScriptTimeoutCommand() {
     WebDriverFixture fixture = new WebDriverFixture(echoCapabilities, nullValueResponder);
 
-    fixture.driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(10));
+    fixture.driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(10));
 
     fixture.verifyCommands(
         new CommandPayload(DriverCommand.SET_TIMEOUT, ImmutableMap.of("script", 10000L)));