[grid][java]: session-timeout set connection timeout in RemoteNode (S…

…eleniumHQ#13854) * [grid][java]: add session-timeout to all kind of Nodes Signed-off-by: Viet Nguyen Duc <[email protected]> * [grid][java]: fix code as suggestions Signed-off-by: Viet Nguyen Duc <[email protected]> * [java][grid]: update test CustomNode can get/set sessionTimeout Signed-off-by: Viet Nguyen Duc <[email protected]> --------- Signed-off-by: Viet Nguyen Duc <[email protected]>
sandeepsuryaprasad · Oct 29, 2024 · 47db100 · 47db100
1 parent a253333
commit 47db100
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 9 deletions.
diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java
@@ -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;
 
@@ -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);
@@ -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);
   }
@@ -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;
 
@@ -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;
@@ -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) {
@@ -162,6 +178,10 @@ public Duration getHeartbeatPeriod() {
     return heartbeatPeriod;
   }
 
+  public Duration getSessionTimeout() {
+    return sessionTimeout;
+  }
+
   public String getVersion() {
     return version;
   }
@@ -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);
 

diff --git a/java/src/org/openqa/selenium/grid/distributor/AddNode.java b/java/src/org/openqa/selenium/grid/distributor/AddNode.java
@@ -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);

diff --git a/java/src/org/openqa/selenium/grid/distributor/GridModel.java b/java/src/org/openqa/selenium/grid/distributor/GridModel.java
@@ -367,6 +367,7 @@ private NodeStatus rewrite(NodeStatus status, Availability availability) {
         status.getSlots(),
         availability,
         status.getHeartbeatPeriod(),
+        status.getSessionTimeout(),
         status.getVersion(),
         status.getOsInfo());
   }
@@ -508,6 +509,7 @@ private void amend(Availability availability, NodeStatus status, Slot slot) {
               newSlots,
               availability,
               status.getHeartbeatPeriod(),
+              status.getSessionTimeout(),
               status.getVersion(),
               status.getOsInfo()));
     } finally {

diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -314,6 +314,7 @@ private void register(NodeStatus status) {
               status.getNodeId(),
               status.getExternalUri(),
               registrationSecret,
+              status.getSessionTimeout(),
               capabilities);
 
       add(remoteNode);

diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java
@@ -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;
@@ -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);
@@ -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;
   }

diff --git a/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java b/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java
@@ -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);
@@ -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
@@ -376,6 +378,7 @@ public NodeStatus getStatus() {
                     : new Session(sessionId, getUri(), stereotype, capabilities, Instant.now()))),
         isDraining() ? DRAINING : UP,
         heartbeatPeriod,
+        getSessionTimeout(),
         getNodeVersion(),
         getOsInfo());
   }

diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java
@@ -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);
 
@@ -906,6 +911,7 @@ public NodeStatus getStatus() {
         slots,
         availability,
         heartbeatPeriod,
+        getSessionTimeout(),
         getNodeVersion(),
         getOsInfo());
   }

diff --git a/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java b/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java
@@ -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;
@@ -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;
@@ -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;
@@ -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));
+    this.client = Require.nonNull("HTTP client factory", clientFactory).createClient(clientConfig);
 
     this.healthCheck = new RemoteCheck();
 

diff --git a/java/src/org/openqa/selenium/redis/GridRedisClient.java b/java/src/org/openqa/selenium/redis/GridRedisClient.java
@@ -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);

diff --git a/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java b/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java
@@ -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",

diff --git a/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java b/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java
@@ -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()));
@@ -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());
     }
   }
 
@@ -345,6 +347,7 @@ void distributorShouldUpdateStateOfExistingNodeWhenNodePublishesStateChange()
                           Instant.now()))),
               UP,
               Duration.ofSeconds(10),
+              status.getSessionTimeout(),
               status.getVersion(),
               status.getOsInfo());
 
@@ -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);
@@ -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());
     }

diff --git a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java
@@ -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",

diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java
@@ -158,6 +158,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
             new NodeId(UUID.randomUUID()),
             uri,
             registrationSecret,
+            local.getSessionTimeout(),
             ImmutableSet.of(caps));
   }
 
@@ -172,6 +173,7 @@ void shouldRefuseToCreateASessionIfNoFactoriesAttached() {
             new NodeId(UUID.randomUUID()),
             uri,
             registrationSecret,
+            local.getSessionTimeout(),
             ImmutableSet.of());
 
     Either<WebDriverException, CreateSessionResponse> response =
@@ -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");
@@ -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 =