Minor refactor of #3431 and add javadocs.

Also removed unnecessary before logic in BaseRemoteProxyTest

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

@@ -34,6 +34,14 @@ public enum SeleniumProtocol {
     this.path = path;
   }
 
+  /**
+   * Get the protocol considering the capabilities provided which may define the property
+   * {@link RegistrationRequest#SELENIUM_PROTOCOL}
+   *
+   * @param capabilities map of capabilities to consider
+   * @return the {@link SeleniumProtocol} or throws a {@link GridException} if the capabilities
+   * does not define a recognized protocol.
+   */
   public static SeleniumProtocol fromCapabilitiesMap(Map<String, ?> capabilities) {
     String type = (String) capabilities.get(SELENIUM_PROTOCOL);
     if (type == null || type.trim().isEmpty()) {
@@ -47,6 +55,13 @@ public static SeleniumProtocol fromCapabilitiesMap(Map<String, ?> capabilities)
     }
   }
 
+  /**
+   * Get the protocol path considering the capabilities provided which may define a new
+   * path via the property {@link RegistrationRequest#PATH}
+   *
+   * @param capabilities map of capabilities to consider
+   * @return the protocol path defined by the capabilities or the value of {@link #getPath}.
+   */
   public String getPathConsideringCapabilitiesMap(Map<String, ?> capabilities) {
     String localPath = (String) capabilities.get(PATH);
     if (localPath != null) {
@@ -55,10 +70,19 @@ public String getPathConsideringCapabilitiesMap(Map<String, ?> capabilities) {
     return path;
   }
 
+  /**
+   * Get the protocol path
+   *
+   * @return the protocol path
+   */
   public String getPath() {
     return path;
   }
 
+  /**
+   * @deprecated use {@link SeleniumProtocol#Selenium#equals(Object)}
+   */
+  @Deprecated
   public boolean isSelenium() {
     return Selenium.equals(this);
   }

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

@@ -155,7 +155,7 @@ public BaseRemoteProxy(RegistrationRequest request, Registry registry) {
         for (String k : capability.asMap().keySet()) {
           c.put(k, capability.getCapability(k));
         }
-        slots.add(newTestSlot( protocol, c));
+        slots.add(createTestSlot(protocol, c));
       }
     }
 

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

@@ -41,14 +41,16 @@
 public interface RemoteProxy extends Comparable<RemoteProxy> {
 
   /**
+   * Create a new TestSlot.
    *
-   * @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.
+   * @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);
+  default TestSlot createTestSlot(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,

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

@@ -32,6 +32,7 @@
 import org.apache.http.message.BasicHttpEntityEnclosingRequest;
 import org.apache.http.message.BasicHttpRequest;
 import org.apache.http.util.EntityUtils;
+import org.openqa.grid.common.SeleniumProtocol;
 import org.openqa.grid.common.exception.ClientGoneException;
 import org.openqa.grid.common.exception.GridException;
 import org.openqa.grid.internal.listeners.CommandListener;
@@ -149,7 +150,7 @@ public boolean isOrphaned() {
 
     // The session needs to have been open for at least the time interval and we need to have not
     // seen any new commands during that time frame.
-    return slot.getProtocol().isSelenium()
+    return slot.getProtocol().equals(SeleniumProtocol.Selenium)
            && elapsedSinceCreation > MAX_IDLE_TIME_BEFORE_CONSIDERED_ORPHANED
            && sessionCreatedAt == lastActivity;
   }

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

@@ -63,7 +63,13 @@ public class TestSlot {
   private boolean showWarning = false;
   private long lastSessionStart = -1;
 
-
+  /**
+   * Create a new test slot specifying a custom protocol path
+   * @param proxy the {@link RemoteProxy} which includes this test slot
+   * @param protocol the {@link SeleniumProtocol} this test slot conforms to
+   * @param path the protocol path this test slot uses
+   * @param capabilities capabilities of this test slot
+   */
   public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, String path,
                   Map<String, Object> capabilities) {
     this.proxy = proxy;
@@ -79,17 +85,25 @@ public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, String path,
     this.capabilities = capabilities;
   }
 
+  /**
+   * Create a new test slot
+   * @param proxy the {@link RemoteProxy} which includes this test slot
+   * @param protocol the {@link SeleniumProtocol} this test slot conforms to
+   * @param capabilities capabilities of this test slot
+   */
   public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, Map<String, Object> capabilities) {
     this(proxy, protocol, protocol.getPathConsideringCapabilitiesMap(capabilities), capabilities);
   }
 
-
+  /**
+   * @return the capabilities of this test slot
+   */
   public Map<String, Object> getCapabilities() {
     return Collections.unmodifiableMap(capabilities);
   }
 
   /**
-   * @return the RemoteProxy that hosts this slot.
+   * @return the {@link RemoteProxy} that hosts this slot.
    */
   public RemoteProxy getProxy() {
     return proxy;
@@ -100,8 +114,8 @@ public RemoteProxy getProxy() {
    * test slot can host the desired capabilities, {@link CapabilityMatcher#matches(Map, Map)} is
    * invoked.
    * <p>
-   * Use {@link GridHubConfiguration#setCapabilityMatcher(CapabilityMatcher)}
-   * on the proxy hosting the test slot to modify the definition of match
+   * Use {@link GridHubConfiguration#capabilityMatcher} on the proxy hosting the test slot to
+   * modify the definition of match
    *
    * @param desiredCapabilities capabilities for the new session
    * @return a new session linked to that testSlot if possible, null otherwise.
@@ -125,8 +139,6 @@ public TestSession getNewSession(Map<String, Object> desiredCapabilities) {
     }
   }
 
-
-
   /**
    * the type of protocol for the TestSlot.Ideally should always be webdriver, but can also be
    * selenium1 protocol for backward compatibility purposes.
@@ -205,15 +217,24 @@ void finishReleaseProcess() {
     }
   }
 
+  /**
+   * Finish releasing all resources so the slot can be reused.
+   */
   public void doFinishRelease() {
     currentSession = null;
     beingReleased = false;
   }
 
+  /**
+   * @return the test session internal key
+   */
   String getInternalKey() {
     return currentSession == null ? null : currentSession.getInternalKey();
   }
 
+  /**
+   * @return invokes after session {@link TestSessionListener} events on this test slot
+   */
   boolean performAfterSessionEvent() {
     // run the pre-release listener
     try {

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

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

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

@@ -5,26 +5,22 @@
 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()));
+    assertEquals("foo/bar", protocol.getPathConsideringCapabilitiesMap(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()));
+    assertEquals("/wd/hub", protocol.getPathConsideringCapabilitiesMap(caps.asMap()));
   }
-
 }

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

@@ -42,32 +42,30 @@
 import java.util.Map;
 
 public class BaseRemoteProxyTest {
-
-  private RemoteProxy p1 = null;
-  private RemoteProxy p2 = null;
-
-  private Map<String, Object> app1Capability = new HashMap<>();
-  private Map<String, Object> app2Capability = new HashMap<>();
-  private Registry registry = Registry.newInstance();
+  private Registry registry;
 
   @Before
-  public void setup() throws Exception {
+  public void before() {
+    registry = Registry.newInstance();
+  }
 
+  @Test
+  public void testEqual() throws Exception {
+    final Map<String, Object> app1Capability = new HashMap<>();
+    final Map<String, Object> app2Capability = new HashMap<>();
     app1Capability.put(CapabilityType.APPLICATION_NAME, "app1");
     app2Capability.put(CapabilityType.APPLICATION_NAME, "app2");
 
-    p1 =
-        RemoteProxyFactory
-            .getNewBasicRemoteProxy(app1Capability, "http://machine1:4444/", registry);
+    final RemoteProxy p1 =
+        RemoteProxyFactory.getNewBasicRemoteProxy(app1Capability, "http://machine1:4444/", registry);
+
     List<Map<String, Object>> caps = new ArrayList<>();
     caps.add(app1Capability);
     caps.add(app2Capability);
-    p2 = RemoteProxyFactory.getNewBasicRemoteProxy(caps, "http://machine4:4444/", registry);
+    final RemoteProxy p2 =
+        RemoteProxyFactory.getNewBasicRemoteProxy(caps, "http://machine4:4444/", registry);
 
-  }
 
-  @Test
-  public void testEqual() {
     assertTrue(p1.equals(p1));
     assertFalse(p1.equals(p2));
   }
@@ -166,6 +164,7 @@ public void proxyWithCustomTestSlot() {
     RegistrationRequest req = RegistrationRequest.build(nodeConfiguration);
     DesiredCapabilities caps = new DesiredCapabilities();
     caps.setCapability(CapabilityType.APPLICATION_NAME, "app1");
+    caps.setCapability(RegistrationRequest.PATH, "/foo/bar/baz");
     req.getConfiguration().capabilities.add(caps);
     req.getConfiguration().proxy = MyCustomProxy.class.getName();
     RemoteProxy p = BaseRemoteProxy.getNewInstance(req,registry);
@@ -181,6 +180,7 @@ public void proxyWithCustomTestSlot() {
     TestSlot slot = session.getSlot();
     assertTrue(slot instanceof MyTestSlot);
     assertTrue(slot.toString().contains("CrazySlot"));
+    assertEquals("/foo/bar/baz", slot.getPath());
   }
 
   @After

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

@@ -23,7 +23,6 @@
 import org.openqa.grid.common.exception.GridException;
 
 public class DetachedRemoteProxy extends BaseRemoteProxy {
-
   public DetachedRemoteProxy(RegistrationRequest request, Registry registry) {
     super(request, registry);
   }
@@ -32,5 +31,4 @@ public DetachedRemoteProxy(RegistrationRequest request, Registry registry) {
   public JsonObject getStatus() throws GridException {
     return null; // just to make sure there is no GridException thrown
   }
-
 }

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

@@ -52,7 +52,7 @@ public String getString() {
   }
 
   @Override
-  public TestSlot newTestSlot(SeleniumProtocol protocol, Map<String, Object> capabilities) {
+  public TestSlot createTestSlot(SeleniumProtocol protocol, Map<String, Object> capabilities) {
     return new MyTestSlot(this,protocol, capabilities);
   }