#549 FIX

src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java

@@ -28,14 +28,17 @@
 import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
 import org.openqa.selenium.SearchContext;
+import org.openqa.selenium.StaleElementReferenceException;
 import org.openqa.selenium.TimeoutException;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ui.FluentWait;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 
 class AppiumElementLocator implements CacheableLocator {
 
@@ -45,9 +48,9 @@ class AppiumElementLocator implements CacheableLocator {
     private final TimeOutDuration originalTimeOutDuration;
     private final WebDriver originalWebDriver;
     private final SearchContext searchContext;
-    private final WaitingFunction waitingFunction;
     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
@@ -71,26 +74,27 @@ public AppiumElementLocator(SearchContext searchContext, By by, boolean shouldCa
         this.originalTimeOutDuration = originalDuration;
         this.by = by;
         this.originalWebDriver = originalWebDriver;
-        waitingFunction = new WaitingFunction(this.searchContext);
+        this.exceptionMessageIfElementNotFound =  "Can't locate an element by this strategy: " + by.toString();
     }
 
     private void changeImplicitlyWaitTimeOut(long newTimeOut, TimeUnit newTimeUnit) {
         originalWebDriver.manage().timeouts().implicitlyWait(newTimeOut, newTimeUnit);
     }
 
-    // This method waits for not empty element list using all defined by
-    private List<WebElement> waitFor() {
-        // When we use complex By strategies (like ChainedBy or ByAll)
-        // there are some problems (StaleElementReferenceException, implicitly
-        // wait time out
-        // for each chain By section, etc)
+    private <T extends Object> T waitFor(Supplier<T> supplier) {
+        WaitingFunction<T> function = new WaitingFunction<>();
         try {
             changeImplicitlyWaitTimeOut(0, TimeUnit.SECONDS);
-            FluentWait<By> wait = new FluentWait<>(by);
+            FluentWait<Supplier<T>> wait = new FluentWait<>(supplier)
+                    .ignoring(NoSuchElementException.class);
             wait.withTimeout(timeOutDuration.getTime(), timeOutDuration.getTimeUnit());
-            return wait.until(waitingFunction);
+            return wait.until(function);
         } catch (TimeoutException e) {
-            return new ArrayList<>();
+            if (function.foundStaleElementReferenceException != null) {
+                throw StaleElementReferenceException
+                        .class.cast(function.foundStaleElementReferenceException);
+            }
+            throw e;
         } finally {
             changeImplicitlyWaitTimeOut(originalTimeOutDuration.getTime(), originalTimeOutDuration.getTimeUnit());
         }
@@ -103,19 +107,17 @@ public WebElement findElement() {
         if (cachedElement != null && shouldCache) {
             return cachedElement;
         }
-        List<WebElement> result = waitFor();
-        if (result.size() == 0) {
-            String message = "Can't locate an element by this strategy: " + by.toString();
-            if (waitingFunction.foundStaleElementReferenceException != null) {
-                throw new NoSuchElementException(message,
-                    waitingFunction.foundStaleElementReferenceException);
+
+        try {
+            WebElement result =  waitFor(() ->
+                    searchContext.findElement(by));
+            if (shouldCache) {
+                cachedElement = result;
             }
-            throw new NoSuchElementException(message);
-        }
-        if (shouldCache) {
-            cachedElement = result.get(0);
+            return result;
+        } catch (TimeoutException | StaleElementReferenceException e) {
+            throw new NoSuchElementException(exceptionMessageIfElementNotFound, e);
         }
-        return result.get(0);
     }
 
     /**
@@ -125,7 +127,20 @@ public List<WebElement> findElements() {
         if (cachedElementList != null && shouldCache) {
             return cachedElementList;
         }
-        List<WebElement> result = waitFor();
+
+        List<WebElement> result;
+        try {
+            result = waitFor(() -> {
+                List<WebElement> list = searchContext.findElements(by);
+                if (list.size() > 0) {
+                    return list;
+                }
+                return null;
+            });
+        } catch (TimeoutException | StaleElementReferenceException e) {
+            result = new ArrayList<>();
+        }
+
         if (shouldCache) {
             cachedElementList = result;
         }
@@ -138,26 +153,19 @@ public List<WebElement> findElements() {
 
 
     // This function waits for not empty element list using all defined by
-    private static class WaitingFunction implements Function<By, List<WebElement>> {
-        private final SearchContext searchContext;
-        Throwable foundStaleElementReferenceException;
-
-        private WaitingFunction(SearchContext searchContext) {
-            this.searchContext = searchContext;
-        }
+    private static class WaitingFunction<T> implements Function<Supplier<T>, T> {
+        private Throwable foundStaleElementReferenceException;
 
-        public List<WebElement> apply(By by) {
-            List<WebElement> result = new ArrayList<>();
-            Throwable shouldBeThrown = null;
-            boolean isRootCauseInvalidSelector;
-            boolean isRootCauseStaleElementReferenceException = false;
+        public T apply(Supplier<T> supplier) {
             foundStaleElementReferenceException = null;
 
             try {
-                result.addAll(searchContext.findElements(by));
+                return supplier.get();
             } catch (Throwable e) {
+                boolean isRootCauseStaleElementReferenceException = false;
+                Throwable shouldBeThrown;
+                boolean isRootCauseInvalidSelector = isInvalidSelectorRootCause(e);
 
-                isRootCauseInvalidSelector = isInvalidSelectorRootCause(e);
                 if (!isRootCauseInvalidSelector) {
                     isRootCauseStaleElementReferenceException = isStaleElementReferenceException(e);
                 }
@@ -168,21 +176,19 @@ public List<WebElement> apply(By by) {
 
                 if (!isRootCauseInvalidSelector & !isRootCauseStaleElementReferenceException) {
                     shouldBeThrown = extractReadableException(e);
+                    if (shouldBeThrown != null) {
+                        if (NoSuchElementException.class.equals(shouldBeThrown.getClass())) {
+                            throw NoSuchElementException.class.cast(shouldBeThrown);
+                        } else {
+                            throw new WebDriverException(shouldBeThrown);
+                        }
+                    } else {
+                        throw new WebDriverException(e);
+                    }
+                } else {
+                    return null;
                 }
             }
-
-            if (shouldBeThrown != null) {
-                if (RuntimeException.class.isAssignableFrom(shouldBeThrown.getClass())) {
-                    throw (RuntimeException) shouldBeThrown;
-                }
-                throw new RuntimeException(shouldBeThrown);
-            }
-
-            if (result.size() > 0) {
-                return result;
-            } else {
-                return null;
-            }
         }
     }
 }

src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java

@@ -32,6 +32,10 @@ public ContentMappedBy(Map<ContentType, By> map) {
         this.map = map;
     }
 
+    @Override public WebElement findElement(SearchContext context) {
+        return context.findElement(map.get(getCurrentContentType(context)));
+    }
+
     @Override public List<WebElement> findElements(SearchContext context) {
         return context.findElements(map.get(getCurrentContentType(context)));
     }

src/main/java/io/appium/java_client/pagefactory/bys/builder/AppiumByBuilder.java

@@ -25,7 +25,6 @@
 import org.openqa.selenium.By;
 import org.openqa.selenium.support.pagefactory.AbstractAnnotations;
 import org.openqa.selenium.support.pagefactory.ByAll;
-import org.openqa.selenium.support.pagefactory.ByChained;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;

src/main/java/io/appium/java_client/pagefactory/bys/builder/ByChained.java

@@ -0,0 +1,72 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.appium.java_client.pagefactory.bys.builder;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import io.appium.java_client.functions.AppiumFunction;
+import org.openqa.selenium.By;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.SearchContext;
+import org.openqa.selenium.TimeoutException;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.FluentWait;
+
+import java.util.Optional;
+
+class ByChained extends org.openqa.selenium.support.pagefactory.ByChained {
+
+    private final By[] bys;
+
+    private static AppiumFunction<SearchContext, WebElement> getSearchingFunction(By by) {
+        return input -> {
+            try {
+                return input.findElement(by);
+            } catch (NoSuchElementException e) {
+                return null;
+            }
+        };
+    }
+
+    public ByChained(By[] bys) {
+        super(bys);
+        checkNotNull(bys);
+        if (bys.length == 0) {
+            throw new IllegalArgumentException("By array should not be empty");
+        }
+        this.bys = bys;
+    }
+
+    @Override
+    public WebElement findElement(SearchContext context) {
+        AppiumFunction<SearchContext, WebElement> searchingFunction = null;
+
+        for (By by: bys) {
+            searchingFunction = Optional.ofNullable(searchingFunction != null
+                    ? searchingFunction.andThen(getSearchingFunction(by)) : null).orElse(getSearchingFunction(by));
+        }
+
+        FluentWait<SearchContext> waiting = new FluentWait<>(context);
+
+        try {
+            checkNotNull(searchingFunction);
+            return waiting.until(searchingFunction);
+        } catch (TimeoutException e) {
+            throw new NoSuchElementException("Cannot locate an element using " + toString());
+        }
+    }
+}

src/test/java/io/appium/java_client/pagefactory_tests/AndroidPageObjectTest.java

@@ -168,6 +168,18 @@ public class AndroidPageObjectTest extends BaseAndroidTest {
     @FindBy(className = "android.widget.TextView")
     private MobileElement cached;
 
+    @AndroidFindBy(uiAutomator = "new UiSelector().resourceId(\"android:id/content\")")
+    @AndroidFindBy(uiAutomator = "new UiSelector().resourceId(\"android:id/list\")")
+    @AndroidFindBy(id = "android:id/Faketext1")
+    @AndroidFindBy(id = "android:id/text1")
+    private WebElement elementFoundByInvalidChainedSelector;
+
+    @AndroidFindBy(uiAutomator = "new UiSelector().resourceId(\"android:id/content\")")
+    @AndroidFindBy(uiAutomator = "new UiSelector().resourceId(\"android:id/list\")")
+    @AndroidFindBy(id = "android:id/Faketext1")
+    @AndroidFindBy(id = "android:id/text1")
+    private List<WebElement> elementsFoundByInvalidChainedSelector;
+
     /**
      * The setting up.
      */
@@ -312,4 +324,13 @@ public class AndroidPageObjectTest extends BaseAndroidTest {
     @Test public void checkCached() {
         assertEquals(cached.getId(), cached.getId());
     }
+
+    @Test(expected = NoSuchElementException.class)
+    public void checkThatElementSearchingThrowsExpectedExceptionIfChainedLocatorIsInvalid() {
+        assertNotNull(elementFoundByInvalidChainedSelector.getAttribute("text"));
+    }
+
+    @Test public void checkThatListSearchingWorksIfChainedLocatorIsInvalid() {
+        assertEquals(0, elementsFoundByInvalidChainedSelector.size());
+    }
 }