Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#764 alternative fix #769

Merged
merged 6 commits into from
Nov 21, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static io.appium.java_client.pagefactory.ThrowableUtil.isInvalidSelectorRootCause;
import static io.appium.java_client.pagefactory.ThrowableUtil.isStaleElementReferenceException;
import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType;
import static java.lang.String.format;


import io.appium.java_client.pagefactory.bys.ContentMappedBy;
Expand All @@ -41,13 +42,15 @@

class AppiumElementLocator implements CacheableLocator {

private static final String exceptionMessageIfElementNotFound = "Can't locate an element by this strategy: %s";

private final boolean shouldCache;
private final By by;
private final TimeOutDuration duration;
private final SearchContext searchContext;
private WebElement cachedElement;
private List<WebElement> cachedElementList;
private final String exceptionMessageIfElementNotFound;

/**
* Creates a new mobile element locator. It instantiates {@link WebElement}
* using @AndroidFindBy (-s), @iOSFindBy (-s) and @FindBy (-s) annotation
Expand All @@ -66,9 +69,18 @@ public AppiumElementLocator(SearchContext searchContext, By by, boolean shouldCa
this.shouldCache = shouldCache;
this.duration = duration;
this.by = by;
this.exceptionMessageIfElementNotFound = "Can't locate an element by this strategy: " + by.toString();
}

/**
* This methods makes sets some settings of the {@link By} according to
* the given instance of {@link SearchContext}. If there is some {@link ContentMappedBy}
* then it is switched to the searching for some html or native mobile element.
* Otherwise nothing happens there.
*
* @param currentBy is some locator strategy
* @param currentContent is an instance of some subclass of the {@link SearchContext}.
* @return the corrected {@link By} for the further searching
*/
private static By getBy(By currentBy, SearchContext currentContent) {
if (!ContentMappedBy.class.isAssignableFrom(currentBy.getClass())) {
return currentBy;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be easier to understand the code if this condition had an explanation comment

Expand Down Expand Up @@ -102,15 +114,16 @@ public WebElement findElement() {
return cachedElement;
}

By bySearching = getBy(this.by, searchContext);
try {
WebElement result = waitFor(() ->
searchContext.findElement(getBy(by, searchContext)));
searchContext.findElement(bySearching));
if (shouldCache) {
cachedElement = result;
}
return result;
} catch (TimeoutException | StaleElementReferenceException e) {
throw new NoSuchElementException(exceptionMessageIfElementNotFound, e);
throw new NoSuchElementException(format(exceptionMessageIfElementNotFound, bySearching.toString()), e);
}
}

Expand All @@ -127,10 +140,7 @@ public List<WebElement> findElements() {
result = waitFor(() -> {
List<WebElement> list = searchContext
.findElements(getBy(by, searchContext));
if (list.size() > 0) {
return list;
}
return null;
return list.size() > 0 ? list : null;
});
} catch (TimeoutException | StaleElementReferenceException e) {
result = new ArrayList<>();
Expand All @@ -147,7 +157,7 @@ public List<WebElement> findElements() {
}

@Override public String toString() {
return String.format("Located by %s", by);
return format("Located by %s", by);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class AppiumFieldDecorator implements FieldDecorator {
IOSElement.class, WindowsElement.class);
public static long DEFAULT_TIMEOUT = 1;
public static TimeUnit DEFAULT_TIMEUNIT = TimeUnit.SECONDS;
private final WebDriver originalDriver;
private final WebDriver webDriver;
private final DefaultFieldDecorator defaultElementFieldDecoracor;
private final AppiumElementLocatorFactory widgetLocatorFactory;
private final String platform;
Expand All @@ -87,12 +87,12 @@ public AppiumFieldDecorator(SearchContext context, long timeout,
* @param duration is a desired duration of the waiting for an element presence.
*/
public AppiumFieldDecorator(SearchContext context, TimeOutDuration duration) {
this.originalDriver = unpackWebDriverFromSearchContext(context);
HasSessionDetails hasSessionDetails = ofNullable(this.originalDriver).map(webDriver -> {
if (!HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) {
this.webDriver = unpackWebDriverFromSearchContext(context);
HasSessionDetails hasSessionDetails = ofNullable(this.webDriver).map(webDriver -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant it's better to use the mapped local variable webDriver -> inside the lambda handler rather than class-level this.originalDriverone.

if (!HasSessionDetails.class.isAssignableFrom(this.webDriver.getClass())) {
return null;
}
return HasSessionDetails.class.cast(originalDriver);
return HasSessionDetails.class.cast(this.webDriver);
}).orElse(null);

if (hasSessionDetails == null) {
Expand Down Expand Up @@ -206,19 +206,19 @@ private Object decorateWidget(Field field) {

if (isAlist) {
return getEnhancedProxy(ArrayList.class,
new WidgetListInterceptor(locator, originalDriver, map, widgetType,
new WidgetListInterceptor(locator, webDriver, map, widgetType,
duration));
}

Constructor<? extends Widget> constructor =
WidgetConstructorUtil.findConvenientConstructor(widgetType);
return getEnhancedProxy(widgetType, new Class[] {constructor.getParameterTypes()[0]},
new Object[] {proxyForAnElement(locator)},
new WidgetInterceptor(locator, originalDriver, null, map, duration));
new WidgetInterceptor(locator, webDriver, null, map, duration));
}

private WebElement proxyForAnElement(ElementLocator locator) {
ElementInterceptor elementInterceptor = new ElementInterceptor(locator, originalDriver);
ElementInterceptor elementInterceptor = new ElementInterceptor(locator, webDriver);
return getEnhancedProxy(getElementClass(platform, automation), elementInterceptor);
}
}