diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java index fb81a8332..36c19e895 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java @@ -18,6 +18,8 @@ import io.appium.java_client.pagefactory.bys.ContentMappedBy; import io.appium.java_client.pagefactory.locator.CacheableLocator; +import io.appium.java_client.support.pagefactory.ByAll; +import io.appium.java_client.support.pagefactory.ByChained; import io.appium.java_client.support.ui.FluentWait; import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; @@ -197,6 +199,74 @@ public List findElements() { return result; } + /** + * First element of each alternative locator after the one that produced {@code resolvedRoot}. + * A widget annotated with {@link LocatorGroupStrategy#ALL_POSSIBLE} is that set of locators. + * Chained locators stay one root: a miss there is not a signal to try another chain. + * + * @param resolvedRoot the root already searched for a nested element + * @return later roots, or an empty list when this locator has no further alternatives + */ + List findFurtherRoots(WebElement resolvedRoot) { + SearchContext context = getSearchContext().orElse(null); + if (resolvedRoot == null || context == null) { + return List.of(); + } + + By[] alternatives = alternativeLocators(mappedLocator(context)); + if (alternatives == null) { + return List.of(); + } + + List furtherRoots = new ArrayList<>(); + boolean skippedResolvedLocator = false; + for (By candidate : alternatives) { + List matches = context.findElements(candidate); + if (matches.isEmpty()) { + continue; + } + if (!skippedResolvedLocator) { + skippedResolvedLocator = true; + continue; + } + furtherRoots.add(matches.get(0)); + } + return furtherRoots; + } + + private By mappedLocator(SearchContext context) { + By effective = getBy(by, context); + if (effective instanceof ContentMappedBy) { + return ((ContentMappedBy) effective).getCurrentLocator(); + } + return effective; + } + + /** + * Locators that are alternatives for one another, or null when {@code by} is a single + * locator or a chain. A chain of one locator is unwrapped so {@code @AndroidFindAll} + * is treated the same way as repeated {@code @AndroidFindBy} annotations. + */ + private static By[] alternativeLocators(By by) { + if (by instanceof ByAll) { + By[] nested = ((ByAll) by).getBys(); + if (nested.length == 1) { + By[] unwrapped = alternativeLocators(nested[0]); + if (unwrapped != null) { + return unwrapped; + } + } + return nested.length > 1 ? nested : null; + } + if (by instanceof ByChained) { + By[] nested = ((ByChained) by).getBys(); + if (nested.length == 1) { + return alternativeLocators(nested[0]); + } + } + return null; + } + @Override public boolean isLookUpCached() { return shouldCache; } diff --git a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java index 91847534e..b914de494 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -21,6 +21,7 @@ import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.support.PageFactory; import org.jspecify.annotations.Nullable; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; @@ -30,6 +31,7 @@ import java.lang.reflect.Modifier; import java.time.Duration; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -90,34 +92,71 @@ public WidgetInterceptor( @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); - WebElement cachedElement = cachedElementReference == null ? null : cachedElementReference.get(); - if (cachedElement == null || !cachedInstances.containsKey(type) - || locator != null && !((CacheableLocator) locator).isLookUpCached() - ) { - cachedElementReference = new WeakReference<>(element); - - Constructor constructor = instantiationMap.get(type); - Class clazz = constructor.getDeclaringClass(); - - if (Modifier.isAbstract(clazz.getModifiers())) { - throw new InstantiationException( - String.format("%s is abstract so it cannot be instantiated", clazz.getName()) - ); + if (isCached(type)) { + return invoke(cachedInstances.get(type), method, args); + } + + rejectAbstractWidget(type); + try { + return bindAndInvoke(type, element, method, args); + } catch (NoSuchElementException firstMiss) { + // The first ALL_POSSIBLE root matched, but the nested lookup did not. + // Try each later root with the same nested locators. + NoSuchElementException lastMiss = firstMiss; + for (WebElement root : furtherRoots(element)) { + try { + return bindAndInvoke(type, root, method, args); + } catch (NoSuchElementException laterMiss) { + lastMiss = laterMiss; + } } + throw lastMiss; + } + } + + private boolean isCached(ContentType type) { + WebElement cachedElement = cachedElementReference == null ? null : cachedElementReference.get(); + if (cachedElement == null || !cachedInstances.containsKey(type)) { + return false; + } + return locator == null || ((CacheableLocator) locator).isLookUpCached(); + } - Widget widget = constructor.newInstance(element); - cachedInstances.put(type, widget); - PageFactory.initElements( - new AppiumFieldDecorator(new WeakReference<>(widget), duration, defaultLocatorGroupStrategy), - widget + private void rejectAbstractWidget(ContentType type) throws InstantiationException { + Class clazz = instantiationMap.get(type).getDeclaringClass(); + if (Modifier.isAbstract(clazz.getModifiers())) { + throw new InstantiationException( + String.format("%s is abstract so it cannot be instantiated", clazz.getName()) ); } + } + + private Object bindAndInvoke(ContentType type, WebElement root, Method method, Object[] args) throws Throwable { + Widget widget = instantiationMap.get(type).newInstance(root); + PageFactory.initElements( + new AppiumFieldDecorator(new WeakReference<>(widget), duration, defaultLocatorGroupStrategy), + widget + ); + Object result = invoke(widget, method, args); + cachedElementReference = new WeakReference<>(root); + cachedInstances.put(type, widget); + return result; + } + + private static Object invoke(Widget widget, Method method, Object[] args) throws Throwable { try { method.setAccessible(true); - return method.invoke(cachedInstances.get(type), args); - } catch (Throwable t) { - throw extractReadableException(t); + return method.invoke(widget, args); + } catch (Throwable thrown) { + throw extractReadableException(thrown); + } + } + + private List furtherRoots(WebElement resolvedRoot) { + if (!(locator instanceof AppiumElementLocator)) { + return List.of(); } + return ((AppiumElementLocator) locator).findFurtherRoots(resolvedRoot); } @Override diff --git a/src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java b/src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java index 534a03916..9aff41a3f 100644 --- a/src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java +++ b/src/main/java/io/appium/java_client/pagefactory/bys/ContentMappedBy.java @@ -55,6 +55,15 @@ public By useContent(@NonNull ContentType type) { return this; } + /** + * Returns the locator selected for the current content type. + * + * @return the locator selected by {@link #useContent(ContentType)} + */ + public By getCurrentLocator() { + return map.get(currentContent); + } + @Override public WebElement findElement(SearchContext context) { return context.findElement(map.get(currentContent)); diff --git a/src/main/java/io/appium/java_client/support/pagefactory/ByAll.java b/src/main/java/io/appium/java_client/support/pagefactory/ByAll.java index 4e2def133..84fadca7b 100644 --- a/src/main/java/io/appium/java_client/support/pagefactory/ByAll.java +++ b/src/main/java/io/appium/java_client/support/pagefactory/ByAll.java @@ -49,6 +49,15 @@ public ByAll(By... bys) { this.bys = bys; } + /** + * Returns the locators searched by this strategy, in order. + * + * @return a copy of the locators + */ + public By[] getBys() { + return bys.clone(); + } + @Override public WebElement findElement(SearchContext context) { for (By by : bys) { diff --git a/src/main/java/io/appium/java_client/support/pagefactory/ByChained.java b/src/main/java/io/appium/java_client/support/pagefactory/ByChained.java index ff9c0e6b9..0b2ec57c8 100644 --- a/src/main/java/io/appium/java_client/support/pagefactory/ByChained.java +++ b/src/main/java/io/appium/java_client/support/pagefactory/ByChained.java @@ -50,6 +50,15 @@ public ByChained(By... bys) { this.bys = bys; } + /** + * Returns the locators applied by this chain, in order. + * + * @return a copy of the locators + */ + public By[] getBys() { + return bys.clone(); + } + @Override public WebElement findElement(SearchContext context) { List elements = findElements(context); diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/WidgetMultipleLocatorLookupTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/WidgetMultipleLocatorLookupTest.java new file mode 100644 index 000000000..0cd5971e7 --- /dev/null +++ b/src/test/java/io/appium/java_client/pagefactory_tests/WidgetMultipleLocatorLookupTest.java @@ -0,0 +1,373 @@ +/* + * 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_tests; + +import io.appium.java_client.pagefactory.AndroidBy; +import io.appium.java_client.pagefactory.AndroidFindAll; +import io.appium.java_client.pagefactory.AndroidFindBy; +import io.appium.java_client.pagefactory.AppiumFieldDecorator; +import io.appium.java_client.pagefactory.HowToUseLocators; +import io.appium.java_client.pagefactory.LocatorGroupStrategy; +import io.appium.java_client.pagefactory.Widget; +import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; +import io.appium.java_client.pagefactory_tests.widget.tests.StubWebElement; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; + +import static io.appium.java_client.pagefactory.LocatorGroupStrategy.ALL_POSSIBLE; +import static io.appium.java_client.support.PageFactory.initElements; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Widgets looked up with {@link LocatorGroupStrategy#ALL_POSSIBLE} must try every widget root + * together with every nested locator. See https://github.com/appium/java-client/issues/1816. + */ +public class WidgetMultipleLocatorLookupTest { + + private static final String WIDGET_1 = "widget1"; + private static final String WIDGET_2 = "widget2"; + private static final String ELEMENT_1 = "element1"; + private static final String ELEMENT_2 = "element2"; + private static final String FOUND_TEXT = "from-second-widget"; + + private static final List EVERY_COMBINATION = List.of( + lookup(WIDGET_1, ELEMENT_1), + lookup(WIDGET_1, ELEMENT_2), + lookup(WIDGET_2, ELEMENT_1), + lookup(WIDGET_2, ELEMENT_2) + ); + + /** + * The nested element exists only under the second widget locator. The first locator still + * matches a root, so both nested locators are searched there and miss. + */ + @Test + public void nestedElementIsFoundUnderALaterWidgetLocator() { + assertFoundAcrossWidgets(new SamplePage(), new SelectiveDriver(WIDGET_1, WIDGET_2)); + } + + /** + * {@code @AndroidFindAll} is the other way to declare the same set of alternative locators. + */ + @Test + public void nestedElementIsFoundUnderALaterAndroidFindAllWidget() { + assertFoundAcrossWidgets(new FindAllPage(), new SelectiveDriver(WIDGET_1, WIDGET_2)); + } + + /** + * The first widget locator matches nothing. {@code ALL_POSSIBLE} should move on to the second + * root and find the nested element there. + */ + @Test + public void nestedElementIsFoundWhenTheFirstWidgetLocatorMisses() { + SelectiveDriver driver = new SelectiveDriver(null, WIDGET_2); + SamplePage page = new SamplePage(); + initElements(new AppiumFieldDecorator(driver, Duration.ZERO), page); + + assertEquals(FOUND_TEXT, page.readNestedElement()); + assertEquals(List.of( + lookup(WIDGET_2, ELEMENT_1), + lookup(WIDGET_2, ELEMENT_2) + ), driver.lookups); + } + + /** + * A nested hit inside the first root must not search the later widget locators. + */ + @Test + public void nestedLookupStopsAtTheFirstWidgetRootThatContainsTheElement() { + SelectiveDriver driver = new SelectiveDriver(WIDGET_2, WIDGET_1); + SamplePage page = new SamplePage(); + initElements(new AppiumFieldDecorator(driver, Duration.ZERO), page); + + assertEquals(FOUND_TEXT, page.readNestedElement()); + assertEquals(List.of( + lookup(WIDGET_1, ELEMENT_1), + lookup(WIDGET_1, ELEMENT_2) + ), driver.lookups); + } + + /** + * Chained widget locators are one path, not alternative roots. + */ + @Test + public void chainedWidgetLocatorsDoNotTryLaterRoots() { + SelectiveDriver driver = new SelectiveDriver(WIDGET_1, WIDGET_2); + ChainedPage page = new ChainedPage(); + initElements(new AppiumFieldDecorator(driver, Duration.ZERO), page); + + assertThrows(NoSuchElementException.class, page::readNestedElement); + assertEquals(List.of(lookup(WIDGET_1, WIDGET_2)), driver.lookups); + } + + /** + * {@link AppiumFieldDecorator}'s default strategy is another way to select {@code ALL_POSSIBLE}. + */ + @Test + public void nestedElementIsFoundWhenAllPossibleIsTheDecoratorDefault() { + SelectiveDriver driver = new SelectiveDriver(WIDGET_1, WIDGET_2); + DefaultStrategyPage page = new DefaultStrategyPage(); + initElements(new AppiumFieldDecorator(driver, Duration.ZERO, ALL_POSSIBLE), page); + + try { + assertEquals(FOUND_TEXT, page.readNestedElement()); + } catch (NoSuchElementException e) { + fail("Nested element was not found across every widget locator. Lookups: " + + driver.lookups + ". " + e.getMessage()); + } + assertEquals(EVERY_COMBINATION, driver.lookups); + } + + private static void assertFoundAcrossWidgets(Readable page, SelectiveDriver driver) { + initElements(new AppiumFieldDecorator(driver, Duration.ZERO), page); + try { + assertEquals(FOUND_TEXT, page.readNestedElement()); + } catch (NoSuchElementException e) { + fail("Nested element was not found across every widget locator. Lookups: " + + driver.lookups + ". " + e.getMessage()); + } + assertEquals(EVERY_COMBINATION, driver.lookups); + } + + private static String lookup(String parent, String child) { + return parent + " -> " + child; + } + + private static String idOf(By by) { + String value = by.toString(); + int separator = value.lastIndexOf(": "); + return separator < 0 ? value : value.substring(separator + 2); + } + + public static class SampleWidget extends Widget { + + @HowToUseLocators(androidAutomation = ALL_POSSIBLE) + @AndroidFindBy(id = ELEMENT_1) + @AndroidFindBy(id = ELEMENT_2) + private WebElement element; + + protected SampleWidget(WebElement element) { + super(element); + } + + /** + * Reads the nested element. The lookup happens while this widget method is running. + * + * @return the nested element text + */ + public String read() { + return element.getText(); + } + } + + public static class SamplePage implements Readable { + + @HowToUseLocators(androidAutomation = ALL_POSSIBLE) + @AndroidFindBy(id = WIDGET_1) + @AndroidFindBy(id = WIDGET_2) + private SampleWidget widget; + + @Override + public String readNestedElement() { + return widget.read(); + } + } + + public static class FindAllWidget extends Widget { + + @AndroidFindAll({ + @AndroidBy(id = ELEMENT_1), + @AndroidBy(id = ELEMENT_2) + }) + private WebElement element; + + protected FindAllWidget(WebElement element) { + super(element); + } + + /** + * Reads the nested element. The lookup happens while this widget method is running. + * + * @return the nested element text + */ + public String read() { + return element.getText(); + } + } + + public static class FindAllPage implements Readable { + + @AndroidFindAll({ + @AndroidBy(id = WIDGET_1), + @AndroidBy(id = WIDGET_2) + }) + private FindAllWidget widget; + + @Override + public String readNestedElement() { + return widget.read(); + } + } + + public static class ChainedPage implements Readable { + + @AndroidFindBy(id = WIDGET_1) + @AndroidFindBy(id = WIDGET_2) + private SampleWidget widget; + + @Override + public String readNestedElement() { + return widget.read(); + } + } + + public static class DefaultStrategyWidget extends Widget { + + @AndroidFindBy(id = ELEMENT_1) + @AndroidFindBy(id = ELEMENT_2) + private WebElement element; + + protected DefaultStrategyWidget(WebElement element) { + super(element); + } + + /** + * Reads the nested element. The lookup happens while this widget method is running. + * + * @return the nested element text + */ + public String read() { + return element.getText(); + } + } + + public static class DefaultStrategyPage implements Readable { + + @AndroidFindBy(id = WIDGET_1) + @AndroidFindBy(id = WIDGET_2) + private DefaultStrategyWidget widget; + + @Override + public String readNestedElement() { + return widget.read(); + } + } + + public interface Readable { + String readNestedElement(); + } + + /** + * Driver whose roots are present or absent by id. The root named by + * {@code presentWithTarget} contains {@code element2}. {@code element1} never matches. + */ + private static final class SelectiveDriver extends AbstractStubWebDriver.StubAndroidDriver { + private final String presentWithoutTarget; + private final String presentWithTarget; + private final List lookups = new ArrayList<>(); + + private SelectiveDriver(String presentWithoutTarget, String presentWithTarget) { + this.presentWithoutTarget = presentWithoutTarget; + this.presentWithTarget = presentWithTarget; + } + + @Override + public List findElements(By by) { + return rootsFor(idOf(by)); + } + + @Override + public StubWebElement findElement(By by) { + if (!(by instanceof By.Remotable)) { + return (StubWebElement) by.findElement(this); + } + List found = findElements(by); + if (found.isEmpty()) { + throw new NoSuchElementException("Cannot locate " + by); + } + return (StubWebElement) found.get(0); + } + + private List rootsFor(String id) { + if (id.equals(presentWithoutTarget)) { + return List.of(new RootElement(this, id, false, lookups)); + } + if (id.equals(presentWithTarget)) { + return List.of(new RootElement(this, id, true, lookups)); + } + return List.of(); + } + } + + private static class RootElement extends StubWebElement { + private final String id; + private final boolean containsTarget; + private final List lookups; + + private RootElement(WebDriver driver, String id, boolean containsTarget, List lookups) { + super(driver, By.id(id)); + this.id = id; + this.containsTarget = containsTarget; + this.lookups = lookups; + } + + @Override + public List findElements(By by) { + String child = idOf(by); + lookups.add(lookup(id, child)); + if (containsTarget && ELEMENT_2.equals(child)) { + return List.of(new TextElement(getWrappedDriver(), FOUND_TEXT)); + } + return List.of(); + } + + @Override + public StubWebElement findElement(By by) { + if (!(by instanceof By.Remotable)) { + return (StubWebElement) by.findElement(this); + } + List found = findElements(by); + if (found.isEmpty()) { + throw new NoSuchElementException("Cannot locate " + by + " under " + id); + } + return (StubWebElement) found.get(0); + } + } + + private static final class TextElement extends StubWebElement { + private final String text; + + private TextElement(WebDriver driver, String text) { + super(driver, By.id("text")); + this.text = text; + } + + @Override + public String getText() { + return text; + } + } +}