-
-
Notifications
You must be signed in to change notification settings - Fork 767
#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.
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
Changes from 5 commits
2ddb61c
60b2aa9
e75a8a5
a19c0de
b784e16
d10f5d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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 | ||
|
|
@@ -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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
|
|
||
| return ContentMappedBy.class.cast(currentBy) | ||
| .useContent(getCurrentContentType(currentContent)); | ||
| } | ||
|
|
||
| private <T> T waitFor(Supplier<T> supplier) { | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -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<>(); | ||
|
|
@@ -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); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
||
|
|
@@ -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, | ||
|
|
@@ -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 -> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant it's better to use the mapped local variable |
||
| if (!HasSessionDetails.class.isAssignableFrom(this.webDriver.getClass())) { | ||
| return null; | ||
| } | ||
| return HasSessionDetails.class.cast(this.webDriver); | ||
| }).orElse(null); | ||
|
|
||
| if (hasSessionDetails == null) { | ||
| platform = null; | ||
| automation = null; | ||
| } else { | ||
| hasSessionDetails = HasSessionDetails.class.cast(originalDriver); | ||
| platform = hasSessionDetails.getPlatformName(); | ||
| automation = hasSessionDetails.getAutomationName(); | ||
| } | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it ok if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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.
*/
public By useContent(@Nonnull ContentType type) {
checkNotNull(type);
currentContent = type;
return this;
} |
||
| } | ||
|
|
||
| @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(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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.valueOffor values of type String?There was a problem hiding this comment.
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
nullvalue