Skip to content

Commit

Permalink
Start rewrite for thread safety, automatic stale element resolution
Browse files Browse the repository at this point in the history
This is a huge refactoring motivated by:
- Wanting to move away from unnecessary dynamic proxies
- Needing thread safe access to driver
- Needing to automatically refind elements if they are stale
- Needing to have targeted drivers and elements
- Needing to add locator information to toString (not done yet, but now
  possible)

It works by creating various implementations of WebElement and WebDriver
that accomplish each of those above needs (more or less). This allows
each need to be focused and individually testable. On the downside, the
real driver is heavily nested, something like:

ThreadedWebDriver
  - driver: ForwardingTargetedWebDriver
    - driver: RefindingWebDriver
      - driver: FirefoxDriver -- real impl

Likewise with WebElements:

ThreadedWebElement
  - element: ForwardingTargetedWebElement
    - element: RefindingWebElement
      - element: RemoteWebElement -- real impl

On the plus side, none of those implementations have to know about any
other. Care must be taken in wiring them up the right way, however.

As of now, not everything is completely tested or verified, but the
fundamentals work.

Known issues:
- Lists of elements are not refinding. Not sure if it's possible.
  - Could/should at least create refinding element if list size == 1
  - Could use javascript to associate each element in the list
    with xpath, and use that to relook up individual elements, but this
    is a little scary.
  • Loading branch information
alechenninger committed Jul 27, 2014
1 parent 18c2cb7 commit c9c4b85
Show file tree
Hide file tree
Showing 25 changed files with 2,060 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@
*/
@FunctionalInterface
public interface ElementConstructor<T extends Element> {
T newElement(Supplier<WebElement> source, ElementFactory converter);
T newElement(WebElement source, ElementFactory converter);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.redhat.darcy.webdriver.internal.TargetedWebDriver;
import com.redhat.darcy.webdriver.internal.TargetedWebDriverFactory;
import com.redhat.darcy.webdriver.internal.TargetedWebDriverParentContext;
import com.redhat.darcy.webdriver.internal.ThreadSafeCachingTargetedWebDriverFactory;
import com.redhat.darcy.webdriver.internal.WebDriverTarget;
import com.redhat.darcy.webdriver.internal.WebDriverTargets;

Expand Down Expand Up @@ -63,8 +64,8 @@ public abstract <E extends WebDriverElement> T withElementImplementation(Class<?
protected static Browser makeBrowser(WebDriver driver, ElementConstructorMap constructorMap) {
WebDriverTarget target = WebDriverTargets.window(driver.getWindowHandle());

TargetedWebDriverFactory targetedWdFactory = new CachingTargetedWebDriverFactory(driver,
target);
TargetedWebDriverFactory targetedWdFactory = new ThreadSafeCachingTargetedWebDriverFactory(
driver, target);
TargetedWebDriver targetedDriver = targetedWdFactory.getTargetedWebDriver(target);

TargetedElementFactoryFactory elementFactoryFactory =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@

import org.openqa.selenium.WebElement;

import java.util.function.Supplier;

public class WebDriverButton extends WebDriverElement implements Button {

public WebDriverButton(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverButton(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,19 @@

import com.redhat.darcy.ui.api.ElementContext;
import com.redhat.darcy.ui.api.elements.Element;
import com.redhat.darcy.util.Caching;
import com.redhat.darcy.ui.api.elements.Findable;
import com.redhat.darcy.webdriver.internal.DefaultWebDriverElementContext;
import com.redhat.darcy.webdriver.internal.ElementFactory;

import org.openqa.selenium.NotFoundException;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.internal.WrapsElement;

import java.util.function.Supplier;

public class WebDriverElement implements Element, Caching, WrapsElement {
private final Supplier<WebElement> source;
public class WebDriverElement implements Element, WrapsElement {
private final WebElement source;
private final ElementFactory elementFactory;

private WebElement cachedElement;

public WebDriverElement(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverElement(WebElement source, ElementFactory elementFactory) {
this.source = source;
this.elementFactory = elementFactory;
}
Expand All @@ -53,27 +49,12 @@ public boolean isDisplayed() {

@Override
public boolean isPresent() {
try {
invalidateCache();
getWrappedElement();
return true;
} catch (NotFoundException e) {
return false;
}
}

@Override
public void invalidateCache() {
cachedElement = null;
return ((Findable) source).isPresent();
}

@Override
public WebElement getWrappedElement() {
if (cachedElement == null) {
cachedElement = source.get();
}

return cachedElement;
return source;
}

public ElementContext getElementContext() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@

import org.openqa.selenium.WebElement;

import java.util.function.Supplier;

public class WebDriverLabel extends WebDriverElement implements Label {
public WebDriverLabel(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverLabel(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@

import org.openqa.selenium.WebElement;

import java.util.function.Supplier;

public class WebDriverLink extends WebDriverElement implements Link {
public WebDriverLink(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverLink(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

public class WebDriverSelect extends WebDriverElement implements Select {

public WebDriverSelect(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverSelect(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

public class WebDriverSelectOption extends WebDriverElement implements SelectOption {

public WebDriverSelectOption(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverSelectOption(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.function.Supplier;

public class WebDriverTextInput extends WebDriverElement implements TextInput {
public WebDriverTextInput(Supplier<WebElement> source, ElementFactory elementFactory) {
public WebDriverTextInput(WebElement source, ElementFactory elementFactory) {
super(source, elementFactory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.redhat.darcy.ui.api.Transition;
import com.redhat.darcy.ui.api.elements.Element;
import com.redhat.darcy.ui.internal.SimpleTransition;
import com.redhat.darcy.util.LazyList;
import com.redhat.darcy.webdriver.WebDriverElementContext;
import com.redhat.darcy.webdriver.locators.ByPartialVisibleText;
import com.redhat.darcy.webdriver.locators.ByVisibleText;
Expand All @@ -38,6 +39,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;

public class DefaultWebDriverElementContext implements WebDriverElementContext {
private final WebElementContext context;
Expand Down Expand Up @@ -69,7 +71,7 @@ public <T> List<T> findAllById(Class<T> type, String id) {

@Override
public <T> T findById(Class<T> type, String id) {
return newElement(type, () -> context.findById(WebElement.class, id));
return newElement(type, context.findById(WebElement.class, id));
}

@Override
Expand All @@ -79,7 +81,7 @@ public <T> List<T> findAllByName(Class<T> type, String name) {

@Override
public <T> T findByName(Class<T> type, String name) {
return newElement(type, () -> context.findByName(WebElement.class, name));
return newElement(type, context.findByName(WebElement.class, name));
}

@Override
Expand All @@ -89,7 +91,7 @@ public <T> List<T> findAllByLinkText(Class<T> type, String linkText) {

@Override
public <T> T findByLinkText(Class<T> type, String linkText) {
return newElement(type, () -> context.findByLinkText(WebElement.class, linkText));
return newElement(type, context.findByLinkText(WebElement.class, linkText));
}

@Override
Expand All @@ -100,7 +102,7 @@ public <T> List<T> findAllByTextContent(Class<T> type, String textContent) {

@Override
public <T> T findByTextContent(Class<T> type, String textContent) {
return newElement(type, () -> context.findByTextContent(WebElement.class, textContent));
return newElement(type, context.findByTextContent(WebElement.class, textContent));
}

@Override
Expand All @@ -112,7 +114,7 @@ public <T> List<T> findAllByPartialTextContent(Class<T> type, String partialText
@Override
public <T> T findByPartialTextContent(Class<T> type, String partialTextContent) {
return newElement(type,
() -> context.findByPartialTextContent(WebElement.class, partialTextContent));
context.findByPartialTextContent(WebElement.class, partialTextContent));
}

@Override
Expand All @@ -122,7 +124,7 @@ public <T> List<T> findAllByXPath(Class<T> type, String xpath) {

@Override
public <T> T findByXPath(Class<T> type, String xpath) {
return newElement(type, () -> context.findByXPath(WebElement.class, xpath));
return newElement(type, context.findByXPath(WebElement.class, xpath));
}

@Override
Expand All @@ -132,7 +134,7 @@ public <T> List<T> findAllByCss(Class<T> type, String css) {

@Override
public <T> T findByCss(Class<T> type, String css) {
return newElement(type, () -> context.findByCss(WebElement.class, css));
return newElement(type, context.findByCss(WebElement.class, css));
}

@Override
Expand All @@ -142,7 +144,7 @@ public <T> List<T> findAllByHtmlTag(Class<T> type, String tag) {

@Override
public <T> T findByHtmlTag(Class<T> type, String tag) {
return newElement(type, () -> context.findByHtmlTag(WebElement.class, tag));
return newElement(type, context.findByHtmlTag(WebElement.class, tag));
}

@Override
Expand All @@ -152,7 +154,7 @@ public <T> List<T> findAllByChained(Class<T> type, Locator... locators) {

@Override
public <T> T findByChained(Class<T> type, Locator... locators) {
return newElement(type, () -> context.findByChained(WebElement.class, locators));
return newElement(type, context.findByChained(WebElement.class, locators));
}

@Override
Expand All @@ -162,7 +164,7 @@ public <T> List<T> findAllByNested(Class<T> type, Element parent, Locator child)

@Override
public <T> T findByNested(Class<T> type, Element parent, Locator child) {
return newElement(type, () -> context.findByNested(WebElement.class, parent, child));
return newElement(type, context.findByNested(WebElement.class, parent, child));
}

@Override
Expand All @@ -178,7 +180,7 @@ public WebDriverElementContext withRootElement(Element root) {
}

@SuppressWarnings("unchecked")
private <T> T newElement(Class<T> type, Supplier<WebElement> source) {
private <T> T newElement(Class<T> type, WebElement source) {
if (!Element.class.isAssignableFrom(type)) {
throw new DarcyException("An ElementContext can only locate Element types: "
+ type.toString());
Expand All @@ -194,7 +196,10 @@ private <T> List<T> newElementList(Class<T> type, Supplier<List<WebElement>> sou
+ type.toString());
}

return (List<T>) elementFactory.newElementList((Class<Element>) type, source);
return new LazyList<>(() -> (List<T>) source.get()
.stream()
.map(s -> elementFactory.newElement((Class<Element>) type, s))
.collect(Collectors.toList()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,5 @@
* Translates from WebElement(s) to Darcy Elements.
*/
public interface ElementFactory {
<T extends Element> T newElement(Class<T> type, Supplier<WebElement> sourceReference);
<T extends Element> List<T> newElementList(Class<T> type,
Supplier<List<WebElement>> sourceReference);
<T extends Element> T newElement(Class<T> type, WebElement sourceReference);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,8 @@ public TargetedElementFactory(TargetedWebDriver driver, ElementConstructorMap el
}

@Override
public <T extends Element> T newElement(Class<T> type, Supplier<WebElement> sourceReference) {
return elementMap.get(type).newElement(
new TargetedWebElementSupplier(sourceReference, driver), this);
}

@Override
public <T extends Element> List<T> newElementList(Class<T> type,
Supplier<List<WebElement>> sourceReference) {
return new LazyList<T>(() -> sourceReference.get()
.stream()
.map(e -> newElement(type, () -> e))
.collect(Collectors.toList()));
public <T extends Element> T newElement(Class<T> type, WebElement source) {
return elementMap.get(type).newElement(source, this);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
Copyright 2014 Red Hat, Inc. and/or its affiliates.
This file is part of darcy-webdriver.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package com.redhat.darcy.webdriver.internal;

import com.redhat.darcy.webdriver.internal.webdriver.ForwardingTargetedWebDriver;
import com.redhat.darcy.webdriver.internal.webdriver.RefindingWebDriver;
import com.redhat.darcy.webdriver.internal.webdriver.ThreadedTargetedWebDriver;

import org.openqa.selenium.WebDriver;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class ThreadSafeCachingTargetedWebDriverFactory implements TargetedWebDriverFactory {
// It's important that all access to driver is gated by same executor
private final ExecutorService executor = Executors.newSingleThreadExecutor();
private final CachingTargetLocator cachingTargetLocator;

public ThreadSafeCachingTargetedWebDriverFactory(WebDriver untargetedDriver,
WebDriverTarget currentTarget) {
cachingTargetLocator = new CachingTargetLocator(currentTarget,
new RefindingWebDriver(untargetedDriver));
}

@Override
public TargetedWebDriver getTargetedWebDriver(WebDriverTarget target) {
// Order is important here: we want atomic operations to be executed in a single threaded
// queue, so driver switching needs to happen in the same queued task as the subsequent
// action. This means that the outer most driver should queue the operations of the
// forwarded driver -- which takes care of the switching.

// Inner to outer:
// 1. Refinding -- finds elements that can refind themselves if stale (these show up because
// CachingTargetLocator is powered by a RefindingDriver).
// 2. Targeted -- targets a specific driver before proceeding, creates elements that do same
// (So with refinding, creates elements that target a driver, then can refind themselves
// if stale).
// 3. Threaded -- Assures atomic, single threaded interaction with driver from any source
return new ThreadedTargetedWebDriver(
new ForwardingTargetedWebDriver(cachingTargetLocator, target), executor);
}
}

0 comments on commit c9c4b85

Please sign in to comment.