From 336d1651e98eabc167eb122f7a45e1f5c4f22b15 Mon Sep 17 00:00:00 2001 From: Ganesh Date: Tue, 3 Nov 2026 06:30:00 +0000 Subject: [PATCH 1/3] feat: allow @OverrideWidget fields declared as interfaces An @OverrideWidget field may be an interface implemented by the platform widget classes. Calls are proxied to the concrete widget for the current content type. --- docs/Page-objects.md | 16 +- .../pagefactory/AppiumFieldDecorator.java | 52 +++-- .../pagefactory/OverrideWidget.java | 19 +- .../pagefactory/OverrideWidgetReader.java | 42 ++-- .../pagefactory/WidgetByBuilder.java | 10 +- .../pagefactory/WidgetInterceptor.java | 6 + .../pagefactory/WidgetListInterceptor.java | 47 +++-- .../pagefactory/utils/ProxyFactory.java | 58 ++++++ .../OverriddenWidgetInterfaceTest.java | 183 ++++++++++++++++++ 9 files changed, 363 insertions(+), 70 deletions(-) create mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java diff --git a/docs/Page-objects.md b/docs/Page-objects.md index c1bc445bc..d16da102e 100644 --- a/docs/Page-objects.md +++ b/docs/Page-objects.md @@ -684,7 +684,7 @@ and then This use case has some restrictions; -- All classes which are declared by the OverrideWidget annotation should be subclasses of the class declared by field +- All classes which are declared by the OverrideWidget annotation should be subclasses of the class declared by the field, or implementations of the interface declared by the field - All classes which are declared by the OverrideWidget should not be abstract. If a declared class is overridden partially like @@ -700,6 +700,20 @@ This use case has some restrictions; //below is the other field/method declaration ``` +The field may be an interface implemented by those classes. Each class named by `@OverrideWidget` must extend `Widget` and implement the interface. A content type that is left as the default cannot be instantiated, so name a class for every content type the field is used with: + +```java +public interface UsersWidget { + void doWork(); +} + +@FindBy(id = "users") +@OverrideWidget(html = HtmlUsersWidget.class, + androidUIAutomator = AndroidUsersWidget.class, + iOSXCUITAutomation = IosUsersWidget.class) +UsersWidget widget; +``` + - for now it is not possible to ```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..1b0b2a641 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -46,6 +46,7 @@ import java.util.Map; import static io.appium.java_client.internal.Preconditions.checkNotNull; +import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedInterfaceProxy; import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.unpackObjectFromSearchContext; import static io.appium.java_client.remote.options.SupportsAutomationNameOption.AUTOMATION_NAME_OPTION; @@ -278,11 +279,12 @@ public Object decorate(ClassLoader ignored, Field field) { @Nullable private Object decorateWidget(Field field) { Class type = field.getType(); - if (!Widget.class.isAssignableFrom(type) && !List.class.isAssignableFrom(type)) { + if (!Widget.class.isAssignableFrom(type) && !List.class.isAssignableFrom(type) + && !isOverriddenWidgetInterface(type, field)) { return null; } - Class widgetType; + Class widgetType; boolean isAlist = false; if (List.class.isAssignableFrom(type)) { isAlist = true; @@ -298,18 +300,17 @@ private Object decorateWidget(Field field) { } if (listType instanceof Class) { - if (!Widget.class.isAssignableFrom((Class) listType)) { + Class listClass = (Class) listType; + if (!Widget.class.isAssignableFrom(listClass) && !isOverriddenWidgetInterface(listClass, field)) { return null; } - //noinspection unchecked - widgetType = (Class) listType; + widgetType = listClass; } else { return null; } } else { - //noinspection unchecked - widgetType = (Class) field.getType(); + widgetType = field.getType(); } CacheableLocator locator = widgetLocatorFactory.createLocator(field); @@ -329,22 +330,39 @@ private Object decorateWidget(Field field) { ); } - Constructor constructor = WidgetConstructorUtil.findConvenientConstructor(widgetType); + WidgetInterceptor interceptor = new WidgetInterceptor( + locator, + webDriverReference, + null, + map, + duration, + defaultLocatorGroupStrategy + ); + if (widgetType.isInterface()) { + return getEnhancedInterfaceProxy(widgetType, interceptor); + } + + //noinspection unchecked + Class widgetClass = (Class) widgetType; + Constructor constructor = WidgetConstructorUtil.findConvenientConstructor(widgetClass); return getEnhancedProxy( - widgetType, + widgetClass, new Class[]{constructor.getParameterTypes()[0]}, new Object[]{proxyForAnElement(locator)}, - new WidgetInterceptor( - locator, - webDriverReference, - null, - map, - duration, - defaultLocatorGroupStrategy - ) + interceptor ); } + /** + * True when {@code type} is a widget contract rather than a {@link WebElement}. + * {@code @OverrideWidget} on an element field keeps the normal element proxy. + */ + private static boolean isOverriddenWidgetInterface(Class type, Field field) { + return type.isInterface() + && !WebElement.class.isAssignableFrom(type) + && field.isAnnotationPresent(OverrideWidget.class); + } + private WebElement proxyForAnElement(ElementLocator locator) { ElementInterceptor elementInterceptor = new ElementInterceptor(locator, webDriverReference); return getEnhancedProxy(AppiumWebElement.class, elementInterceptor); diff --git a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java index 53bdee8f0..1569a03f9 100644 --- a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java +++ b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java @@ -23,7 +23,8 @@ /** * This annotation should mark an abstract/certain Widget object class - * or field that declares some abstract/certain Widget object class. + * or field that declares some abstract/certain Widget object class or an interface + * implemented by those classes. * io.appium.java_client.pagefactory.Widget is the Appium-specific extension of the * Page Object design pattern. * About the Page Object design pattern please read these documents: @@ -33,8 +34,8 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD}) public @interface OverrideWidget { /** * This property is designed for HTML (browser or web view) content. - * A declared class should not be abstract. Declared class also should be a subclass - * of an annotated class/class which is declared by an annotated field. + * A declared class should not be abstract. It must be a subclass of the field type, + * or an implementation of the field type when that type is an interface. * * @return a class which extends {@link io.appium.java_client.pagefactory.Widget} */ @@ -42,8 +43,8 @@ /** * This property is designed for Android native content. - * A declared class should not be abstract. Declared class also should be a subclass - * of an annotated class/class which is declared by an annotated field. + * A declared class should not be abstract. It must be a subclass of the field type, + * or an implementation of the field type when that type is an interface. * * @return a class which extends {@link io.appium.java_client.pagefactory.Widget} */ @@ -52,8 +53,8 @@ /** * This property is designed for iOS native content when * {@link io.appium.java_client.remote.AutomationName#IOS_XCUI_TEST} automation is used. - * A declared class should not be abstract. Declared class also should be a subclass - * of an annotated class/class which is declared by an annotated field. + * A declared class should not be abstract. It must be a subclass of the field type, + * or an implementation of the field type when that type is an interface. * * @return a class which extends {@link io.appium.java_client.pagefactory.Widget} */ @@ -61,8 +62,8 @@ /** * This property is designed for Windows native content. - * A declared class should not be abstract. Declared class also should be a subclass - * of an annotated class/class which is declared by an annotated field. + * A declared class should not be abstract. It must be a subclass of the field type, + * or an implementation of the field type when that type is an interface. * * @return a class which extends {@link io.appium.java_client.pagefactory.Widget} */ diff --git a/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java b/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java index 09984729c..79e6cb28e 100644 --- a/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java +++ b/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java @@ -40,15 +40,14 @@ class OverrideWidgetReader { private OverrideWidgetReader() { } - @SuppressWarnings("unchecked") - private static Class getConvenientClass(Class declaredClass, - AnnotatedElement annotatedElement, String method) { - Class convenientClass; + private static Class getConvenientClass(Class declaredClass, + AnnotatedElement annotatedElement, String method) { + Class convenientClass; OverrideWidget overrideWidget = annotatedElement.getAnnotation(OverrideWidget.class); try { if (overrideWidget == null || (convenientClass = - (Class) OverrideWidget.class + (Class) OverrideWidget.class .getDeclaredMethod(method).invoke(overrideWidget)) .equals(EMPTY)) { convenientClass = declaredClass; @@ -68,13 +67,12 @@ private static Class getConvenientClass(Class getDefaultOrHTMLWidgetClass( - Class declaredClass, AnnotatedElement annotatedElement) { + static Class getDefaultOrHTMLWidgetClass(Class declaredClass, AnnotatedElement annotatedElement) { return getConvenientClass(declaredClass, annotatedElement, HTML); } - static Class getMobileNativeWidgetClass(Class declaredClass, - AnnotatedElement annotatedElement, String platform) { + static Class getMobileNativeWidgetClass(Class declaredClass, + AnnotatedElement annotatedElement, String platform) { String transformedPlatform = String.valueOf(platform).toUpperCase(ROOT).trim(); if (ANDROID.equalsIgnoreCase(transformedPlatform)) { @@ -92,22 +90,30 @@ static Class getMobileNativeWidgetClass(Class findConstructor(Class clazz) { + if (clazz == null || clazz.isInterface() || !Widget.class.isAssignableFrom(clazz)) { + return null; + } + return findConvenientConstructor((Class) clazz); + } + private static Constructor getConstructorOfADefaultOrHTMLWidget( - Class declaredClass, AnnotatedElement annotatedElement) { - Class clazz = - getDefaultOrHTMLWidgetClass(declaredClass, annotatedElement); - return findConvenientConstructor(clazz); + Class declaredClass, AnnotatedElement annotatedElement) { + return findConstructor(getDefaultOrHTMLWidgetClass(declaredClass, annotatedElement)); } private static Constructor getConstructorOfAMobileNativeWidgets( - Class declaredClass, AnnotatedElement annotatedElement, String platform) { - Class clazz = - getMobileNativeWidgetClass(declaredClass, annotatedElement, platform); - return findConvenientConstructor(clazz); + Class declaredClass, AnnotatedElement annotatedElement, String platform) { + return findConstructor(getMobileNativeWidgetClass(declaredClass, annotatedElement, platform)); } protected static Map> read( - Class declaredClass, AnnotatedElement annotatedElement, String platform) { + Class declaredClass, AnnotatedElement annotatedElement, String platform) { Map> result = new HashMap<>(); result.put(ContentType.HTML_OR_DEFAULT, getConstructorOfADefaultOrHTMLWidget(declaredClass, annotatedElement)); diff --git a/src/main/java/io/appium/java_client/pagefactory/WidgetByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/WidgetByBuilder.java index 17a2ed48a..211658ea6 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetByBuilder.java @@ -70,7 +70,6 @@ private static Class getClassFromAListField(Field field) { return (Class) listType; } - @SuppressWarnings("unchecked") private By getByFromDeclaredClass(WhatIsNeeded whatIsNeeded) { AnnotatedElement annotatedElement = annotatedElementContainer.getAnnotated(); Field field = (Field) annotatedElement; @@ -87,15 +86,12 @@ private By getByFromDeclaredClass(WhatIsNeeded whatIsNeeded) { Class convenientClass; if (whatIsNeeded.equals(WhatIsNeeded.DEFAULT_OR_HTML)) { - convenientClass = - getDefaultOrHTMLWidgetClass((Class) declaredClass, field); + convenientClass = getDefaultOrHTMLWidgetClass(declaredClass, field); } else { - convenientClass = - getMobileNativeWidgetClass((Class) declaredClass, field, - platform); + convenientClass = getMobileNativeWidgetClass(declaredClass, field, platform); } - while (result == null && !convenientClass.equals(Object.class)) { + while (result == null && convenientClass != null && !convenientClass.equals(Object.class)) { setAnnotated(convenientClass); if (whatIsNeeded.equals(WhatIsNeeded.DEFAULT_OR_HTML)) { result = super.buildDefaultBy(); 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..6bf8560a6 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -97,6 +97,12 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr cachedElementReference = new WeakReference<>(element); Constructor constructor = instantiationMap.get(type); + if (constructor == null) { + throw new InstantiationException(String.format( + "No concrete @OverrideWidget class is declared for %s. " + + "A field typed as an interface needs a widget class for this content.", + type)); + } Class clazz = constructor.getDeclaringClass(); if (Modifier.isAbstract(clazz.getModifiers())) { diff --git a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java index cdaca4721..4a1cfddf4 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -34,6 +34,7 @@ import java.util.stream.Collectors; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; +import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedInterfaceProxy; import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; import static java.util.Optional.ofNullable; @@ -41,8 +42,8 @@ /** Proxy interceptor class for lists of widgets. */ public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final Map> instantiationMap; - private final List cachedWidgets = new ArrayList<>(); - private final Class declaredType; + private final List cachedWidgets = new ArrayList<>(); + private final Class declaredType; private final Duration duration; private final LocatorGroupStrategy defaultLocatorGroupStrategy; private final WeakReference driver; @@ -54,14 +55,14 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { * @param locator the locator of the widget elements * @param driver the reference to the driver * @param instantiationMap the widget constructors mapped by the content type - * @param declaredType the declared widget type + * @param declaredType the declared widget class, or an interface implemented by the override classes * @param duration the timeout of waiting for an element presence */ public WidgetListInterceptor( @Nullable CacheableLocator locator, WeakReference driver, Map> instantiationMap, - Class declaredType, + Class declaredType, Duration duration ) { this( @@ -78,7 +79,7 @@ public WidgetListInterceptor( @Nullable CacheableLocator locator, WeakReference driver, Map> instantiationMap, - Class declaredType, + Class declaredType, Duration duration, LocatorGroupStrategy defaultLocatorGroupStrategy ) { @@ -104,21 +105,21 @@ protected Object getObject(List elements, Method method, Object[] ar ContentType type = null; for (WebElement element : elements) { type = ofNullable(type).orElseGet(() -> getCurrentContentType(element)); - Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; WeakReference elementReference = new WeakReference<>(element); - cachedWidgets.add( - getEnhancedProxy( - declaredType, params, new Object[] {element}, - new WidgetInterceptor( - null, - driver, - elementReference, - instantiationMap, - duration, - defaultLocatorGroupStrategy - ) - ) + WidgetInterceptor interceptor = new WidgetInterceptor( + null, + driver, + elementReference, + instantiationMap, + duration, + defaultLocatorGroupStrategy ); + if (declaredType.isInterface()) { + cachedWidgets.add(getEnhancedInterfaceProxy(declaredType, interceptor)); + } else { + Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; + cachedWidgets.add(proxyWidget(declaredType, params, element, interceptor)); + } cachedElementReferences.add(elementReference); } } @@ -128,4 +129,14 @@ protected Object getObject(List elements, Method method, Object[] ar throw extractReadableException(t); } } + + @SuppressWarnings("unchecked") + private static Widget proxyWidget( + Class declaredType, + Class[] params, + WebElement element, + WidgetInterceptor interceptor + ) { + return getEnhancedProxy((Class) declaredType, params, new Object[] {element}, interceptor); + } } diff --git a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java index b0ee40c7e..1a0fa87c3 100644 --- a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java @@ -16,18 +16,28 @@ package io.appium.java_client.pagefactory.utils; +import io.appium.java_client.proxy.HasMethodCallListeners; +import io.appium.java_client.proxy.Interceptor; import io.appium.java_client.proxy.MethodCallListener; +import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import net.bytebuddy.implementation.FieldAccessor; +import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.matcher.ElementMatcher; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; import static io.appium.java_client.proxy.Helpers.OBJECT_METHOD_NAMES; import static io.appium.java_client.proxy.Helpers.createProxy; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -39,6 +49,8 @@ public final class ProxyFactory { private static final Set NON_PROXYABLE_METHODS = setWithout( OBJECT_METHOD_NAMES, "toString", "equals", "hashCode" ); + private static final Map, Class> CACHED_INTERFACE_PROXIES = + Collections.synchronizedMap(new WeakHashMap<>()); @SafeVarargs private static Set setWithout(@SuppressWarnings("SameParameterValue") Set source, T... items) { @@ -97,4 +109,50 @@ public static T getEnhancedProxy( extraMatcher ); } + + /** + * Creates a proxy that implements {@code iface} and forwards its public methods to {@code listener}. + * Used when an {@code @OverrideWidget} field is declared as an interface. + * + * @param iface an interface type + * @param listener receives the proxied calls + * @return a proxy that implements {@code iface} + */ + public static Object getEnhancedInterfaceProxy(Class iface, MethodCallListener listener) { + if (iface == null || !iface.isInterface()) { + throw new IllegalArgumentException("Expected an interface, got " + iface); + } + if (listener == null) { + throw new IllegalArgumentException("Listener must not be null"); + } + Class proxyClass = CACHED_INTERFACE_PROXIES.computeIfAbsent(iface, ProxyFactory::loadInterfaceProxy); + try { + Object result = proxyClass.getDeclaredConstructor().newInstance(); + ((HasMethodCallListeners) result).setMethodCallListeners(new MethodCallListener[] {listener}); + return result; + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(String.format("Unable to create a proxy of %s", iface.getName()), e); + } + } + + private static Class loadInterfaceProxy(Class iface) { + ClassLoader loader = iface.getClassLoader() == null + ? ProxyFactory.class.getClassLoader() + : iface.getClassLoader(); + ElementMatcher methods = isPublic().and(not( + namedOneOf(NON_PROXYABLE_METHODS.toArray(new String[0])) + )); + //noinspection resource + return new ByteBuddy() + .subclass(Object.class) + .implement(iface) + .method(methods) + .intercept(MethodDelegation.to(Interceptor.class)) + .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) + .implement(HasMethodCallListeners.class) + .intercept(FieldAccessor.ofBeanProperty()) + .make() + .load(loader, ClassLoadingStrategy.Default.WRAPPER) + .getLoaded(); + } } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java new file mode 100644 index 000000000..366ad5ec0 --- /dev/null +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java @@ -0,0 +1,183 @@ +package io.appium.java_client.pagefactory_tests.widget.tests.combined; + +import io.appium.java_client.pagefactory.AndroidFindBy; +import io.appium.java_client.pagefactory.AppiumFieldDecorator; +import io.appium.java_client.pagefactory.OverrideWidget; +import io.appium.java_client.pagefactory.Widget; +import io.appium.java_client.pagefactory.iOSXCUITFindBy; +import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; +import io.appium.java_client.support.FindBy; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; + +import java.util.List; +import java.util.stream.Stream; + +import static io.appium.java_client.support.PageFactory.initElements; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class OverriddenWidgetInterfaceTest { + + static Stream drivers() { + return Stream.of( + Arguments.of(new AbstractStubWebDriver.StubAndroidDriver(), "android"), + Arguments.of(new AbstractStubWebDriver.StubIOSXCUITDriver(), "ios"), + Arguments.of(new AbstractStubWebDriver.StubBrowserDriver(), "html"), + Arguments.of(new AbstractStubWebDriver.StubAndroidBrowserOrWebViewDriver(), "html") + ); + } + + @ParameterizedTest + @MethodSource("drivers") + void delegatesInterfaceMethodToPlatformWidget(WebDriver driver, String expected) { + var page = new Page(); + initElements(new AppiumFieldDecorator(driver), page); + + assertEquals(expected, page.widget.platformAction()); + assertEquals(2, page.widgets.size()); + page.widgets.forEach(widget -> assertEquals(expected, widget.platformAction())); + } + + @Test + void usesLocatorDeclaredOnThePlatformWidgetClass() { + var page = new ClassLocatedPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertEquals("android", page.widget.platformAction()); + assertTrue(page.widget.toString().contains("android-root")); + assertEquals(2, page.widgets.size()); + page.widgets.forEach(widget -> assertEquals("android", widget.platformAction())); + } + + @Test + void allowsPartialOverrideForTheCurrentContent() { + var page = new IosOnlyPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubIOSXCUITDriver()), page); + + assertEquals("ios", page.widget.platformAction()); + } + + @Test + void missingContentOverrideFailsWhenThatContentIsUsed() { + var page = new IosOnlyPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + InstantiationException error = assertThrows(InstantiationException.class, () -> page.widget.platformAction()); + assertTrue(error.getMessage().contains("NATIVE_MOBILE_SPECIFIC")); + } + + @Test + void rejectsOverrideClassThatDoesNotImplementTheInterface() { + var page = new MismatchedPage(); + + IllegalArgumentException error = assertThrows( + IllegalArgumentException.class, + () -> initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page) + ); + assertTrue(error.getCause().getMessage().contains("is not assignable from")); + } + + @Test + void leavesUnannotatedInterfaceFieldUnset() { + var page = new PlainPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertNull(page.untouched); + } + + static class Page { + @OverrideWidget(html = HtmlWidget.class, + androidUIAutomator = AndroidWidget.class, + iOSXCUITAutomation = IosWidget.class) + @FindBy(id = "widget") + private ActionWidget widget; + + @OverrideWidget(html = HtmlWidget.class, + androidUIAutomator = AndroidWidget.class, + iOSXCUITAutomation = IosWidget.class) + @FindBy(id = "widget") + private List widgets; + } + + static class ClassLocatedPage { + @OverrideWidget(html = HtmlWidget.class, + androidUIAutomator = AndroidWidget.class, + iOSXCUITAutomation = IosWidget.class) + private ActionWidget widget; + + @OverrideWidget(html = HtmlWidget.class, + androidUIAutomator = AndroidWidget.class, + iOSXCUITAutomation = IosWidget.class) + private List widgets; + } + + static class IosOnlyPage { + @OverrideWidget(iOSXCUITAutomation = IosWidget.class) + @FindBy(id = "widget") + private ActionWidget widget; + } + + static class MismatchedPage { + @OverrideWidget(androidUIAutomator = UnrelatedWidget.class, + iOSXCUITAutomation = IosWidget.class, + html = HtmlWidget.class) + @FindBy(id = "widget") + private ActionWidget widget; + } + + static class PlainPage { + private ActionWidget untouched; + } + + public interface ActionWidget { + String platformAction(); + } + + @AndroidFindBy(uiAutomator = "android-root") + public static class AndroidWidget extends Widget implements ActionWidget { + protected AndroidWidget(WebElement element) { + super(element); + } + + @Override + public String platformAction() { + return "android"; + } + } + + @iOSXCUITFindBy(iOSNsPredicate = "ios-root") + public static class IosWidget extends Widget implements ActionWidget { + protected IosWidget(WebElement element) { + super(element); + } + + @Override + public String platformAction() { + return "ios"; + } + } + + public static class HtmlWidget extends Widget implements ActionWidget { + protected HtmlWidget(WebElement element) { + super(element); + } + + @Override + public String platformAction() { + return "html"; + } + } + + public static class UnrelatedWidget extends Widget { + protected UnrelatedWidget(WebElement element) { + super(element); + } + } +} From 24359f8869bab0c68c2fb2a4c0891984c456c375 Mon Sep 17 00:00:00 2001 From: Ganesh Date: Sat, 7 Nov 2026 08:45:49 +0000 Subject: [PATCH 2/3] fix: keep interface widget proxies usable across loaders Hold a list item's element until its widget exists, generate interface proxies in Helpers, and reject non-public interfaces or disconnected class loaders with IllegalArgumentException. --- docs/Page-objects.md | 2 +- .../pagefactory/OverrideWidget.java | 2 +- .../pagefactory/WidgetInterceptor.java | 12 +- .../pagefactory/utils/ProxyFactory.java | 50 +---- .../io/appium/java_client/proxy/Helpers.java | 117 +++++++--- .../InterfaceProxyLoadingTest.java | 95 ++++++++ .../OverriddenWidgetInterfaceTest.java | 203 ++++++++++++++++++ .../tests/combined/PackagePrivateWidget.java | 5 + 8 files changed, 414 insertions(+), 72 deletions(-) create mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/InterfaceProxyLoadingTest.java create mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/PackagePrivateWidget.java diff --git a/docs/Page-objects.md b/docs/Page-objects.md index d16da102e..ccfb036e7 100644 --- a/docs/Page-objects.md +++ b/docs/Page-objects.md @@ -700,7 +700,7 @@ This use case has some restrictions; //below is the other field/method declaration ``` -The field may be an interface implemented by those classes. Each class named by `@OverrideWidget` must extend `Widget` and implement the interface. A content type that is left as the default cannot be instantiated, so name a class for every content type the field is used with: +The field may be an interface implemented by those classes. Each class named by `@OverrideWidget` must extend `Widget` and implement the interface. The interface must be public. The page object and this library must be visible to loaders that delegate to each other; otherwise proxy creation fails with `IllegalArgumentException` instead of a linkage error. A content type that is left as the default cannot be instantiated, so name a class for every content type the field is used with: ```java public interface UsersWidget { diff --git a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java index 1569a03f9..7343a834c 100644 --- a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java +++ b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java @@ -23,7 +23,7 @@ /** * This annotation should mark an abstract/certain Widget object class - * or field that declares some abstract/certain Widget object class or an interface + * or field that declares some abstract/certain Widget object class or a public interface * implemented by those classes. * io.appium.java_client.pagefactory.Widget is the Appium-specific extension of the * Page Object design pattern. 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 6bf8560a6..97a4694f3 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -35,7 +35,6 @@ import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; -import static java.util.Optional.ofNullable; /** Proxy interceptor class for widgets. */ public class WidgetInterceptor extends InterceptorOfASingleElement { @@ -45,6 +44,8 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Duration duration; private final LocatorGroupStrategy defaultLocatorGroupStrategy; private WeakReference cachedElementReference; + // Interface list proxies do not store the element. Hold it until the widget, which keeps it, exists. + private WebElement elementUntilWidgetCreated; /** * Creates a new interceptor. @@ -82,6 +83,7 @@ public WidgetInterceptor( ) { super(locator, driverReference); this.cachedElementReference = cachedElementReference; + this.elementUntilWidgetCreated = cachedElementReference == null ? null : cachedElementReference.get(); this.instantiationMap = instantiationMap; this.duration = duration; this.defaultLocatorGroupStrategy = defaultLocatorGroupStrategy; @@ -117,6 +119,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr new AppiumFieldDecorator(new WeakReference<>(widget), duration, defaultLocatorGroupStrategy), widget ); + elementUntilWidgetCreated = null; } try { method.setAccessible(true); @@ -128,9 +131,14 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr @Override public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { - WebElement element = ofNullable(cachedElementReference).map(WeakReference::get).orElse(null); + WebElement element = referencedElement(); return locator == null && element != null ? getObject(element, method, args) : super.call(obj, method, args, original); } + + private WebElement referencedElement() { + WebElement cached = cachedElementReference == null ? null : cachedElementReference.get(); + return cached != null ? cached : elementUntilWidgetCreated; + } } diff --git a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java index 1a0fa87c3..f2e72bee0 100644 --- a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java @@ -16,28 +16,18 @@ package io.appium.java_client.pagefactory.utils; -import io.appium.java_client.proxy.HasMethodCallListeners; -import io.appium.java_client.proxy.Interceptor; import io.appium.java_client.proxy.MethodCallListener; -import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.modifier.Visibility; -import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; -import net.bytebuddy.implementation.FieldAccessor; -import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.matcher.ElementMatcher; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.WeakHashMap; import static io.appium.java_client.proxy.Helpers.OBJECT_METHOD_NAMES; import static io.appium.java_client.proxy.Helpers.createProxy; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -49,8 +39,9 @@ public final class ProxyFactory { private static final Set NON_PROXYABLE_METHODS = setWithout( OBJECT_METHOD_NAMES, "toString", "equals", "hashCode" ); - private static final Map, Class> CACHED_INTERFACE_PROXIES = - Collections.synchronizedMap(new WeakHashMap<>()); + private static final ElementMatcher INTERFACE_METHODS = not( + namedOneOf(NON_PROXYABLE_METHODS.toArray(new String[0])) + ); @SafeVarargs private static Set setWithout(@SuppressWarnings("SameParameterValue") Set source, T... items) { @@ -112,7 +103,7 @@ public static T getEnhancedProxy( /** * Creates a proxy that implements {@code iface} and forwards its public methods to {@code listener}. - * Used when an {@code @OverrideWidget} field is declared as an interface. + * The interface must be public. Its loader and this library's loader must be able to see each other. * * @param iface an interface type * @param listener receives the proxied calls @@ -125,34 +116,9 @@ public static Object getEnhancedInterfaceProxy(Class iface, MethodCallListene if (listener == null) { throw new IllegalArgumentException("Listener must not be null"); } - Class proxyClass = CACHED_INTERFACE_PROXIES.computeIfAbsent(iface, ProxyFactory::loadInterfaceProxy); - try { - Object result = proxyClass.getDeclaredConstructor().newInstance(); - ((HasMethodCallListeners) result).setMethodCallListeners(new MethodCallListener[] {listener}); - return result; - } catch (ReflectiveOperationException e) { - throw new IllegalStateException(String.format("Unable to create a proxy of %s", iface.getName()), e); - } - } - - private static Class loadInterfaceProxy(Class iface) { - ClassLoader loader = iface.getClassLoader() == null - ? ProxyFactory.class.getClassLoader() - : iface.getClassLoader(); - ElementMatcher methods = isPublic().and(not( - namedOneOf(NON_PROXYABLE_METHODS.toArray(new String[0])) - )); - //noinspection resource - return new ByteBuddy() - .subclass(Object.class) - .implement(iface) - .method(methods) - .intercept(MethodDelegation.to(Interceptor.class)) - .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) - .implement(HasMethodCallListeners.class) - .intercept(FieldAccessor.ofBeanProperty()) - .make() - .load(loader, ClassLoadingStrategy.Default.WRAPPER) - .getLoaded(); + @SuppressWarnings("unchecked") + Class type = (Class) iface; + return createProxy(type, new Object[] {}, new Class[] {}, Collections.singletonList(listener), + INTERFACE_METHODS); } } diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index 8ee21706d..c32b0fea4 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -29,6 +29,7 @@ import org.jspecify.annotations.Nullable; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -56,6 +57,8 @@ public class Helpers { // the performance and to avoid extensive memory usage for our case, where // the amount of instrumented proxy classes we create is low in comparison to the amount // of proxy instances. + // The key is a signature object. The generated class does not reference that object, + // so collecting the weak key does not depend on the interface or widget class. private static final Map> CACHED_PROXY_CLASSES = Collections.synchronizedMap(new WeakHashMap<>()); @@ -71,10 +74,11 @@ private Helpers() { * are expected to be skipped if they throw * {@link io.appium.java_client.proxy.NotImplementedException}. * - * @param cls the class to which the proxy should be created. - * Must not be an interface. + * @param cls the class to proxy, or a public interface. + * A non-public interface is rejected. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. + * Must be empty when {@code cls} is an interface. * @param constructorArgTypes Array of constructor argument types. Must * represent types of constructorArgs. * @param listeners One or more method invocation listeners. @@ -103,8 +107,8 @@ public static T createProxy( * {@link io.appium.java_client.proxy.NotImplementedException}. * !!! This API is designed for private usage. * - * @param cls The class to which the proxy should be created. - * Must not be an interface. + * @param cls The class to proxy, or a public interface. + * A non-public interface is rejected. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. * @param constructorArgTypes Array of constructor argument types. Must @@ -121,6 +125,16 @@ public static T createProxy( Collection listeners, @Nullable ElementMatcher extraMethodMatcher ) { + requireNonNull(cls, "Class must not be null"); + if (cls.isInterface()) { + Preconditions.checkArgument(Modifier.isPublic(cls.getModifiers()), String.format( + "Cannot proxy non-public interface %s. The generated proxy is defined in its own class loader, " + + "so it cannot implement a package-private or protected interface. Declare %s public.", + cls.getName(), cls.getSimpleName() + )); + Preconditions.checkArgument(constructorArgs.length == 0 && constructorArgTypes.length == 0, + cls.getName() + " is an interface, so the proxy has no constructor arguments"); + } var signature = ProxyClassSignature.of(cls, constructorArgTypes, extraMethodMatcher); var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, k -> { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, @@ -130,22 +144,9 @@ public static T createProxy( ) ); Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); - requireNonNull(cls, "Class must not be null"); - Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); ElementMatcher.Junction matcher = ElementMatchers.isPublic(); - //noinspection resource - return new ByteBuddy() - .subclass(cls) - .method(extraMethodMatcher == null ? matcher : matcher.and(extraMethodMatcher)) - .intercept(MethodDelegation.to(Interceptor.class)) - // https://github.com/raphw/byte-buddy/blob/2caef35c172897cbdd21d163c55305a64649ce41/byte-buddy-dep/src/test/java/net/bytebuddy/ByteBuddyTutorialExamplesTest.java#L346 - .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) - .implement(HasMethodCallListeners.class).intercept(FieldAccessor.ofBeanProperty()) - .make() - .load(Helpers.class.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) - .getLoaded() - .asSubclass(cls); + return defineProxyClass(cls, extraMethodMatcher == null ? matcher : matcher.and(extraMethodMatcher)); }); try { @@ -171,9 +172,9 @@ public static T createProxy( * **public** method of the given class is invoked. Overridden callbacks * are expected to be skipped if they throw NotImplementedException. * - * @param cls the class to which the proxy should be created. - * Must not be an interface. Must expose a constructor - * without arguments. + * @param cls the class to proxy, or a public interface. + * A class must expose a constructor without arguments. + * A non-public interface is rejected. * @param listeners One or more method invocation listeners. * @param Any class derived from Object * @return Proxy instance @@ -190,9 +191,9 @@ public static T createProxy(Class cls, Collection lis * **public** method of the given class is invoked. Overridden callbacks * are expected to be skipped if they throw NotImplementedException. * - * @param cls the class to which the proxy should be created. - * Must not be an interface. Must expose a constructor - * without arguments. + * @param cls the class to proxy, or a public interface. + * A class must expose a constructor without arguments. + * A non-public interface is rejected. * @param listener Method invocation listener. * @param Any class derived from Object * @return Proxy instance @@ -209,8 +210,8 @@ public static T createProxy(Class cls, MethodCallListener listener) { * **public** method of the given class is invoked. Overridden callbacks * are expected to be skipped if they throw NotImplementedException. * - * @param cls the class to which the proxy should be created. - * Must not be an interface. + * @param cls the class to proxy, or a public interface. + * A non-public interface is rejected. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. * @param constructorArgTypes Array of constructor argument types. Must @@ -228,6 +229,70 @@ public static T createProxy( return createProxy(cls, constructorArgs, constructorArgTypes, Collections.singletonList(listener)); } + /** + * Builds a proxy class for a class or a public interface. + * An interface proxy subclasses {@link Object} and is loaded where both sides are visible. + */ + private static Class defineProxyClass( + Class cls, + ElementMatcher methodMatcher + ) { + boolean interfaceProxy = cls.isInterface(); + ClassLoader loader = interfaceProxy ? loaderForInterfaceProxy(cls) : Helpers.class.getClassLoader(); + var byteBuddy = new ByteBuddy(); + var subclassed = byteBuddy.subclass(interfaceProxy ? Object.class : cls); + var instrumented = interfaceProxy ? subclassed.implement(cls) : subclassed; + //noinspection resource + return instrumented + .method(methodMatcher) + .intercept(MethodDelegation.to(Interceptor.class)) + // https://github.com/raphw/byte-buddy/blob/2caef35c172897cbdd21d163c55305a64649ce41/byte-buddy-dep/src/test/java/net/bytebuddy/ByteBuddyTutorialExamplesTest.java#L346 + .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) + .implement(HasMethodCallListeners.class).intercept(FieldAccessor.ofBeanProperty()) + .make() + .load(loader, ClassLoadingStrategy.Default.WRAPPER) + .getLoaded() + .asSubclass(cls); + } + + /** + * Prefers the library loader, matching class proxies, when it can see the interface. + * Otherwise uses the interface loader when that loader can see {@link Interceptor}. + */ + private static ClassLoader loaderForInterfaceProxy(Class iface) { + ClassLoader libraryLoader = Helpers.class.getClassLoader(); + if (sameTypeVisible(libraryLoader, iface)) { + return libraryLoader; + } + ClassLoader interfaceLoader = iface.getClassLoader(); + if (interfaceLoader != null && sameTypeVisible(interfaceLoader, Interceptor.class)) { + return interfaceLoader; + } + throw new IllegalArgumentException(String.format( + "Cannot proxy %s. The generated proxy must see both this interface and %s. " + + "Interface class loader: %s. Library class loader: %s. " + + "Load the page object and io.appium.java_client through loaders that delegate to each other.", + iface.getName(), + Interceptor.class.getName(), + interfaceLoader, + libraryLoader + )); + } + + private static boolean sameTypeVisible(ClassLoader loader, Class type) { + if (type.getClassLoader() == loader) { + return true; + } + if (loader == null) { + return false; + } + try { + return Class.forName(type.getName(), false, loader) == type; + } catch (ClassNotFoundException e) { + return false; + } + } + @Value(staticConstructor = "of") private static class ProxyClassSignature { Class cls; diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/InterfaceProxyLoadingTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/InterfaceProxyLoadingTest.java new file mode 100644 index 000000000..8bd674c2f --- /dev/null +++ b/src/test/java/io/appium/java_client/pagefactory_tests/InterfaceProxyLoadingTest.java @@ -0,0 +1,95 @@ +package io.appium.java_client.pagefactory_tests; + +import io.appium.java_client.proxy.Helpers; +import io.appium.java_client.proxy.MethodCallListener; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Map; +import java.util.concurrent.Callable; + +import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedInterfaceProxy; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class InterfaceProxyLoadingTest { + + public interface Probe { + String ping(); + } + + @Test + void reusesOneProxyClassForTheSameInterface() { + MethodCallListener listener = new MethodCallListener() { + }; + Object first = getEnhancedInterfaceProxy(Probe.class, listener); + Object second = getEnhancedInterfaceProxy(Probe.class, listener); + + assertSame(first.getClass(), second.getClass()); + } + + @Test + void cacheKeyIsNotTheInterfaceClass() throws Exception { + getEnhancedInterfaceProxy(Probe.class, new MethodCallListener() { + }); + Field cacheField = Helpers.class.getDeclaredField("CACHED_PROXY_CLASSES"); + cacheField.setAccessible(true); + Map cache = (Map) cacheField.get(null); + + assertTrue(cache.keySet().stream().noneMatch(key -> key instanceof Class)); + assertThrows(NoSuchFieldException.class, () -> Class + .forName("io.appium.java_client.pagefactory.utils.ProxyFactory") + .getDeclaredField("CACHED_INTERFACE_PROXIES")); + } + + @Test + void proxiesPublicInterfaceLoadedByAChildOfTheLibraryLoader() throws Exception { + ClassLoader child = new URLClassLoader(new URL[0], Helpers.class.getClassLoader()); + Class iface = new ByteBuddy() + .makeInterface() + .name("io.appium.java_client.pagefactory_tests.ChildProbe") + .defineMethod("ping", String.class, Visibility.PUBLIC) + .withoutCode() + .make() + .load(child, ClassLoadingStrategy.Default.WRAPPER) + .getLoaded(); + Object proxy = getEnhancedInterfaceProxy(iface, new MethodCallListener() { + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) { + return "ping".equals(method.getName()) ? "pong" : MethodCallListener.UNSET; + } + }); + + assertEquals("pong", iface.getMethod("ping").invoke(proxy)); + } + + @Test + void rejectsInterfaceWhoseLoaderCannotSeeTheLibrary() throws Exception { + ClassLoader isolated = new URLClassLoader(new URL[0], ClassLoader.getPlatformClassLoader()); + Class iface = new ByteBuddy() + .makeInterface() + .name("io.appium.java_client.pagefactory_tests.IsolatedProbe") + .defineMethod("ping", String.class, Visibility.PUBLIC) + .withoutCode() + .make() + .load(isolated, ClassLoadingStrategy.Default.WRAPPER) + .getLoaded(); + + IllegalArgumentException error = assertThrows( + IllegalArgumentException.class, + () -> getEnhancedInterfaceProxy(iface, new MethodCallListener() { + }) + ); + assertTrue(error.getMessage().contains("class loader")); + assertNull(error.getCause()); + } +} diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java index 366ad5ec0..de28af00a 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java @@ -4,8 +4,11 @@ import io.appium.java_client.pagefactory.AppiumFieldDecorator; import io.appium.java_client.pagefactory.OverrideWidget; import io.appium.java_client.pagefactory.Widget; +import io.appium.java_client.pagefactory.WidgetInterceptor; import io.appium.java_client.pagefactory.iOSXCUITFindBy; import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; +import io.appium.java_client.proxy.HasMethodCallListeners; +import io.appium.java_client.proxy.MethodCallListener; import io.appium.java_client.support.FindBy; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -14,11 +17,14 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import java.lang.ref.WeakReference; +import java.lang.reflect.Field; import java.util.List; import java.util.stream.Stream; import static io.appium.java_client.support.PageFactory.initElements; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -41,10 +47,71 @@ void delegatesInterfaceMethodToPlatformWidget(WebDriver driver, String expected) initElements(new AppiumFieldDecorator(driver), page); assertEquals(expected, page.widget.platformAction()); + assertEquals(expected + "-title", page.widget.title()); assertEquals(2, page.widgets.size()); page.widgets.forEach(widget -> assertEquals(expected, widget.platformAction())); } + @Test + void singleFieldObjectMethodsUseTheLocatorAndProxyIdentity() { + var page = new Page(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertTrue(page.widget.toString().contains("Located by")); + assertTrue(page.widget.equals(page.widget)); + assertFalse(page.widget.equals(null)); + assertEquals(page.widget.hashCode(), page.widget.hashCode()); + } + + @Test + void listItemObjectMethodsAreDispatchedToTheWidget() { + var page = new IdentityPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + IdentityContract item = page.widgets.get(0); + + assertEquals("id-widget", item.toString()); + assertEquals(17, item.hashCode()); + assertTrue(item.equals("token")); + assertFalse(item.equals("other")); + } + + @Test + void listItemWorksAfterItsWeakElementReferenceIsCleared() throws Exception { + var page = new Page(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + ActionWidget item = page.widgets.get(0); + clearWeakElementReference(item); + + assertEquals("android", item.platformAction()); + } + + @Test + void nestedFindByOnTheConcreteWidgetIsInitialized() { + var page = new NestedPage(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertTrue(page.widget.labelText().contains("label")); + } + + @Test + void packagePrivateInterfaceFailsBeforeProxyGeneration() { + var page = new HiddenPage(); + + IllegalArgumentException error = assertThrows( + IllegalArgumentException.class, + () -> initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page) + ); + assertTrue(error.getMessage().contains("public")); + assertNull(error.getCause()); + } + + private static void clearWeakElementReference(Object proxy) throws Exception { + MethodCallListener listener = ((HasMethodCallListeners) proxy).getMethodCallListeners()[0]; + Field reference = WidgetInterceptor.class.getDeclaredField("cachedElementReference"); + reference.setAccessible(true); + reference.set(listener, new WeakReference(null)); + } + @Test void usesLocatorDeclaredOnThePlatformWidgetClass() { var page = new ClassLocatedPage(); @@ -136,8 +203,43 @@ static class PlainPage { private ActionWidget untouched; } + static class IdentityPage { + @OverrideWidget(html = IdentityHtmlWidget.class, + androidUIAutomator = IdentityAndroidWidget.class, + iOSXCUITAutomation = IdentityIosWidget.class) + @FindBy(id = "widget") + private List widgets; + } + + static class NestedPage { + @OverrideWidget(html = NestedHtmlWidget.class, + androidUIAutomator = NestedAndroidWidget.class, + iOSXCUITAutomation = NestedIosWidget.class) + private NestedContract widget; + } + + static class HiddenPage { + @OverrideWidget(html = HiddenWidget.class, + androidUIAutomator = HiddenWidget.class, + iOSXCUITAutomation = HiddenWidget.class) + @FindBy(id = "widget") + private PackagePrivateWidget widget; + } + public interface ActionWidget { String platformAction(); + + default String title() { + return "default"; + } + } + + public interface IdentityContract { + String platformAction(); + } + + public interface NestedContract { + String labelText(); } @AndroidFindBy(uiAutomator = "android-root") @@ -150,6 +252,11 @@ protected AndroidWidget(WebElement element) { public String platformAction() { return "android"; } + + @Override + public String title() { + return "android-title"; + } } @iOSXCUITFindBy(iOSNsPredicate = "ios-root") @@ -162,6 +269,11 @@ protected IosWidget(WebElement element) { public String platformAction() { return "ios"; } + + @Override + public String title() { + return "ios-title"; + } } public static class HtmlWidget extends Widget implements ActionWidget { @@ -173,6 +285,11 @@ protected HtmlWidget(WebElement element) { public String platformAction() { return "html"; } + + @Override + public String title() { + return "html-title"; + } } public static class UnrelatedWidget extends Widget { @@ -180,4 +297,90 @@ protected UnrelatedWidget(WebElement element) { super(element); } } + + public static class IdentityAndroidWidget extends Widget implements IdentityContract { + protected IdentityAndroidWidget(WebElement element) { + super(element); + } + + @Override + public String platformAction() { + return "android"; + } + + @Override + public String toString() { + return "id-widget"; + } + + @Override + public int hashCode() { + return 17; + } + + @Override + public boolean equals(Object obj) { + return "token".equals(obj); + } + } + + public static class IdentityIosWidget extends IdentityAndroidWidget { + protected IdentityIosWidget(WebElement element) { + super(element); + } + } + + public static class IdentityHtmlWidget extends IdentityAndroidWidget { + protected IdentityHtmlWidget(WebElement element) { + super(element); + } + } + + @AndroidFindBy(uiAutomator = "android-root") + public static class NestedAndroidWidget extends Widget implements NestedContract { + @FindBy(id = "label") + private WebElement label; + + protected NestedAndroidWidget(WebElement element) { + super(element); + } + + @Override + public String labelText() { + return label.toString(); + } + } + + public static class NestedIosWidget extends Widget implements NestedContract { + protected NestedIosWidget(WebElement element) { + super(element); + } + + @Override + public String labelText() { + return "ios"; + } + } + + public static class NestedHtmlWidget extends Widget implements NestedContract { + protected NestedHtmlWidget(WebElement element) { + super(element); + } + + @Override + public String labelText() { + return "html"; + } + } + + public static class HiddenWidget extends Widget implements PackagePrivateWidget { + protected HiddenWidget(WebElement element) { + super(element); + } + + @Override + public String platformAction() { + return "hidden"; + } + } } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/PackagePrivateWidget.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/PackagePrivateWidget.java new file mode 100644 index 000000000..04868007c --- /dev/null +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/PackagePrivateWidget.java @@ -0,0 +1,5 @@ +package io.appium.java_client.pagefactory_tests.widget.tests.combined; + +interface PackagePrivateWidget { + String platformAction(); +} From a732abbad80ceb6d38907badec72a55d78cd264d Mon Sep 17 00:00:00 2001 From: Ganesh Date: Sat, 7 Nov 2026 08:51:44 +0000 Subject: [PATCH 3/3] test: run un-overridden widget defaults on the real widget Document that a public interface is accepted when either class loader can see the other side, and that other cases throw IllegalArgumentException. --- docs/Page-objects.md | 2 +- .../pagefactory/OverrideWidget.java | 5 ++- .../pagefactory/utils/ProxyFactory.java | 5 ++- .../io/appium/java_client/proxy/Helpers.java | 31 +++++++++++++++---- .../OverriddenWidgetInterfaceTest.java | 16 ++++++++++ 5 files changed, 50 insertions(+), 9 deletions(-) diff --git a/docs/Page-objects.md b/docs/Page-objects.md index ccfb036e7..0ca7ea83c 100644 --- a/docs/Page-objects.md +++ b/docs/Page-objects.md @@ -700,7 +700,7 @@ This use case has some restrictions; //below is the other field/method declaration ``` -The field may be an interface implemented by those classes. Each class named by `@OverrideWidget` must extend `Widget` and implement the interface. The interface must be public. The page object and this library must be visible to loaders that delegate to each other; otherwise proxy creation fails with `IllegalArgumentException` instead of a linkage error. A content type that is left as the default cannot be instantiated, so name a class for every content type the field is used with: +The field may be an interface implemented by those classes. Each class named by `@OverrideWidget` must extend `Widget` and implement the interface. The interface must be public. Creation succeeds when this library's class loader can see that interface, or when the interface's class loader can see this library. A package-private or protected interface, and loaders that cannot see either way, fail with `IllegalArgumentException` instead of a linkage error. A content type that is left as the default cannot be instantiated, so name a class for every content type the field is used with: ```java public interface UsersWidget { diff --git a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java index 7343a834c..3d01f6900 100644 --- a/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java +++ b/src/main/java/io/appium/java_client/pagefactory/OverrideWidget.java @@ -24,7 +24,10 @@ /** * This annotation should mark an abstract/certain Widget object class * or field that declares some abstract/certain Widget object class or a public interface - * implemented by those classes. + * implemented by those classes. A package-private or protected interface is rejected with + * {@link IllegalArgumentException}. A public interface is accepted when this library's + * class loader can see it, or when its class loader can see this library. Otherwise + * initialization throws {@link IllegalArgumentException}. * io.appium.java_client.pagefactory.Widget is the Appium-specific extension of the * Page Object design pattern. * About the Page Object design pattern please read these documents: diff --git a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java index f2e72bee0..8507416df 100644 --- a/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/utils/ProxyFactory.java @@ -103,7 +103,10 @@ public static T getEnhancedProxy( /** * Creates a proxy that implements {@code iface} and forwards its public methods to {@code listener}. - * The interface must be public. Its loader and this library's loader must be able to see each other. + * The interface must be public. Creation succeeds when this library's class loader can see + * {@code iface}, or when {@code iface}'s class loader can see this library. + * A package-private or protected interface, or loaders that cannot see either way, + * is rejected with {@link IllegalArgumentException}. * * @param iface an interface type * @param listener receives the proxied calls diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index c32b0fea4..bcf38c357 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -43,6 +43,11 @@ /** * Helpers for creating transparent proxies of classes. + * + *

A public interface is proxied when this library's class loader can see the interface, + * or when the interface's class loader can see this library. + * A package-private or protected interface, or a public interface for which neither is true, + * is rejected with {@link IllegalArgumentException}. */ public class Helpers { /** @@ -75,7 +80,10 @@ private Helpers() { * {@link io.appium.java_client.proxy.NotImplementedException}. * * @param cls the class to proxy, or a public interface. - * A non-public interface is rejected. + * A non-public interface is rejected with + * {@link IllegalArgumentException}. A public interface is also + * rejected unless this library's loader can see it, or its loader + * can see this library. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. * Must be empty when {@code cls} is an interface. @@ -108,7 +116,10 @@ public static T createProxy( * !!! This API is designed for private usage. * * @param cls The class to proxy, or a public interface. - * A non-public interface is rejected. + * A non-public interface is rejected with + * {@link IllegalArgumentException}. A public interface is also + * rejected unless this library's loader can see it, or its loader + * can see this library. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. * @param constructorArgTypes Array of constructor argument types. Must @@ -174,7 +185,9 @@ public static T createProxy( * * @param cls the class to proxy, or a public interface. * A class must expose a constructor without arguments. - * A non-public interface is rejected. + * A non-public interface is rejected with {@link IllegalArgumentException}. + * A public interface is also rejected unless this library's loader can see it, + * or its loader can see this library. * @param listeners One or more method invocation listeners. * @param Any class derived from Object * @return Proxy instance @@ -193,7 +206,9 @@ public static T createProxy(Class cls, Collection lis * * @param cls the class to proxy, or a public interface. * A class must expose a constructor without arguments. - * A non-public interface is rejected. + * A non-public interface is rejected with {@link IllegalArgumentException}. + * A public interface is also rejected unless this library's loader can see it, + * or its loader can see this library. * @param listener Method invocation listener. * @param Any class derived from Object * @return Proxy instance @@ -211,7 +226,10 @@ public static T createProxy(Class cls, MethodCallListener listener) { * are expected to be skipped if they throw NotImplementedException. * * @param cls the class to proxy, or a public interface. - * A non-public interface is rejected. + * A non-public interface is rejected with + * {@link IllegalArgumentException}. A public interface is also + * rejected unless this library's loader can see it, or its loader + * can see this library. * @param constructorArgs Array of constructor arguments. Could be an * empty array if the class provides a constructor without arguments. * @param constructorArgTypes Array of constructor argument types. Must @@ -271,7 +289,8 @@ private static ClassLoader loaderForInterfaceProxy(Class iface) { throw new IllegalArgumentException(String.format( "Cannot proxy %s. The generated proxy must see both this interface and %s. " + "Interface class loader: %s. Library class loader: %s. " - + "Load the page object and io.appium.java_client through loaders that delegate to each other.", + + "Either this library's loader must see the interface, " + + "or the interface's loader must see this library.", iface.getName(), Interceptor.class.getName(), interfaceLoader, diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java index de28af00a..00be00f76 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/OverriddenWidgetInterfaceTest.java @@ -52,6 +52,15 @@ void delegatesInterfaceMethodToPlatformWidget(WebDriver driver, String expected) page.widgets.forEach(widget -> assertEquals(expected, widget.platformAction())); } + @Test + void defaultMethodNotOverriddenRunsOnTheRealWidgetForSingleAndList() { + var page = new Page(); + initElements(new AppiumFieldDecorator(new AbstractStubWebDriver.StubAndroidDriver()), page); + + assertEquals("AndroidWidget:android", page.widget.described()); + assertEquals("AndroidWidget:android", page.widgets.get(0).described()); + } + @Test void singleFieldObjectMethodsUseTheLocatorAndProxyIdentity() { var page = new Page(); @@ -232,6 +241,13 @@ public interface ActionWidget { default String title() { return "default"; } + + /** + * Not overridden by the platform widgets. {@code getClass} shows which instance ran it. + */ + default String described() { + return getClass().getSimpleName() + ":" + platformAction(); + } } public interface IdentityContract {