Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[grid][java]: session-timeout set connection timeout in RemoteNode #13854

Merged
merged 11 commits into from
May 6, 2024
23 changes: 22 additions & 1 deletion java/src/org/openqa/selenium/grid/data/NodeStatus.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class NodeStatus {
private final Set<Slot> slots;
private final Availability availability;
private final Duration heartbeatPeriod;
private final Duration sessionTimeout;
private final String version;
private final Map<String, String> osInfo;

Expand All @@ -52,6 +53,7 @@ public NodeStatus(
Set<Slot> slots,
Availability availability,
Duration heartbeatPeriod,
Duration sessionTimeout,
String version,
Map<String, String> osInfo) {
this.nodeId = Require.nonNull("Node id", nodeId);
Expand All @@ -62,6 +64,7 @@ public NodeStatus(
this.slots = unmodifiableSet(new HashSet<>(Require.nonNull("Slots", slots)));
this.availability = Require.nonNull("Availability", availability);
this.heartbeatPeriod = heartbeatPeriod;
this.sessionTimeout = sessionTimeout;
this.version = Require.nonNull("Grid Node version", version);
this.osInfo = Require.nonNull("Node host OS info", osInfo);
}
Expand All @@ -73,6 +76,7 @@ public static NodeStatus fromJson(JsonInput input) {
Set<Slot> slots = null;
Availability availability = null;
Duration heartbeatPeriod = null;
Duration sessionTimeout = null;
String version = null;
Map<String, String> osInfo = null;

Expand All @@ -87,6 +91,10 @@ public static NodeStatus fromJson(JsonInput input) {
heartbeatPeriod = Duration.ofMillis(input.read(Long.class));
break;

case "sessionTimeout":
sessionTimeout = Duration.ofMillis(input.read(Long.class));
break;

case "nodeId":
nodeId = input.read(NodeId.class);
break;
Expand Down Expand Up @@ -119,7 +127,15 @@ public static NodeStatus fromJson(JsonInput input) {
input.endObject();

return new NodeStatus(
nodeId, externalUri, maxSessions, slots, availability, heartbeatPeriod, version, osInfo);
nodeId,
externalUri,
maxSessions,
slots,
availability,
heartbeatPeriod,
sessionTimeout,
version,
osInfo);
}

public boolean hasCapability(Capabilities caps, SlotMatcher slotMatcher) {
Expand Down Expand Up @@ -162,6 +178,10 @@ public Duration getHeartbeatPeriod() {
return heartbeatPeriod;
}

public Duration getSessionTimeout() {
return sessionTimeout;
}

public String getVersion() {
return version;
}
Expand Down Expand Up @@ -212,6 +232,7 @@ private Map<String, Object> toJson() {
toReturn.put("slots", slots);
toReturn.put("availability", availability);
toReturn.put("heartbeatPeriod", heartbeatPeriod.toMillis());
toReturn.put("sessionTimeout", sessionTimeout.toMillis());
toReturn.put("version", version);
toReturn.put("osInfo", osInfo);

Expand Down
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/grid/distributor/AddNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public HttpResponse execute(HttpRequest req) {
status.getNodeId(),
status.getExternalUri(),
registrationSecret,
status.getSessionTimeout(),
status.getSlots().stream().map(Slot::getStereotype).collect(Collectors.toSet()));

distributor.add(node);
Expand Down
2 changes: 2 additions & 0 deletions java/src/org/openqa/selenium/grid/distributor/GridModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ private NodeStatus rewrite(NodeStatus status, Availability availability) {
status.getSlots(),
availability,
status.getHeartbeatPeriod(),
status.getSessionTimeout(),
status.getVersion(),
status.getOsInfo());
}
Expand Down Expand Up @@ -508,6 +509,7 @@ private void amend(Availability availability, NodeStatus status, Slot slot) {
newSlots,
availability,
status.getHeartbeatPeriod(),
status.getSessionTimeout(),
status.getVersion(),
status.getOsInfo()));
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ private void register(NodeStatus status) {
status.getNodeId(),
status.getExternalUri(),
registrationSecret,
status.getSessionTimeout(),
capabilities);

add(remoteNode);
Expand Down
10 changes: 9 additions & 1 deletion java/src/org/openqa/selenium/grid/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.net.URI;
import java.time.Duration;
import java.util.Map;
import java.util.ServiceLoader;
import java.util.Set;
Expand Down Expand Up @@ -116,13 +117,16 @@ public abstract class Node implements HasReadyState, Routable {
protected final Tracer tracer;
private final NodeId id;
private final URI uri;
private final Duration sessionTimeout;
private final Route routes;
protected boolean draining;

protected Node(Tracer tracer, NodeId id, URI uri, Secret registrationSecret) {
protected Node(
Tracer tracer, NodeId id, URI uri, Secret registrationSecret, Duration sessionTimeout) {
this.tracer = Require.nonNull("Tracer", tracer);
this.id = Require.nonNull("Node id", id);
this.uri = Require.nonNull("URI", uri);
this.sessionTimeout = Require.positive("Session timeout", sessionTimeout);
Require.nonNull("Registration secret", registrationSecret);

RequiresSecretFilter requiresSecret = new RequiresSecretFilter(registrationSecret);
Expand Down Expand Up @@ -246,6 +250,10 @@ public TemporaryFilesystem getDownloadsFilesystem(UUID uuid) throws IOException

public abstract HealthCheck getHealthCheck();

public Duration getSessionTimeout() {
return sessionTimeout;
}

public boolean isDraining() {
return draining;
}
Expand Down
5 changes: 4 additions & 1 deletion java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,13 @@ private OneShotNode(
EventBus events,
Secret registrationSecret,
Duration heartbeatPeriod,
Duration sessionTimeout,
NodeId id,
URI uri,
URI gridUri,
Capabilities stereotype,
WebDriverInfo driverInfo) {
super(tracer, id, uri, registrationSecret);
super(tracer, id, uri, registrationSecret, Require.positive(sessionTimeout));

this.heartbeatPeriod = heartbeatPeriod;
this.events = Require.nonNull("Event bus", events);
Expand Down Expand Up @@ -169,6 +170,7 @@ public static Node create(Config config) {
eventOptions.getEventBus(),
secretOptions.getRegistrationSecret(),
nodeOptions.getHeartbeatPeriod(),
nodeOptions.getSessionTimeout(),
new NodeId(UUID.randomUUID()),
serverOptions.getExternalUri(),
nodeOptions
Expand Down Expand Up @@ -376,6 +378,7 @@ public NodeStatus getStatus() {
: new Session(sessionId, getUri(), stereotype, capabilities, Instant.now()))),
isDraining() ? DRAINING : UP,
heartbeatPeriod,
getSessionTimeout(),
getNodeVersion(),
getOsInfo());
}
Expand Down
8 changes: 7 additions & 1 deletion java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,12 @@ protected LocalNode(
List<SessionSlot> factories,
Secret registrationSecret,
boolean managedDownloadsEnabled) {
super(tracer, new NodeId(UUID.randomUUID()), uri, registrationSecret);
super(
tracer,
new NodeId(UUID.randomUUID()),
uri,
registrationSecret,
Require.positive(sessionTimeout));

this.bus = Require.nonNull("Event bus", bus);

Expand Down Expand Up @@ -906,6 +911,7 @@ public NodeStatus getStatus() {
slots,
availability,
heartbeatPeriod,
getSessionTimeout(),
getNodeVersion(),
getOsInfo());
}
Expand Down
11 changes: 8 additions & 3 deletions java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.openqa.selenium.grid.data.Availability.DRAINING;
import static org.openqa.selenium.grid.data.Availability.UP;
import static org.openqa.selenium.net.Urls.fromUri;
import static org.openqa.selenium.remote.http.ClientConfig.defaultConfig;
import static org.openqa.selenium.remote.http.Contents.asJson;
import static org.openqa.selenium.remote.http.Contents.reader;
import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
Expand All @@ -35,6 +36,7 @@
import java.io.Reader;
import java.io.UncheckedIOException;
import java.net.URI;
import java.time.Duration;
import java.util.Collection;
import java.util.Map;
import java.util.Objects;
Expand All @@ -60,6 +62,7 @@
import org.openqa.selenium.json.Json;
import org.openqa.selenium.json.JsonInput;
import org.openqa.selenium.remote.SessionId;
import org.openqa.selenium.remote.http.ClientConfig;
import org.openqa.selenium.remote.http.Filter;
import org.openqa.selenium.remote.http.HttpClient;
import org.openqa.selenium.remote.http.HttpHandler;
Expand All @@ -83,13 +86,15 @@ public RemoteNode(
NodeId id,
URI externalUri,
Secret registrationSecret,
Duration sessionTimeout,
Collection<Capabilities> capabilities) {
super(tracer, id, externalUri, registrationSecret);
super(tracer, id, externalUri, registrationSecret, sessionTimeout);
this.externalUri = Require.nonNull("External URI", externalUri);
this.capabilities = ImmutableSet.copyOf(capabilities);

this.client =
Require.nonNull("HTTP client factory", clientFactory).createClient(fromUri(externalUri));
ClientConfig clientConfig =
defaultConfig().readTimeout(this.getSessionTimeout()).baseUrl(fromUri(externalUri));
VietND96 marked this conversation as resolved.
Show resolved Hide resolved
this.client = Require.nonNull("HTTP client factory", clientFactory).createClient(clientConfig);

this.healthCheck = new RemoteCheck();

Expand Down
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/redis/GridRedisClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public Optional<NodeStatus> getNode(NodeId id) {
node.getSlots(),
node.getAvailability(),
node.getHeartbeatPeriod(),
node.getSessionTimeout(),
node.getVersion(),
node.getOsInfo());
return Optional.of(resultNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ void ensureRoundTripWorks() throws URISyntaxException {
Instant.now()))),
UP,
Duration.ofSeconds(10),
Duration.ofSeconds(300),
"4.0.0",
ImmutableMap.of(
"name", "Max OS X",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ void shouldBeAbleToRegisterACustomNode() throws URISyntaxException {
bus,
new NodeId(UUID.randomUUID()),
externalUrl.toURI(),
Duration.ofSeconds(300),
c ->
new Session(
new SessionId(UUID.randomUUID()), sessionUri, stereotype, c, Instant.now()));
Expand Down Expand Up @@ -193,6 +194,7 @@ void shouldBeAbleToRegisterACustomNode() throws URISyntaxException {

NodeStatus status = getOnlyElement(distributor.getStatus().getNodes());
assertEquals(1, getStereotypes(status).get(CAPS));
assertEquals(Duration.ofSeconds(300), status.getSessionTimeout());
}
}

Expand Down Expand Up @@ -345,6 +347,7 @@ void distributorShouldUpdateStateOfExistingNodeWhenNodePublishesStateChange()
Instant.now()))),
UP,
Duration.ofSeconds(10),
status.getSessionTimeout(),
status.getVersion(),
status.getOsInfo());

Expand Down Expand Up @@ -374,8 +377,12 @@ static class CustomNode extends Node {
private Session running;

protected CustomNode(
EventBus bus, NodeId nodeId, URI uri, Function<Capabilities, Session> factory) {
super(DefaultTestTracer.createTracer(), nodeId, uri, registrationSecret);
EventBus bus,
NodeId nodeId,
URI uri,
Duration sessionTimeout,
Function<Capabilities, Session> factory) {
super(DefaultTestTracer.createTracer(), nodeId, uri, registrationSecret, sessionTimeout);

this.bus = bus;
this.factory = Objects.requireNonNull(factory);
Expand Down Expand Up @@ -468,6 +475,7 @@ public NodeStatus getStatus() {
new Slot(new SlotId(getId(), UUID.randomUUID()), CAPS, Instant.now(), sess)),
UP,
Duration.ofSeconds(10),
getSessionTimeout(),
getNodeVersion(),
getOsInfo());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ private NodeStatus createNode(List<Capabilities> stereotypes, int count, int cur
ImmutableSet.copyOf(slots),
UP,
Duration.ofSeconds(10),
Duration.ofSeconds(300),
"4.0.0",
ImmutableMap.of(
"name", "Max OS X",
Expand Down
4 changes: 4 additions & 0 deletions java/test/org/openqa/selenium/grid/node/NodeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
new NodeId(UUID.randomUUID()),
uri,
registrationSecret,
local.getSessionTimeout(),
ImmutableSet.of(caps));
}

Expand All @@ -172,6 +173,7 @@ void shouldRefuseToCreateASessionIfNoFactoriesAttached() {
new NodeId(UUID.randomUUID()),
uri,
registrationSecret,
local.getSessionTimeout(),
ImmutableSet.of());

Either<WebDriverException, CreateSessionResponse> response =
Expand Down Expand Up @@ -223,6 +225,7 @@ public boolean test(Capabilities capabilities) {
new NodeId(UUID.randomUUID()),
uri,
registrationSecret,
local.getSessionTimeout(),
ImmutableSet.of(caps));

ImmutableCapabilities wrongCaps = new ImmutableCapabilities("browserName", "burger");
Expand Down Expand Up @@ -346,6 +349,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
new NodeId(UUID.randomUUID()),
uri,
registrationSecret,
local.getSessionTimeout(),
ImmutableSet.of(caps));

Either<WebDriverException, CreateSessionResponse> response =
Expand Down