Skip to content

Commit 51b503d

Browse files
asolntsevasolntsev
authored
[java] fix some nullability warnings (#17362)
* fix nullability warnings about `Values.get` After introducing JSpecify nullability annotations, IDE shows some reasonable warnings about nullable/nonnullable values. This commit marks method `Values.get` as @nullable and fixes all its usages. * fix few more nullability warnings
1 parent 265d061 commit 51b503d

19 files changed

Lines changed: 121 additions & 83 deletions

File tree

‎java/src/org/openqa/selenium/events/Event.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Objects;
2121
import java.util.StringJoiner;
2222
import java.util.UUID;
23+
import org.jspecify.annotations.Nullable;
2324
import org.openqa.selenium.internal.Require;
2425
import org.openqa.selenium.json.Json;
2526
import org.openqa.selenium.json.JsonOutput;
@@ -31,11 +32,11 @@ public class Event {
3132
private final EventName eventName;
3233
private final String data;
3334

34-
public Event(EventName eventName, Object data) {
35+
public Event(EventName eventName, @Nullable Object data) {
3536
this(UUID.randomUUID(), eventName, data);
3637
}
3738

38-
public Event(UUID id, EventName eventName, Object data) {
39+
public Event(UUID id, EventName eventName, @Nullable Object data) {
3940
this.id = Require.nonNull("Message id", id);
4041
this.eventName = Require.nonNull("Event type", eventName);
4142

‎java/src/org/openqa/selenium/grid/Bootstrap.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
import java.net.MalformedURLException;
2828
import java.net.URL;
2929
import java.net.URLClassLoader;
30+
import java.nio.file.DirectoryStream;
31+
import java.nio.file.Files;
32+
import java.nio.file.Path;
3033
import java.security.AccessController;
3134
import java.security.PrivilegedAction;
3235
import java.util.ArrayList;
@@ -90,16 +93,19 @@ private static ClassLoader createExtendedClassLoader(String ext) {
9093
continue;
9194
}
9295

93-
if (file.isDirectory()) {
94-
File[] files = file.listFiles();
95-
if (files == null) {
96-
LOG.warning("Cannot list files in directory: " + file);
97-
} else {
98-
for (File subdirFile : files) {
96+
Path dir = file.toPath();
97+
98+
if (Files.isDirectory(dir)) {
99+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(dir)) {
100+
for (Path path : stream) {
101+
File subdirFile = path.toFile();
99102
if (subdirFile.isFile() && subdirFile.getName().endsWith(".jar")) {
100103
jars.add(subdirFile);
101104
}
102105
}
106+
} catch (IOException e) {
107+
LOG.warning(
108+
() -> String.format("Cannot list files in directory: %s due to: %s", file, e));
103109
}
104110
} else {
105111
jars.add(file);

‎java/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public RemoteDistributor add(Node node) {
7575

7676
HttpResponse response = client.with(addSecret).execute(request);
7777

78-
Values.get(response, Void.class);
78+
Values.parse(response);
7979

8080
LOG.info(String.format("Added node %s.", node.getId()));
8181

@@ -91,7 +91,7 @@ public boolean drain(NodeId nodeId) {
9191

9292
HttpResponse response = client.with(addSecret).execute(request);
9393

94-
return Values.get(response, Boolean.class);
94+
return Values.parseBoolean(response);
9595
}
9696

9797
@Override
@@ -102,7 +102,7 @@ public void remove(NodeId nodeId) {
102102

103103
HttpResponse response = client.with(addSecret).execute(request);
104104

105-
Values.get(response, Void.class);
105+
Values.parse(response);
106106
}
107107

108108
@Override
@@ -112,7 +112,7 @@ public DistributorStatus getStatus() {
112112

113113
HttpResponse response = client.execute(request);
114114

115-
return Values.get(response, DistributorStatus.class);
115+
return Values.parse(response, DistributorStatus.class);
116116
}
117117

118118
@Override
@@ -125,7 +125,7 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(
125125
HttpResponse res = client.execute(req);
126126

127127
if (res.isSuccessful()) {
128-
return Either.right(Values.get(res, CreateSessionResponse.class));
128+
return Either.right(Values.parse(res, CreateSessionResponse.class));
129129
} else {
130130
return Either.left(Values.get(res, SessionNotCreatedException.class));
131131
}

‎java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public boolean isSessionOwner(SessionId id) {
207207

208208
HttpResponse res = client.with(addSecret).execute(req);
209209

210-
return Boolean.TRUE.equals(Values.get(res, Boolean.class));
210+
return Values.parseBoolean(res);
211211
}
212212

213213
@Override
@@ -219,7 +219,7 @@ public boolean tryAcquireConnection(SessionId id) {
219219

220220
HttpResponse res = client.with(addSecret).execute(req);
221221

222-
return Boolean.TRUE.equals(Values.get(res, Boolean.class));
222+
return Values.parseBoolean(res);
223223
}
224224

225225
@Override
@@ -231,7 +231,7 @@ public void releaseConnection(SessionId id) {
231231

232232
HttpResponse res = client.with(addSecret).execute(req);
233233

234-
Values.get(res, Void.class);
234+
Values.parse(res);
235235
}
236236

237237
@Override
@@ -243,7 +243,7 @@ public Session getSession(SessionId id) throws NoSuchSessionException {
243243

244244
HttpResponse res = client.with(addSecret).execute(req);
245245

246-
return Require.nonNull("Session", Values.get(res, Session.class));
246+
return Values.parse(res, Session.class);
247247
}
248248

249249
@Override
@@ -274,7 +274,7 @@ public void stop(SessionId id) throws NoSuchSessionException {
274274

275275
HttpResponse res = client.with(addSecret).execute(req);
276276

277-
Values.get(res, Void.class);
277+
Values.parse(res);
278278
}
279279

280280
@Override

‎java/src/org/openqa/selenium/grid/sessionmap/remote/RemoteSessionMap.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static org.openqa.selenium.remote.http.HttpMethod.POST;
2525

2626
import java.io.UncheckedIOException;
27-
import java.lang.reflect.Type;
2827
import java.net.MalformedURLException;
2928
import java.net.URI;
3029
import org.jspecify.annotations.Nullable;
@@ -114,7 +113,7 @@ public void remove(SessionId id) {
114113
}
115114

116115
@Nullable
117-
private <T> T makeRequest(HttpRequest request, Type typeOfT) {
116+
private <T> T makeRequest(HttpRequest request, Class<T> typeOfT) {
118117
HttpTracing.inject(tracer, tracer.getCurrentContext(), request);
119118

120119
HttpResponse response = client.execute(request);

‎java/src/org/openqa/selenium/grid/sessionqueue/remote/RemoteNewSessionQueue.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public boolean retryAddToQueue(SessionRequest request) {
121121
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
122122
upstream.setContent(Contents.asJson(request));
123123
HttpResponse response = client.with(addSecret).execute(upstream);
124-
return Values.get(response, Boolean.class);
124+
return Values.parseBoolean(response);
125125
}
126126

127127
@Override
@@ -156,7 +156,7 @@ public List<SessionRequest> getNextAvailable(Map<Capabilities, Long> stereotypes
156156
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
157157
HttpResponse response = client.with(addSecret).execute(upstream);
158158

159-
return Values.get(response, SESSION_REQUEST_TYPE);
159+
return Require.nonNull("Next session request", Values.get(response, SESSION_REQUEST_TYPE));
160160
}
161161

162162
@Override
@@ -178,7 +178,7 @@ public boolean complete(
178178

179179
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
180180
HttpResponse response = client.with(addSecret).execute(upstream);
181-
return Values.get(response, Boolean.class);
181+
return Values.parseBoolean(response);
182182
}
183183

184184
@Override
@@ -187,7 +187,7 @@ public int clearQueue() {
187187
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
188188
HttpResponse response = client.with(addSecret).execute(upstream);
189189

190-
return Values.get(response, Integer.class);
190+
return Values.parse(response, Integer.class);
191191
}
192192

193193
@Override
@@ -196,7 +196,7 @@ public List<SessionRequestCapability> getQueueContents() {
196196
HttpTracing.inject(tracer, tracer.getCurrentContext(), upstream);
197197
HttpResponse response = client.execute(upstream);
198198

199-
return Values.get(response, QUEUE_CONTENTS_TYPE);
199+
return Require.nonNull("Queue contents", Values.get(response, QUEUE_CONTENTS_TYPE));
200200
}
201201

202202
@Override

‎java/src/org/openqa/selenium/grid/web/Values.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import java.io.Reader;
2727
import java.io.UncheckedIOException;
2828
import java.lang.reflect.Type;
29+
import org.jspecify.annotations.Nullable;
30+
import org.openqa.selenium.internal.Require;
2931
import org.openqa.selenium.json.Json;
3032
import org.openqa.selenium.json.JsonInput;
3133
import org.openqa.selenium.remote.ErrorCodec;
@@ -36,6 +38,19 @@ public class Values {
3638
private static final Json JSON = new Json();
3739
private static final ErrorCodec ERRORS = ErrorCodec.createDefault();
3840

41+
public static <T> T parse(HttpResponse response, Class<T> classOfT) {
42+
return Require.nonNull(classOfT.getSimpleName(), get(response, classOfT));
43+
}
44+
45+
public static void parse(HttpResponse response) {
46+
get(response, Void.class);
47+
}
48+
49+
public static boolean parseBoolean(HttpResponse response) {
50+
return Boolean.TRUE.equals(get(response, Boolean.class));
51+
}
52+
53+
@Nullable
3954
public static <T> T get(HttpResponse response, Type typeOfT) {
4055
try (Reader reader = reader(response);
4156
JsonInput input = JSON.newInput(reader)) {

‎java/src/org/openqa/selenium/io/Zip.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
import java.io.IOException;
2727
import java.io.InputStream;
2828
import java.io.OutputStream;
29+
import java.nio.file.DirectoryStream;
30+
import java.nio.file.Files;
31+
import java.nio.file.Path;
2932
import java.nio.file.attribute.FileTime;
3033
import java.util.Base64;
3134
import java.util.logging.Level;
@@ -54,12 +57,15 @@ public static String zip(File input) throws IOException {
5457

5558
private static void addToZip(String basePath, ZipOutputStream zos, File toAdd)
5659
throws IOException {
57-
if (toAdd.isDirectory()) {
58-
File[] files = toAdd.listFiles();
59-
if (files != null) {
60-
for (File file : files) {
61-
addToZip(basePath, zos, file);
60+
Path dirPath = toAdd.toPath();
61+
62+
if (Files.isDirectory(dirPath)) {
63+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(dirPath)) {
64+
for (Path path : stream) {
65+
addToZip(basePath, zos, path.toFile());
6266
}
67+
} catch (IOException e) {
68+
LOG.warning(() -> String.format("Failed to read directory %s for zipping: %s", toAdd, e));
6369
}
6470
} else {
6571
try (FileInputStream fis = new FileInputStream(toAdd)) {

‎java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,10 @@ protected void defineCommand(String name, CommandSpec spec) {
322322
}
323323

324324
private String buildUri(
325-
String commandName, SessionId sessionId, Map<String, ?> parameters, CommandSpec spec) {
325+
String commandName,
326+
@Nullable SessionId sessionId,
327+
Map<String, ?> parameters,
328+
CommandSpec spec) {
326329
StringBuilder builder = new StringBuilder();
327330
for (String part : spec.pathSegments) {
328331
if (part.isEmpty()) {
@@ -340,10 +343,12 @@ private String buildUri(
340343
}
341344

342345
private String getParameter(
343-
String parameterName, String commandName, SessionId sessionId, Map<String, ?> parameters) {
346+
String parameterName,
347+
String commandName,
348+
@Nullable SessionId sessionId,
349+
Map<String, ?> parameters) {
344350
if ("sessionId".equals(parameterName)) {
345-
Require.argument("Session id", sessionId)
346-
.nonNull("Session ID may not be null for command %s", commandName);
351+
Require.nonNull("Session id", sessionId, "may not be null for command %s", commandName);
347352
return sessionId.toString();
348353
}
349354

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.assertj.core.api.Assertions.assertThat;
2121
import static org.assertj.core.api.Assertions.assertThatCode;
2222
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
23+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2324
import static org.openqa.selenium.WaitingConditions.newWindowIsOpened;
2425
import static org.openqa.selenium.support.ui.ExpectedConditions.alertIsPresent;
2526
import static org.openqa.selenium.support.ui.ExpectedConditions.presenceOfElementLocated;
@@ -117,8 +118,10 @@ void testShouldThrowIllegalArgumentExceptionWhenKeysNull() {
117118
driver.findElement(By.id("alert")).click();
118119
Alert alert = wait.until(alertIsPresent());
119120

120-
assertThatExceptionOfType(IllegalArgumentException.class)
121-
.isThrownBy(() -> alert.sendKeys(null));
121+
//noinspection DataFlowIssue
122+
assertThatThrownBy(() -> alert.sendKeys(null))
123+
.isInstanceOf(IllegalArgumentException.class)
124+
.hasMessage("Keys to send should be a not null CharSequence");
122125
}
123126

124127
@Test

0 commit comments

Comments
 (0)