Skip to content

#764 alternative fix#769

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.

Sign up for GitHub

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

Merged
TikhomirovSergey merged 6 commits into from
Nov 21, 2017
Merged

#764 alternative fix #769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ddb61c
#764 FIX.
TikhomirovSergey Nov 14, 2017
60b2aa9
#764 FIX. AppiumFieldDecorator. Code style improvement
TikhomirovSergey Nov 15, 2017
e75a8a5
#764 FIX.
TikhomirovSergey Nov 16, 2017
a19c0de
#764 FIX: DefaultGenericMobileDriver: toString method was changed
TikhomirovSergey Nov 17, 2017
b784e16
#764 FIX: improving of some minor issues.
TikhomirovSergey Nov 18, 2017
d10f5d1
#764 FIX: improving of some minor issues.
TikhomirovSergey Nov 19, 2017
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
4 changes: 2 additions & 2 deletions src/main/java/io/appium/java_client/DefaultGenericMobileDriver.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -153,7 +153,7 @@ public List findElementsByXPath(String using) {

@Override
public String toString() {
return String.format("%s: %s", getPlatformName(),
getAutomationName());
return String.format("%s, Capabilities: %s", getClass().getCanonicalName(),
getCapabilities().asMap().toString());
}
}
19 changes: 11 additions & 8 deletions src/main/java/io/appium/java_client/internal/ElementMap.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,9 +16,10 @@

package io.appium.java_client.internal;

import static org.apache.commons.lang3.StringUtils.isBlank;

import com.google.common.collect.ImmutableMap;

import io.appium.java_client.HasSessionDetails;
import io.appium.java_client.MobileElement;
import io.appium.java_client.android.AndroidElement;
import io.appium.java_client.ios.IOSElement;
Expand DownExpand Up@@ -68,17 +69,19 @@ public Class<? extends RemoteWebElement> getElementClass() {
}

/**
* @param hasSessionDetails something that implements {@link io.appium.java_client.HasSessionDetails}.
* @return subclass of {@link io.appium.java_client.MobileElement} that convenient to current session details.
* @param platform is the mobile platform. See {@link MobilePlatform}.
* @param automation is the mobile automation type. See {@link AutomationName}
* @return subclass of {@link org.openqa.selenium.remote.RemoteWebElement} that convenient
* to current session details.
*/
public static Class<? extends RemoteWebElement> getElementClass(HasSessionDetails hasSessionDetails) {
if (hasSessionDetails == null) {
public static Class<? extends RemoteWebElement> getElementClass(String platform, String automation) {
if (isBlank(platform) && isBlank(automation)) {
return RemoteWebElement.class;
}
ElementMap element = Optional.ofNullable(mobileElementMap.get(String
.valueOf(hasSessionDetails.getAutomationName()).toLowerCase().trim()))
ElementMap element = Optional.ofNullable(mobileElementMap.get(
String.valueOf(platform).toLowerCase().trim()))

@mykola-mokhnachmykola-mokhnachNov 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does it make any sense to use String.valueOf for values of type String?

@TikhomirovSergeyTikhomirovSergeyNov 18, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@mykola-mokhnach It may return null value

.orElseGet(() -> mobileElementMap
.get(String.valueOf(hasSessionDetails.getPlatformName()).toLowerCase().trim()));
.get(String.valueOf(automation).toLowerCase().trim()));
if (element == null) {
return RemoteWebElement.class;
}
Expand Down
12 changes: 8 additions & 4 deletions src/main/java/io/appium/java_client/internal/JsonToMobileElementConverter.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,7 +37,10 @@
public class JsonToMobileElementConverter extends JsonToWebElementConverter {

protected final RemoteWebDriver driver;
private final HasSessionDetails hasSessionDetails;

private final String platform;
private final String automation;


/**
* @param driver an instance of {@link org.openqa.selenium.remote.RemoteWebDriver} subclass
Expand All@@ -46,7 +49,8 @@ public class JsonToMobileElementConverter extends JsonToWebElementConverter {
public JsonToMobileElementConverter(RemoteWebDriver driver, HasSessionDetails hasSessionDetails) {
super(driver);
this.driver = driver;
this.hasSessionDetails = hasSessionDetails;
this.platform = hasSessionDetails.getPlatformName();
this.automation = hasSessionDetails.getAutomationName();
}

/**
Expand DownExpand Up@@ -80,9 +84,9 @@ public Object apply(Object result) {
return result;
}

protected RemoteWebElement newMobileElement() {
private RemoteWebElement newMobileElement() {
Class<? extends RemoteWebElement> target;
target = getElementClass(hasSessionDetails);
target = getElementClass(platform, automation);
try {
Constructor<? extends RemoteWebElement> constructor = target.getDeclaredConstructor();
constructor.setAccessible(true);
Expand Down
42 changes: 32 additions & 10 deletions src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,8 +19,11 @@
import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException;
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;
import io.appium.java_client.pagefactory.locator.CacheableLocator;

import org.openqa.selenium.By;
Expand All@@ -39,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@@ -64,7 +69,25 @@ 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;
}

return ContentMappedBy.class.cast(currentBy)
.useContent(getCurrentContentType(currentContent));
}

private <T> T waitFor(Supplier<T> supplier) {
Expand All@@ -91,15 +114,16 @@ public WebElement findElement() {
return cachedElement;
}

By bySearching = getBy(this.by, searchContext);
try {
WebElement result = waitFor(() ->
searchContext.findElement(by));
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@@ -114,11 +138,9 @@ public List<WebElement> findElements() {
List<WebElement> result;
try {
result = waitFor(() -> {
List<WebElement> list = searchContext.findElements(by);
if (list.size() > 0) {
return list;
}
return null;
List<WebElement> list = searchContext
.findElements(getBy(by, searchContext));
return list.size() > 0 ? list : null;
});
} catch (TimeoutException | StaleElementReferenceException e) {
result = new ArrayList<>();
Expand All@@ -135,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
26 changes: 15 additions & 11 deletions src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy;
import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility
.unpackWebDriverFromSearchContext;
import static java.util.Optional.ofNullable;

import com.google.common.collect.ImmutableList;

Expand DownExpand Up@@ -64,13 +65,12 @@ 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;
private final String automation;
private final TimeOutDuration duration;
private final HasSessionDetails hasSessionDetails;


public AppiumFieldDecorator(SearchContext context, long timeout,
Expand All@@ -87,14 +87,18 @@ 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);
if (originalDriver == null
|| !HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) {
hasSessionDetails = null;
this.webDriver = unpackWebDriverFromSearchContext(context);
HasSessionDetails hasSessionDetails = ofNullable(this.webDriver).map(webDriver -> {
if (!HasSessionDetails.class.isAssignableFrom(webDriver.getClass())) {
return null;
}
return HasSessionDetails.class.cast(webDriver);
}).orElse(null);

if (hasSessionDetails == null) {
platform = null;
automation = null;
} else {
hasSessionDetails = HasSessionDetails.class.cast(originalDriver);
platform = hasSessionDetails.getPlatformName();
automation = hasSessionDetails.getAutomationName();
}
Expand DownExpand Up@@ -202,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);
return getEnhancedProxy(getElementClass(hasSessionDetails), elementInterceptor);
ElementInterceptor elementInterceptor = new ElementInterceptor(locator, webDriver);
return getEnhancedProxy(getElementClass(platform, automation), elementInterceptor);
}
}
4 changes: 3 additions & 1 deletion src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,7 @@

import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException;
import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType;
import static java.util.Optional.ofNullable;

import io.appium.java_client.pagefactory.bys.ContentType;
import io.appium.java_client.pagefactory.interceptors.InterceptorOfAListOfElements;
Expand DownExpand Up@@ -59,8 +60,9 @@ class WidgetListInterceptor extends InterceptorOfAListOfElements {
cachedElements = elements;
cachedWidgets.clear();

ContentType type = null;
for (WebElement element : cachedElements) {
ContentType type = getCurrentContentType(element);
type = ofNullable(type).orElseGet(() -> getCurrentContentType(element));
Class<?>[] params =
new Class<?>[] {instantiationMap.get(type).getParameterTypes()[0]};
cachedWidgets.add(ProxyFactory
Expand Down
31 changes: 18 additions & 13 deletions src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,40 +16,45 @@

package io.appium.java_client.pagefactory.bys;

import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType;
import static com.google.common.base.Preconditions.checkNotNull;
import static io.appium.java_client.pagefactory.bys.ContentType.NATIVE_MOBILE_SPECIFIC;

import org.openqa.selenium.By;
import org.openqa.selenium.SearchContext;
import org.openqa.selenium.WebElement;

import java.util.List;
import java.util.Map;
import javax.annotation.Nonnull;

public class ContentMappedBy extends By {
private final Map<ContentType, By> map;
private ContentType currentContent = NATIVE_MOBILE_SPECIFIC;

public ContentMappedBy(Map<ContentType, By> map) {
this.map = map;
}

/**
* This method sets required content type for the further searching.
* @param type required content type {@link ContentType}
* @return self-reference.
*/
public By useContent(@Nonnull ContentType type) {
checkNotNull(type);
currentContent = type;
return this;
}

@Override public WebElement findElement(SearchContext context) {
return context.findElement(map.get(getCurrentContentType(context)));
return context.findElement(map.get(currentContent));

@mykola-mokhnachmykola-mokhnachNov 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it ok if map.get returns null in any of these methods?

@TikhomirovSergeyTikhomirovSergeyNov 18, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It is not possible. It will use NATIVE_MOBILE_SPECIFIC as it is defined by default or it will use HTML_OR_DEFAULT when there is no SearchContext which also implements ContextArare or HasSessionDetails. Also it will use HTML_OR_DEFAULT when HasSessionDetails.isBrowser returns true or current context != NATIVE_CONTEXT.

Also there is the method

/** * This method sets required content type for the further searching. * @param type required content type {@link ContentType} * @return self-reference. */publicByuseContent(@NonnullContentTypetype) {
checkNotNull(type);
currentContent = type;
returnthis;
}

}

@Override public List<WebElement> findElements(SearchContext context) {
return context.findElements(map.get(getCurrentContentType(context)));
return context.findElements(map.get(currentContent));
}

@Override public String toString() {
By defaultBy = map.get(ContentType.HTML_OR_DEFAULT);
By nativeBy = map.get(ContentType.NATIVE_MOBILE_SPECIFIC);

if (defaultBy.equals(nativeBy)) {
return defaultBy.toString();
}

return "Locator map: " + "\n"
+ "- native content: \"" + nativeBy.toString() + "\" \n"
+ "- html content: \"" + defaultBy.toString() + "\"";
return map.get(currentContent).toString();
}
}
23 changes: 11 additions & 12 deletions src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,6 +19,7 @@
import static io.appium.java_client.pagefactory.bys.ContentType.HTML_OR_DEFAULT;
import static io.appium.java_client.pagefactory.bys.ContentType.NATIVE_MOBILE_SPECIFIC;
import static java.util.Optional.ofNullable;
import static org.apache.commons.lang3.StringUtils.containsIgnoreCase;

import io.appium.java_client.HasSessionDetails;
import io.appium.java_client.pagefactory.bys.ContentType;
Expand DownExpand Up@@ -77,7 +78,8 @@ public static WebDriver unpackWebDriverFromSearchContext(SearchContext searchCon
* extension/implementation.
* Note: if you want to use your own implementation then it should
* implement {@link org.openqa.selenium.ContextAware} or
* {@link org.openqa.selenium.internal.WrapsDriver}
* {@link org.openqa.selenium.internal.WrapsDriver} or
* {@link HasSessionDetails}
* @return current content type. It depends on current context. If current context is
* NATIVE_APP it will return
* {@link io.appium.java_client.pagefactory.bys.ContentType#NATIVE_MOBILE_SPECIFIC}.
Expand All@@ -93,20 +95,17 @@ public static ContentType getCurrentContentType(SearchContext context) {
if (HasSessionDetails.class.isAssignableFrom(driver.getClass())) {
HasSessionDetails hasSessionDetails = HasSessionDetails.class.cast(driver);

if (hasSessionDetails.isBrowser()) {
return HTML_OR_DEFAULT;
if (!hasSessionDetails.isBrowser()) {
return NATIVE_MOBILE_SPECIFIC;
}
return NATIVE_MOBILE_SPECIFIC;
}

if (!ContextAware.class.isAssignableFrom(driver.getClass())) { //it is desktop browser
return HTML_OR_DEFAULT;
}

ContextAware contextAware = ContextAware.class.cast(driver);
String currentContext = contextAware.getContext();
if (currentContext.contains(NATIVE_APP_PATTERN)) {
return NATIVE_MOBILE_SPECIFIC;
if (ContextAware.class.isAssignableFrom(driver.getClass())) { //it is desktop browser
ContextAware contextAware = ContextAware.class.cast(driver);
String currentContext = contextAware.getContext();
if (containsIgnoreCase(currentContext, NATIVE_APP_PATTERN)) {
return NATIVE_MOBILE_SPECIFIC;
}
}

return HTML_OR_DEFAULT;
Expand Down