Skip to content

Commit 0aaa401

Browse files
committed
Rework how By works
With this change, each WebDriver implementation needs to take responsibility for handling locators. In the case of the classes that extend `RemoteWebDriver` (which is most of them), we first check to see if a locator directly supports remote location. If it does, we try to find the element that way first. If that fails in a way that indicates that the remote end didn't understand the search mechanism, we fall back to passing the search context to the locator. In order to be as efficient as possible, drivers store a map of which search strategy should work.
1 parent e43cce3 commit 0aaa401

File tree

9 files changed

+590
-362
lines changed

9 files changed

+590
-362
lines changed

java/client/src/org/openqa/selenium/By.java

Lines changed: 139 additions & 213 deletions
Large diffs are not rendered by default.

java/client/src/org/openqa/selenium/remote/DriverCommand.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ static CommandPayload DELETE_COOKIE(String name) {
7373
String DELETE_ALL_COOKIES = "deleteAllCookies";
7474

7575
String FIND_ELEMENT = "findElement";
76-
static CommandPayload FIND_ELEMENT(String strategy, String value) {
76+
static CommandPayload FIND_ELEMENT(String strategy, Object value) {
7777
return new CommandPayload(FIND_ELEMENT, ImmutableMap.of("using", strategy, "value", value));
7878
}
7979
String FIND_ELEMENTS = "findElements";
80-
static CommandPayload FIND_ELEMENTS(String strategy, String value) {
80+
static CommandPayload FIND_ELEMENTS(String strategy, Object value) {
8181
return new CommandPayload(FIND_ELEMENTS, ImmutableMap.of("using", strategy, "value", value));
8282
}
8383
String FIND_CHILD_ELEMENT = "findChildElement";

java/client/src/org/openqa/selenium/remote/RemoteWebDriver.java

Lines changed: 193 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,17 @@
2727
import org.openqa.selenium.Dimension;
2828
import org.openqa.selenium.HasCapabilities;
2929
import org.openqa.selenium.ImmutableCapabilities;
30+
import org.openqa.selenium.InvalidArgumentException;
3031
import org.openqa.selenium.JavascriptExecutor;
3132
import org.openqa.selenium.MutableCapabilities;
3233
import org.openqa.selenium.NoSuchElementException;
3334
import org.openqa.selenium.NoSuchFrameException;
3435
import org.openqa.selenium.NoSuchWindowException;
3536
import org.openqa.selenium.OutputType;
37+
import org.openqa.selenium.Pdf;
3638
import org.openqa.selenium.Platform;
3739
import org.openqa.selenium.Point;
40+
import org.openqa.selenium.PrintsPage;
3841
import org.openqa.selenium.SearchContext;
3942
import org.openqa.selenium.SessionNotCreatedException;
4043
import org.openqa.selenium.TakesScreenshot;
@@ -55,8 +58,6 @@
5558
import org.openqa.selenium.logging.Logs;
5659
import org.openqa.selenium.logging.NeedsLocalLogs;
5760
import org.openqa.selenium.print.PrintOptions;
58-
import org.openqa.selenium.Pdf;
59-
import org.openqa.selenium.PrintsPage;
6061
import org.openqa.selenium.remote.internal.WebElementToJsonConverter;
6162
import org.openqa.selenium.virtualauthenticator.Credential;
6263
import org.openqa.selenium.virtualauthenticator.HasVirtualAuthenticator;
@@ -69,6 +70,7 @@
6970
import java.util.Collection;
7071
import java.util.Collections;
7172
import java.util.Date;
73+
import java.util.HashMap;
7274
import java.util.HashSet;
7375
import java.util.LinkedHashSet;
7476
import java.util.List;
@@ -92,10 +94,22 @@ public class RemoteWebDriver implements WebDriver, JavascriptExecutor, HasInputD
9294
HasCapabilities, Interactive, TakesScreenshot,
9395
HasVirtualAuthenticator, PrintsPage {
9496

95-
// TODO(dawagner): This static logger should be unified with the per-instance localLogs
97+
// TODO: This static logger should be unified with the per-instance localLogs
9698
private static final Logger logger = Logger.getLogger(RemoteWebDriver.class.getName());
9799
private Level level = Level.FINE;
98100

101+
private Map<Class<? extends By>, Mechanism> remotableBys = new HashMap<>();
102+
// We would do this in either `init` or a constructor, but there are
103+
// multiple constructors and we occasionally add a new one, and `init`
104+
// may be overridden by the user
105+
{
106+
remotableBys.put(By.cssSelector("").getClass(), Mechanism.REMOTE);
107+
remotableBys.put(By.linkText("").getClass(), Mechanism.REMOTE);
108+
remotableBys.put(By.partialLinkText("").getClass(), Mechanism.REMOTE);
109+
remotableBys.put(By.tagName("").getClass(), Mechanism.REMOTE);
110+
remotableBys.put(By.xpath("").getClass(), Mechanism.REMOTE);
111+
}
112+
99113
private ErrorHandler errorHandler = new ErrorHandler();
100114
private CommandExecutor executor;
101115
private Capabilities capabilities;
@@ -112,7 +126,7 @@ public class RemoteWebDriver implements WebDriver, JavascriptExecutor, HasInputD
112126

113127
// For cglib
114128
protected RemoteWebDriver() {
115-
this.capabilities=init(new ImmutableCapabilities());
129+
this.capabilities = init(new ImmutableCapabilities());
116130
}
117131

118132
public RemoteWebDriver(Capabilities capabilities) {
@@ -129,7 +143,7 @@ public RemoteWebDriver(CommandExecutor executor, Capabilities capabilities) {
129143
}
130144
this.executor = executor;
131145

132-
capabilities=init(capabilities);
146+
capabilities = init(capabilities);
133147

134148
if (executor instanceof NeedsLocalLogs) {
135149
((NeedsLocalLogs) executor).setLocalLogs(localLogs);
@@ -342,69 +356,121 @@ public Pdf print(PrintOptions printOptions) throws WebDriverException {
342356

343357
@Override
344358
public WebElement findElement(By locator) {
345-
if (locator instanceof By.StandardLocator) {
346-
return ((By.StandardLocator) locator).findElement(this, this::findElement);
347-
} else {
348-
return locator.findElement(this);
359+
Require.nonNull("Locator", locator);
360+
361+
return findElement(this, this, locator);
362+
}
363+
364+
WebElement findElement(RemoteWebDriver parent, SearchContext context, By locator) {
365+
Mechanism mechanism = remotableBys.get(locator.getClass());
366+
if (mechanism != null) {
367+
WebElement element = mechanism.findElement(parent, context, locator);
368+
return massage(parent, context, element, locator);
369+
}
370+
371+
// Attempt to find the element remotely
372+
if (locator instanceof By.Remotable) {
373+
try {
374+
WebElement element = Mechanism.REMOTE.findElement(parent, context, locator);
375+
remotableBys.put(locator.getClass(), Mechanism.REMOTE);
376+
return massage(parent, context, element, locator);
377+
} catch (NoSuchElementException e) {
378+
remotableBys.put(locator.getClass(), Mechanism.REMOTE);
379+
throw e;
380+
} catch (InvalidArgumentException e) {
381+
// Fall through
382+
}
383+
}
384+
385+
try {
386+
WebElement element = Mechanism.CONTEXT.findElement(parent, context, locator);
387+
remotableBys.put(locator.getClass(), Mechanism.CONTEXT);
388+
return massage(parent, context, element, locator);
389+
} catch (NoSuchElementException e) {
390+
remotableBys.put(locator.getClass(), Mechanism.CONTEXT);
391+
throw e;
349392
}
350393
}
351394

352395
@Override
353396
public List<WebElement> findElements(By locator) {
354-
if (locator instanceof By.StandardLocator) {
355-
return ((By.StandardLocator) locator).findElements(this, this::findElements);
356-
} else {
357-
return locator.findElements(this);
358-
}
397+
Require.nonNull("Locator", locator);
398+
399+
return findElements(this, this, locator);
359400
}
360401

361-
protected WebElement findElement(String by, String using) {
362-
if (using == null) {
363-
throw new IllegalArgumentException("Cannot find elements when the selector is null.");
402+
public List<WebElement> findElements(RemoteWebDriver parent, SearchContext context, By locator) {
403+
Mechanism mechanism = remotableBys.get(locator.getClass());
404+
if (mechanism != null) {
405+
List<WebElement> elements = mechanism.findElements(parent, context, locator);
406+
elements.forEach(e -> massage(parent, context, e, locator));
407+
return elements;
364408
}
365409

366-
Response response = execute(DriverCommand.FIND_ELEMENT(by, using));
367-
Object value = response.getValue();
368-
if (value == null) { // see https://github.com/SeleniumHQ/selenium/issues/5809
369-
throw new NoSuchElementException(String.format("Cannot locate an element using %s=%s", by, using));
410+
// Attempt to find the element remotely
411+
if (locator instanceof By.Remotable) {
412+
try {
413+
List<WebElement> elements = Mechanism.REMOTE.findElements(parent, context, locator);
414+
remotableBys.put(locator.getClass(), Mechanism.REMOTE);
415+
elements.forEach(e -> massage(parent, context, e, locator));
416+
return elements;
417+
} catch (NoSuchElementException e) {
418+
remotableBys.put(locator.getClass(), Mechanism.REMOTE);
419+
throw e;
420+
} catch (InvalidArgumentException e) {
421+
// Fall through
422+
}
370423
}
371-
if (!(value instanceof WebElement)) {
372-
throw new WebDriverException("Returned value cannot be converted to WebElement: " + value);
424+
425+
try {
426+
List<WebElement> elements = Mechanism.CONTEXT.findElements(parent, context, locator);
427+
remotableBys.put(locator.getClass(), Mechanism.CONTEXT);
428+
elements.forEach(e -> massage(parent, context, e, locator));
429+
return elements;
430+
} catch (NoSuchElementException e) {
431+
remotableBys.put(locator.getClass(), Mechanism.CONTEXT);
432+
throw e;
373433
}
374-
WebElement element = (WebElement) value;
375-
setFoundBy(this, element, by, using);
376-
return element;
377434
}
378435

379-
protected void setFoundBy(SearchContext context, WebElement element, String by, String using) {
380-
if (element instanceof RemoteWebElement) {
381-
RemoteWebElement remoteElement = (RemoteWebElement) element;
382-
remoteElement.setFoundBy(context, by, using);
383-
remoteElement.setFileDetector(getFileDetector());
436+
private WebElement massage(RemoteWebDriver parent, SearchContext context, WebElement element, By locator) {
437+
if (!(element instanceof RemoteWebElement)) {
438+
return element;
439+
}
440+
441+
RemoteWebElement remoteElement = (RemoteWebElement) element;
442+
if (locator instanceof By.Remotable) {
443+
By.Remotable.Parameters params = ((By.Remotable) locator).getRemoteParameters();
444+
remoteElement.setFoundBy(context, params.using(), String.valueOf(params.value()));
384445
}
446+
remoteElement.setFileDetector(parent.getFileDetector());
447+
remoteElement.setParent(parent);
448+
449+
return remoteElement;
385450
}
386451

387-
@SuppressWarnings("unchecked")
452+
/**
453+
* @deprecated Rely on using {@link By.Remotable} instead
454+
*/
455+
@Deprecated
456+
protected WebElement findElement(String by, String using) {
457+
throw new UnsupportedOperationException("`findElement` has been replaced by usages of " + By.Remotable.class);
458+
}
459+
460+
/**
461+
* @deprecated Rely on using {@link By.Remotable} instead
462+
*/
463+
@Deprecated
388464
protected List<WebElement> findElements(String by, String using) {
389-
if (using == null) {
390-
throw new IllegalArgumentException("Cannot find elements when the selector is null.");
391-
}
465+
throw new UnsupportedOperationException("`findElement` has been replaced by usages of " + By.Remotable.class);
466+
}
392467

393-
Response response = execute(DriverCommand.FIND_ELEMENTS(by, using));
394-
Object value = response.getValue();
395-
if (value == null) { // see https://github.com/SeleniumHQ/selenium/issues/4555
396-
return Collections.emptyList();
397-
}
398-
List<WebElement> allElements;
399-
try {
400-
allElements = (List<WebElement>) value;
401-
} catch (ClassCastException ex) {
402-
throw new WebDriverException("Returned value cannot be converted to List<WebElement>: " + value, ex);
403-
}
404-
for (WebElement element : allElements) {
405-
setFoundBy(this, element, by, using);
468+
protected void setFoundBy(SearchContext context, WebElement element, String by, String using) {
469+
if (element instanceof RemoteWebElement) {
470+
RemoteWebElement remoteElement = (RemoteWebElement) element;
471+
remoteElement.setFoundBy(context, by, using);
472+
remoteElement.setFileDetector(getFileDetector());
406473
}
407-
return allElements;
408474
}
409475

410476
// Misc
@@ -1111,4 +1177,82 @@ public String toString() {
11111177
platform,
11121178
getSessionId());
11131179
}
1180+
1181+
private enum Mechanism {
1182+
CONTEXT {
1183+
@Override
1184+
WebElement findElement(RemoteWebDriver parent, SearchContext context, By locator) {
1185+
return locator.findElement(context);
1186+
}
1187+
1188+
@Override
1189+
List<WebElement> findElements(RemoteWebDriver parent, SearchContext context, By locator) {
1190+
return locator.findElements(context);
1191+
}
1192+
},
1193+
REMOTE {
1194+
@Override
1195+
WebElement findElement(RemoteWebDriver parent, SearchContext context, By locator) {
1196+
String commandName;
1197+
Map<String, Object> params = new HashMap<>();
1198+
1199+
By.Remotable.Parameters fromLocator = ((By.Remotable) locator).getRemoteParameters();
1200+
params.put("using", fromLocator.using());
1201+
params.put("value", fromLocator.value());
1202+
1203+
if (context instanceof RemoteWebElement) {
1204+
commandName = DriverCommand.FIND_CHILD_ELEMENT;
1205+
params.put("id", ((RemoteWebElement) context).getId());
1206+
} else {
1207+
commandName = DriverCommand.FIND_ELEMENT;
1208+
}
1209+
1210+
Response response = parent.execute(new CommandPayload(commandName, params));
1211+
Object value = response.getValue();
1212+
if (value == null) {
1213+
throw new NoSuchElementException("Unable to find element with locator " + locator);
1214+
}
1215+
try {
1216+
return (WebElement) value;
1217+
} catch (ClassCastException ex) {
1218+
throw new WebDriverException(
1219+
"Returned value cannot be converted to WebElement: " + value, ex);
1220+
}
1221+
}
1222+
1223+
@Override
1224+
List<WebElement> findElements(RemoteWebDriver parent, SearchContext context, By locator) {
1225+
String commandName;
1226+
Map<String, Object> params = new HashMap<>();
1227+
1228+
By.Remotable.Parameters fromLocator = ((By.Remotable) locator).getRemoteParameters();
1229+
params.put("using", fromLocator.using());
1230+
params.put("value", fromLocator.value());
1231+
1232+
if (context instanceof RemoteWebElement) {
1233+
commandName = DriverCommand.FIND_CHILD_ELEMENTS;
1234+
params.put("id", ((RemoteWebElement) context).getId());
1235+
} else {
1236+
commandName = DriverCommand.FIND_ELEMENTS;
1237+
}
1238+
1239+
Response response = parent.execute(new CommandPayload(commandName, params));
1240+
Object value = response.getValue();
1241+
if (value == null) { // see https://github.com/SeleniumHQ/selenium/issues/4555
1242+
return Collections.emptyList();
1243+
}
1244+
try {
1245+
@SuppressWarnings("unchecked") List<WebElement> toReturn = (List<WebElement>) value;
1246+
return toReturn;
1247+
} catch (ClassCastException ex) {
1248+
throw new WebDriverException(
1249+
"Returned value cannot be converted to WebElement: " + value, ex);
1250+
}
1251+
}
1252+
}
1253+
;
1254+
1255+
abstract WebElement findElement(RemoteWebDriver parent, SearchContext context, By locator);
1256+
abstract List<WebElement> findElements(RemoteWebDriver parent, SearchContext context, By locator);
1257+
}
11141258
}

0 commit comments

Comments
 (0)