Allow for customisation of TestSlot (SeleniumHQ#3431)

The current implementation does not let downstream
consumers of the Grid provide a customised way of
injecting a TestSlot.
This is very useful when one needs to build an On-Demand Grid,
wherein there are no real proxies, but a ghost proxy stands
for the real proxy and based on new session requests,
nodes are spun off.

* Fixed this by defining a method for instantiating a
TestSlot so that downstream RemoteProxy implementations
can choose to provide their own customised TestSlots.

* enhanced the SeleniumProtocol enum to house all
logic related to forming the protocol and the path
from a desired capability into it.

* exposed the matches() method from TestSlot.
Downstream consumers of TestSlot would need access
to alter the logic of matches() method because
sometimes TestSlots would blindly need to return
true for all calls to matches() invocation [ This
is true when one is building a ghost proxy which doesn’t
represent any real node, but stands as a proxy for
something such as a docker daemon from where the
actual sessions would be honoured ]

Without this, end-users would be forced to have their
customized TestSlots reside in the same package
as that of TestSlot (org.openqa.grid.internal)

java/server/src/org/openqa/grid/common/SeleniumProtocol.java

@@ -17,8 +17,47 @@
 
 package org.openqa.grid.common;
 
+import static org.openqa.grid.common.RegistrationRequest.PATH;
+import static org.openqa.grid.common.RegistrationRequest.SELENIUM_PROTOCOL;
+
+import org.openqa.grid.common.exception.GridException;
+
+import java.util.Arrays;
+import java.util.Map;
+
 public enum SeleniumProtocol {
-  Selenium, WebDriver;
+  Selenium("/selenium-server/driver"),
+  WebDriver("/wd/hub");
+  private String path;
+
+  SeleniumProtocol(String path) {
+    this.path = path;
+  }
+
+  public static SeleniumProtocol fromCapabilitiesMap(Map<String, ?> capabilities) {
+    String type = (String) capabilities.get(SELENIUM_PROTOCOL);
+    if (type == null || type.trim().isEmpty()) {
+      return WebDriver;
+    }
+    try {
+      return SeleniumProtocol.valueOf(type);
+    } catch (IllegalArgumentException e) {
+      throw new GridException(type + " isn't a valid protocol type for grid. Valid values :[" +
+                              Arrays.toString(values()) + "]", e);
+    }
+  }
+
+  public String getPathConsideringCapabilitiesMap(Map<String, ?> capabilities) {
+    String localPath = (String) capabilities.get(PATH);
+    if (localPath != null) {
+      return localPath;
+    }
+    return path;
+  }
+
+  public String getPath() {
+    return path;
+  }
 
   public boolean isSelenium() {
     return Selenium.equals(this);

java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java

@@ -18,8 +18,6 @@
 package org.openqa.grid.internal;
 
 import static org.openqa.grid.common.RegistrationRequest.MAX_INSTANCES;
-import static org.openqa.grid.common.RegistrationRequest.PATH;
-import static org.openqa.grid.common.RegistrationRequest.SELENIUM_PROTOCOL;
 
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
@@ -144,8 +142,7 @@ public BaseRemoteProxy(RegistrationRequest request, Registry registry) {
     for (DesiredCapabilities capability : capabilities) {
       Object maxInstance = capability.getCapability(MAX_INSTANCES);
 
-      SeleniumProtocol protocol = getProtocol(capability);
-      String path = getPath(capability);
+      SeleniumProtocol protocol = SeleniumProtocol.fromCapabilitiesMap(capability.asMap());
 
       if (maxInstance == null) {
         log.warning("Max instance not specified. Using default = 1 instance");
@@ -158,45 +155,13 @@ public BaseRemoteProxy(RegistrationRequest request, Registry registry) {
         for (String k : capability.asMap().keySet()) {
           c.put(k, capability.getCapability(k));
         }
-        slots.add(new TestSlot(this, protocol, path, c));
+        slots.add(newTestSlot( protocol, c));
       }
     }
 
     this.testSlots = Collections.unmodifiableList(slots);
   }
 
-  private SeleniumProtocol getProtocol(DesiredCapabilities capability) {
-    String type = (String) capability.getCapability(SELENIUM_PROTOCOL);
-
-    SeleniumProtocol protocol;
-    if (type == null) {
-      protocol = SeleniumProtocol.WebDriver;
-    } else {
-      try {
-        protocol = SeleniumProtocol.valueOf(type);
-      } catch (IllegalArgumentException e) {
-        throw new GridException(
-            type + " isn't a valid protocol type for grid. See SeleniumProtocol enum.", e);
-      }
-    }
-    return protocol;
-  }
-
-  private String getPath(DesiredCapabilities capability) {
-    String type = (String) capability.getCapability(PATH);
-    if (type == null) {
-      switch (getProtocol(capability)) {
-        case Selenium:
-          return "/selenium-server/driver";
-        case WebDriver:
-          return "/wd/hub";
-        default:
-          throw new GridException("Protocol not supported.");
-      }
-    }
-    return type;
-  }
-
   public void setupTimeoutListener() {
     cleanUpThread = null;
     if (this instanceof TimeoutListener) {
@@ -270,7 +235,7 @@ public void run() {
     }
 
     void cleanUpAllSlots() {
-      for (TestSlot slot : testSlots) {
+      for (TestSlot slot : getTestSlots()) {
         try {
           cleanUpSlot(slot);
         } catch (Throwable t) {
@@ -342,7 +307,7 @@ public TestSession getNewSession(Map<String, Object> requestedCapability) {
       return null;
     }
     // any slot left for the given app ?
-    for (TestSlot testslot : testSlots) {
+    for (TestSlot testslot : getTestSlots()) {
       TestSession session = testslot.getNewSession(requestedCapability);
 
       if (session != null) {
@@ -355,7 +320,7 @@ public TestSession getNewSession(Map<String, Object> requestedCapability) {
   public int getTotalUsed() {
     int totalUsed = 0;
 
-    for (TestSlot slot : testSlots) {
+    for (TestSlot slot : getTestSlots()) {
       if (slot.getSession() != null) {
         totalUsed++;
       }
@@ -365,7 +330,7 @@ public int getTotalUsed() {
   }
 
   public boolean hasCapability(Map<String, Object> requestedCapability) {
-    for (TestSlot slot : testSlots) {
+    for (TestSlot slot : getTestSlots()) {
       if (slot.matches(requestedCapability)) {
         return true;
       }
@@ -544,7 +509,7 @@ public float getResourceUsageInPercent() {
 
   public long getLastSessionStart() {
     long last = -1;
-    for (TestSlot slot : testSlots) {
+    for (TestSlot slot : getTestSlots()) {
       last = Math.max(last, slot.getLastSessionStart());
     }
     return last;

java/server/src/org/openqa/grid/internal/RemoteProxy.java

@@ -20,7 +20,7 @@
 import com.google.gson.JsonObject;
 
 import org.openqa.grid.common.RegistrationRequest;
-import org.openqa.grid.common.exception.GridException;
+import org.openqa.grid.common.SeleniumProtocol;
 import org.openqa.grid.internal.utils.CapabilityMatcher;
 import org.openqa.grid.internal.utils.HtmlRenderer;
 import org.openqa.grid.internal.utils.configuration.GridNodeConfiguration;
@@ -39,6 +39,16 @@
  * only support Firefox.
  */
 public interface RemoteProxy extends Comparable<RemoteProxy> {
+
+  /**
+   *
+   * @param protocol - A {@link SeleniumProtocol} object that identifies the request flavor.
+   * @param capabilities - the type of test the client is interested in performing.
+   * @return - The entity on a proxy that can host a test session.
+   */
+  default TestSlot newTestSlot(SeleniumProtocol protocol, Map<String, Object> capabilities) {
+    return new TestSlot(this, protocol,capabilities);
+  }
   /**
    * Each test running on the node will occupy a test slot.  A test slot can either be in use (have a session) or be
    * available for scheduling (no associated session).  This method allows retrieving the total state of the node,
@@ -158,9 +168,8 @@ public interface RemoteProxy extends Comparable<RemoteProxy> {
    *
    * @return the node status.
    *
-   * @throws GridException if the node is down.
    */
-  JsonObject getStatus() throws GridException;
+  JsonObject getStatus() ;
 
   /**
    * Checks if the node has the capability requested.

java/server/src/org/openqa/grid/internal/TestSlot.java

@@ -79,6 +79,11 @@ public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, String path,
     this.capabilities = capabilities;
   }
 
+  public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, Map<String, Object> capabilities) {
+    this(proxy, protocol, protocol.getPathConsideringCapabilitiesMap(capabilities), capabilities);
+  }
+
+
   public Map<String, Object> getCapabilities() {
     return Collections.unmodifiableMap(capabilities);
   }
@@ -147,7 +152,7 @@ public String getPath() {
    * @return true if the desired capabilities matches for the
    *         {@link RemoteProxy#getCapabilityHelper()}
    */
-  boolean matches(Map<String, Object> desiredCapabilities) {
+  public boolean matches(Map<String, Object> desiredCapabilities) {
     return matcher.matches(capabilities, desiredCapabilities);
   }
 

java/server/test/org/openqa/grid/common/CommonTests.java

@@ -21,5 +21,5 @@
 import org.junit.runners.Suite;
 
 @RunWith(Suite.class)
-@Suite.SuiteClasses(RegistrationRequestTest.class)
+@Suite.SuiteClasses( {RegistrationRequestTest.class,SeleniumProtocolTest.class})
 public class CommonTests {}

java/server/test/org/openqa/grid/common/SeleniumProtocolTest.java

@@ -0,0 +1,30 @@
+package org.openqa.grid.common;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import java.util.Map;
+
+public class SeleniumProtocolTest {
+
+  @Test
+  public void getPathTest() {
+
+    //Ensuring that when path is specified via capabilities, that is what we get back in return.
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability(RegistrationRequest.SELENIUM_PROTOCOL, SeleniumProtocol.WebDriver.toString());
+    caps.setCapability(RegistrationRequest.PATH, "foo/bar");
+    SeleniumProtocol protocol = SeleniumProtocol.fromCapabilitiesMap(caps.asMap());
+    assertEquals(SeleniumProtocol.WebDriver, protocol);
+    assertEquals("foo/bar", protocol.getPathConsideringCapabilitiesMap((Map<String, Object>) caps.asMap()));
+
+    //Ensuring that by default we parse the protocol as WebDriver and we get back its default path.
+    caps = new DesiredCapabilities();
+    protocol = SeleniumProtocol.fromCapabilitiesMap(caps.asMap());
+    assertEquals(SeleniumProtocol.WebDriver, protocol);
+    assertEquals("/wd/hub", protocol.getPathConsideringCapabilitiesMap((Map<String, Object>) caps.asMap()));
+  }
+
+}

java/server/test/org/openqa/grid/internal/BaseRemoteProxyTest.java

@@ -30,7 +30,9 @@
 import org.junit.Test;
 import org.openqa.grid.common.RegistrationRequest;
 import org.openqa.grid.common.exception.GridException;
+import org.openqa.grid.internal.mock.GridHelper;
 import org.openqa.grid.internal.utils.configuration.GridNodeConfiguration;
+import org.openqa.grid.web.servlet.handler.RequestHandler;
 import org.openqa.selenium.remote.CapabilityType;
 import org.openqa.selenium.remote.DesiredCapabilities;
 
@@ -158,6 +160,28 @@ public void timeouts() {
     assertEquals(23000, p.getTimeOut());
   }
 
+  @Test
+  public void proxyWithCustomTestSlot() {
+    GridNodeConfiguration nodeConfiguration = new GridNodeConfiguration();
+    RegistrationRequest req = RegistrationRequest.build(nodeConfiguration);
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability(CapabilityType.APPLICATION_NAME, "app1");
+    req.getConfiguration().capabilities.add(caps);
+    req.getConfiguration().proxy = MyCustomProxy.class.getName();
+    RemoteProxy p = BaseRemoteProxy.getNewInstance(req,registry);
+    Map<String, Object> app1 = new HashMap<>();
+    app1.put(CapabilityType.APPLICATION_NAME, "app1");
+    app1.put("slotName", "CrazySlot");
+
+    registry.add(p);
+    RequestHandler newSessionRequest = GridHelper.createNewSessionHandler(registry, app1);
+
+    newSessionRequest.process();
+    TestSession session = newSessionRequest.getSession();
+    TestSlot slot = session.getSlot();
+    assertTrue(slot instanceof MyTestSlot);
+    assertTrue(slot.toString().contains("CrazySlot"));
+  }
 
   @After
   public void teardown() {

java/server/test/org/openqa/grid/internal/MyCustomProxy.java

@@ -18,9 +18,11 @@
 package org.openqa.grid.internal;
 
 import org.openqa.grid.common.RegistrationRequest;
+import org.openqa.grid.common.SeleniumProtocol;
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.Map;
 
 public class MyCustomProxy extends BaseRemoteProxy {
 
@@ -49,4 +51,18 @@ public String getString() {
     return MY_STRING;
   }
 
+  @Override
+  public TestSlot newTestSlot(SeleniumProtocol protocol, Map<String, Object> capabilities) {
+    return new MyTestSlot(this,protocol, capabilities);
+  }
+
+  @Override
+  public TestSession getNewSession(Map<String, Object> requestedCapability) {
+    TestSession session =  super.getNewSession(requestedCapability);
+    TestSlot slot = session.getSlot();
+    if (requestedCapability.containsKey("slotName") && slot instanceof MyTestSlot) {
+      ((MyTestSlot)slot).setSlotName(requestedCapability.get("slotName").toString());
+    }
+    return session;
+  }
 }

java/server/test/org/openqa/grid/internal/MyTestSlot.java

@@ -0,0 +1,23 @@
+package org.openqa.grid.internal;
+
+import org.openqa.grid.common.SeleniumProtocol;
+
+import java.util.Map;
+
+public class MyTestSlot extends TestSlot {
+  private String slotName;
+
+  public MyTestSlot(RemoteProxy proxy, SeleniumProtocol protocol,
+                    Map<String, Object> capabilities) {
+    super(proxy, protocol, capabilities);
+  }
+
+  public void setSlotName(String slotName) {
+    this.slotName = slotName;
+  }
+
+  @Override
+  public String toString() {
+    return slotName + super.toString();
+  }
+}