From 500a2087d3c7b816c776ae8e24c7f906d498bd67 Mon Sep 17 00:00:00 2001 From: Nacy Dev Date: Mon, 2 Nov 2026 22:57:51 +0700 Subject: [PATCH 1/5] feat(pagefactory): support decorating By and AppiumBy fields --- .../pagefactory/AppiumFieldDecorator.java | 76 ++++++- .../pagefactory/DefaultElementByBuilder.java | 3 + .../ByFieldDecorationTest.java | 196 ++++++++++++++++++ 3 files changed, 269 insertions(+), 6 deletions(-) create mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java index 907e7a177..e6d4b173e 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -16,7 +16,9 @@ package io.appium.java_client.pagefactory; +import io.appium.java_client.AppiumBy; import io.appium.java_client.internal.CapabilityHelpers; +import io.appium.java_client.pagefactory.bys.ContentMappedBy; import io.appium.java_client.pagefactory.bys.ContentType; import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.remote.AppiumWebElement; @@ -27,6 +29,7 @@ import io.appium.java_client.support.pagefactory.FieldDecorator; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; +import org.openqa.selenium.By; import org.openqa.selenium.HasCapabilities; import org.openqa.selenium.SearchContext; import org.openqa.selenium.WebDriver; @@ -47,6 +50,7 @@ import static io.appium.java_client.internal.Preconditions.checkNotNull; import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; +import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.unpackObjectFromSearchContext; import static io.appium.java_client.remote.options.SupportsAutomationNameOption.AUTOMATION_NAME_OPTION; import static java.time.Duration.ofSeconds; @@ -54,12 +58,13 @@ /** * Default decorator for use with PageFactory. Will decorate 1) all of the - * WebElement fields and 2) List of WebElement that have + * WebElement fields, 2) List of WebElement that have * {@literal @AndroidFindBy}, {@literal @AndroidFindBys}, or * {@literal @iOSFindBy/@iOSFindBys} annotation with a proxy that locates the - * elements using the passed in ElementLocatorFactory. - * Please pay attention: fields of {@link WebElement} or {@link AppiumWebElement} - * to use with this decorator + * elements using the passed in ElementLocatorFactory, and 3) {@link By} / {@link AppiumBy} + * fields annotated with Appium or Selenium locator annotations. + * Please pay attention: fields of {@link WebElement}, {@link AppiumWebElement}, + * {@link By}, or {@link AppiumBy} are supported by this decorator. */ public class AppiumFieldDecorator implements FieldDecorator { @@ -71,9 +76,12 @@ public class AppiumFieldDecorator implements FieldDecorator { public static final Duration DEFAULT_WAITING_TIMEOUT = ofSeconds(1); private static final ThreadLocal CURRENT_DEFAULT_STRATEGY = ThreadLocal.withInitial(() -> LocatorGroupStrategy.CHAIN); + private final SearchContext searchContext; + private final WeakReference searchContextReference; private final WeakReference webDriverReference; private final DefaultFieldDecorator defaultElementFieldDecorator; private final AppiumElementLocatorFactory widgetLocatorFactory; + private final DefaultElementByBuilder elementByBuilder; private final String platform; private final String automation; private final Duration duration; @@ -114,6 +122,8 @@ public AppiumFieldDecorator( Duration duration, LocatorGroupStrategy defaultLocatorGroupStrategy ) { + this.searchContext = context; + this.searchContextReference = null; this.webDriverReference = requireWebDriverReference(context); this.platform = readStringCapability(context, CapabilityType.PLATFORM_NAME); this.automation = readStringCapability(context, AUTOMATION_NAME_OPTION); @@ -122,9 +132,10 @@ public AppiumFieldDecorator( defaultLocatorGroupStrategy, "Default locator group strategy must not be null" ); + this.elementByBuilder = new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy); defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( - context, duration, new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy) + context, duration, this.elementByBuilder )); widgetLocatorFactory = new AppiumElementLocatorFactory( context, duration, new WidgetByBuilder(platform, automation, this.defaultLocatorGroupStrategy) @@ -170,6 +181,8 @@ public AppiumFieldDecorator(SearchContext context, LocatorGroupStrategy defaultL LocatorGroupStrategy defaultLocatorGroupStrategy ) { var cr = contextReference.get(); + this.searchContext = null; + this.searchContextReference = contextReference; this.webDriverReference = requireWebDriverReference(cr); this.platform = readStringCapability(cr, CapabilityType.PLATFORM_NAME); this.automation = readStringCapability(cr, AUTOMATION_NAME_OPTION); @@ -178,11 +191,12 @@ public AppiumFieldDecorator(SearchContext context, LocatorGroupStrategy defaultL defaultLocatorGroupStrategy, "Default locator group strategy must not be null" ); + this.elementByBuilder = new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy); defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( contextReference, duration, - new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy) + this.elementByBuilder )); widgetLocatorFactory = new AppiumElementLocatorFactory( contextReference, @@ -265,6 +279,9 @@ public Object decorate(ClassLoader ignored, Field field) { CURRENT_DEFAULT_STRATEGY.set(defaultLocatorGroupStrategy); try { Object result = decorateWidget(field); + if (result == null) { + result = decorateBy(field); + } return result == null ? defaultElementFieldDecorator.decorate(ignored, field) : result; } finally { if (previous == LocatorGroupStrategy.CHAIN) { @@ -275,6 +292,53 @@ public Object decorate(ClassLoader ignored, Field field) { } } + @Nullable + private Object decorateBy(Field field) { + Class type = field.getType(); + if (!By.class.isAssignableFrom(type)) { + return null; + } + + elementByBuilder.setAnnotated(field); + try { + elementByBuilder.assertValidAnnotations(); + By defaultBy = elementByBuilder.buildDefaultBy(); + By mobileNativeBy = elementByBuilder.buildMobileNativeBy(); + if (defaultBy == null && mobileNativeBy == null) { + return null; + } + + if (ContentMappedBy.class.isAssignableFrom(type)) { + return elementByBuilder.buildBy(); + } + + By result; + if (defaultBy == null) { + result = mobileNativeBy; + } else if (mobileNativeBy == null) { + result = defaultBy; + } else { + SearchContext currentContext = searchContext != null + ? searchContext + : (searchContextReference != null ? searchContextReference.get() : null); + ContentType contentType = currentContext != null + ? getCurrentContentType(currentContext) + : ContentType.NATIVE_MOBILE_SPECIFIC; + result = contentType == ContentType.HTML_OR_DEFAULT ? defaultBy : mobileNativeBy; + } + + if (!type.isInstance(result)) { + throw new IllegalArgumentException(String.format( + "Field '%s' of type %s is not compatible with built locator of type %s", + field.getName(), type.getName(), result.getClass().getName() + )); + } + return result; + } finally { + elementByBuilder.setAnnotated(null); + } + } + @Nullable private Object decorateWidget(Field field) { Class type = field.getType(); diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java index b50c093ae..97cae5e53 100644 --- a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java @@ -102,6 +102,9 @@ private static By buildMobileBy(LocatorGroupStrategy locatorGroupStrategy, By[] if (bys.length == 0) { return null; } + if (bys.length == 1) { + return bys[0]; + } if (LocatorGroupStrategy.ALL_POSSIBLE.equals(locatorGroupStrategy)) { return new ByAll(bys); } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java new file mode 100644 index 000000000..34d9e6cad --- /dev/null +++ b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java @@ -0,0 +1,196 @@ +/* + * 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.AppiumBy; +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.AndroidFindBys; +import io.appium.java_client.pagefactory.AppiumFieldDecorator; +import io.appium.java_client.pagefactory.HowToUseLocators; +import io.appium.java_client.pagefactory.Widget; +import io.appium.java_client.pagefactory.bys.ContentMappedBy; +import io.appium.java_client.pagefactory.bys.builder.ByChained; +import io.appium.java_client.pagefactory.iOSXCUITBy; +import io.appium.java_client.pagefactory.iOSXCUITFindAll; +import io.appium.java_client.pagefactory.iOSXCUITFindBy; +import io.appium.java_client.pagefactory.iOSXCUITFindBys; +import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; +import io.appium.java_client.support.FindBy; +import io.appium.java_client.support.pagefactory.ByAll; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +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.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class ByFieldDecorationTest { + + public static class CrossPlatformByPage { + @AndroidFindBy(id = "something_android") + @iOSXCUITFindBy(accessibility = "something_ios") + private By somethingBy; + + @AndroidFindBy(id = "appium_android") + @iOSXCUITFindBy(accessibility = "appium_ios") + private AppiumBy somethingAppiumBy; + + @FindBy(id = "web_only") + @AndroidFindBy(id = "native_android") + @iOSXCUITFindBy(accessibility = "native_ios") + private By hybridBy; + + @FindBy(css = ".web-button") + private By webFallbackBy; + + @AndroidFindBy(id = "chain1") + @AndroidFindBy(id = "chain2") + @iOSXCUITFindBy(id = "chain1") + @iOSXCUITFindBy(id = "chain2") + private By multiLocatorBy; + + @HowToUseLocators(androidAutomation = ALL_POSSIBLE, iOSXCUITAutomation = ALL_POSSIBLE) + @AndroidFindBy(id = "any1") + @AndroidFindBy(id = "any2") + @iOSXCUITFindBy(id = "any1") + @iOSXCUITFindBy(id = "any2") + private By allPossibleBy; + + @AndroidFindBys({@AndroidBy(id = "parent"), @AndroidBy(id = "child")}) + @iOSXCUITFindBys({@iOSXCUITBy(id = "parent"), @iOSXCUITBy(id = "child")}) + private By findBysField; + + @AndroidFindAll({@AndroidBy(id = "opt1"), @AndroidBy(id = "opt2")}) + @iOSXCUITFindAll({@iOSXCUITBy(id = "opt1"), @iOSXCUITBy(id = "opt2")}) + private By findAllField; + + @FindBy(id = "mapped_web") + @AndroidFindBy(id = "mapped_android") + private ContentMappedBy contentMappedBy; + + @AndroidFindBy(id = "widget_root") + @iOSXCUITFindBy(id = "widget_root") + private ByHoldingWidget widget; + + private final By preInitializedBy = By.id("constant_id"); + + private By unannotatedNullBy; + } + + public static class ByHoldingWidget extends Widget { + @AndroidFindBy(accessibility = "inner_android") + @iOSXCUITFindBy(iOSNsPredicate = "name == 'inner_ios'") + private AppiumBy innerAppiumBy; + + protected ByHoldingWidget(WebElement element) { + super(element); + } + + public AppiumBy getInnerAppiumBy() { + return innerAppiumBy; + } + } + + public static class IncompatibleByTypePage { + @AndroidFindBy(xpath = "//android.widget.TextView") + private AppiumBy xpathAssignedToAppiumBy; + } + + @Test + public void decoratesByAndAppiumByFieldsOnAndroid() { + CrossPlatformByPage page = new CrossPlatformByPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertEquals(AppiumBy.id("something_android"), page.somethingBy); + assertEquals(AppiumBy.id("appium_android"), page.somethingAppiumBy); + assertEquals(AppiumBy.id("native_android"), page.hybridBy); + assertEquals(By.cssSelector(".web-button"), page.webFallbackBy); + assertInstanceOf(ByChained.class, page.multiLocatorBy); + assertInstanceOf(ByAll.class, page.allPossibleBy); + assertInstanceOf(ByChained.class, page.findBysField); + assertInstanceOf(ByAll.class, page.findAllField); + assertNotNull(page.contentMappedBy); + assertEquals(AppiumBy.accessibilityId("inner_android"), page.widget.getInnerAppiumBy()); + assertEquals(By.id("constant_id"), page.preInitializedBy); + assertNull(page.unannotatedNullBy); + } + + @Test + public void decoratesByAndAppiumByFieldsOnIOS() { + CrossPlatformByPage page = new CrossPlatformByPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubIOSXCUITDriver()), page); + + assertEquals(AppiumBy.accessibilityId("something_ios"), page.somethingBy); + assertEquals(AppiumBy.accessibilityId("appium_ios"), page.somethingAppiumBy); + assertEquals(AppiumBy.accessibilityId("native_ios"), page.hybridBy); + assertEquals(By.cssSelector(".web-button"), page.webFallbackBy); + assertInstanceOf(ByChained.class, page.multiLocatorBy); + assertInstanceOf(ByAll.class, page.allPossibleBy); + assertInstanceOf(ByChained.class, page.findBysField); + assertInstanceOf(ByAll.class, page.findAllField); + assertEquals(AppiumBy.iOSNsPredicateString("name == 'inner_ios'"), page.widget.getInnerAppiumBy()); + assertEquals(By.id("constant_id"), page.preInitializedBy); + assertNull(page.unannotatedNullBy); + } + + @Test + public void selectsDefaultByInBrowserOrWebViewContext() { + CrossPlatformByPage browserPage = new CrossPlatformByPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubBrowserDriver()), browserPage); + + assertNull(browserPage.somethingBy); + assertNull(browserPage.somethingAppiumBy); + assertEquals(By.id("web_only"), browserPage.hybridBy); + assertEquals(By.cssSelector(".web-button"), browserPage.webFallbackBy); + assertEquals(By.id("constant_id"), browserPage.preInitializedBy); + + CrossPlatformByPage webViewPage = new CrossPlatformByPage(); + initElements( + new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidBrowserOrWebViewDriver()), + webViewPage + ); + assertEquals(By.id("web_only"), webViewPage.hybridBy); + assertEquals(AppiumBy.id("something_android"), webViewPage.somethingBy); + } + + @Test + public void respectsProgrammaticDefaultLocatorGroupStrategyForByFields() { + CrossPlatformByPage page = new CrossPlatformByPage(); + initElements( + new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver(), ALL_POSSIBLE), + page + ); + assertInstanceOf(ByAll.class, page.multiLocatorBy); + } + + @Test + public void throwsWhenBuiltByDoesNotMatchDeclaredSubType() { + IncompatibleByTypePage page = new IncompatibleByTypePage(); + assertThrows( + IllegalArgumentException.class, + () -> initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page) + ); + } +} + From 1ea13d053d5b73d8763f852a28adfd7bb81c5e98 Mon Sep 17 00:00:00 2001 From: Nacy Dev Date: Tue, 3 Nov 2026 00:19:21 +0700 Subject: [PATCH 2/5] fix(pagefactory): use lazy ContentMappedBy resolution and address review notes --- .../pagefactory/AppiumFieldDecorator.java | 112 ++++-- .../pagefactory/DefaultElementByBuilder.java | 3 - .../pagefactory/bys/ContentMappedBy.java | 3 + .../java_client/support/PageFactory.java | 23 +- .../ByFieldDecorationTest.java | 339 +++++++++++++++--- 5 files changed, 391 insertions(+), 89 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java index e6d4b173e..8f486a428 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -23,6 +23,12 @@ import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.remote.AppiumWebElement; import io.appium.java_client.remote.CapabilityType; +import io.appium.java_client.support.FindAll; +import io.appium.java_client.support.FindBy; +import io.appium.java_client.support.FindBys; +import io.appium.java_client.support.PageFactory; +import io.appium.java_client.support.pagefactory.ByAll; +import io.appium.java_client.support.pagefactory.ByChained; import io.appium.java_client.support.pagefactory.DefaultFieldDecorator; import io.appium.java_client.support.pagefactory.ElementLocator; import io.appium.java_client.support.pagefactory.ElementLocatorFactory; @@ -38,6 +44,7 @@ import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; @@ -50,7 +57,6 @@ import static io.appium.java_client.internal.Preconditions.checkNotNull; import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; -import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.unpackObjectFromSearchContext; import static io.appium.java_client.remote.options.SupportsAutomationNameOption.AUTOMATION_NAME_OPTION; import static java.time.Duration.ofSeconds; @@ -76,8 +82,6 @@ public class AppiumFieldDecorator implements FieldDecorator { public static final Duration DEFAULT_WAITING_TIMEOUT = ofSeconds(1); private static final ThreadLocal CURRENT_DEFAULT_STRATEGY = ThreadLocal.withInitial(() -> LocatorGroupStrategy.CHAIN); - private final SearchContext searchContext; - private final WeakReference searchContextReference; private final WeakReference webDriverReference; private final DefaultFieldDecorator defaultElementFieldDecorator; private final AppiumElementLocatorFactory widgetLocatorFactory; @@ -122,8 +126,6 @@ public AppiumFieldDecorator( Duration duration, LocatorGroupStrategy defaultLocatorGroupStrategy ) { - this.searchContext = context; - this.searchContextReference = null; this.webDriverReference = requireWebDriverReference(context); this.platform = readStringCapability(context, CapabilityType.PLATFORM_NAME); this.automation = readStringCapability(context, AUTOMATION_NAME_OPTION); @@ -181,8 +183,6 @@ public AppiumFieldDecorator(SearchContext context, LocatorGroupStrategy defaultL LocatorGroupStrategy defaultLocatorGroupStrategy ) { var cr = contextReference.get(); - this.searchContext = null; - this.searchContextReference = contextReference; this.webDriverReference = requireWebDriverReference(cr); this.platform = readStringCapability(cr, CapabilityType.PLATFORM_NAME); this.automation = readStringCapability(cr, AUTOMATION_NAME_OPTION); @@ -295,43 +295,30 @@ public Object decorate(ClassLoader ignored, Field field) { @Nullable private Object decorateBy(Field field) { Class type = field.getType(); - if (!By.class.isAssignableFrom(type)) { + int modifiers = field.getModifiers(); + if (!By.class.isAssignableFrom(type) + || Modifier.isStatic(modifiers) + || Modifier.isFinal(modifiers) + || isFieldAlreadyInitialized(field)) { return null; } elementByBuilder.setAnnotated(field); try { - elementByBuilder.assertValidAnnotations(); - By defaultBy = elementByBuilder.buildDefaultBy(); - By mobileNativeBy = elementByBuilder.buildMobileNativeBy(); - if (defaultBy == null && mobileNativeBy == null) { - return null; - } - if (ContentMappedBy.class.isAssignableFrom(type)) { return elementByBuilder.buildBy(); } - - By result; - if (defaultBy == null) { - result = mobileNativeBy; - } else if (mobileNativeBy == null) { - result = defaultBy; - } else { - SearchContext currentContext = searchContext != null - ? searchContext - : (searchContextReference != null ? searchContextReference.get() : null); - ContentType contentType = currentContext != null - ? getCurrentContentType(currentContext) - : ContentType.NATIVE_MOBILE_SPECIFIC; - result = contentType == ContentType.HTML_OR_DEFAULT ? defaultBy : mobileNativeBy; + if (!hasLocatorAnnotations(field)) { + return null; + } + if (type.isAssignableFrom(ContentMappedBy.class)) { + return elementByBuilder.buildBy(); } - if (!type.isInstance(result)) { - throw new IllegalArgumentException(String.format( - "Field '%s' of type %s is not compatible with built locator of type %s", - field.getName(), type.getName(), result.getClass().getName() - )); + elementByBuilder.assertValidAnnotations(); + By result = coerceToType(type, elementByBuilder.buildMobileNativeBy()); + if (result == null) { + result = coerceToType(type, elementByBuilder.buildDefaultBy()); } return result; } finally { @@ -339,6 +326,63 @@ private Object decorateBy(Field field) { } } + private static boolean isFieldAlreadyInitialized(Field field) { + Object currentPage = PageFactory.getCurrentPage(); + if (currentPage == null || !field.getDeclaringClass().isInstance(currentPage)) { + return false; + } + try { + field.setAccessible(true); + return field.get(currentPage) != null; + } catch (IllegalAccessException | RuntimeException e) { + return false; + } + } + + private static boolean hasLocatorAnnotations(Field field) { + return SeleniumAnnotationsCompat.isPresent(field, FindBy.class) + || SeleniumAnnotationsCompat.isPresent(field, FindBys.class) + || SeleniumAnnotationsCompat.isPresent(field, FindAll.class) + || field.getAnnotationsByType(AndroidFindBy.class).length > 0 + || field.getAnnotationsByType(AndroidFindBys.class).length > 0 + || field.getAnnotationsByType(AndroidFindAll.class).length > 0 + || field.getAnnotationsByType(iOSXCUITFindBy.class).length > 0 + || field.getAnnotationsByType(iOSXCUITFindBys.class).length > 0 + || field.getAnnotationsByType(iOSXCUITFindAll.class).length > 0; + } + + @Nullable + private static By coerceToType(Class type, @Nullable By candidate) { + if (candidate == null) { + return null; + } + if (type.isInstance(candidate)) { + return candidate; + } + By unwrapped = unwrapSingleBy(candidate); + return type.isInstance(unwrapped) ? unwrapped : null; + } + + @Nullable + private static By unwrapSingleBy(@Nullable By by) { + By current = by; + while (current instanceof ByChained || current instanceof ByAll) { + Class holderClass = current instanceof ByAll ? ByAll.class : ByChained.class; + try { + Field bysField = holderClass.getDeclaredField("bys"); + bysField.setAccessible(true); + By[] bys = (By[]) bysField.get(current); + if (bys == null || bys.length != 1) { + break; + } + current = bys[0]; + } catch (ReflectiveOperationException e) { + break; + } + } + return current; + } + @Nullable private Object decorateWidget(Field field) { Class type = field.getType(); diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java index 97cae5e53..b50c093ae 100644 --- a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java @@ -102,9 +102,6 @@ private static By buildMobileBy(LocatorGroupStrategy locatorGroupStrategy, By[] if (bys.length == 0) { return null; } - if (bys.length == 1) { - return bys[0]; - } if (LocatorGroupStrategy.ALL_POSSIBLE.equals(locatorGroupStrategy)) { return new ByAll(bys); } 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..1a9ddba88 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 @@ -26,6 +26,7 @@ import java.util.Map; import static io.appium.java_client.pagefactory.bys.ContentType.NATIVE_MOBILE_SPECIFIC; +import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; import static java.util.Objects.requireNonNull; /** Locator which selects the underlying locator by the current content type. */ @@ -57,11 +58,13 @@ public By useContent(@NonNull ContentType type) { @Override public WebElement findElement(SearchContext context) { + useContent(getCurrentContentType(context)); return context.findElement(map.get(currentContent)); } @Override public List findElements(SearchContext context) { + useContent(getCurrentContentType(context)); return context.findElements(map.get(currentContent)); } diff --git a/src/main/java/io/appium/java_client/support/PageFactory.java b/src/main/java/io/appium/java_client/support/PageFactory.java index b469fd281..c14d80963 100644 --- a/src/main/java/io/appium/java_client/support/PageFactory.java +++ b/src/main/java/io/appium/java_client/support/PageFactory.java @@ -31,7 +31,7 @@ */ public final class PageFactory { - private static final ThreadLocal>> PAGE_CLASS_STACK = + private static final ThreadLocal> PAGE_STACK = ThreadLocal.withInitial(ArrayDeque::new); private PageFactory() { @@ -46,7 +46,20 @@ private PageFactory() { */ @Nullable public static Class getCurrentPageClass() { - return PAGE_CLASS_STACK.get().peek(); + Object page = PAGE_STACK.get().peek(); + return page == null ? null : page.getClass(); + } + + /** + * Returns the page object currently being initialized by + * {@link #initElements(FieldDecorator, Object)} on the current thread, or {@code null} + * if no page initialization is active. + * + * @return the current page object or {@code null} + */ + @Nullable + public static Object getCurrentPage() { + return PAGE_STACK.get().peek(); } /** @@ -69,8 +82,8 @@ public static void initElements(ElementLocatorFactory factory, Object page) { * @param page The object to decorate the fields of */ public static void initElements(FieldDecorator decorator, Object page) { - Deque> stack = PAGE_CLASS_STACK.get(); - stack.push(page.getClass()); + Deque stack = PAGE_STACK.get(); + stack.push(page); try { Class proxyIn = page.getClass(); while (proxyIn != Object.class) { @@ -80,7 +93,7 @@ public static void initElements(FieldDecorator decorator, Object page) { } finally { stack.pop(); if (stack.isEmpty()) { - PAGE_CLASS_STACK.remove(); + PAGE_STACK.remove(); } } } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java index 34d9e6cad..c391a83f9 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java @@ -24,6 +24,7 @@ import io.appium.java_client.pagefactory.AppiumFieldDecorator; import io.appium.java_client.pagefactory.HowToUseLocators; import io.appium.java_client.pagefactory.Widget; +import io.appium.java_client.pagefactory.WithTimeout; import io.appium.java_client.pagefactory.bys.ContentMappedBy; import io.appium.java_client.pagefactory.bys.builder.ByChained; import io.appium.java_client.pagefactory.iOSXCUITBy; @@ -31,22 +32,161 @@ import io.appium.java_client.pagefactory.iOSXCUITFindBy; import io.appium.java_client.pagefactory.iOSXCUITFindBys; import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; +import io.appium.java_client.pagefactory_tests.widget.tests.StubWebElement; +import io.appium.java_client.remote.SupportsContextSwitching; +import io.appium.java_client.support.ByIdOrName; +import io.appium.java_client.support.CacheLookup; +import io.appium.java_client.support.FindAll; import io.appium.java_client.support.FindBy; +import io.appium.java_client.support.FindBys; import io.appium.java_client.support.pagefactory.ByAll; import org.junit.jupiter.api.Test; import org.openqa.selenium.By; +import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import java.time.temporal.ChronoUnit; +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.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; public class ByFieldDecorationTest { + private interface ByRecorder { + List recordedBys(); + } + + private static class RecordingStubElement extends StubWebElement { + private final List recordedBys; + + RecordingStubElement(WebDriver driver, By by, List recordedBys) { + super(driver, by); + this.recordedBys = recordedBys; + } + + @Override + public RecordingStubElement findElement(By by) { + if (by instanceof ContentMappedBy) { + return (RecordingStubElement) by.findElement(this); + } + recordedBys.add(by); + return new RecordingStubElement(getWrappedDriver(), by, recordedBys); + } + + @Override + public List findElements(By by) { + if (by instanceof ContentMappedBy) { + return by.findElements(this); + } + recordedBys.add(by); + return List.of( + new RecordingStubElement(getWrappedDriver(), by, recordedBys), + new RecordingStubElement(getWrappedDriver(), by, recordedBys) + ); + } + } + + private static class ContextSwitchingAndroidDriver + extends AbstractStubWebDriver.StubAndroidDriver + implements SupportsContextSwitching, ByRecorder { + private final List recordedBys = new ArrayList<>(); + private String currentContext = "NATIVE_APP"; + private int contextCheckCount; + + @Override + public List recordedBys() { + return recordedBys; + } + + @Override + public WebDriver context(String name) { + this.currentContext = name; + return this; + } + + @Override + public boolean isBrowser() { + contextCheckCount++; + return !"NATIVE_APP".equals(currentContext); + } + + @Override + public String getContext() { + contextCheckCount++; + return currentContext; + } + + int getContextCheckCount() { + return contextCheckCount; + } + + @Override + public StubWebElement findElement(By by) { + if (by instanceof ContentMappedBy) { + return (StubWebElement) by.findElement(this); + } + recordedBys.add(by); + return new RecordingStubElement(this, by, recordedBys); + } + + @Override + public List findElements(By by) { + if (by instanceof ContentMappedBy) { + return by.findElements(this); + } + recordedBys.add(by); + return List.of( + new RecordingStubElement(this, by, recordedBys), + new RecordingStubElement(this, by, recordedBys) + ); + } + } + + private static class RecordingIOSDriver + extends AbstractStubWebDriver.StubIOSXCUITDriver + implements ByRecorder { + private final List recordedBys = new ArrayList<>(); + + @Override + public List recordedBys() { + return recordedBys; + } + + @Override + public StubWebElement findElement(By by) { + if (by instanceof ContentMappedBy) { + return (StubWebElement) by.findElement(this); + } + recordedBys.add(by); + return new RecordingStubElement(this, by, recordedBys); + } + + @Override + public List findElements(By by) { + if (by instanceof ContentMappedBy) { + return by.findElements(this); + } + recordedBys.add(by); + return List.of( + new RecordingStubElement(this, by, recordedBys), + new RecordingStubElement(this, by, recordedBys) + ); + } + } + + private static By lastRecordedBy(ByRecorder recorder, Runnable lookup) { + recorder.recordedBys().clear(); + lookup.run(); + List bys = recorder.recordedBys(); + return bys.get(bys.size() - 1); + } + public static class CrossPlatformByPage { @AndroidFindBy(id = "something_android") @iOSXCUITFindBy(accessibility = "something_ios") @@ -68,32 +208,47 @@ public static class CrossPlatformByPage { @AndroidFindBy(id = "chain2") @iOSXCUITFindBy(id = "chain1") @iOSXCUITFindBy(id = "chain2") - private By multiLocatorBy; + private ByChained multiLocatorBy; @HowToUseLocators(androidAutomation = ALL_POSSIBLE, iOSXCUITAutomation = ALL_POSSIBLE) @AndroidFindBy(id = "any1") @AndroidFindBy(id = "any2") @iOSXCUITFindBy(id = "any1") @iOSXCUITFindBy(id = "any2") - private By allPossibleBy; + private ByAll allPossibleBy; @AndroidFindBys({@AndroidBy(id = "parent"), @AndroidBy(id = "child")}) @iOSXCUITFindBys({@iOSXCUITBy(id = "parent"), @iOSXCUITBy(id = "child")}) - private By findBysField; + private ByChained findBysField; @AndroidFindAll({@AndroidBy(id = "opt1"), @AndroidBy(id = "opt2")}) @iOSXCUITFindAll({@iOSXCUITBy(id = "opt1"), @iOSXCUITBy(id = "opt2")}) - private By findAllField; + private ByAll findAllField; @FindBy(id = "mapped_web") @AndroidFindBy(id = "mapped_android") private ContentMappedBy contentMappedBy; + private ContentMappedBy unannotatedContentMappedBy; + + @CacheLookup + @WithTimeout(time = 5, chronoUnit = ChronoUnit.SECONDS) + @FindBy(id = "cached_web") + @AndroidFindBy(id = "cached_android") + private By cachedAndTimedBy; + @AndroidFindBy(id = "widget_root") @iOSXCUITFindBy(id = "widget_root") private ByHoldingWidget widget; - private final By preInitializedBy = By.id("constant_id"); + @FindBy(id = "ignored_final") + private final By finalAnnotatedBy = By.id("constant_id"); + + @FindBy(id = "ignored_static") + private static By staticAnnotatedBy = By.id("static_id"); + + @FindBy(id = "ignored_preinit") + private By preInitializedAnnotatedBy = By.id("preserved_id"); private By unannotatedNullBy; } @@ -112,85 +267,175 @@ public AppiumBy getInnerAppiumBy() { } } - public static class IncompatibleByTypePage { + public static class IncompatibleBySubTypePage { @AndroidFindBy(xpath = "//android.widget.TextView") private AppiumBy xpathAssignedToAppiumBy; + + @AndroidFindBy(id = "first") + @AndroidFindBy(id = "second") + private AppiumBy chainedAssignedToAppiumBy; + + @AndroidFindBy(id = "valid_android") + private AppiumBy validAppiumBy; + } + + public static class ConflictingSeleniumAnnotationsByPage { + @FindBy(id = "one") + @FindBys({@FindBy(id = "two"), @FindBy(id = "three")}) + private By conflictingBy; + } + + public static class ConflictingSeleniumOnAppiumByPage { + @FindBy(id = "one") + @FindAll({@FindBy(id = "two"), @FindBy(id = "three")}) + private AppiumBy conflictingAppiumBy; } @Test - public void decoratesByAndAppiumByFieldsOnAndroid() { + public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { + ContextSwitchingAndroidDriver driver = new ContextSwitchingAndroidDriver(); CrossPlatformByPage page = new CrossPlatformByPage(); - initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + initElements(new AppiumFieldDecorator(driver), page); + + // No remote context round-trips happen at initElements time + assertEquals(0, driver.getContextCheckCount()); + + assertInstanceOf(ContentMappedBy.class, page.somethingBy); + assertInstanceOf(ContentMappedBy.class, page.hybridBy); + assertInstanceOf(ContentMappedBy.class, page.webFallbackBy); + assertInstanceOf(ContentMappedBy.class, page.contentMappedBy); + assertInstanceOf(ContentMappedBy.class, page.unannotatedContentMappedBy); + assertInstanceOf(ContentMappedBy.class, page.cachedAndTimedBy); - assertEquals(AppiumBy.id("something_android"), page.somethingBy); assertEquals(AppiumBy.id("appium_android"), page.somethingAppiumBy); - assertEquals(AppiumBy.id("native_android"), page.hybridBy); - assertEquals(By.cssSelector(".web-button"), page.webFallbackBy); assertInstanceOf(ByChained.class, page.multiLocatorBy); assertInstanceOf(ByAll.class, page.allPossibleBy); assertInstanceOf(ByChained.class, page.findBysField); assertInstanceOf(ByAll.class, page.findAllField); - assertNotNull(page.contentMappedBy); assertEquals(AppiumBy.accessibilityId("inner_android"), page.widget.getInnerAppiumBy()); - assertEquals(By.id("constant_id"), page.preInitializedBy); + + // Pre-initialized, final, static, and unannotated By fields are left untouched + assertEquals(By.id("constant_id"), page.finalAnnotatedBy); + assertEquals(By.id("static_id"), CrossPlatformByPage.staticAnnotatedBy); + assertEquals(By.id("preserved_id"), page.preInitializedAnnotatedBy); assertNull(page.unannotatedNullBy); + + // In NATIVE_APP context, hybridBy and somethingBy resolve to native locators + assertEquals( + new ByChained(new By[]{AppiumBy.id("native_android")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.hybridBy)).toString() + ); + assertEquals( + new ByChained(new By[]{AppiumBy.id("something_android")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() + ); + assertEquals( + By.id("unannotatedContentMappedBy"), + lastRecordedBy(driver, () -> driver.findElement(page.unannotatedContentMappedBy)) + ); + + // Switching to WEBVIEW_1 updates the same By and ContentMappedBy fields lazily + driver.context("WEBVIEW_1"); + assertEquals( + By.id("web_only"), + lastRecordedBy(driver, () -> driver.findElement(page.hybridBy)) + ); + assertEquals( + By.id("web_only"), + lastRecordedBy(driver, () -> driver.findElements(page.hybridBy)) + ); + assertEquals( + By.id("mapped_web"), + lastRecordedBy(driver, () -> driver.findElement(page.contentMappedBy)) + ); + // Field with only @AndroidFindBy falls back to ByIdOrName in webview context + assertEquals( + new ByIdOrName("somethingBy").toString(), + lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() + ); + assertEquals( + new ByIdOrName("unannotatedContentMappedBy").toString(), + lastRecordedBy(driver, () -> driver.findElement(page.unannotatedContentMappedBy)).toString() + ); + + // Switching back to NATIVE_APP restores native locators + driver.context("NATIVE_APP"); + assertEquals( + new ByChained(new By[]{AppiumBy.id("native_android")}).toString(), + lastRecordedBy(driver, () -> driver.findElements(page.hybridBy)).toString() + ); } @Test public void decoratesByAndAppiumByFieldsOnIOS() { + RecordingIOSDriver driver = new RecordingIOSDriver(); CrossPlatformByPage page = new CrossPlatformByPage(); - initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubIOSXCUITDriver()), page); + initElements(new AppiumFieldDecorator(driver), page); - assertEquals(AppiumBy.accessibilityId("something_ios"), page.somethingBy); assertEquals(AppiumBy.accessibilityId("appium_ios"), page.somethingAppiumBy); - assertEquals(AppiumBy.accessibilityId("native_ios"), page.hybridBy); - assertEquals(By.cssSelector(".web-button"), page.webFallbackBy); + assertEquals( + new ByChained(new By[]{AppiumBy.accessibilityId("something_ios")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() + ); + assertEquals( + new ByChained(new By[]{AppiumBy.accessibilityId("native_ios")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.hybridBy)).toString() + ); + assertEquals( + By.cssSelector(".web-button"), + lastRecordedBy(driver, () -> driver.findElement(page.webFallbackBy)) + ); assertInstanceOf(ByChained.class, page.multiLocatorBy); assertInstanceOf(ByAll.class, page.allPossibleBy); assertInstanceOf(ByChained.class, page.findBysField); assertInstanceOf(ByAll.class, page.findAllField); assertEquals(AppiumBy.iOSNsPredicateString("name == 'inner_ios'"), page.widget.getInnerAppiumBy()); - assertEquals(By.id("constant_id"), page.preInitializedBy); + assertEquals(By.id("constant_id"), page.finalAnnotatedBy); + assertEquals(By.id("preserved_id"), page.preInitializedAnnotatedBy); assertNull(page.unannotatedNullBy); } - @Test - public void selectsDefaultByInBrowserOrWebViewContext() { - CrossPlatformByPage browserPage = new CrossPlatformByPage(); - initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubBrowserDriver()), browserPage); - - assertNull(browserPage.somethingBy); - assertNull(browserPage.somethingAppiumBy); - assertEquals(By.id("web_only"), browserPage.hybridBy); - assertEquals(By.cssSelector(".web-button"), browserPage.webFallbackBy); - assertEquals(By.id("constant_id"), browserPage.preInitializedBy); - - CrossPlatformByPage webViewPage = new CrossPlatformByPage(); - initElements( - new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidBrowserOrWebViewDriver()), - webViewPage - ); - assertEquals(By.id("web_only"), webViewPage.hybridBy); - assertEquals(AppiumBy.id("something_android"), webViewPage.somethingBy); - } - @Test public void respectsProgrammaticDefaultLocatorGroupStrategyForByFields() { + ContextSwitchingAndroidDriver driver = new ContextSwitchingAndroidDriver(); CrossPlatformByPage page = new CrossPlatformByPage(); - initElements( - new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver(), ALL_POSSIBLE), - page + initElements(new AppiumFieldDecorator(driver, ALL_POSSIBLE), page); + + assertEquals( + new ByAll(new By[]{AppiumBy.id("native_android")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.hybridBy)).toString() ); - assertInstanceOf(ByAll.class, page.multiLocatorBy); + assertEquals( + new ByAll(new By[]{AppiumBy.id("something_android")}).toString(), + lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() + ); + } + + @Test + public void leavesIncompatibleNarrowSubTypeFieldsNullWithoutAbortingPageInit() { + IncompatibleBySubTypePage page = new IncompatibleBySubTypePage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertNull(page.xpathAssignedToAppiumBy); + assertNull(page.chainedAssignedToAppiumBy); + assertEquals(AppiumBy.id("valid_android"), page.validAppiumBy); } @Test - public void throwsWhenBuiltByDoesNotMatchDeclaredSubType() { - IncompatibleByTypePage page = new IncompatibleByTypePage(); + public void throwsOnConflictingAnnotationsOnByFields() { assertThrows( IllegalArgumentException.class, - () -> initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page) + () -> initElements( + new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), + new ConflictingSeleniumAnnotationsByPage() + ) + ); + assertThrows( + IllegalArgumentException.class, + () -> initElements( + new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), + new ConflictingSeleniumOnAppiumByPage() + ) ); } } - From 342f11a82e8c97204d01adcc357062694e68e7a3 Mon Sep 17 00:00:00 2001 From: Nacy Dev Date: Tue, 3 Nov 2026 04:28:43 +0700 Subject: [PATCH 3/5] fix(pagefactory): make By decoration thread-safe and preserve explicit content types --- .../pagefactory/AppiumFieldDecorator.java | 93 ++++++------------- .../pagefactory/DefaultElementByBuilder.java | 16 ++++ .../pagefactory/bys/ContentMappedBy.java | 18 ++-- .../java_client/support/PageFactory.java | 23 +---- .../support/pagefactory/ByAll.java | 9 ++ .../support/pagefactory/ByChained.java | 9 ++ .../ByFieldDecorationTest.java | 60 +++++++++--- 7 files changed, 124 insertions(+), 104 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java index 8f486a428..eb2ed7e7c 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -23,10 +23,6 @@ import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.remote.AppiumWebElement; import io.appium.java_client.remote.CapabilityType; -import io.appium.java_client.support.FindAll; -import io.appium.java_client.support.FindBy; -import io.appium.java_client.support.FindBys; -import io.appium.java_client.support.PageFactory; import io.appium.java_client.support.pagefactory.ByAll; import io.appium.java_client.support.pagefactory.ByChained; import io.appium.java_client.support.pagefactory.DefaultFieldDecorator; @@ -85,7 +81,6 @@ public class AppiumFieldDecorator implements FieldDecorator { private final WeakReference webDriverReference; private final DefaultFieldDecorator defaultElementFieldDecorator; private final AppiumElementLocatorFactory widgetLocatorFactory; - private final DefaultElementByBuilder elementByBuilder; private final String platform; private final String automation; private final Duration duration; @@ -134,10 +129,9 @@ public AppiumFieldDecorator( defaultLocatorGroupStrategy, "Default locator group strategy must not be null" ); - this.elementByBuilder = new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy); defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( - context, duration, this.elementByBuilder + context, duration, new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy) )); widgetLocatorFactory = new AppiumElementLocatorFactory( context, duration, new WidgetByBuilder(platform, automation, this.defaultLocatorGroupStrategy) @@ -191,12 +185,11 @@ public AppiumFieldDecorator(SearchContext context, LocatorGroupStrategy defaultL defaultLocatorGroupStrategy, "Default locator group strategy must not be null" ); - this.elementByBuilder = new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy); defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( contextReference, duration, - this.elementByBuilder + new DefaultElementByBuilder(platform, automation, this.defaultLocatorGroupStrategy) )); widgetLocatorFactory = new AppiumElementLocatorFactory( contextReference, @@ -298,57 +291,27 @@ private Object decorateBy(Field field) { int modifiers = field.getModifiers(); if (!By.class.isAssignableFrom(type) || Modifier.isStatic(modifiers) - || Modifier.isFinal(modifiers) - || isFieldAlreadyInitialized(field)) { + || Modifier.isFinal(modifiers)) { return null; } - elementByBuilder.setAnnotated(field); - try { - if (ContentMappedBy.class.isAssignableFrom(type)) { - return elementByBuilder.buildBy(); - } - if (!hasLocatorAnnotations(field)) { - return null; - } - if (type.isAssignableFrom(ContentMappedBy.class)) { - return elementByBuilder.buildBy(); - } - - elementByBuilder.assertValidAnnotations(); - By result = coerceToType(type, elementByBuilder.buildMobileNativeBy()); - if (result == null) { - result = coerceToType(type, elementByBuilder.buildDefaultBy()); - } - return result; - } finally { - elementByBuilder.setAnnotated(null); - } - } - - private static boolean isFieldAlreadyInitialized(Field field) { - Object currentPage = PageFactory.getCurrentPage(); - if (currentPage == null || !field.getDeclaringClass().isInstance(currentPage)) { - return false; + DefaultElementByBuilder builder = new DefaultElementByBuilder( + platform, automation, defaultLocatorGroupStrategy + ); + builder.setAnnotated(field); + if (!builder.hasLocatorAnnotations()) { + return null; } - try { - field.setAccessible(true); - return field.get(currentPage) != null; - } catch (IllegalAccessException | RuntimeException e) { - return false; + if (type.isAssignableFrom(ContentMappedBy.class)) { + return builder.buildBy(); } - } - private static boolean hasLocatorAnnotations(Field field) { - return SeleniumAnnotationsCompat.isPresent(field, FindBy.class) - || SeleniumAnnotationsCompat.isPresent(field, FindBys.class) - || SeleniumAnnotationsCompat.isPresent(field, FindAll.class) - || field.getAnnotationsByType(AndroidFindBy.class).length > 0 - || field.getAnnotationsByType(AndroidFindBys.class).length > 0 - || field.getAnnotationsByType(AndroidFindAll.class).length > 0 - || field.getAnnotationsByType(iOSXCUITFindBy.class).length > 0 - || field.getAnnotationsByType(iOSXCUITFindBys.class).length > 0 - || field.getAnnotationsByType(iOSXCUITFindAll.class).length > 0; + builder.assertValidAnnotations(); + By result = coerceToType(type, builder.buildMobileNativeBy()); + if (result == null) { + result = coerceToType(type, builder.buildDefaultBy()); + } + return result; } @Nullable @@ -366,19 +329,19 @@ private static By coerceToType(Class type, @Nullable By candidate) { @Nullable private static By unwrapSingleBy(@Nullable By by) { By current = by; - while (current instanceof ByChained || current instanceof ByAll) { - Class holderClass = current instanceof ByAll ? ByAll.class : ByChained.class; - try { - Field bysField = holderClass.getDeclaredField("bys"); - bysField.setAccessible(true); - By[] bys = (By[]) bysField.get(current); - if (bys == null || bys.length != 1) { - break; - } - current = bys[0]; - } catch (ReflectiveOperationException e) { + while (true) { + By[] bys; + if (current instanceof ByChained) { + bys = ((ByChained) current).getBys(); + } else if (current instanceof ByAll) { + bys = ((ByAll) current).getBys(); + } else { + break; + } + if (bys.length != 1) { break; } + current = bys[0]; } return current; } diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java index b50c093ae..9fe174c7d 100644 --- a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java @@ -322,6 +322,22 @@ public boolean isLookupCached() { return SeleniumAnnotationsCompat.isPresent(annotatedElement, CacheLookup.class); } + boolean hasLocatorAnnotations() { + AnnotatedElement annotatedElement = annotatedElementContainer.getAnnotated(); + if (annotatedElement == null) { + return false; + } + return SeleniumAnnotationsCompat.isPresent(annotatedElement, FindBy.class) + || SeleniumAnnotationsCompat.isPresent(annotatedElement, FindBys.class) + || SeleniumAnnotationsCompat.isPresent(annotatedElement, FindAll.class) + || annotatedElement.getAnnotationsByType(AndroidFindBy.class).length > 0 + || annotatedElement.getAnnotationsByType(AndroidFindBys.class).length > 0 + || annotatedElement.getAnnotationsByType(AndroidFindAll.class).length > 0 + || annotatedElement.getAnnotationsByType(iOSXCUITFindBy.class).length > 0 + || annotatedElement.getAnnotationsByType(iOSXCUITFindBys.class).length > 0 + || annotatedElement.getAnnotationsByType(iOSXCUITFindAll.class).length > 0; + } + private By returnMappedBy(By byDefault, By nativeAppBy) { Map contentMap = new HashMap<>(); contentMap.put(ContentType.HTML_OR_DEFAULT, byDefault); 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 1a9ddba88..b9c94497e 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 @@ -18,6 +18,7 @@ import lombok.EqualsAndHashCode; import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; import org.openqa.selenium.By; import org.openqa.selenium.SearchContext; import org.openqa.selenium.WebElement; @@ -33,7 +34,9 @@ @EqualsAndHashCode(callSuper = true) public class ContentMappedBy extends By { private final Map map; - private ContentType currentContent = NATIVE_MOBILE_SPECIFIC; + @EqualsAndHashCode.Exclude + @Nullable + private ContentType currentContent; /** * Creates a new locator. @@ -56,20 +59,23 @@ public By useContent(@NonNull ContentType type) { return this; } + private ContentType resolveContentType(SearchContext context) { + return currentContent != null ? currentContent : getCurrentContentType(context); + } + @Override public WebElement findElement(SearchContext context) { - useContent(getCurrentContentType(context)); - return context.findElement(map.get(currentContent)); + return context.findElement(map.get(resolveContentType(context))); } @Override public List findElements(SearchContext context) { - useContent(getCurrentContentType(context)); - return context.findElements(map.get(currentContent)); + return context.findElements(map.get(resolveContentType(context))); } @Override public String toString() { - return map.get(currentContent).toString(); + ContentType contentType = currentContent != null ? currentContent : NATIVE_MOBILE_SPECIFIC; + return map.get(contentType).toString(); } } diff --git a/src/main/java/io/appium/java_client/support/PageFactory.java b/src/main/java/io/appium/java_client/support/PageFactory.java index c14d80963..b469fd281 100644 --- a/src/main/java/io/appium/java_client/support/PageFactory.java +++ b/src/main/java/io/appium/java_client/support/PageFactory.java @@ -31,7 +31,7 @@ */ public final class PageFactory { - private static final ThreadLocal> PAGE_STACK = + private static final ThreadLocal>> PAGE_CLASS_STACK = ThreadLocal.withInitial(ArrayDeque::new); private PageFactory() { @@ -46,20 +46,7 @@ private PageFactory() { */ @Nullable public static Class getCurrentPageClass() { - Object page = PAGE_STACK.get().peek(); - return page == null ? null : page.getClass(); - } - - /** - * Returns the page object currently being initialized by - * {@link #initElements(FieldDecorator, Object)} on the current thread, or {@code null} - * if no page initialization is active. - * - * @return the current page object or {@code null} - */ - @Nullable - public static Object getCurrentPage() { - return PAGE_STACK.get().peek(); + return PAGE_CLASS_STACK.get().peek(); } /** @@ -82,8 +69,8 @@ public static void initElements(ElementLocatorFactory factory, Object page) { * @param page The object to decorate the fields of */ public static void initElements(FieldDecorator decorator, Object page) { - Deque stack = PAGE_STACK.get(); - stack.push(page); + Deque> stack = PAGE_CLASS_STACK.get(); + stack.push(page.getClass()); try { Class proxyIn = page.getClass(); while (proxyIn != Object.class) { @@ -93,7 +80,7 @@ public static void initElements(FieldDecorator decorator, Object page) { } finally { stack.pop(); if (stack.isEmpty()) { - PAGE_STACK.remove(); + PAGE_CLASS_STACK.remove(); } } } 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..84f3e5362 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 composite locator. + * + * @return a copy of the underlying locator array + */ + public By[] getBys() { + return bys == null ? new By[0] : 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..f2b5e0e13 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 that form this chain. + * + * @return a copy of the underlying locator array + */ + public By[] getBys() { + return bys == null ? new By[0] : bys.clone(); + } + @Override public WebElement findElement(SearchContext context) { List elements = findElements(context); diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java index c391a83f9..736eaae96 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java @@ -48,8 +48,11 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static io.appium.java_client.pagefactory.LocatorGroupStrategy.ALL_POSSIBLE; +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 io.appium.java_client.support.PageFactory.initElements; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -231,6 +234,11 @@ public static class CrossPlatformByPage { private ContentMappedBy unannotatedContentMappedBy; + private ContentMappedBy preInitializedContentMappedBy = new ContentMappedBy(Map.of( + HTML_OR_DEFAULT, By.id("preset_mapped_web"), + NATIVE_MOBILE_SPECIFIC, AppiumBy.id("preset_mapped_native") + )); + @CacheLookup @WithTimeout(time = 5, chronoUnit = ChronoUnit.SECONDS) @FindBy(id = "cached_web") @@ -247,8 +255,7 @@ public static class CrossPlatformByPage { @FindBy(id = "ignored_static") private static By staticAnnotatedBy = By.id("static_id"); - @FindBy(id = "ignored_preinit") - private By preInitializedAnnotatedBy = By.id("preserved_id"); + private By preInitializedBy = By.id("preserved_id"); private By unannotatedNullBy; } @@ -304,7 +311,6 @@ public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { assertInstanceOf(ContentMappedBy.class, page.hybridBy); assertInstanceOf(ContentMappedBy.class, page.webFallbackBy); assertInstanceOf(ContentMappedBy.class, page.contentMappedBy); - assertInstanceOf(ContentMappedBy.class, page.unannotatedContentMappedBy); assertInstanceOf(ContentMappedBy.class, page.cachedAndTimedBy); assertEquals(AppiumBy.id("appium_android"), page.somethingAppiumBy); @@ -314,11 +320,18 @@ public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { assertInstanceOf(ByAll.class, page.findAllField); assertEquals(AppiumBy.accessibilityId("inner_android"), page.widget.getInnerAppiumBy()); - // Pre-initialized, final, static, and unannotated By fields are left untouched + // Pre-initialized, final, static, and unannotated By / ContentMappedBy fields are left untouched assertEquals(By.id("constant_id"), page.finalAnnotatedBy); assertEquals(By.id("static_id"), CrossPlatformByPage.staticAnnotatedBy); - assertEquals(By.id("preserved_id"), page.preInitializedAnnotatedBy); + assertEquals(By.id("preserved_id"), page.preInitializedBy); assertNull(page.unannotatedNullBy); + assertNull(page.unannotatedContentMappedBy); + assertEquals( + AppiumBy.id("preset_mapped_native"), + lastRecordedBy(driver, () -> driver.findElement(page.preInitializedContentMappedBy)) + ); + + final int initialHashCode = page.hybridBy.hashCode(); // In NATIVE_APP context, hybridBy and somethingBy resolve to native locators assertEquals( @@ -329,10 +342,6 @@ public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { new ByChained(new By[]{AppiumBy.id("something_android")}).toString(), lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() ); - assertEquals( - By.id("unannotatedContentMappedBy"), - lastRecordedBy(driver, () -> driver.findElement(page.unannotatedContentMappedBy)) - ); // Switching to WEBVIEW_1 updates the same By and ContentMappedBy fields lazily driver.context("WEBVIEW_1"); @@ -353,17 +362,37 @@ public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { new ByIdOrName("somethingBy").toString(), lastRecordedBy(driver, () -> driver.findElement(page.somethingBy)).toString() ); - assertEquals( - new ByIdOrName("unannotatedContentMappedBy").toString(), - lastRecordedBy(driver, () -> driver.findElement(page.unannotatedContentMappedBy)).toString() - ); - // Switching back to NATIVE_APP restores native locators + // Switching back to NATIVE_APP restores native locators, and hashCode remains stable driver.context("NATIVE_APP"); assertEquals( new ByChained(new By[]{AppiumBy.id("native_android")}).toString(), lastRecordedBy(driver, () -> driver.findElements(page.hybridBy)).toString() ); + assertEquals(initialHashCode, page.hybridBy.hashCode()); + } + + @Test + public void explicitUseContentIsHonoredWithoutExtraContextChecksOrHashCodeMutation() { + ContextSwitchingAndroidDriver driver = new ContextSwitchingAndroidDriver(); + ContentMappedBy mappedBy = new ContentMappedBy(Map.of( + HTML_OR_DEFAULT, By.id("web_target"), + NATIVE_MOBILE_SPECIFIC, AppiumBy.id("native_target") + )); + final int hashBefore = mappedBy.hashCode(); + + // Explicitly select native content even when driver is in WEBVIEW_1 + driver.context("WEBVIEW_1"); + mappedBy.useContent(NATIVE_MOBILE_SPECIFIC); + assertEquals(hashBefore, mappedBy.hashCode()); + + final int checksBefore = driver.getContextCheckCount(); + assertEquals( + AppiumBy.id("native_target"), + lastRecordedBy(driver, () -> driver.findElement(mappedBy)) + ); + assertEquals(checksBefore, driver.getContextCheckCount()); + assertEquals(hashBefore, mappedBy.hashCode()); } @Test @@ -391,8 +420,9 @@ public void decoratesByAndAppiumByFieldsOnIOS() { assertInstanceOf(ByAll.class, page.findAllField); assertEquals(AppiumBy.iOSNsPredicateString("name == 'inner_ios'"), page.widget.getInnerAppiumBy()); assertEquals(By.id("constant_id"), page.finalAnnotatedBy); - assertEquals(By.id("preserved_id"), page.preInitializedAnnotatedBy); + assertEquals(By.id("preserved_id"), page.preInitializedBy); assertNull(page.unannotatedNullBy); + assertNull(page.unannotatedContentMappedBy); } @Test From b5dd09d3deb86710708eb61c31d6af8938724a87 Mon Sep 17 00:00:00 2001 From: Nacy Dev Date: Wed, 4 Nov 2026 03:18:58 +0700 Subject: [PATCH 4/5] fix(pagefactory): preserve initialized By fields and refine ContentMappedBy --- .../pagefactory/AppiumFieldDecorator.java | 40 +++++++++++++++---- .../pagefactory/DefaultElementByBuilder.java | 40 +++++-------------- .../pagefactory/bys/ContentMappedBy.java | 33 +++++++++++++-- .../java_client/support/PageFactory.java | 34 +++++++++++++--- .../ByFieldDecorationTest.java | 26 +++++++++--- 5 files changed, 120 insertions(+), 53 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java index eb2ed7e7c..20e9eee95 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -23,6 +23,7 @@ import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.remote.AppiumWebElement; import io.appium.java_client.remote.CapabilityType; +import io.appium.java_client.support.PageFactory; import io.appium.java_client.support.pagefactory.ByAll; import io.appium.java_client.support.pagefactory.ByChained; import io.appium.java_client.support.pagefactory.DefaultFieldDecorator; @@ -36,6 +37,8 @@ import org.openqa.selenium.SearchContext; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; @@ -63,13 +66,18 @@ * WebElement fields, 2) List of WebElement that have * {@literal @AndroidFindBy}, {@literal @AndroidFindBys}, or * {@literal @iOSFindBy/@iOSFindBys} annotation with a proxy that locates the - * elements using the passed in ElementLocatorFactory, and 3) {@link By} / {@link AppiumBy} - * fields annotated with Appium or Selenium locator annotations. + * elements using the passed in ElementLocatorFactory, and 3) non-static, non-final, + * uninitialized {@link By} / {@link AppiumBy} fields annotated with Appium or Selenium + * locator annotations. When a field is declared with a narrow {@link By} subtype + * (such as {@link AppiumBy}) and the built locator cannot be coerced to that subtype, + * a warning is logged and the field is left {@code null}. * Please pay attention: fields of {@link WebElement}, {@link AppiumWebElement}, * {@link By}, or {@link AppiumBy} are supported by this decorator. */ public class AppiumFieldDecorator implements FieldDecorator { + private static final Logger LOG = LoggerFactory.getLogger(AppiumFieldDecorator.class); + private static final List> AVAILABLE_ELEMENT_CLASSES = List.of( WebElement.class, AppiumWebElement.class @@ -291,7 +299,8 @@ private Object decorateBy(Field field) { int modifiers = field.getModifiers(); if (!By.class.isAssignableFrom(type) || Modifier.isStatic(modifiers) - || Modifier.isFinal(modifiers)) { + || Modifier.isFinal(modifiers) + || PageFactory.isCurrentPageFieldInitialized(field)) { return null; } @@ -299,17 +308,32 @@ private Object decorateBy(Field field) { platform, automation, defaultLocatorGroupStrategy ); builder.setAnnotated(field); - if (!builder.hasLocatorAnnotations()) { + builder.assertValidAnnotations(); + + By mobileNativeBy = builder.buildMobileNativeBy(); + By defaultBy = builder.buildDefaultBy(); + if (mobileNativeBy == null && defaultBy == null) { return null; } if (type.isAssignableFrom(ContentMappedBy.class)) { - return builder.buildBy(); + return builder.buildMappedBy(defaultBy, mobileNativeBy); } - builder.assertValidAnnotations(); - By result = coerceToType(type, builder.buildMobileNativeBy()); + By result = coerceToType(type, mobileNativeBy); if (result == null) { - result = coerceToType(type, builder.buildDefaultBy()); + result = coerceToType(type, defaultBy); + } + if (result == null) { + By built = mobileNativeBy != null ? unwrapSingleBy(mobileNativeBy) : unwrapSingleBy(defaultBy); + LOG.warn( + "Cannot decorate field '{}.{}' of narrow type {}: built locator {} ({}) is not assignable to {}", + field.getDeclaringClass().getName(), + field.getName(), + type.getName(), + built, + built != null ? built.getClass().getName() : "null", + type.getName() + ); } return result; } diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java index 9fe174c7d..c24415f25 100644 --- a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java @@ -322,22 +322,6 @@ public boolean isLookupCached() { return SeleniumAnnotationsCompat.isPresent(annotatedElement, CacheLookup.class); } - boolean hasLocatorAnnotations() { - AnnotatedElement annotatedElement = annotatedElementContainer.getAnnotated(); - if (annotatedElement == null) { - return false; - } - return SeleniumAnnotationsCompat.isPresent(annotatedElement, FindBy.class) - || SeleniumAnnotationsCompat.isPresent(annotatedElement, FindBys.class) - || SeleniumAnnotationsCompat.isPresent(annotatedElement, FindAll.class) - || annotatedElement.getAnnotationsByType(AndroidFindBy.class).length > 0 - || annotatedElement.getAnnotationsByType(AndroidFindBys.class).length > 0 - || annotatedElement.getAnnotationsByType(AndroidFindAll.class).length > 0 - || annotatedElement.getAnnotationsByType(iOSXCUITFindBy.class).length > 0 - || annotatedElement.getAnnotationsByType(iOSXCUITFindBys.class).length > 0 - || annotatedElement.getAnnotationsByType(iOSXCUITFindAll.class).length > 0; - } - private By returnMappedBy(By byDefault, By nativeAppBy) { Map contentMap = new HashMap<>(); contentMap.put(ContentType.HTML_OR_DEFAULT, byDefault); @@ -345,34 +329,30 @@ private By returnMappedBy(By byDefault, By nativeAppBy) { return new ContentMappedBy(contentMap); } - @Override - public By buildBy() { - assertValidAnnotations(); - - By defaultBy = buildDefaultBy(); - By mobileNativeBy = buildMobileNativeBy(); - + By buildMappedBy(@Nullable By defaultBy, @Nullable By mobileNativeBy) { String idOrName = ((Field) annotatedElementContainer.getAnnotated()).getName(); if (defaultBy == null && mobileNativeBy == null) { - defaultBy = new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); - mobileNativeBy = new By.ById(idOrName); - return returnMappedBy(defaultBy, mobileNativeBy); + return returnMappedBy(new ByIdOrName(idOrName), new By.ById(idOrName)); } if (defaultBy == null) { - defaultBy = new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); - return returnMappedBy(defaultBy, mobileNativeBy); + return returnMappedBy(new ByIdOrName(idOrName), mobileNativeBy); } if (mobileNativeBy == null) { - mobileNativeBy = defaultBy; - return returnMappedBy(defaultBy, mobileNativeBy); + return returnMappedBy(defaultBy, defaultBy); } return returnMappedBy(defaultBy, mobileNativeBy); } + @Override + public By buildBy() { + assertValidAnnotations(); + return buildMappedBy(buildDefaultBy(), buildMobileNativeBy()); + } + private static class AnnotationComparator implements Comparator { private static Method getPriorityMethod(Class clazz) { 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 b9c94497e..224896768 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 @@ -25,16 +25,30 @@ import java.util.List; import java.util.Map; +import java.util.Objects; +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 io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; import static java.util.Objects.requireNonNull; -/** Locator which selects the underlying locator by the current content type. */ +/** + * Locator which selects the underlying locator by the active {@link ContentType}. + * + *

When a content type is explicitly pinned via {@link #useContent(ContentType)}, + * {@link #findElement(SearchContext)}, {@link #findElements(SearchContext)}, and + * {@link #toString()} use that pinned entry. When no content type has been set, + * {@link #findElement(SearchContext)} and {@link #findElements(SearchContext)} + * auto-detect the content type from the target {@link SearchContext} via + * {@link io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility#getCurrentContentType(SearchContext)} + * (resolving to {@link ContentType#NATIVE_MOBILE_SPECIFIC} for native mobile contexts and + * {@link ContentType#HTML_OR_DEFAULT} for web/browser contexts or plain {@link SearchContext} + * implementations that implement neither {@link io.appium.java_client.HasBrowserCheck} nor + * {@link io.appium.java_client.remote.SupportsContextSwitching}). + */ @EqualsAndHashCode(callSuper = true) public class ContentMappedBy extends By { private final Map map; - @EqualsAndHashCode.Exclude @Nullable private ContentType currentContent; @@ -75,7 +89,18 @@ public List findElements(SearchContext context) { @Override public String toString() { - ContentType contentType = currentContent != null ? currentContent : NATIVE_MOBILE_SPECIFIC; - return map.get(contentType).toString(); + if (currentContent != null) { + return map.get(currentContent).toString(); + } + By defaultBy = map.get(HTML_OR_DEFAULT); + By nativeBy = map.get(NATIVE_MOBILE_SPECIFIC); + if (Objects.equals(defaultBy, nativeBy) && nativeBy != null) { + return nativeBy.toString(); + } + return String.format( + "ContentMappedBy(%s=%s, %s=%s)", + HTML_OR_DEFAULT, defaultBy, + NATIVE_MOBILE_SPECIFIC, nativeBy + ); } } diff --git a/src/main/java/io/appium/java_client/support/PageFactory.java b/src/main/java/io/appium/java_client/support/PageFactory.java index b469fd281..1d7d562ef 100644 --- a/src/main/java/io/appium/java_client/support/PageFactory.java +++ b/src/main/java/io/appium/java_client/support/PageFactory.java @@ -31,7 +31,7 @@ */ public final class PageFactory { - private static final ThreadLocal>> PAGE_CLASS_STACK = + private static final ThreadLocal> PAGE_OBJECT_STACK = ThreadLocal.withInitial(ArrayDeque::new); private PageFactory() { @@ -46,7 +46,31 @@ private PageFactory() { */ @Nullable public static Class getCurrentPageClass() { - return PAGE_CLASS_STACK.get().peek(); + Object current = PAGE_OBJECT_STACK.get().peek(); + return current != null ? current.getClass() : null; + } + + /** + * Checks whether the given {@code field} already has a non-null value on the page object + * currently being initialized by {@link #initElements(FieldDecorator, Object)} on the + * current thread. + * + * @param field the field to inspect + * @return {@code true} if the field is already initialized (or cannot be inspected + * reflectively) on the active page object; {@code false} otherwise + */ + public static boolean isCurrentPageFieldInitialized(Field field) { + for (Object page : PAGE_OBJECT_STACK.get()) { + if (field.getDeclaringClass().isInstance(page)) { + try { + field.setAccessible(true); + return field.get(page) != null; + } catch (IllegalAccessException | RuntimeException e) { + return true; + } + } + } + return false; } /** @@ -69,8 +93,8 @@ public static void initElements(ElementLocatorFactory factory, Object page) { * @param page The object to decorate the fields of */ public static void initElements(FieldDecorator decorator, Object page) { - Deque> stack = PAGE_CLASS_STACK.get(); - stack.push(page.getClass()); + Deque stack = PAGE_OBJECT_STACK.get(); + stack.push(page); try { Class proxyIn = page.getClass(); while (proxyIn != Object.class) { @@ -80,7 +104,7 @@ public static void initElements(FieldDecorator decorator, Object page) { } finally { stack.pop(); if (stack.isEmpty()) { - PAGE_CLASS_STACK.remove(); + PAGE_OBJECT_STACK.remove(); } } } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java index 736eaae96..12b729c81 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/ByFieldDecorationTest.java @@ -56,8 +56,10 @@ import static io.appium.java_client.support.PageFactory.initElements; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ByFieldDecorationTest { @@ -234,6 +236,8 @@ public static class CrossPlatformByPage { private ContentMappedBy unannotatedContentMappedBy; + @FindBy(id = "ignored_mapped_web") + @AndroidFindBy(id = "ignored_mapped_native") private ContentMappedBy preInitializedContentMappedBy = new ContentMappedBy(Map.of( HTML_OR_DEFAULT, By.id("preset_mapped_web"), NATIVE_MOBILE_SPECIFIC, AppiumBy.id("preset_mapped_native") @@ -255,6 +259,9 @@ public static class CrossPlatformByPage { @FindBy(id = "ignored_static") private static By staticAnnotatedBy = By.id("static_id"); + @FindBy(id = "ignored_pre_initialized") + @AndroidFindBy(id = "ignored_pre_initialized_android") + @iOSXCUITFindBy(accessibility = "ignored_pre_initialized_ios") private By preInitializedBy = By.id("preserved_id"); private By unannotatedNullBy; @@ -374,25 +381,32 @@ public void decoratesByAndAppiumByFieldsAndSwitchesContextLazilyOnAndroid() { @Test public void explicitUseContentIsHonoredWithoutExtraContextChecksOrHashCodeMutation() { - ContextSwitchingAndroidDriver driver = new ContextSwitchingAndroidDriver(); - ContentMappedBy mappedBy = new ContentMappedBy(Map.of( + Map locatorMap = Map.of( HTML_OR_DEFAULT, By.id("web_target"), NATIVE_MOBILE_SPECIFIC, AppiumBy.id("native_target") - )); - final int hashBefore = mappedBy.hashCode(); + ); + ContentMappedBy mappedBy = new ContentMappedBy(locatorMap); + ContentMappedBy webPinnedBy = new ContentMappedBy(locatorMap); + webPinnedBy.useContent(HTML_OR_DEFAULT); + + assertTrue(mappedBy.toString().contains("HTML_OR_DEFAULT=")); + assertTrue(mappedBy.toString().contains("NATIVE_MOBILE_SPECIFIC=")); // Explicitly select native content even when driver is in WEBVIEW_1 + ContextSwitchingAndroidDriver driver = new ContextSwitchingAndroidDriver(); driver.context("WEBVIEW_1"); mappedBy.useContent(NATIVE_MOBILE_SPECIFIC); - assertEquals(hashBefore, mappedBy.hashCode()); + assertEquals(AppiumBy.id("native_target").toString(), mappedBy.toString()); + assertNotEquals(webPinnedBy, mappedBy); + final int pinnedHash = mappedBy.hashCode(); final int checksBefore = driver.getContextCheckCount(); assertEquals( AppiumBy.id("native_target"), lastRecordedBy(driver, () -> driver.findElement(mappedBy)) ); assertEquals(checksBefore, driver.getContextCheckCount()); - assertEquals(hashBefore, mappedBy.hashCode()); + assertEquals(pinnedHash, mappedBy.hashCode()); } @Test From f5ae34117cc264a8f14e5701d82bc72e4570b42c Mon Sep 17 00:00:00 2001 From: Nacy Dev Date: Wed, 4 Nov 2026 14:37:38 +0700 Subject: [PATCH 5/5] fix(pagefactory): return false on inaccessible field check and polish ContentMappedBy --- .../java_client/pagefactory/AppiumFieldDecorator.java | 3 ++- .../java_client/pagefactory/bys/ContentMappedBy.java | 11 ++++++----- .../io/appium/java_client/support/PageFactory.java | 6 +++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java index 20e9eee95..2244e4b79 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -68,7 +68,8 @@ * {@literal @iOSFindBy/@iOSFindBys} annotation with a proxy that locates the * elements using the passed in ElementLocatorFactory, and 3) non-static, non-final, * uninitialized {@link By} / {@link AppiumBy} fields annotated with Appium or Selenium - * locator annotations. When a field is declared with a narrow {@link By} subtype + * locator annotations (pre-initialized {@link By} fields are preserved when initialized via + * {@link PageFactory#initElements}). When a field is declared with a narrow {@link By} subtype * (such as {@link AppiumBy}) and the built locator cannot be coerced to that subtype, * a warning is logged and the field is left {@code null}. * Please pay attention: fields of {@link WebElement}, {@link AppiumWebElement}, 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 224896768..cbe8d3394 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 @@ -73,10 +73,6 @@ public By useContent(@NonNull ContentType type) { return this; } - private ContentType resolveContentType(SearchContext context) { - return currentContent != null ? currentContent : getCurrentContentType(context); - } - @Override public WebElement findElement(SearchContext context) { return context.findElement(map.get(resolveContentType(context))); @@ -98,9 +94,14 @@ public String toString() { return nativeBy.toString(); } return String.format( - "ContentMappedBy(%s=%s, %s=%s)", + "%s(%s=%s, %s=%s)", + getClass().getSimpleName(), HTML_OR_DEFAULT, defaultBy, NATIVE_MOBILE_SPECIFIC, nativeBy ); } + + private ContentType resolveContentType(SearchContext context) { + return currentContent != null ? currentContent : getCurrentContentType(context); + } } diff --git a/src/main/java/io/appium/java_client/support/PageFactory.java b/src/main/java/io/appium/java_client/support/PageFactory.java index 1d7d562ef..80a02f9ed 100644 --- a/src/main/java/io/appium/java_client/support/PageFactory.java +++ b/src/main/java/io/appium/java_client/support/PageFactory.java @@ -56,8 +56,8 @@ public static Class getCurrentPageClass() { * current thread. * * @param field the field to inspect - * @return {@code true} if the field is already initialized (or cannot be inspected - * reflectively) on the active page object; {@code false} otherwise + * @return {@code true} if the field can be inspected and already holds a non-null value + * on the active page object; {@code false} otherwise */ public static boolean isCurrentPageFieldInitialized(Field field) { for (Object page : PAGE_OBJECT_STACK.get()) { @@ -66,7 +66,7 @@ public static boolean isCurrentPageFieldInitialized(Field field) { field.setAccessible(true); return field.get(page) != null; } catch (IllegalAccessException | RuntimeException e) { - return true; + return false; } } }