Skip to content

Commit 6704db0

Browse files
iampopovichpujagani
iampopovich
and
pujagani
authored
[java] minor performance improvements and code cleanup (#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]>
1 parent aca918b commit 6704db0

15 files changed

+41
-59
lines changed

‎java/src/org/openqa/selenium/Cookie.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public Cookie(
129129
String sameSite) {
130130
this.name = name;
131131
this.value = value;
132-
this.path = path == null || "".equals(path) ? "/" : path;
132+
this.path = path == null || path.isEmpty() ? "/" : path;
133133

134134
this.domain = stripPort(domain);
135135
this.isSecure = isSecure;
@@ -203,7 +203,7 @@ private static String stripPort(String domain) {
203203
}
204204

205205
public void validate() {
206-
if (name == null || "".equals(name) || value == null || path == null) {
206+
if (name == null || name.isEmpty() || value == null || path == null) {
207207
throw new IllegalArgumentException(
208208
"Required attributes are not set or " + "any non-null attribute set to null");
209209
}

‎java/src/org/openqa/selenium/chromium/ChromiumOptions.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,7 @@ protected Object getExtraCapability(String capabilityName) {
252252
return null;
253253
}
254254

255-
Map<String, Object> options = new TreeMap<>();
256-
experimentalOptions.forEach(options::put);
255+
Map<String, Object> options = new TreeMap<>(experimentalOptions);
257256

258257
if (binary != null) {
259258
options.put("binary", binary);

‎java/src/org/openqa/selenium/firefox/FileExtension.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ public Iterator<String> getPrefixes(String uri) {
209209
id = idNode.getTextContent();
210210
}
211211

212-
if (id == null || "".equals(id.trim())) {
212+
if (id == null || id.trim().isEmpty()) {
213213
throw new FileNotFoundException("Cannot install extension with ID: " + id);
214214
}
215215
return id;

‎java/src/org/openqa/selenium/grid/commands/CompletionCommand.java

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,8 @@ private void outputZshCompletions(PrintStream out) {
126126
.sorted(Comparator.comparing(CliCommand::getName))
127127
.forEach(
128128
cmd -> {
129-
out.println(
130-
String.format(
131-
" '%s:%s'",
132-
cmd.getName(), cmd.getDescription().replace("'", "'\\''")));
129+
out.printf(
130+
" '%s:%s'%n", cmd.getName(), cmd.getDescription().replace("'", "'\\''"));
133131
});
134132

135133
out.println(" )");
@@ -143,8 +141,8 @@ private void outputZshCompletions(PrintStream out) {
143141
.forEach(
144142
cmd -> {
145143
String shellName = cmd.getName().replace('-', '_');
146-
out.println(String.format(" (%s)", cmd.getName()));
147-
out.println(String.format(" _selenium_%s", shellName));
144+
out.printf(" (%s)%n", cmd.getName());
145+
out.printf(" _selenium_%s%n", shellName);
148146
out.println(" ;;");
149147
});
150148

@@ -155,7 +153,7 @@ private void outputZshCompletions(PrintStream out) {
155153

156154
allCommands.forEach(
157155
(cmd, options) -> {
158-
out.println(String.format("_selenium_%s() {", cmd.getName().replace('-', '_')));
156+
out.printf("_selenium_%s() {%n", cmd.getName().replace('-', '_'));
159157
out.println(" args=(");
160158

161159
options.stream()
@@ -170,17 +168,16 @@ private void outputZshCompletions(PrintStream out) {
170168
}
171169

172170
if (opt.flags().size() == 1) {
173-
out.println(
174-
String.format(
175-
" '%s[%s]%s'",
176-
opt.flags().iterator().next(), quotedDesc, getZshType(opt)));
171+
out.printf(
172+
" '%s[%s]%s'%n",
173+
opt.flags().iterator().next(), quotedDesc, getZshType(opt));
177174
} else {
178175
out.print(" '");
179176
out.print(opt.flags.stream().collect(joining(" ", "(", ")")));
180177
out.print("'");
181178
out.print(opt.flags.stream().collect(joining(",", "{", "}")));
182179
out.print("'");
183-
out.print(String.format("[%s]", quotedDesc));
180+
out.printf("[%s]", quotedDesc);
184181
out.print(getZshType(opt));
185182
out.print("'\n");
186183
}

‎java/src/org/openqa/selenium/grid/config/ConcatenatingConfig.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class ConcatenatingConfig implements Config {
3737
private final Map<String, String> values;
3838

3939
public ConcatenatingConfig(String prefix, char separator, Map<?, ?> values) {
40-
this.prefix = prefix == null || "".equals(prefix) ? "" : (prefix + separator);
40+
this.prefix = prefix == null || prefix.isEmpty() ? "" : (prefix + separator);
4141
this.separator = separator;
4242

4343
this.values =

‎java/src/org/openqa/selenium/logging/LogLevelMapping.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public static String getName(Level level) {
7171
}
7272

7373
public static Level toLevel(String logLevelName) {
74-
if (logLevelName == null || "".equals(logLevelName)) {
74+
if (logLevelName == null || logLevelName.isEmpty()) {
7575
// Default the log level to info.
7676
return Level.INFO;
7777
}

‎java/src/org/openqa/selenium/print/PrintOptions.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ public void setPageRanges(String firstRange, String... ranges) {
6767

6868
this.pageRanges[0] = firstRange;
6969

70-
for (int i = 1; i < ranges.length; i++) {
71-
this.pageRanges[i] = ranges[i - 1];
72-
}
70+
if (ranges.length > 0) System.arraycopy(ranges, 0, this.pageRanges, 1, ranges.length - 1);
7371
}
7472

7573
public void setPageRanges(List<String> ranges) {

‎java/src/org/openqa/selenium/remote/RemoteLogs.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,7 @@ public Set<String> getAvailableLogTypes() {
106106
@SuppressWarnings("unchecked")
107107
List<String> rawList = (List<String>) raw;
108108
Set<String> builder = new LinkedHashSet<>();
109-
for (String logType : rawList) {
110-
builder.add(logType);
111-
}
109+
builder.addAll(rawList);
112110
builder.addAll(getAvailableLocalLogs());
113111
return Set.copyOf(builder);
114112
}

‎java/src/org/openqa/selenium/support/events/EventFiringDecorator.java

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.lang.reflect.Method;
2222
import java.util.Arrays;
2323
import java.util.List;
24+
import java.util.Objects;
2425
import java.util.logging.Level;
2526
import java.util.logging.Logger;
2627
import org.openqa.selenium.Alert;
@@ -245,14 +246,10 @@ private void fireBeforeEvents(
245246
int argsLength = args != null ? args.length : 0;
246247
Object[] args2 = new Object[argsLength + 1];
247248
args2[0] = target.getOriginal();
248-
for (int i = 0; i < argsLength; i++) {
249-
args2[i + 1] = args[i];
250-
}
249+
if (args != null) System.arraycopy(args, 0, args2, 1, argsLength);
251250

252251
Method m = findMatchingMethod(listener, methodName, args2);
253-
if (m != null) {
254-
callListenerMethod(m, listener, args2);
255-
}
252+
if (m != null) callListenerMethod(m, listener, args2);
256253
}
257254

258255
private void fireAfterEvents(
@@ -261,20 +258,15 @@ private void fireAfterEvents(
261258

262259
boolean isVoid =
263260
method.getReturnType() == Void.TYPE || method.getReturnType() == WebDriver.Timeouts.class;
261+
264262
int argsLength = args != null ? args.length : 0;
265263
Object[] args2 = new Object[argsLength + 1 + (isVoid ? 0 : 1)];
266264
args2[0] = target.getOriginal();
267-
for (int i = 0; i < argsLength; i++) {
268-
args2[i + 1] = args[i];
269-
}
270-
if (!isVoid) {
271-
args2[args2.length - 1] = res;
272-
}
265+
if (args != null) System.arraycopy(Objects.requireNonNull(args), 0, args2, 1, argsLength);
266+
if (!isVoid) args2[args2.length - 1] = res;
273267

274268
Method m = findMatchingMethod(listener, methodName, args2);
275-
if (m != null) {
276-
callListenerMethod(m, listener, args2);
277-
}
269+
if (m != null) callListenerMethod(m, listener, args2);
278270

279271
try {
280272
if (target.getOriginal() instanceof WebDriver) {

‎java/test/org/openqa/selenium/ExecutingAsyncJavascriptTest.java

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import static org.openqa.selenium.testing.drivers.Browser.SAFARI;
2929

3030
import java.time.Duration;
31-
import java.util.Arrays;
3231
import java.util.Iterator;
3332
import java.util.List;
3433
import org.junit.jupiter.api.BeforeEach;
@@ -45,7 +44,7 @@ class ExecutingAsyncJavascriptTest extends JupiterTestBase {
4544
public void setUp() {
4645
assumeTrue(driver instanceof JavascriptExecutor);
4746
executor = (JavascriptExecutor) driver;
48-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
47+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
4948
}
5049

5150
@Test
@@ -56,7 +55,7 @@ public void setUp() {
5655
public void shouldSetAndGetScriptTimeout() {
5756
Duration timeout = driver.manage().timeouts().getScriptTimeout();
5857
assertThat(timeout).hasMillis(30000);
59-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(3000));
58+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(3000));
6059
Duration timeout2 = driver.manage().timeouts().getScriptTimeout();
6160
assertThat(timeout2).hasMillis(3000);
6261
}
@@ -185,7 +184,7 @@ public void shouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout() {
185184
@Test
186185
@NotYetImplemented(SAFARI)
187186
public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
188-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(500));
187+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(500));
189188
driver.get(pages.ajaxyPage);
190189
assertThatExceptionOfType(ScriptTimeoutException.class)
191190
.isThrownBy(
@@ -199,7 +198,7 @@ public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
199198
@Ignore(IE)
200199
public void shouldDetectPageLoadsWhileWaitingOnAnAsyncScriptAndReturnAnError() {
201200
driver.get(pages.ajaxyPage);
202-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(100));
201+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(100));
203202
assertThatExceptionOfType(WebDriverException.class)
204203
.isThrownBy(
205204
() -> executor.executeAsyncScript("window.location = '" + pages.dynamicPage + "';"));
@@ -244,7 +243,7 @@ public void shouldCatchErrorsWithMessageAndStacktraceWhenExecutingInitialScript(
244243
t -> {
245244
Throwable rootCause = getRootCause(t);
246245
assertThat(rootCause).hasMessageContaining("errormessage");
247-
assertThat(Arrays.asList(rootCause.getStackTrace()))
246+
assertThat(List.of(rootCause.getStackTrace()))
248247
.extracting(StackTraceElement::getMethodName)
249248
.contains("functionB");
250249
});
@@ -266,7 +265,7 @@ void shouldBeAbleToExecuteAsynchronousScripts() {
266265
"There should only be 1 DIV at this point, which is used for the butter message")
267266
.isEqualTo(1);
268267

269-
driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(15));
268+
driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(15));
270269
String text =
271270
(String)
272271
executor.executeAsyncScript(
@@ -317,7 +316,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
317316
+ "xhr.send('');"; // empty string to stop firefox 3 from choking
318317

319318
driver.get(pages.ajaxyPage);
320-
driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(3));
319+
driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(3));
321320
String response = (String) executor.executeAsyncScript(script, pages.sleepingPage + "?time=2");
322321
assertThat(response.trim())
323322
.isEqualTo("<html><head><title>Done</title></head><body>Slept for 2s</body></html>");
@@ -331,7 +330,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
331330
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
332331
public void throwsIfScriptTriggersAlert() {
333332
driver.get(pages.simpleTestPage);
334-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
333+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
335334
assertThatExceptionOfType(UnhandledAlertException.class)
336335
.isThrownBy(
337336
() ->
@@ -350,7 +349,7 @@ public void throwsIfScriptTriggersAlert() {
350349
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
351350
public void throwsIfAlertHappensDuringScript() {
352351
driver.get(pages.slowLoadingAlertPage);
353-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
352+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
354353
assertThatExceptionOfType(UnhandledAlertException.class)
355354
.isThrownBy(() -> executor.executeAsyncScript("setTimeout(arguments[0], 1000);"));
356355
// Shouldn't throw
@@ -365,7 +364,7 @@ public void throwsIfAlertHappensDuringScript() {
365364
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
366365
public void throwsIfScriptTriggersAlertWhichTimesOut() {
367366
driver.get(pages.simpleTestPage);
368-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
367+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
369368
assertThatExceptionOfType(UnhandledAlertException.class)
370369
.isThrownBy(
371370
() ->
@@ -383,7 +382,7 @@ public void throwsIfScriptTriggersAlertWhichTimesOut() {
383382
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
384383
public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
385384
driver.get(pages.slowLoadingAlertPage);
386-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
385+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
387386
assertThatExceptionOfType(UnhandledAlertException.class)
388387
.isThrownBy(() -> executor.executeAsyncScript(""));
389388
// Shouldn't throw
@@ -397,7 +396,7 @@ public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
397396
@Ignore(FIREFOX)
398397
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
399398
public void includesAlertTextInUnhandledAlertException() {
400-
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
399+
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
401400
String alertText = "Look! An alert!";
402401
assertThatExceptionOfType(UnhandledAlertException.class)
403402
.isThrownBy(

‎java/test/org/openqa/selenium/ExecutingJavascriptTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ public void testShouldThrowAnExceptionWithMessageAndStacktraceWhenTheJavascriptI
264264
t -> {
265265
Throwable rootCause = getRootCause(t);
266266
assertThat(rootCause).hasMessageContaining("errormessage");
267-
assertThat(Arrays.asList(rootCause.getStackTrace()))
267+
assertThat(List.of(rootCause.getStackTrace()))
268268
.extracting(StackTraceElement::getMethodName)
269269
.contains("functionB");
270270
});

‎java/test/org/openqa/selenium/build/BazelBuild.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void build(String target) {
5454
return;
5555
}
5656

57-
if (target == null || "".equals(target)) {
57+
if (target == null || target.isEmpty()) {
5858
throw new IllegalStateException("No targets specified");
5959
}
6060
LOG.info("\nBuilding " + target + " ...");

‎java/test/org/openqa/selenium/grid/router/SessionQueueGridWithTimeoutTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.net.URISyntaxException;
3131
import java.time.Duration;
3232
import java.time.Instant;
33-
import java.util.Arrays;
3433
import java.util.List;
3534
import java.util.concurrent.Callable;
3635
import java.util.concurrent.ExecutionException;
@@ -163,7 +162,7 @@ void shouldBeAbleToDeleteTimedoutSessions() {
163162
try {
164163
Callable<HttpResponse> sessionCreationTask = () -> createSession(caps);
165164
List<Future<HttpResponse>> futureList =
166-
fixedThreadPoolService.invokeAll(Arrays.asList(sessionCreationTask));
165+
fixedThreadPoolService.invokeAll(List.of(sessionCreationTask));
167166

168167
for (Future<HttpResponse> future : futureList) {
169168
HttpResponse httpResponse = future.get(10, SECONDS);

‎java/test/org/openqa/selenium/ie/InternetExplorerDriverTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ void testPersistentHoverCanBeTurnedOff() throws Exception {
138138
// Intentionally wait to make sure hover DOES NOT persist.
139139
Thread.sleep(1000);
140140

141-
wait.until(d -> item.getText().equals(""));
141+
wait.until(d -> item.getText().isEmpty());
142142

143143
assertThat(item.getText()).isEmpty();
144144
}

‎java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ void canHandleGetTimeoutsCommand() {
623623
void canHandleSetScriptTimeoutCommand() {
624624
WebDriverFixture fixture = new WebDriverFixture(echoCapabilities, nullValueResponder);
625625

626-
fixture.driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(10));
626+
fixture.driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(10));
627627

628628
fixture.verifyCommands(
629629
new CommandPayload(DriverCommand.SET_TIMEOUT, ImmutableMap.of("script", 10000L)));

0 commit comments

Comments
 (0)