Skip to content

Commit f052b7b

Browse files
shs96cshs96c
committed
Strongly type the SessionRequest
1 parent 6657964 commit f052b7b

26 files changed

+555
-591
lines changed

‎java/client/src/org/openqa/selenium/remote/NewSessionPayload.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public Stream<Capabilities> stream() {
318318
}
319319
}
320320

321-
public ImmutableSet<Dialect> getDownstreamDialects() {
321+
public Set<Dialect> getDownstreamDialects() {
322322
return dialects.isEmpty() ? ImmutableSet.of(DEFAULT_DIALECT) : dialects;
323323
}
324324

‎java/client/src/org/openqa/selenium/remote/http/Contents.java

+11
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import org.openqa.selenium.internal.Require;
2626
import org.openqa.selenium.json.Json;
27+
import org.openqa.selenium.json.JsonInput;
2728
import org.openqa.selenium.json.JsonOutput;
2829

2930
import java.io.ByteArrayInputStream;
@@ -33,6 +34,7 @@
3334
import java.io.InputStreamReader;
3435
import java.io.Reader;
3536
import java.io.UncheckedIOException;
37+
import java.lang.reflect.Type;
3638
import java.nio.charset.Charset;
3739
import java.util.function.Supplier;
3840

@@ -123,6 +125,15 @@ public static Supplier<InputStream> asJson(Object obj) {
123125
return utf8String(builder);
124126
}
125127

128+
public static <T> T fromJson(HttpMessage<?> message, Type typeOfT) {
129+
try (Reader reader = reader(message);
130+
JsonInput input = JSON.newInput(reader)) {
131+
return input.read(typeOfT);
132+
} catch (IOException e) {
133+
throw new UncheckedIOException(e);
134+
}
135+
}
136+
126137
public static Supplier<InputStream> memoize(Supplier<InputStream> delegate) {
127138
return new MemoizedSupplier(delegate);
128139
}

‎java/server/src/org/openqa/selenium/grid/distributor/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ java_library(
2121
"//java/server/src/org/openqa/selenium/grid/node/remote",
2222
"//java/server/src/org/openqa/selenium/grid/security",
2323
"//java/server/src/org/openqa/selenium/grid/sessionmap",
24+
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
2425
# Default implementation of the session map. Loaded reflectively
2526
"//java/server/src/org/openqa/selenium/grid/sessionmap/remote",
2627
"//java/server/src/org/openqa/selenium/status",

‎java/server/src/org/openqa/selenium/grid/distributor/Distributor.java

+16-24
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@
3434
import org.openqa.selenium.grid.security.RequiresSecretFilter;
3535
import org.openqa.selenium.grid.security.Secret;
3636
import org.openqa.selenium.grid.sessionmap.SessionMap;
37+
import org.openqa.selenium.grid.sessionqueue.SessionRequest;
3738
import org.openqa.selenium.internal.Either;
3839
import org.openqa.selenium.internal.Require;
3940
import org.openqa.selenium.json.Json;
40-
import org.openqa.selenium.remote.NewSessionPayload;
4141
import org.openqa.selenium.remote.SessionId;
4242
import org.openqa.selenium.remote.http.HttpClient;
4343
import org.openqa.selenium.remote.http.HttpRequest;
@@ -53,13 +53,10 @@
5353
import org.openqa.selenium.remote.tracing.Tracer;
5454
import org.openqa.selenium.status.HasReadyState;
5555

56-
import java.io.IOException;
57-
import java.io.Reader;
5856
import java.io.UncheckedIOException;
5957
import java.util.HashMap;
6058
import java.util.Iterator;
6159
import java.util.Map;
62-
import java.util.Objects;
6360
import java.util.Set;
6461
import java.util.UUID;
6562
import java.util.concurrent.locks.Lock;
@@ -73,11 +70,9 @@
7370
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
7471
import static org.openqa.selenium.remote.RemoteTags.SESSION_ID;
7572
import static org.openqa.selenium.remote.RemoteTags.SESSION_ID_EVENT;
76-
import static org.openqa.selenium.remote.http.Contents.reader;
7773
import static org.openqa.selenium.remote.http.Route.delete;
7874
import static org.openqa.selenium.remote.http.Route.get;
7975
import static org.openqa.selenium.remote.http.Route.post;
80-
import static org.openqa.selenium.remote.tracing.HttpTracing.newSpanAsChildOf;
8176
import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
8277

8378
/**
@@ -160,24 +155,21 @@ protected Distributor(
160155
.with(new SpanDecorator(tracer, req -> "distributor.status")));
161156
}
162157

163-
public Either<SessionNotCreatedException, CreateSessionResponse> newSession(HttpRequest request)
158+
public Either<SessionNotCreatedException, CreateSessionResponse> newSession(SessionRequest request)
164159
throws SessionNotCreatedException {
160+
Require.nonNull("Requests to process", request);
165161

166-
Span span = newSpanAsChildOf(tracer, request, "distributor.create_session_response");
162+
Span span = tracer.getCurrentContext().createSpan("distributor.create_session_response");
167163
Map<String, EventAttributeValue> attributeMap = new HashMap<>();
168-
try (
169-
Reader reader = reader(request);
170-
NewSessionPayload payload = NewSessionPayload.create(reader)) {
171-
Objects.requireNonNull(payload, "Requests to process must be set.");
172-
164+
try {
173165
attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(),
174166
EventAttribute.setValue(getClass().getName()));
175167

176-
Iterator<Capabilities> iterator = payload.stream().iterator();
177-
attributeMap.put("request.payload", EventAttribute.setValue(payload.toString()));
168+
Iterator<Capabilities> iterator = request.getDesiredCapabilities().iterator();
169+
attributeMap.put("request.payload", EventAttribute.setValue(request.getDesiredCapabilities().toString()));
178170
String sessionReceivedMessage = "Session request received by the distributor";
179171
span.addEvent(sessionReceivedMessage, attributeMap);
180-
LOG.info(String.format("%s: \n %s", sessionReceivedMessage, payload));
172+
LOG.info(String.format("%s: \n %s", sessionReceivedMessage, request.getDesiredCapabilities()));
181173

182174
if (!iterator.hasNext()) {
183175
SessionNotCreatedException exception =
@@ -192,7 +184,7 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Http
192184

193185
Either<SessionNotCreatedException, CreateSessionResponse> selected;
194186
CreateSessionRequest firstRequest = new CreateSessionRequest(
195-
payload.getDownstreamDialects(),
187+
request.getDownstreamDialects(),
196188
iterator.next(),
197189
ImmutableMap.of("span", span));
198190

@@ -208,7 +200,7 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Http
208200
if (!hostsWithCaps) {
209201
String errorMessage = String.format(
210202
"No Node supports the required capabilities: %s",
211-
payload.stream().map(Capabilities::toString)
203+
request.getDesiredCapabilities().stream().map(Capabilities::toString)
212204
.collect(Collectors.joining(", ")));
213205
SessionNotCreatedException exception = new SessionNotCreatedException(errorMessage);
214206
span.setAttribute(AttributeKey.ERROR.getKey(), true);
@@ -229,7 +221,7 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Http
229221
String errorMessage =
230222
String.format(
231223
"Unable to find provider for session: %s",
232-
payload.stream().map(Capabilities::toString)
224+
request.getDesiredCapabilities().stream().map(Capabilities::toString)
233225
.collect(Collectors.joining(", ")));
234226
SessionNotCreatedException exception = new RetrySessionRequestException(errorMessage);
235227
selected = Either.left(exception);
@@ -270,14 +262,13 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Http
270262
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
271263

272264
return Either.left(e);
273-
} catch (IOException e) {
265+
} catch (UncheckedIOException e) {
274266
span.setAttribute(AttributeKey.ERROR.getKey(), true);
275267
span.setStatus(Status.UNKNOWN);
276268

277269
EXCEPTION.accept(attributeMap, e);
278270
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
279-
EventAttribute.setValue("Unknown error in LocalDistributor while creating session: " +
280-
e.getMessage()));
271+
EventAttribute.setValue("Unknown error in LocalDistributor while creating session: " + e.getMessage()));
281272
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
282273

283274
return Either.left(new SessionNotCreatedException(e.getMessage(), e));
@@ -296,8 +287,9 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(Http
296287

297288
protected abstract Set<NodeStatus> getAvailableNodes();
298289

299-
protected abstract Either<SessionNotCreatedException, CreateSessionResponse> reserve(SlotId slot,
300-
CreateSessionRequest request);
290+
protected abstract Either<SessionNotCreatedException, CreateSessionResponse> reserve(
291+
SlotId slot,
292+
CreateSessionRequest request);
301293

302294
@Override
303295
public boolean test(HttpRequest httpRequest) {

‎java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

+7-9
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.openqa.selenium.grid.distributor.local;
1919

2020
import com.google.common.collect.ImmutableSet;
21-
2221
import org.openqa.selenium.Beta;
2322
import org.openqa.selenium.Capabilities;
2423
import org.openqa.selenium.ImmutableCapabilities;
@@ -59,11 +58,11 @@
5958
import org.openqa.selenium.grid.sessionmap.SessionMap;
6059
import org.openqa.selenium.grid.sessionmap.config.SessionMapOptions;
6160
import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
61+
import org.openqa.selenium.grid.sessionqueue.SessionRequest;
6262
import org.openqa.selenium.grid.sessionqueue.config.NewSessionQueuerOptions;
6363
import org.openqa.selenium.internal.Either;
6464
import org.openqa.selenium.internal.Require;
6565
import org.openqa.selenium.remote.http.HttpClient;
66-
import org.openqa.selenium.remote.http.HttpRequest;
6766
import org.openqa.selenium.remote.tracing.AttributeKey;
6867
import org.openqa.selenium.remote.tracing.EventAttribute;
6968
import org.openqa.selenium.remote.tracing.EventAttributeValue;
@@ -94,7 +93,6 @@
9493
import static com.google.common.collect.ImmutableSet.toImmutableSet;
9594
import static org.openqa.selenium.grid.data.Availability.DOWN;
9695
import static org.openqa.selenium.grid.data.Availability.DRAINING;
97-
import static org.openqa.selenium.remote.tracing.HttpTracing.newSpanAsChildOf;
9896

9997
public class LocalDistributor extends Distributor {
10098

@@ -395,10 +393,10 @@ public void run() {
395393
if (hasCapacity) {
396394
RequestId reqId = requestIds.poll();
397395
if (reqId != null) {
398-
Optional<HttpRequest> optionalHttpRequest = sessionRequests.remove(reqId);
396+
Optional<SessionRequest> maybeRequest = sessionRequests.remove(reqId);
399397
// Check if polling the queue did not return null
400-
if (optionalHttpRequest.isPresent()) {
401-
handleNewSessionRequest(optionalHttpRequest.get(), reqId);
398+
if (maybeRequest.isPresent()) {
399+
handleNewSessionRequest(maybeRequest.get(), reqId);
402400
} else {
403401
fireSessionRejectedEvent(
404402
"Unable to poll request from the new session request queue.",
@@ -412,8 +410,8 @@ public void run() {
412410
}
413411
}
414412

415-
private void handleNewSessionRequest(HttpRequest sessionRequest, RequestId reqId) {
416-
try (Span span = newSpanAsChildOf(tracer, sessionRequest, "distributor.poll_queue")) {
413+
private void handleNewSessionRequest(SessionRequest sessionRequest, RequestId reqId) {
414+
try (Span span = tracer.getCurrentContext().createSpan("distributor.poll_queue")) {
417415
Map<String, EventAttributeValue> attributeMap = new HashMap<>();
418416
attributeMap.put(
419417
AttributeKey.LOGGER_CLASS.getKey(),
@@ -439,7 +437,7 @@ private void handleNewSessionRequest(HttpRequest sessionRequest, RequestId reqId
439437
SessionNotCreatedException exception = response.left();
440438

441439
if (exception instanceof RetrySessionRequestException) {
442-
boolean retried = sessionRequests.retryAddToQueue(sessionRequest, reqId);
440+
boolean retried = sessionRequests.retryAddToQueue(sessionRequest);
443441

444442
attributeMap.put("request.retry_add", EventAttribute.setValue(retried));
445443
span.addEvent("Retry adding to front of queue. No slot available.", attributeMap);

‎java/server/src/org/openqa/selenium/grid/sessionqueue/AddBackToSessionQueue.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import org.openqa.selenium.grid.data.RequestId;
2727
import org.openqa.selenium.internal.Require;
28+
import org.openqa.selenium.remote.http.Contents;
2829
import org.openqa.selenium.remote.http.HttpHandler;
2930
import org.openqa.selenium.remote.http.HttpRequest;
3031
import org.openqa.selenium.remote.http.HttpResponse;
@@ -50,7 +51,9 @@ public HttpResponse execute(HttpRequest req) {
5051
HTTP_REQUEST.accept(span, req);
5152
span.setAttribute(AttributeKey.REQUEST_ID.getKey(), id.toString());
5253

53-
boolean value = newSessionQueuer.retryAddToQueue(req, id);
54+
SessionRequest sessionRequest = Contents.fromJson(req, SessionRequest.class);
55+
56+
boolean value = newSessionQueuer.retryAddToQueue(sessionRequest);
5457

5558
span.setAttribute("request.retry", value);
5659

‎java/server/src/org/openqa/selenium/grid/sessionqueue/AddToSessionQueue.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.openqa.selenium.remote.tracing.Tags.HTTP_RESPONSE;
2323

2424
import org.openqa.selenium.internal.Require;
25+
import org.openqa.selenium.remote.http.Contents;
2526
import org.openqa.selenium.remote.http.HttpHandler;
2627
import org.openqa.selenium.remote.http.HttpRequest;
2728
import org.openqa.selenium.remote.http.HttpResponse;
@@ -43,7 +44,7 @@ public HttpResponse execute(HttpRequest req) {
4344
try (Span span = newSpanAsChildOf(tracer, req, "sessionqueuer.add")) {
4445
HTTP_REQUEST.accept(span, req);
4546

46-
HttpResponse response = newSessionQueuer.addToQueue(req);
47+
HttpResponse response = newSessionQueuer.addToQueue(Contents.fromJson(req, SessionRequest.class));
4748

4849
HTTP_RESPONSE.accept(span, response);
4950

‎java/server/src/org/openqa/selenium/grid/sessionqueue/GetNewSessionResponse.java

+8-12
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@
1717

1818
package org.openqa.selenium.grid.sessionqueue;
1919

20-
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
21-
import static java.util.Collections.singletonMap;
22-
import static org.openqa.selenium.remote.http.Contents.asJson;
23-
import static org.openqa.selenium.remote.http.Contents.bytes;
24-
2520
import org.openqa.selenium.events.EventBus;
2621
import org.openqa.selenium.grid.data.NewSessionErrorResponse;
2722
import org.openqa.selenium.grid.data.NewSessionRejectedEvent;
@@ -30,13 +25,10 @@
3025
import org.openqa.selenium.grid.data.NewSessionResponseEvent;
3126
import org.openqa.selenium.grid.data.RequestId;
3227
import org.openqa.selenium.internal.Require;
33-
import org.openqa.selenium.remote.http.HttpRequest;
3428
import org.openqa.selenium.remote.http.HttpResponse;
35-
import org.openqa.selenium.remote.tracing.Tracer;
3629

3730
import java.util.Map;
3831
import java.util.Optional;
39-
import java.util.UUID;
4032
import java.util.concurrent.ConcurrentHashMap;
4133
import java.util.concurrent.CountDownLatch;
4234
import java.util.concurrent.locks.Lock;
@@ -45,6 +37,11 @@
4537
import java.util.logging.Level;
4638
import java.util.logging.Logger;
4739

40+
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
41+
import static java.util.Collections.singletonMap;
42+
import static org.openqa.selenium.remote.http.Contents.asJson;
43+
import static org.openqa.selenium.remote.http.Contents.bytes;
44+
4845
public class GetNewSessionResponse {
4946

5047
private static final Logger LOG = Logger.getLogger(GetNewSessionResponse.class.getName());
@@ -107,16 +104,15 @@ private void setErrorResponse(NewSessionErrorResponse sessionResponse) {
107104
}
108105
}
109106

110-
public HttpResponse add(HttpRequest request) {
107+
public HttpResponse add(SessionRequest request) {
111108
Require.nonNull("New Session request", request);
112109

113110
CountDownLatch latch = new CountDownLatch(1);
114-
UUID uuid = UUID.randomUUID();
115-
RequestId requestId = new RequestId(uuid);
111+
RequestId requestId = request.getRequestId();
116112
NewSessionRequest requestIdentifier = new NewSessionRequest(requestId, latch);
117113
knownRequests.put(requestId, requestIdentifier);
118114

119-
if (!sessionRequests.offerLast(request, requestId)) {
115+
if (!sessionRequests.offerLast(request)) {
120116
return internalErrorResponse(
121117
"Session request could not be created. Error while adding to the session queue.");
122118
}

‎java/server/src/org/openqa/selenium/grid/sessionqueue/NewSessionQueue.java

+5-7
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ protected NewSessionQueue(Tracer tracer, Duration retryInterval, Duration reques
4242
this.requestTimeout = Require.nonNull("Session request timeout", requestTimeout);
4343
}
4444

45-
public abstract boolean offerLast(HttpRequest request, RequestId requestId);
45+
public abstract boolean offerLast(SessionRequest request);
4646

47-
public abstract boolean offerFirst(HttpRequest request, RequestId requestId);
47+
public abstract boolean offerFirst(SessionRequest request);
4848

49-
public abstract Optional<HttpRequest> remove(RequestId requestId);
49+
public abstract Optional<SessionRequest> remove(RequestId requestId);
5050

5151
public abstract int clear();
5252

@@ -57,13 +57,11 @@ protected NewSessionQueue(Tracer tracer, Duration retryInterval, Duration reques
5757
public void addRequestHeaders(HttpRequest request, RequestId reqId) {
5858
long timestamp = Instant.now().getEpochSecond();
5959
request.addHeader(SESSIONREQUEST_TIMESTAMP_HEADER, Long.toString(timestamp));
60-
6160
request.addHeader(SESSIONREQUEST_ID_HEADER, reqId.toString());
6261
}
6362

64-
public boolean hasRequestTimedOut(HttpRequest request) {
65-
String enqueTimestampStr = request.getHeader(SESSIONREQUEST_TIMESTAMP_HEADER);
66-
Instant enque = Instant.ofEpochSecond(Long.parseLong(enqueTimestampStr));
63+
public boolean hasRequestTimedOut(SessionRequest request) {
64+
Instant enque = request.getEnqueued();
6765
Instant deque = Instant.now();
6866
Duration duration = Duration.between(enque, deque);
6967

0 commit comments

Comments
 (0)