[grid] Improving fallback locators code

Author: diemol

java/server/src/org/openqa/selenium/grid/node/CustomLocatorHandler.java

@@ -157,7 +157,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
         .orElseThrow(() -> new IllegalArgumentException("Cannot locate session ID from " + req.getUri())));
 
     SearchContext context = null;
-    RemoteWebElement element = null;
+    RemoteWebElement element;
     boolean findMultiple = false;
     UrlTemplate.Match match = FIND_ELEMENT.match(req.getUri());
     if (match != null) {
@@ -194,7 +194,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
       throw new IllegalStateException("Unable to determine locator context: " + req);
     }
 
-    Object toReturn = null;
+    Object toReturn;
     By by = customLocator.apply(value);
     if (findMultiple) {
       toReturn = context.findElements(by);

java/server/src/org/openqa/selenium/grid/node/locators/FallbackLocators.java renamed to java/server/src/org/openqa/selenium/grid/node/locators/ById.java

@@ -22,42 +22,29 @@
 import com.google.auto.service.AutoService;
 
 import org.openqa.selenium.By;
+import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.locators.CustomLocator;
 
 import java.util.stream.Stream;
 
-public class FallbackLocators {
-
-  @AutoService(CustomLocator.class)
-  public static class ById extends CustomLocator {
-    @Override
-    public String getLocatorName() {
-      return "id";
-    }
-
-    @Override
-    public By createBy(Object usingParameter) {
-      String id = Stream.of(
-        String.valueOf(usingParameter)
-          .split("\\s+")).map(str -> "#" + str).collect(joining(" "));
-      return By.cssSelector(id);
-    }
-  }
+/**
+ * A class implementing {link @CustomLocator} and to be used as a fallback locator on the server
+ * side.
+ */
 
-  @AutoService(CustomLocator.class)
-  public static class ByName extends CustomLocator {
-    @Override
-    public String getLocatorName() {
-      return "name";
-    }
-
-    @Override
-    public By createBy(Object usingParameter) {
-      String name = String.valueOf(usingParameter).replace("'", "\\'");
-      return By.cssSelector(String.format("*[name='%s']", name));
-    }
+@AutoService(CustomLocator.class)
+public class ById extends CustomLocator {
+  @Override
+  public String getLocatorName() {
+    return "id";
   }
 
-
-  // asJson.put("value", String.format("*[name='%s']", name.replace("'", "\\'")));
+  @Override
+  public By createBy(Object usingParameter) {
+    Require.argument("Locator value", usingParameter).instanceOf(String.class);
+    String id = Stream.of(
+      String.valueOf(usingParameter)
+        .split("\\s+")).map(str -> "#" + str).collect(joining(" "));
+    return By.cssSelector(id);
+  }
 }

java/server/src/org/openqa/selenium/grid/node/locators/ByName.java

@@ -0,0 +1,44 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.node.locators;
+
+import com.google.auto.service.AutoService;
+
+import org.openqa.selenium.By;
+import org.openqa.selenium.internal.Require;
+import org.openqa.selenium.remote.locators.CustomLocator;
+
+/**
+ * A class implementing {link @CustomLocator} and to be used as a fallback locator on the server
+ * side.
+ */
+
+@AutoService(CustomLocator.class)
+public class ByName extends CustomLocator {
+  @Override
+  public String getLocatorName() {
+    return "name";
+  }
+
+  @Override
+  public By createBy(Object usingParameter) {
+    Require.argument("Locator value", usingParameter).instanceOf(String.class);
+    String name = String.valueOf(usingParameter).replace("'", "\\'");
+    return By.cssSelector(String.format("*[name='%s']", name));
+  }
+}

java/server/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java

@@ -32,7 +32,7 @@
 import org.openqa.selenium.events.local.GuavaEventBus;
 import org.openqa.selenium.grid.data.Session;
 import org.openqa.selenium.grid.node.local.LocalNode;
-import org.openqa.selenium.grid.node.locators.FallbackLocators;
+import org.openqa.selenium.grid.node.locators.ById;
 import org.openqa.selenium.grid.security.Secret;
 import org.openqa.selenium.grid.testing.TestSessionFactory;
 import org.openqa.selenium.grid.web.ErrorFilter;
@@ -281,7 +281,7 @@ public void shouldFallbackToUseById() {
     HttpHandler handler = new CustomLocatorHandler(
       node,
       registrationSecret,
-      singleton(new FallbackLocators.ById()));
+      singleton(new ById()));
 
     HttpResponse res = handler.execute(
       new HttpRequest(POST, "/session/1234/elements")